Make sure kubelet gets restarted after package version change

[danifr]

Fix #524

[puppet-community-rangefinder]

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.

[danifr]

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.

[danifr]

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

[daianamezdrea]

Hi @danifr, thank you ! Yes, I'll have a look, I'll try to debug it locally and see what is not working fine

[daianamezdrea]

Hi @danifr, I pushed some changes to see if the specs are going to pass

[danifr]

@daianamezdrea OMG thanks a lot!!!

[CLAassistant]

All committers have signed the CLA.

[danifr]

Hello again, can we get this change merged and have it available within the next release? Thank you!

[danifr]

@daianamezdrea Hi! sorry for insisting, did you have some time to look at this? I think we can safely merge it into main

[daianamezdrea]

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

[danifr]

Hi @daianamezdrea happy new year! is there something I can do to have this merged?

[puppet-community-rangefinder]

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.

[chelnak]

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.

[puppet-community-rangefinder]

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.

[chelnak]

Looks like we have some conflicts. Could you rebase on current main please?

[github-actions]

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.

[danifr]

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.

[deric]

@danifr For failing tests see #565

[danifr]

Still failing... I will try removing @daianamezdrea commit from this PR. Let's see

[danifr]

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.

[jordanbreen28]

@danifr looks like there are some merge conflicts. Could you resolve these so we can assess how to proceed with this.

[danifr]

I just did it. Thanks for the heads-up :)

[GSPatton]

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!

[danifr]

@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?