-
Notifications
You must be signed in to change notification settings - Fork 83
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 privateCA support for containerd images #2079
Conversation
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.
Please add tests to this change
data: | ||
update-containerd-certs.sh: | | ||
#!/usr/bin/env sh | ||
if [ ! -f /hostcontainerd/config.toml ]; then |
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.
theoretically I think this file could be totally absent and need to be created, but if we have tested on Azure, AWS, and GCP I'm fine with crossing that bridge when we come to it. In reality, I would expect it always exists.
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.
yes this file always exists in all cloud provider only difference is the content
values.yaml
Outdated
@@ -19,6 +19,10 @@ global: | |||
privateCaCertsAddToHost: | |||
enabled: false | |||
hostDirectory: /etc/docker/certs.d | |||
addToContainerd: false | |||
containerdConfigPath: /etc/containerd/certs.d |
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.
key name should be something else as that sounds like the path that should put to containerd.toml itself. Maybe rename to containerdCertsConfigPath or similar?
@@ -19,6 +19,10 @@ global: | |||
privateCaCertsAddToHost: | |||
enabled: false | |||
hostDirectory: /etc/docker/certs.d | |||
addToContainerd: false | |||
containerdConfigPath: /etc/containerd/certs.d | |||
containerdnodeAffinitys: [] |
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.
Are these actually templated into the daemonset anywhere?
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.
Needs to be tested in QA on all three major cloud-providers as part of the release process for both new install and upgrade path.
|
Description
Adds the ability to support privateCA for containerd images
Related Issues
https://github.com/astronomer/issues/issues/6022
Testing
tested on AWS, GCP, Azure, local clusters
Merging
release-0.34