-
Notifications
You must be signed in to change notification settings - Fork 524
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
restart modified host containers #1722
restart modified host containers #1722
Conversation
Currently I have tested host-container settings and proxy setting which I think might affect host-container restart. If you think there are another more settings that would affect host-container restart, Please let me know! I would like to do more test. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🍄
@@ -397,6 +406,10 @@ where | |||
if host_containerd_unit.is_active()? && !systemd_unit.is_enabled()? { | |||
command(constants::HOST_CTR_BIN, &["clean-up", "--container-id", name])?; | |||
} | |||
else if host_containerd_unit.is_active()? && systemd_unit.is_enabled()? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change should be done in a different part. For a little context, we only want to enable host-containers' units during the multi-user.target
execution.
With your current approach, during the execution of preconfigured.target
, and with user data as:
[settings.host-containers.control]
source = "<an-image>"
This condition will be false:
host_containerd_unit.is_active()? && systemd_unit.is_enabled()?
And since we have this conditional:
match command(constants::SYSTEMCTL_BIN, &["get-default"])?.trim().as_ref() {
"multi-user.target" => {
debug!("Enabling and starting container: '{}'", unit_name);
systemd_unit.enable_and_start()?
}
_ => {
debug!("Enabling: '{}'", unit_name);
systemd_unit.enable()?
}
};
The unit will be just enabled, not started. Now, say you have a bootstrap container that changes the host-containers' settings, this new condition will now be true, since the unit was enabled during preconfigured.target
:
host_containerd_unit.is_active()? && systemd_unit.is_enabled()?
Thus, your change effectively affects when host containers' units are started. I think a better approach is to modify the enable_and_start
method, since it will only be executed when multi-user.target
is the current target.
I think within that method, you should check if the unit is active with self.active()?
, if it is, then you can run your method try_reload_or_restart
.
This will keep the consistency of only starting host container units in multi-user.target
.
When a host container is running and its settings have changed, bottlerocket will restart and apply new changes to this host container.
9b75bb4
to
3bd666b
Compare
Push above address @arnaldo2792 's comment which it should only affect containers in the multi-user.target. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥾
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⛰️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🖌️
When a host container is running and its settings have changed,
bottlerocket will restart and apply new changes to this host container.
Issue number:
#1531
Description of changes:
If a host container is running, and its settings have changed, we should restart it so the settings take effect.
Testing done:
start a host-container and change some setting that might affect it like superpower, image URL, proxy....
Control container
source:
apiclient set -j '{"host-containers": {"control": {"source": "328549459982.dkr.ecr.us-west-2.amazonaws.com/bottlerocket-control:v0.5.0"}}}'
result:
Superpower:
apiclient set -j '{"host-containers": {"control": {"superpowered": true}}}'
result:
user-data:
result
Admin container
source:
apiclient set -j '{"host-containers": {"admin": {"source": "328549459982.dkr.ecr.us-west-2.amazonaws.com/bottlerocket-admin:v0.7.1"}}}'
result:
superpowered: When I set to false, I'll unable to access to sheltie. If anyway that I can check the log?
But when I set from false to true, Host-container restart
apiclient set -j '{"host-containers": {"admin": {"superpowered": true}}}'
result:
user-data:
result
Proxy:
settings.network.http_proxy=35.84.178.198: 3128
result
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.