这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@akatrevorjay
Copy link
Contributor

In the event that we cannot stop a container, we should try to kill it, and if that then fails, we should continue to the next $oldid , otherwise one container not actually being there or not able to be stopped will prevent the rest from being cleaned up.

The way this is, if a container ever fails to stop, it will never kill it anyway, due to being executed with -e.
This fixes that as well with a simple or.

The bigger issue at hand here is that if you change your container process types around it doesn't really cleanup after itself, leaving CONTAINER.$type.$nid and PORT.$type.$nid files around, but I didn't get around to addressing that.

…one of the old containers.

Also actually attempt to kill if stop has a bad exit code.
@michaelshobbs
Copy link
Member

Thanks for the PR. 👍 for me. @josegonzalez thoughts here?

@josegonzalez
Copy link
Member

Reminds me of that docker_sleep thing I wrote.

@akatrevorjay is there a way we can log a message when the kill fails? Maybe a hook call...

@michaelshobbs
Copy link
Member

Oh yeah. Where is that thing? Was it just in a gist somewhere or do you have it in a repo?

The docker stop/kill loop gets disown'd. I'm not sure we want to introduce a pluginhook call "out-of-band" like that. Sounds like a race-condition waiting to happen. Just my thoughts though.

@josegonzalez
Copy link
Member

Is there a way we can introduce general logging of this?

@michaelshobbs
Copy link
Member

We could trigger an event

@josegonzalez
Copy link
Member

Lets do that then. I want to have something there for our users so that at least it can be logged/detected and taken care of, even if out of band.

@michaelshobbs
Copy link
Member

👍
Implementation thoughts

  • add a new events plugin pluginhook
  • call it after every kill/stop

@michaelshobbs
Copy link
Member

@akatrevorjay let me know if you're comfortable adding the logging or if you'd prefer assistance

@josegonzalez
Copy link
Member

@michaelshobbs pretty sure @akatrevorjay won't get to the logging bit, so if you want to take the reins and merge this, go ahead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants