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.

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

[v2.9][v1.29] Rancher K8s v1.29.0 update #45380

Open
wants to merge 9 commits into
base: release/v2.9
Choose a base branch
from
Open

[v2.9][v1.29] Rancher K8s v1.29.0 update #45380

wants to merge 9 commits into from
+1,184 −10,678

Conversation

krunalhinguu
Copy link
Contributor

@krunalhinguu krunalhinguu commented May 6, 2024 •
edited

Issue:

Problem

  • To support Kubernetes 1.29 we need to bump all the k8s libraries to 1.29

Solution

  • Bumping all Kubernetes libraries to 1.29
  • Removed all dependencies related to PodSecurityPolicy
  • Removed all dependencies related to PodSecurityPolicyTemplateProjectBinding
  • Removed all dependencies related to PodSecurityPolicyTemplateName
  • Removed code to enable PSP
  • forceAdopt is renamed to takeOwnership as part of helm 3.14.3
  • Bumped golangci-lint version. go1.22 support golangci/golangci-lint#4273

Testing

rke2
rke
k3s

Engineering Testing

Manual Testing

Automated Testing

  • Test types added/modified:
    • Unit
    • Integration (Go Framework)
    • Integration (v2prov Framework)
    • Validation (Go Framework)
    • Other - Explain: EXPLAIN
    • None
    • REMOVE NOT APPLICABLE BULLET POINTS ABOVE
  • If "None" - Reason: EXPLAIN THE REASON
  • If "None" - GH Issue/PR: LINK TO GH ISSUE/PR TO ADD TESTS

Summary: TODO

QA Testing Considerations

Regressions Considerations

TODO

Existing / newly added automated tests that provide evidence there are no regressions:

  • TODO

@krunalhinguu krunalhinguu changed the title Rancher 1.29 [Draft][Testing]Rancher 1.29 - testing purpose only May 6, 2024
@mitulshah-suse mitulshah-suse marked this pull request as draft May 7, 2024 04:44
@krunalhinguu krunalhinguu force-pushed the rancher-1.29 branch 5 times, most recently from 4009a83 to 25b37e6 Compare May 8, 2024 06:37
@krunalhinguu krunalhinguu changed the title [Draft][Testing]Rancher 1.29 - testing purpose only [v2.9][v1.29] Rancher K8s v1.29.0 update May 8, 2024
@krunalhinguu krunalhinguu mentioned this pull request May 8, 2024
Open
38 tasks
@krunalhinguu krunalhinguu force-pushed the rancher-1.29 branch 3 times, most recently from 7bccba4 to 2d177ae Compare May 15, 2024 04:31
@chiukapoor chiukapoor force-pushed the rancher-1.29 branch 2 times, most recently from c59803d to 3cec011 Compare May 21, 2024 06:16
@chiukapoor chiukapoor marked this pull request as ready for review May 21, 2024 08:34
@chiukapoor chiukapoor requested review from jiaqiluo, kinarashah, snasovich and a team May 21, 2024 08:34
kinarashah
kinarashah reviewed May 21, 2024
View reviewed changes
go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
kinarashah
kinarashah requested changes May 21, 2024
View reviewed changes
Copy link
Member

@kinarashah kinarashah left a comment

Choose a reason for hiding this comment

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

@krunalhinguu We also need to update kubeVersion in Chart.yaml to add support for 1.29

rancher/chart/Chart.yaml

Line 6 in 3c48a19

kubeVersion: < 1.29.0-0

jiaqiluo
jiaqiluo requested changes May 22, 2024
View reviewed changes
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.

Such a huge PR, good job!
It looks good to me overall, and I have a few suggestions and questions.

go.mod
@@ -2,6 +2,8 @@ module github.com/rancher/rancher

go 1.22

replace github.com/rancher/shepherd => github.com/caliskanugur/shepherd v0.0.0-20240507144611-b10827fbb1f1
Copy link
Member

Choose a reason for hiding this comment

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

This will be switched back to rancher/shepherd, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, once this pull request is merged, automation team will proceed to update rancher/shepherd, which has a dependency on rancher/apis. Afterward, I'll ensure that this is updated with the appropriate rancher/shepherd tag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to keep track of this : #45564

Copy link
Member

Choose a reason for hiding this comment

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

Cool, it sounds good to me.

pkg/api/norman/customization/project/project_actions.go Show resolved Hide resolved
pkg/api/steve/catalog/types/rest.go Show resolved Hide resolved
tests/validation/tests/v3_api/resource/terraform/k3s/master/install_k3s_master.sh Outdated Show resolved Hide resolved
@mitulshah-suse mitulshah-suse requested a review from a team May 22, 2024 05:10
@MbolotSuse MbolotSuse removed their request for review May 22, 2024 15:50
@jiaqiluo jiaqiluo requested a review from a team May 22, 2024 18:08
@kinarashah
Copy link
Member

Need @maxsokolovsky's review around PSP removal before merging.

@snasovich
Copy link
Collaborator

@krunalhinguu , thank you for updating the PR. Unfortunately CI is failing after latest force-push that updated error handling packages usage - https://drone-pr.rancher.io/rancher/rancher/39365/6/2. Could you please resolve this?

@krunalhinguu
Copy link
Contributor Author

@snasovich looks like all the builds are failing in release-v2.9, I have to check the reason

maxsokolovsky
maxsokolovsky reviewed May 28, 2024
View reviewed changes
Copy link
Contributor

@maxsokolovsky maxsokolovsky left a comment •
edited

Choose a reason for hiding this comment

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

A couple of questions:

  1. Are there any ClusterRoles and ClusterRoleBindings with permissions to perform actions on PSPs that may need to be removed?
  2. If yes, do we know if these bindings only act on PSP objects and no others?

I am not seeing any cleanup of these remaining objects, and we might want that.

Cleanup is usually done by running custom code or Job when the agent starts up. See this example: #42325

maxsokolovsky
maxsokolovsky reviewed May 28, 2024
View reviewed changes
Copy link
Contributor

@maxsokolovsky maxsokolovsky left a comment

Choose a reason for hiding this comment

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

Another question is about CRDs, CRs, and PSP resources themselves.

In case of CRDs, any deletions will trigger K8s to delete custom resources of that kind. So those PSP Bindings will get deleted when the CRD for them is deleted. But what about PSPs themselves? Those are not strictly CRDs but resources of a K8s group. Are they treated the same as CRs when a CRD is deleted? If the K8s API server doesn't know about PSPs, does it delete them all?

@snasovich
Copy link
Collaborator

@maxsokolovsky , thank you for the feedback.
I'll let @krunalhinguu respond materially but if there are non-trivial changes that need to be done (like creating migration that cleans up CRs/CRBs and any other objects referencing PSPs) we may want to split it into a separate issue to unblock progress and testing on base 1.29 support and unblock work on 1.30 support. That issue will still target the same v2.9-Next1 milestone so it won't get missed. Having a separate issue will also ensure we have more specific test cases for these scenarios.

@krunalhinguu (cc: @mitulshah-suse ), please let us know if in fact we require these non-trivial changes to address @maxsokolovsky's concerns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxsokolovsky maxsokolovsky maxsokolovsky left review comments

@jiaqiluo jiaqiluo jiaqiluo approved these changes

@kinarashah kinarashah kinarashah approved these changes

@snasovich snasovich Awaiting requested review from snasovich

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

Successfully merging this pull request may close these issues.

None yet
6 participants
@krunalhinguu @kinarashah @snasovich @jiaqiluo @maxsokolovsky @chiukapoor