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

Update k8s libraries to v1.31.1 #6681

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

antoninbas
Copy link
Contributor

@antoninbas antoninbas commented Sep 19, 2024

  • Update K8s libraries to v1.31.1
  • Update sigs.k8s.io/controller-runtime to v0.19.0
  • Update controller-gen to v0.16.3
  • Use ubuntu:24.04 as base image for codegen

Some of the key changes that were caused by these updates:

  • Most functions in k8s.io/client-go/util/workqueue have been deprecated and were replaced with typed functions (using Go generics)
  • The Version field no longer exists in the generic apiserver config. It has been replaced with EffectiveVersion (this is because kube-apiserver now supports version emulation for more robust K8s version upgrades). EffectiveVersion is not required (Version used to be optional), and I am not sure whether that was an intentional change. EffectiveVersion is used (instead of Version) to configure the /version route. At the moment we have no intention of supporting version emulation in the Antrea Controller, but we could in the future. As far as I can tell, other changes relating to version emulation support do not affect Antrea.
  • The arguments to code generators have changed quite a bit.
  • The code generated for client-go (by client-gen) is now more generic (it leverages Go generics), so as a result the generated code is less verbose.
  • controller-gen now requires uniqueness for controller names (across all controllers in the program), for metrics reporting purposes. Because of this, we had to assign unique names manually to all controllers (and the name validation needs to be explicitly skipped in unit tests).

@antoninbas antoninbas force-pushed the update-k8s-libraries-to-v1.31.1 branch 3 times, most recently from b0dfc1f to 638eeaa Compare September 20, 2024 21:41
@antoninbas
Copy link
Contributor Author

/test-multicluster-e2e

@antoninbas antoninbas marked this pull request as ready for review September 20, 2024 22:50
@antoninbas
Copy link
Contributor Author

@tnqn @luolanzone it's a big PR, but I have tried to keep things organized in different commits to facilitate the review

@antoninbas
Copy link
Contributor Author

/test-all

@antoninbas antoninbas added this to the Antrea v2.2 release milestone Sep 20, 2024
tnqn
tnqn previously approved these changes Sep 24, 2024
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the work. Glad to get rid of workqueue item type assertion.

@@ -561,7 +560,7 @@ func TestStaleControllerNoRaceWithResourceImportReconciler(t *testing.T) {

stopCh := make(chan struct{})
defer close(stopCh)
q := workqueue.NewRateLimitingQueue(workqueue.DefaultItemBasedRateLimiter())
q := workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedItemBasedRateLimiter[*mcv1alpha1.ResourceImport]())
Copy link
Member

Choose a reason for hiding this comment

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

Not introduced to the PR, but using a pointer as the workqueue item is a bad usage as multiple events of a single resource could be handled concurrently, though this test creates a single event for each resource.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is at least one other place where a pointer is used (this time not in test code): https://github.com/antrea-io/antrea/blob/main/pkg/util/podstore/podstore.go
I can follow up on this after this PR. In practice, when the pointer is received from the informer through event handlers, does the pointer for a given resource change across updates?

Upgrade sigs.k8s.io/controller-runtime to v0.19.0

Remove GO111MODULE=on in Dockerfile

Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Use GetClient() in clientset expansions

Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
admission.Decoder is now an interface

Missing method in Manager mock

Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Ensure uniqueness of MC controller names

Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Generate OpenAPI spec for k8s.io/apimachinery/pkg/version
This is required because by setting EffectiveVersion (which is now
necessary), we enable the /version route, so we need the OpenAPI spec
for the response type (version.Info). All antrea-controller APIs require
an OpenAPI spec.

Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
@antoninbas
Copy link
Contributor Author

/test-all

@antoninbas
Copy link
Contributor Author

/test-multicluster-e2e

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.

2 participants