Added -y option to kamal traefik reboot command #730
Conversation
igor-alexandrov
force-pushed
the
confirming_dialogs
branch
from
ef929a3
to
bbf9529
Compare
lib/kamal/cli/traefik.rb
Outdated
| execute *KAMAL.traefik.stop, raise_on_non_zero_exit: false | ||
| execute *KAMAL.traefik.remove_container | ||
| execute *KAMAL.traefik.run | ||
| confirming "Reboot Traefik on hosts? (stop container, remove container, start new container). This will cause a brief outage." do |
There was a problem hiding this comment.
I was missing a question here (same as in other two places)
| confirming "Reboot Traefik on hosts? (stop container, remove container, start new container). This will cause a brief outage." do | |
| confirming "Reboot Traefik on hosts? (stop container, remove container, start new container). This will cause a brief outage. Are you sure?" do |
There was a problem hiding this comment.
@Sija I am not a native speaker, but there is already a question at the beginning of the phrase.
There was a problem hiding this comment.
We want something short and finish with a question.
@brunoprietog's point about rolling deploys is valid, though it doesn't guarantee that there won't be outages. Depending on how the load balancing is set up across hosts you could still get an errors for requests in flight or while the load balancer spots the outage. The Traefik reboot hooks are there so you can remove and drain the nodes before rebooting.
Maybe:
This will cause a brief outage on each host. Are you sure?
We can leave it to the user to infer whether those outages are concurrent or sequential based on the rolling argument.
There was a problem hiding this comment.
@djmb, you found the perfect words. I updated the text.
lib/kamal/cli/traefik.rb
Outdated
| execute *KAMAL.traefik.stop, raise_on_non_zero_exit: false | ||
| execute *KAMAL.traefik.remove_container | ||
| execute *KAMAL.traefik.run | ||
| confirming "Reboot Traefik on hosts? (stop container, remove container, start new container). This will cause a brief outage." do |
There was a problem hiding this comment.
I think this is not necessarily true. If the rolling option is used and there are multiple hosts the app will not crash, only one host will become unavailable for a short time.
|
Thanks @igor-alexandrov! |
Based on the discussion in #680, I added the new method
confirming,which accepts the block of code and prompts the user with a question that should be answered either "y" or "n."I replaced existing
askcalls and added confirmation tokamal traefik rebootcommand.