SwarmUI icon indicating copy to clipboard operation
SwarmUI copied to clipboard

Adding --rm to launch-docker.sh

Open fperdigon opened this issue 1 year ago • 3 comments

Adding --rm will ensure that the container is deleted when stopped. This avoids to manually delete the container created in the previous run before launching the server with this script

fperdigon avatar Aug 01 '24 03:08 fperdigon

cc @nightgrey for comment re is --rm here a good idea?

iirc there was an intentional decision to not including that, but it's been long enough that I'm not sure offhand. Iirc it was something like --rm is intended for short lived temporary instances, but Swarm is generally a long term reused application that you don't want a full rebuild of every time you restart

Note also that launch-docker is just one of two docker usage examples (alongside the compose version), you can of course happily just duplicate the file and launch in your own way. So the question of what args to use here isn't what's best for you, but rather what's best for the most common average user.

mcmonkey4eva avatar Aug 01 '24 16:08 mcmonkey4eva

I've added the following to the launch-docker.sh to address this issue, but i believe your solution is more robust.

#!/usr/bin/env bash

# Remove existing container if it exists
if [ $(docker ps -aq -f name=stableswarmui) ]; then
    docker rm -f stableswarmui
fi

isiahw1 avatar Aug 07 '24 05:08 isiahw1

@mcmonkey4eva

cc @nightgrey for comment re is --rm here a good idea?

iirc there was an intentional decision to not including that, but it's been long enough that I'm not sure offhand. Iirc it was something like --rm is intended for short lived temporary instances, but Swarm is generally a long term reused application that you don't want a full rebuild of every time you restart

Note also that launch-docker is just one of two docker usage examples (alongside the compose version), you can of course happily just duplicate the file and launch in your own way.

So the question of what args to use here isn't what's best for you, but rather what's best for the most common average user.

Hey Alex, sorry for the delayed response!

It has been a bit since I last used StableSwarm, but given the volumes and if I'm remembering correctly, any potential data is saved in there, meaning it technically wouldn't hurt, but I also don't understand why you would want to do that. Could you explain why, @fperdigon and @isiahw1?

As for me personally, as you've said, as a user, I just re-used the already existing container and would not have wanted to rebuild it every time, except if there were updates, but then I'd have to pull again anyway.

I also haven't seen --rm being a default for docker scripts in other projects, but I could be wrong, I mostly use containers for personal purposes - might be different for others.

Given that, I'd agree with your reply, but I'd also be curious to hear what the reasoning or benefits might be.

nightgrey avatar Oct 07 '24 09:10 nightgrey

I'd like to bump this topic. As for a dummy like me, the launch from docker says to run 'sudo ./launch-docker.sh'

However once you're done and close it, when you go to run it again you get 'docker: Error response from daemon: Conflict. The container name "/swarmui" is already in use by container "6463c66fb988d0204b74c8c390a9217101952b4cb4171e9cb69d326459a44480". You have to remove (or rename) that container to be able to reuse that name. See 'docker run --help'.'

So I have to manually run 'sudo docker rm swarmui' between each time I launch this, unless I'm missing something.

Someguitarist avatar Dec 06 '24 18:12 Someguitarist

yeah okay, it seems --rm is valid enough here and probably better as a default behavior for users that don't do more manual control of their docker setups.

ps @Someguitarist I don't think you're meant to use sudo here though (again to be clear I don't know enough about docker to really know, I just know I don't use sudo when I test it and the docs for it don't use sudo)

mcmonkey4eva avatar Dec 06 '24 19:12 mcmonkey4eva

... but this PR was written on the github web code editor and never tested. Ow. Formatting errors

mcmonkey4eva avatar Dec 06 '24 19:12 mcmonkey4eva

@Someguitarist

I'd like to bump this topic. As for a dummy like me, the launch from docker says to run 'sudo ./launch-docker.sh'

However once you're done and close it, when you go to run it again you get 'docker: Error response from daemon: Conflict. The container name "/swarmui" is already in use by container "6463c66fb988d0204b74c8c390a9217101952b4cb4171e9cb69d326459a44480". You have to remove (or rename) that container to be able to reuse that name.

See 'docker run --help'.'

So I have to manually run 'sudo docker rm swarmui' between each time I launch this, unless I'm missing something.

That makes a lot of sense! 😃

Sorry, I didn't think of this in my response, I think I barely used that script, and it has been a while since I setup StableSwarm... my bad!

Also, Alex is right - it's recommended to not use sudo, but rather adding a docker group & adding your user to it.

nightgrey avatar Dec 14 '24 18:12 nightgrey

oh, hi. Since my prior comment in here i spent a bunch of time actually learning docker. Wrote up https://github.com/mcmonkeyprojects/SwarmUI/blob/master/docs/Docker.md as a full explainer of using Swarm with Docker. The sudo bit is covered by the docker post-install instructions @ https://docs.docker.com/engine/install/linux-postinstall/

mcmonkey4eva avatar Dec 14 '24 18:12 mcmonkey4eva