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

Use separate containerd configs in aws-k8s and aws-dev #589

Merged
merged 1 commit into from
Dec 16, 2019

Conversation

tjkirch
Copy link
Contributor

@tjkirch tjkirch commented Dec 16, 2019

We were failing to write the containerd config in the dev image because
pod-infra-container-image wasn't set -- because it lives in the Kubernetes
settings we don't have in the dev image.

We don't need a pod-infra-container-image (or similar) setting in the dev image
because we use containerd through Docker which doesn't use it. So, the
clearest change is to split the containerd config file, removing the irrelevant
cri-plugin settings from the dev version, including the
pod-infra-container-image setting.


Testing done:

Built and launched an aws-k8s image; it connected to my cluster and pods worked. Logging in, I see the config files:

bash-5.0# ls -l /usr/share/templates/containerd-config-toml*
-rw-r--r-- 1 root root 337 Dec 16 20:23 /usr/share/templates/containerd-config-toml_aws-dev
-rw-r--r-- 1 root root 920 Dec 16 20:23 /usr/share/templates/containerd-config-toml_aws-k8s

...and confirmed that /etc/containerd/config.toml is the k8s version with sandbox_image set based on pod-infra-container-image.

Then I built an aws-dev image, connected and confirmed Docker containers still run OK. Logging in, I see the same two config file templates, and confirmed that /etc/containerd/config.toml is the new shorter version for dev:

bash-5.0# cat /etc/containerd/config.toml 
version = 2
root = "/var/lib/containerd"
state = "/run/containerd"
disabled_plugins = [
    "io.containerd.snapshotter.v1.aufs",
    "io.containerd.snapshotter.v1.zfs",
    "io.containerd.snapshotter.v1.devmapper",
]

[grpc]
address = "/run/containerd/containerd.sock"

[plugins."io.containerd.internal.v1.opt"]
path = "/opt/containerd"

We were failing to write the containerd config in the dev image because
pod-infra-container-image wasn't set -- because it lives in the Kubernetes
settings we don't have in the dev image.

We don't need a pod-infra-container-image (or similar) setting in the dev image
because we use containerd through Docker which doesn't use it.  So, the
clearest change is to split the containerd config file, removing the irrelevant
cri-plugin settings from the dev version, including the
pod-infra-container-image setting.
@tjkirch tjkirch requested review from jhaynes and etungsten December 16, 2019 22:14
Copy link
Contributor

@jhaynes jhaynes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love multiple config files and the .spec install-splosion that this causes... but since both of those are fairly straightforward to undo, ⛵️ 🚀.

Copy link
Contributor

@etungsten etungsten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

@tjkirch tjkirch merged commit 4783862 into develop Dec 16, 2019
@tjkirch tjkirch deleted the dev-containerd branch December 16, 2019 23:26
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