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

Add registry configuration breakout to containerd/config.toml #1369

Merged
merged 5 commits into from
Dec 16, 2021

Conversation

phealy
Copy link
Collaborator

@phealy phealy commented Dec 9, 2021

Adds a config entry to reference /etc/containerd/certs.d, which can contain per registry configuration files. This will allow customers to drop off config files with a DaemonSet that will modify how containerd acts when it connects to certain registries.

Enabled scenarios:

  • Adding a trusted root CA for a given registry, enabling private registry CAs
  • Adding a client certificate for mutual TLS against a registry
  • Redirecting a registry URL to a pull-through registry proxy seamlessly (so anything going to docker.io actually goes to mydockerproxy.mydomain.com)

Fixes AKS/1940 and helps with some of the people in AKS/2259 for Linux nodepools.

See:

Adds a config entry to reference /etc/containerd/certs.d, which
can contain registry configuration and trusted root CAs for use
when connecting to private registries.
@phealy phealy force-pushed the pahealy/add-containerd-certsd-config branch from 5930dcf to d6b53dd Compare December 9, 2021 17:01
@phealy phealy marked this pull request as draft December 9, 2021 17:11
@phealy
Copy link
Collaborator Author

phealy commented Dec 10, 2021

I tested this in a standalone and it seems to work great - the config is now limited to 1.22.x only where we have containerd 1.5, which is where the option support came in.

Only issue I see - none of our actual test cases in AgentBaker seem to build 1.22 clusters right now, so none of the test data includes the config statement. I'm not quite sure what the best way to add that test case in would be.

@phealy phealy marked this pull request as ready for review December 10, 2021 16:28
@alexeldeib
Copy link
Contributor

k8s version is in the describle table tests here:

DescribeTable("Generated customData and CSE", func(folder, k8sVersion string, configUpdator func(*datamodel.NodeBootstrappingConfiguration)) {
cs := &datamodel.ContainerService{
Location: "southcentralus",
Type: "Microsoft.ContainerService/ManagedClusters",
Properties: &datamodel.Properties{
OrchestratorProfile: &datamodel.OrchestratorProfile{
OrchestratorType: datamodel.Kubernetes,
OrchestratorVersion: k8sVersion,

a case like this with 1.22+containerd would be good to cover this scenario I think:

Entry("AKSUbuntu1804 with NoneCNI", "AKSUbuntu1804+NoneCNI", "1.20.7", func(config *datamodel.NodeBootstrappingConfiguration) {
config.ContainerService.Properties.AgentPoolProfiles[0].KubernetesConfig = &datamodel.KubernetesConfig{
ContainerRuntime: datamodel.Containerd,
}
config.ContainerService.Properties.OrchestratorProfile.KubernetesConfig.NetworkPlugin = datamodel.NetworkPluginNone
}))

otherwise PR lgtm, appreciate it :)

Copy link
Contributor

@alexeldeib alexeldeib left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for adding a test case!

@phealy
Copy link
Collaborator Author

phealy commented Dec 16, 2021

@alexeldeib I don't have write access, so would please you merge or help me fix that?

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.

Provide a configuration to define a registry mirror on AKS worker nodes
2 participants