Skip to content

K3s package rework #176

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 5, 2025
Merged

K3s package rework #176

merged 1 commit into from
Aug 5, 2025

Conversation

susesgartner
Copy link
Contributor

PR to rework the k3s package to use similar logic to the rke2 package.

Changes:

  1. All the changes listed in RKE2 package re-work for recurring runs. #169 .
  2. Updated logic in both RKE2/K3s packages to utilize the standard client helper function.

@susesgartner susesgartner requested a review from a team as a code owner July 31, 2025 21:30
markusewalker
markusewalker reviewed Jul 31, 2025
View reviewed changes
Copy link
Contributor

@markusewalker markusewalker left a comment

Choose a reason for hiding this comment

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

Left few comments, but else looks good. Will await the freeform job.

Josh-Diamond
Josh-Diamond reviewed Jul 31, 2025
View reviewed changes
navsalehi
navsalehi previously approved these changes Aug 1, 2025
View reviewed changes
Josh-Diamond
Josh-Diamond previously approved these changes Aug 1, 2025
View reviewed changes
markusewalker
markusewalker previously approved these changes Aug 1, 2025
View reviewed changes
@susesgartner susesgartner dismissed stale reviews from markusewalker and Josh-Diamond via 679988f August 4, 2025 17:33
@susesgartner susesgartner requested a review from a team as a code owner August 4, 2025 17:33
markusewalker
markusewalker previously approved these changes Aug 5, 2025
View reviewed changes
slickwarren
slickwarren previously approved these changes Aug 5, 2025
View reviewed changes
Copy link
Collaborator

@slickwarren slickwarren left a comment

Choose a reason for hiding this comment

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

none of my comments are blocking, but I do have some clarifying questions for my own understanding and some comments on syntax/formatting. LGTM for pit team

validation/provisioning/k3s/defaults/defaults.yaml Outdated
Comment on lines 64 to 68
sshPath: "<>"

awsCredentials:
secretKey: "<>"
accessKey: "<>"
Copy link
Collaborator

Choose a reason for hiding this comment

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

does it make sense to have these in 'defaults' if they don't have a default value set?

Copy link
Contributor Author

@susesgartner susesgartner Aug 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I just added them to indicate that those fields are required for the objects in question. I suppose maybe I could replace <> with "< required >" if that would make it clearer.

validation/provisioning/k3s/hardened_test.go
machinePools []provisioninginput.MachinePools
scanProfileName string
}{
{"K3S_CIS_1.9_Profile|3_etcd|2_cp|3_worker", k.standardUserClient, nodeRolesStandard, "k3s-cis-1.9-profile"},
Copy link
Collaborator

Choose a reason for hiding this comment

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

for CIS profile, I doesn't that change somewhat often? If so, wouldn't it make sense to make this less hardcoded in some way, i.e. a regex on the profile version or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to rework this test at some point since the new chart is called compliance. I am working on updating that but it will be in a future PR

validation/provisioning/k3s/node_driver_test.go
@@ -0,0 +1,110 @@
//go:build validation || recurring
Copy link
Collaborator

Choose a reason for hiding this comment

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

how are you guys using tags, 'recurring' seems too vague imo. Do you have some cadence that you run the recurring tests on? If so, do you run the same set (everything with 'recurring' tag) every time, or a subset of these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now we are going to run all of them and we are planning on a weekly/bi-weekly cadence. As we continue to upgrade our recurring runs we might change the tags later but that should be easy to change down the road.

validation/provisioning/k3s/psact_test.go
Comment on lines +56 to +58
func TestPSACT(t *testing.T) {
t.Parallel()
k := psactSetup(t)
Copy link
Collaborator

Choose a reason for hiding this comment

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

so you're still using testing.T, but not using the builtin setupSuite. Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

setupSuite is specific to testify. It is not built in as far as i'm aware.

@susesgartner susesgartner dismissed stale reviews from slickwarren and markusewalker via 46ca35f August 5, 2025 17:49
@susesgartner susesgartner force-pushed the K3S-rework branch 2 times, most recently from 46ca35f to 3991baa Compare August 5, 2025 17:58
@susesgartner
k3s rework
fc31d9a 
review fixes

updates

update go.mod

updates

update defaults
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 5, 2025

CustomClusterProvisioningTestSuite TearDownSuite
CustomClusterProvisioningTestSuite SetupSuite
CustomClusterProvisioningTestSuite TestProvisioningK3SCustomCluster
CustomClusterProvisioningTestSuite TestProvisioningK3SCustomClusterDynamicInput
HardenedK3SClusterProvisioningTestSuite TearDownSuite
HardenedK3SClusterProvisioningTestSuite SetupSuite
HardenedK3SClusterProvisioningTestSuite TestProvisioningK3SHardenedCluster
K3SNodeDriverProvisioningTestSuite TearDownSuite
K3SNodeDriverProvisioningTestSuite SetupSuite
K3SNodeDriverProvisioningTestSuite TestProvisioningK3SCluster
K3SNodeDriverProvisioningTestSuite TestProvisioningK3SClusterDynamicInput
K3SPSACTTestSuite TearDownSuite
K3SPSACTTestSuite SetupSuite
K3SPSACTTestSuite SetupSuite
K3SPSACTTestSuite SetupSuite
K3SPSACTTestSuite SetupSuite
K3SPSACTTestSuite SetupSuite
K3SPSACTTestSuite SetupSuite
K3SPSACTTestSuite SetupSuite
K3SPSACTTestSuite SetupSuite
K3SPSACTTestSuite SetupSuite
K3SPSACTTestSuite TestK3SPSACTNodeDriverCluster
K3SPSACTTestSuite TestK3SPSACTNodeDriverCluster
K3SPSACTTestSuite TestK3SPSACTNodeDriverCluster
K3SPSACTTestSuite TestK3SPSACTNodeDriverCluster
K3SPSACTTestSuite TestK3SPSACTNodeDriverCluster
K3SPSACTTestSuite TestK3SPSACTNodeDriverCluster
ClusterTemplateTestSuite TearDownSuite
ClusterTemplateTestSuite SetupSuite
ClusterTemplateTestSuite SetupSuite
ClusterTemplateTestSuite SetupSuite
ClusterTemplateTestSuite SetupSuite
ClusterTemplateTestSuite SetupSuite
ClusterTemplateTestSuite SetupSuite
ClusterTemplateTestSuite TestProvisionK3STemplateCluster
ClusterTemplateTestSuite TestProvisionK3STemplateCluster
ClusterTemplateTestSuite TestProvisionK3STemplateCluster
ClusterTemplateTestSuite TestProvisionK3STemplateCluster
ClusterTemplateTestSuite TestProvisionK3STemplateCluster
ClusterTemplateTestSuite TestProvisionK3STemplateCluster
ClusterTemplateTestSuite TestProvisionK3STemplateCluster
ClusterTemplateTestSuite TestProvisionK3STemplateCluster

TestSuites above were modified. TestSuites below use modified code from this PR.

TestCustomClusterK3SProvisioningTestSuite
TestCustom
TestDynamicCustom
TestDynamicNodeDriver
TestHardenedK3SClusterProvisioningTestSuite
TestHardened
TestNodeDriver
TestK3SProvisioningTestSuite
TestPSACT
TestK3SPSACTTestSuite
TestTemplate
TestClusterTemplateTestSuite
TestDynamicNodeDriver
TestTemplate
TestTemplate

@susesgartner susesgartner merged commit 075e3ba into rancher:main Aug 5, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markusewalker markusewalker markusewalker approved these changes

@navsalehi navsalehi navsalehi approved these changes

@slickwarren slickwarren slickwarren left review comments

@Josh-Diamond Josh-Diamond Josh-Diamond left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@susesgartner @slickwarren @Josh-Diamond @markusewalker @navsalehi