puppetlabs-kubernetes
puppetlabs-kubernetes copied to clipboard
Make sure kubelet gets restarted after package version change
Fix https://github.com/puppetlabs/puppetlabs-kubernetes/issues/524
kubernetes::packages is a class
that may have no external impact to Forge modules.
kubernetes::service is a class
that may have no external impact to Forge modules.
This module is declared in 0 of 576 indexed public Puppetfiles
.
These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.
Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.
Hmm I am not sure why the tests failed.
I am doing something similar to: https://github.com/puppetlabs/puppetlabs-kubernetes/blob/main/manifests/packages.pp#L357 (referencing a Service declared in the service.pp class)
I someone can give me a hint I'd appreciate it.
Well, I don't know what I can do to make the tests pass... I tried a few different approaches but I always get an error. I wonder if the tests are OK.
Just so you know, same patch, running on my infra, works as expected (kubelet services gets restarted after a package update):
Notice: /Stage[main]/Kubernetes::Packages/Package[kubelet]/ensure: ensure changed '1.20.8-0' to '1.21.0' (corrective)
Info: /Stage[main]/Kubernetes::Packages/Package[kubelet]: Scheduling refresh of Service[kubelet]
Notice: /Stage[main]/Kubernetes::Packages/Package[kubectl]/ensure: ensure changed '1.20.8-0' to '1.21.0' (corrective)
Notice: /Stage[main]/Kubernetes::Packages/Package[kubeadm]/ensure: ensure changed '1.20.8-0' to '1.21.0' (corrective)
@daianamezdrea can you please review it? Thanks
Hi @danifr, thank you ! Yes, I'll have a look, I'll try to debug it locally and see what is not working fine
Hi @danifr, I pushed some changes to see if the specs are going to pass
@daianamezdrea OMG thanks a lot!!!
Hello again, can we get this change merged and have it available within the next release? Thank you!
@daianamezdrea Hi! sorry for insisting, did you have some time to look at this? I think we can safely merge it into main
Hi @danifr, I required @carabasdaniel review on the changes I've made and he said that the dependencies added transform the unit tests in acceptance (with the changes I did) so I need to re-think the issue, sorry for the late response, looking now again how to approach it
Hi @daianamezdrea happy new year! is there something I can do to have this merged?
This PR has been marked as stale because it has been open for a while and has had no recent activity. If this PR is still important to you please drop a comment below and we will add this to our backlog to complete. Otherwise, it will be closed in 7 days.
Is there something else I can do so we can get this one merged?
kubernetes::packages is a class
that may have no external impact to Forge modules.
kubernetes::service is a class
that may have no external impact to Forge modules.
This module is declared in 0 of 579 indexed public Puppetfiles
.
These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.
Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.
Looks like there were some failing tests on the last run.
I've re kicked them so we can get a fresh view of what is happening.
kubernetes::packages is a class
that may have no external impact to Forge modules.
kubernetes::service is a class
that may have no external impact to Forge modules.
This module is declared in 0 of 579 indexed public Puppetfiles
.
These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.
Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.
Looks like we have some conflicts. Could you rebase on current main please?
Hello! 👋
This pull request has been open for a while and has had no recent activity. We've labelled it with attention-needed
so that we can get a clear view of which PRs need our attention.
If you are waiting on a response from us we will try and address your comments on a future Community Day.
Alternatively, if it is no longer relevant to you please close the PR with a comment.
Please note that if a pull request receives no update for 7 after it has been labelled, it will be closed. We are always happy to re-open pull request if they have been closed in error.
Hi @chelnak, sorry I missed you comment. I just rebased from main and the tests are still failing...
I don't understand why though... the only relevant change that this PR introduces is:
https://github.com/puppetlabs/puppetlabs-kubernetes/pull/525/files#diff-98b7734749457d0f7b07bfccf04f7316e31a48cdf3e02a46cb3cb8209daecb29R128
Maybe you know what's going on.
@danifr For failing tests see #565
Still failing... I will try removing @daianamezdrea commit from this PR. Let's see
Blows my mind that is fails with:
# Puppet::Parser::Compiler::CatalogValidationError:
# Could not find resource 'Service[kubectl]' in parameter 'notify' (file: /home/runner/work/puppetlabs-kubernetes/puppetlabs-kubernetes/spec/fixtures/modules/kubernetes/manifests/packages.pp, line: 368)
Because of: https://github.com/puppetlabs/puppetlabs-kubernetes/pull/525/files#diff-7395f2f3aeb84af1465802a40aed5e8226f466887bccbf9a8e4599ea73f50964R386
But it does not say anything about: https://github.com/puppetlabs/puppetlabs-kubernetes/blob/main/manifests/packages.pp#L357 that is basically the same thing.
@danifr looks like there are some merge conflicts. Could you resolve these so we can assess how to proceed with this.
I just did it. Thanks for the heads-up :)
hey @danifr. I pulled your PR and ran the tests locally. The tests fail on your branch but pass on the master branch so the failures are due to your changes.
The failing tests complain about missing k8s-related files e.g. expected to contain File[/etc/systemd/system/kubelet.service.d/20-cloud.conf] with content =~ /--cloud-provider=openstack/
etc. This may be a good starting point.
Hopefully this helps!
@GSPatton sorry did not have time to work on this. I can open a new PR in the future if I managed to get it working.
I need to figure out how to run the test locally, so I can make sure the test pass before opening a PR. Do you have some docs or info on how to run them locally?