You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds an additional mount of the /local/host-containers/NAME directory as /.bottlerocket/host-containers/current to provide a generic persistent storage location that isn't tied to the name of the container. This makes it easier for scripts to utilize the storage.
Testing done:
Built aws-ecs-1 ami and launched instance.
Instance connected to ecs cluster.
Test task deployed successfully.
Connected to control container via ssm session.
Verified that /.bottlerocket/host-containers/ contained both a control directory and a current directory.
Enabled and launched admin container.
Connected to admin container via ssh.
Verified that /.bottlerocket/host-containers/ contained both an admin directory and a current directory.
Verified that /.bottlerocket/host-containers/admin/ and /.bottlerocket/host-containers/current/ had the same contents.
Ran sudo sheltie to verify root shell was still available.
Checked for failed systemd units.
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.
If the goal is to let scripts know where the host-container's persistent storage location is, maybe we can utilize environment variables instead of trying to mount the same thing twice. https://pkg.go.dev/github.com/ktock/containerd/oci#WithEnv
So something like,
If the goal is to let scripts know where the host-container's persistent storage location is, maybe we can utilize environment variables instead of trying to mount the same thing twice. https://pkg.go.dev/github.com/ktock/containerd/oci#WithEnv
So something like,
Or just set "HOST_CONTAINER_NAME="containerID so the examples we have in the README using that variable would work literally. It feels slightly more troublesome to me than just having a fixed directory name, because variables add other complications, but not huge.
There are definitely pros and cons to both methods. I'm leaning more towards what @etungsten is saying with the ENV, but using oci.WithEnv([]string{"HOST_CONTAINER_NAME"=containerID})". It keeps our current documentation correct and users don't need to unlearn anything. Is anyone strongly opposed?
There are definitely pros and cons to both methods. I'm leaning more towards what @etungsten is saying with the ENV, but using oci.WithEnv([]string{"HOST_CONTAINER_NAME"=containerID})". It keeps our current documentation correct and users don't need to unlearn anything. Is anyone strongly opposed?
Is it going to be possible to symlink /var/lib/amazon/ssm to the correct host-containers/FOO in bottlerocket-os/bottlerocket-control-container#12 in Dockerfile if this uses a variable instead of a mount? The variable wouldn't be present during the container build. It could be done in start_control_ssm.sh but it's better to do that statically where possible.
Right, the symlink would have to be created in start_control_ssm.sh, but I don't think that is necessarily a bad thing. The folder it is symlinking to is created in start_control_ssm.sh, so it may be better to keep that logic closer together.
Moved persistent mount logic into it's own function. This function mounts the legacy persistant storage location and mounts a current storage location only if the host container does not also share the name current.
This adds an additional mount of the /local/host-containers/NAME
directory as /.bottlerocket/host-containers/current to provide a generic
persistent storage location that isn't tied to the name of the
container. This makes it easier for scripts to utilize the storage.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
jpculp
force-pushed
the
add-generic-mount-to-host-containers
branch
from
Issue number:
N/A
Description of changes:
This adds an additional mount of the
/local/host-containers/NAMEdirectory as/.bottlerocket/host-containers/currentto provide a generic persistent storage location that isn't tied to the name of the container. This makes it easier for scripts to utilize the storage.Testing done:
aws-ecs-1ami and launched instance./.bottlerocket/host-containers/contained both acontroldirectory and acurrentdirectory./.bottlerocket/host-containers/contained both anadmindirectory and acurrentdirectory./.bottlerocket/host-containers/admin/and/.bottlerocket/host-containers/current/had the same contents.sudo sheltieto verify root shell was still available.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.