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

bump system agent, fix revive errors, update golangci-lint config file #199

Merged
merged 2 commits into from
Aug 1, 2024

Conversation

HarrisonWAffel
Copy link
Contributor

@HarrisonWAffel HarrisonWAffel commented Jul 17, 2024

Issue rancher/rancher#46219, rancher/rancher#46220

Summary

Several Kubernetes dependencies are outdated and contain CVEs. These dependencies are indirect and are used mostly by the embedded system agent. In order to use the most recent dependency versions, the system agent dependency should also be updated, as we do not know if there are any regressions or changes in behavior when combining an older version of the system agent with newer dependency versions.

Occurred changes and/or fixed issues

Bumped the system agent and the related k8s dependencies. We now use 1.27.x for all Kubernetes dependencies, which is inline with the most recent system agent version 0.3.7.

Recent versions of the system agent have introduced a concept of 'interlock files', which were designed to improve consistency when executing plans. Currently, there are no Windows specific plans which utilize these files, so the applyinator.NewApplyinator function signature intentionally omits that parameter.

v0.3.7 of the system-agent changed the signature of k8splan.Watch, which now requires an additional boolean. Unfortunately, Rancher has not been properly updated to propagate an environment variable or other value to specify what the value of that boolean should be. Due to this, the value is currently set to false to ensure there are no behavior changes.

Additionally, the CI has been updated to accommodate the extra time required to lint the updated dependencies.

Technical notes summary

Areas or cases that should be tested

When a new version of wins is cut and vendored into Rancher, we should test all basic provisioning use cases.

Areas which could experience regressions

Screenshot/Video

@HarrisonWAffel HarrisonWAffel marked this pull request as ready for review July 17, 2024 21:16
@HarrisonWAffel HarrisonWAffel requested a review from a team July 17, 2024 21:21
@snasovich
Copy link

We will be releasing system-agent v0.3.7 shortly as part of 2.9.0 release so let's bump to it once it's available.

@HarrisonWAffel
Copy link
Contributor Author

HarrisonWAffel commented Jul 30, 2024

v0.3.7 has been released and I'm currently testing the use of that version in wins. I'll update the PR once I can confirm that there are no issues with v0.3.7 on windows due to the agent tls mode feature.

Copy link
Member

@jiaqiluo jiaqiluo left a comment

Choose a reason for hiding this comment

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

lgtm

@jiaqiluo jiaqiluo requested a review from a team July 31, 2024 17:22
@HarrisonWAffel
Copy link
Contributor Author

HarrisonWAffel commented Jul 31, 2024

I've tested the system agent v0.3.7 using a custom build of wins and confirmed that I was able to provision a windows cluster, deploy workloads, and upgrade that windows cluster successfully using rancher v2.9.0-rc7.

However, the latest version of the system agent has changed the k8splan.Watch function signature to include a boolean for strict tls verification. The initial implementation of that feature has not properly passed the required environment variables to windows nodes, which means there is no way to know what the proper value is for the given cluster. For this reason, I have set the value to false, so that there is no change in behavior for existing clusters.

While testing the new version of the system agent, and determining why the environment variable was not being passed, I stumbled on this old windows issue rancher/windows#181. This issue was not properly tested, and does not work as intended. This is likely one of the reasons that the strict tls verification environment variable is not being passed to wins, and why the wins binary does not upgrade alongside Rancher versions.

@HarrisonWAffel HarrisonWAffel merged commit fdc2367 into rancher:main Aug 1, 2024
3 checks passed
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.

4 participants