Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

auto-update: allow to use custom command when stopping container #14413

Closed
gdesmott opened this issue May 30, 2022 · 11 comments
Closed

auto-update: allow to use custom command when stopping container #14413

gdesmott opened this issue May 30, 2022 · 11 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@gdesmott
Copy link

/kind feature

Description

I manually implemented a script very similar to podman-auto-update. The only thing blocking me from switching to it is the ability to use a custom command to stop the running container.
I send a request to my server telling it to set itself in "maintenance" mode so it will stop accepting new requests from clients and automatically shutdown itself when all the clients have been disconnected. This ensure seamless upgrades as clients are not disconnected right away when upgrading.

Would it be possible to have something like this with auto-update? Something a bit similar to watchtower's hooks may do.

@openshift-ci openshift-ci bot added the kind/feature Categorizes issue or PR as related to a new feature. label May 30, 2022
@vrothberg
Copy link
Member

Thanks for reaching out, @gdesmott!

What you could do is to change the generated systemd unit:

Before

ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id
ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id

After

ExecStop= *** CUSTOM SHUTDOWN LOGIX ***
ExecStopPost=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id
ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id

Note that customizing the generated units will leave supported territory. Any chance your workload can handle SIGTERM? That's what podman stop sends to the container initially.

@gdesmott
Copy link
Author

Thanks for your quick reply @vrothberg :)

Note that customizing the generated units will leave supported territory.

Yes, I'd rather keep my own update script than doing such error-prone hacks.

Any chance your workload can handle SIGTERM? That's what podman stop sends to the container initially.

I considered that, but my container has a entrypoint.sh script starting the binary server and I didn't find a way to easily forward signals to the actual binary.

@vrothberg
Copy link
Member

@gdesmott, can you elaborate on the problems you faced trying to forward the signal? Did you run the container with --init?

That is the only idea (for a supported solution) I have at the moment.

@gdesmott
Copy link
Author

Did you run the container with --init?

I did not. Will that automatically signal from entrypoint.sh to the server binary it started?

@vrothberg
Copy link
Member

I did not. Will that automatically signal from entrypoint.sh to the server binary it started?

I can't tell for sure but it's worth testing. The --init binary will only forward to spawned process but some shells exec on the last command instead of forking (see #13324 (comment)), so it may "just work".

@Luap99
Copy link
Member

Luap99 commented May 30, 2022

I don't think that adding hooks to podman auto-update is more useful then just running the commands before/afterwards.

You can also just put this into a script:

custom command
podman auto-update
custom command

@vrothberg
Copy link
Member

@Luap99, I don't that fits the use case. The "custom stop command" should only be executed during auto-update. If there's nothing to update, nothing should happen.

@Luap99
Copy link
Member

Luap99 commented May 30, 2022

Ah, that's true.

@vrothberg
Copy link
Member

I did not. Will that automatically signal from entrypoint.sh to the server binary it started?

I can't tell for sure but it's worth testing. The --init binary will only forward to spawned process but some shells exec on the last command instead of forking (see #13324 (comment)), so it may "just work".

@gdesmott did it work using --init?

@gdesmott
Copy link
Author

I didn't have a chance to test it yet. It's definitely on my TODO list but it makes take a while, so feel free to close this ticket for now if you think adding such feature does not make sense.

@vrothberg
Copy link
Member

Sounds good to me. Please let us know how it worked out.

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

No branches or pull requests

3 participants