-
Notifications
You must be signed in to change notification settings - Fork 12
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
Update k8s/cri-o repositories #94
Conversation
@@ -8,16 +8,13 @@ | |||
Boolean $manage_container_manager = $k8s::manage_container_manager, | |||
K8s::Container_runtimes $container_manager = $k8s::container_manager, | |||
String[1] $crio_version = $k8s::version.split('\.')[0, 2].join('.'), | |||
Boolean $use_kubic_repos = false |
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 document the new parameter with puppet-strings
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.
Happy to do all the tidy bits once I get consensus that this is an approach ya'all agree with. If I can get pre-approval that going this direction is agreeable (dropping all obsolete k8s version support, etc) I'm happy to invest a few hours making it perfect.
So this is only intended to hit the target of "should work for you if you want to test it" and "something for us to discuss"
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.
Personally not against dropping kubic entirely, optionally even with a catalog fail condition on Kubernetes <1.24 versions, since those versions are long since deprecated.
Unfortunately the migration from kubic to the new isv:kubernetes repos can be difficult to automate, since some packages have migrated concerns between them.
On Debian, a complete uninstall of cri-o and conmon is necessary before the isv:kubernetes cri-o package can be installed - as one example.
thanks for the PR, can you add some unit tests to verify the new behaviour? |
Remove path fixes that don't exist in any version of crio packages I can find Remove conditionals around long-obsolete/unsupported versions Allow use of the deprecated kubic repos, default to official k8s packages
84f9347
to
234a796
Compare
The |
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.
Have finally the chance to sit down and test things with the new repos, found a few issues.
} else { | ||
$kubernetes_name = 'kubernetes' | ||
$kubernetes_repo = "https://pkgs.k8s.io/core:/stable:/v${crio_version}/deb" | ||
$kubernetes_key = 'kubernetes-apt-keyring.gpg' |
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.
This needs to be kubernetes-apt-keyring.asc
$kubernetes_key = 'kubernetes-apt-keyring.gpg' | ||
$crio_name = 'cri-o' | ||
$crio_url = "https://pkgs.k8s.io/addons:/cri-o:/stable:/v${crio_version}/deb" | ||
$crio_key = 'cri-o-apt-keyring.gpg' |
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.
This needs to be cri-o-apt-keyring.asc
@@ -8,16 +8,13 @@ | |||
Boolean $manage_container_manager = $k8s::manage_container_manager, | |||
K8s::Container_runtimes $container_manager = $k8s::container_manager, | |||
String[1] $crio_version = $k8s::version.split('\.')[0, 2].join('.'), | |||
Boolean $use_kubic_repos = false |
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.
Personally not against dropping kubic entirely, optionally even with a catalog fail condition on Kubernetes <1.24 versions, since those versions are long since deprecated.
Unfortunately the migration from kubic to the new isv:kubernetes repos can be difficult to automate, since some packages have migrated concerns between them.
On Debian, a complete uninstall of cri-o and conmon is necessary before the isv:kubernetes cri-o package can be installed - as one example.
Superseded by #103 |
Pull Request (PR) description
This Pull Request (PR) fixes the following issues
Fixes #77
Fixes #47 (for Ubuntu)
Some comments
I included the ability to use the kubic repos for existing implementations (with a value change) based on this comment #77 (comment) however given the number of changes around k8s repos since 1.28 I personally think it would be better to make a major version cut and simply say that it drops support for the kubic repos which are deprecated IMHO YMMV
Obviously this PR only addresses Debian. I didn't want to invest the time until we have some consensus on the approach, at which time I'll update the PR to fix the repos for all platforms.