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

Vendor k/*@v0.19 and controller-runtime@v0.7 #3393

Merged
merged 27 commits into from Jan 14, 2021

Conversation

timebertt
Copy link
Member

@timebertt timebertt commented Jan 13, 2021

How to categorize this PR?

/area open-source quality
/kind task
/priority normal

What this PR does / why we need it:

This PR vendors all the kubernetes dependencies in version v0.19.6 and controller-runtime version v0.7.0.
See #3109 for more details.

Which issue(s) this PR fixes:
This is only the strictly required part of #3109

Special notes for your reviewer:
I tried to work in different commits to ease the review of this monstrous PR, each one of them addresses mainly one breaking change in upstream (though, it's not 100% accurately separated, just to ease the review).
/squash

gardener-seed-admission-controller currently uses some custom decoding and encoding logic for admission request and review, but uses the controller-runtime types for passing them down to the handlers.
The upstream types were changed to admission/v1 (ref kubernetes-sigs/controller-runtime#1284).
I didn't want to go through the hassle of adapting our logic to handle both admission versions, I rather removed the custom logic and migrated the handlers to use the upstream implementation.
I haven't migrated to the controller-runtime manager/webhook server yet and also haven't done the same change in the gardener-admission-controller, as it wasn't strictly required and I wanted to keep this PR limited to the required changes in the upgrade process.

Release note:

⚠️ Go dependencies to `kubernetes/*` and `kubernetes-sigs/controller-runtime` were updated to `v0.19.6` and `v0.7.0` respectively. This imposes a lot of consequent breaking changes to go projects vendoring gardener/gardener. If your project/extension vendors gardener/gardener, please read the dedicated section in [this issue](https://github.com/gardener/gardener/issues/3109) carefully when upgrading your dependencies.

@timebertt timebertt requested review from a team as code owners January 13, 2021 07:59
@gardener-robot gardener-robot added area/open-source Open Source (community, enablement, contributions, conferences, CNCF, etc.) related area/quality Output qualification (tests, checks, scans, automation in general, etc.) related merge/squash labels Jan 13, 2021
@gardener-robot
Copy link

@timebertt Label kind/pain does not exist.

@timuthy
Copy link
Contributor

timuthy commented Jan 13, 2021

/assign

@rfranzke
Copy link
Member

rfranzke commented Jan 13, 2021

/assign
/blocker

@timebertt
Copy link
Member Author

I had a quick chat with @mvladev about #3372 (comment).
The way I addressed it in this PR should be fine for now, as we don't use the Extenders field and its anyways only a renaming.
@mvladev wants to copy the k8s.io/kube-scheduler/config/v1 as well and set things straight in this regard, but will do it after this PR.

Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

Well, first of all: Kudos and thanks a lot for this great PR. It's well structured and the dedicated commits helped tremendously to review it. It was a very smooth experience!

I've mainly found a few nits, not much. I've three "critical" questions when it comes to supporting old Kubernetes versions now that you upgraded certain APIs to new versions that are not available in older Kubernetes versions (which we still support, though).

pkg/gardenlet/bootstrap/bootstrap.go Show resolved Hide resolved
pkg/client/kubernetes/runtime_client.go Outdated Show resolved Hide resolved
extensions/pkg/controller/utils.go Show resolved Hide resolved
extensions/pkg/controller/utils.go Show resolved Hide resolved
pkg/utils/kubernetes/client/client.go Show resolved Hide resolved
pkg/seedadmission/extension_crds.go Show resolved Hide resolved
pkg/seedadmission/pod_scheduler_name.go Outdated Show resolved Hide resolved
pkg/seedadmission/pod_scheduler_name_test.go Outdated Show resolved Hide resolved
pkg/seedadmission/pod_scheduler_name.go Outdated Show resolved Hide resolved
@timebertt
Copy link
Member Author

Changing the default warning handler might be an alternative in this case: https://kubernetes.io/blog/2020/09/03/warnings/#customize-client-handling

Yes, this seems like the better option for now.

I added a deduplicating warnings handler to all components.
@ialidzhikov @timuthy @rfranzke PTAL :)

Copy link
Member

@ialidzhikov ialidzhikov left a comment

Choose a reason for hiding this comment

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

/lgtm

Copy link
Contributor

@timuthy timuthy left a comment

Choose a reason for hiding this comment

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

/lgtm

@timebertt
Copy link
Member Author

Nice! Thanks a lot guys for your thorough and fast reviews! ❤️
This means we are good to merge, once CI has passed, right?

@rfranzke
Copy link
Member

😱 Probably :)

@timebertt timebertt merged commit 03794c4 into gardener:master Jan 14, 2021
@timebertt timebertt deleted the vendor/kk-cr branch January 14, 2021 15:13
ezeeyahoo pushed a commit to ezeeyahoo/gardener that referenced this pull request Feb 17, 2021
* Vendor k/k@v1.19

* make generate after vendoring k/code-generator

* Adapt gardenlet boostrap and cert rotation code

* Vendor sigs.k8s.io/controller-runtime@v0.7.0

* Remove pkg/client/kubernetes/utils in favor of manager.Options.ClientDisableCacheFor

* Remove extensions/pkg/event in favor of simplified event.GenericEvent struct

* Replace runtime.Object with client.{Object,ObjectList}

* Adapt OperationAnnotationWrapper to client.Object

* Change ClientMap signatures to use contexts

* Adapt controllers to new reconciler.Reconcile interface

Adapt extension controllers to new reconciler.Reconcile interface

* Adapt event log helpers to new event type

* Adapt extensions mappers

* Adapt extensions webhooks to admissionv1

* Adapt seed admission webhooks

* Migrate seed-admission-controller to controller-runtime handlers

* Adapt and streamline signal handling

* Try to fix k-scheduler stuff under third_party

* Fix case sensitivity of gnostic dependency

* Fix bug in controller worker creation

* Address some first comments from rfranzke

* Address some first comments from timuthy

* Migrate shoot extension webhook handler to controller-runtime handler

* Make gardenlet bootstrap code fallback to certificates/v1beta1

* Address some first comments from ialidzhikov

* Pin sigs.k8s.io/structured-merge-diff/v3 back to fork

* Typo in test

* Deduplicate client-go warnings in all components
@gardener-robot gardener-robot added priority/3 Priority (lower number equals higher priority) and removed priority/3 Priority (lower number equals higher priority) labels Mar 8, 2021
krgostev pushed a commit to krgostev/gardener that referenced this pull request Apr 21, 2022
* Vendor k/k@v1.19

* make generate after vendoring k/code-generator

* Adapt gardenlet boostrap and cert rotation code

* Vendor sigs.k8s.io/controller-runtime@v0.7.0

* Remove pkg/client/kubernetes/utils in favor of manager.Options.ClientDisableCacheFor

* Remove extensions/pkg/event in favor of simplified event.GenericEvent struct

* Replace runtime.Object with client.{Object,ObjectList}

* Adapt OperationAnnotationWrapper to client.Object

* Change ClientMap signatures to use contexts

* Adapt controllers to new reconciler.Reconcile interface

Adapt extension controllers to new reconciler.Reconcile interface

* Adapt event log helpers to new event type

* Adapt extensions mappers

* Adapt extensions webhooks to admissionv1

* Adapt seed admission webhooks

* Migrate seed-admission-controller to controller-runtime handlers

* Adapt and streamline signal handling

* Try to fix k-scheduler stuff under third_party

* Fix case sensitivity of gnostic dependency

* Fix bug in controller worker creation

* Address some first comments from rfranzke

* Address some first comments from timuthy

* Migrate shoot extension webhook handler to controller-runtime handler

* Make gardenlet bootstrap code fallback to certificates/v1beta1

* Address some first comments from ialidzhikov

* Pin sigs.k8s.io/structured-merge-diff/v3 back to fork

* Typo in test

* Deduplicate client-go warnings in all components
krgostev pushed a commit to krgostev/gardener that referenced this pull request Jul 5, 2022
* Vendor k/k@v1.19

* make generate after vendoring k/code-generator

* Adapt gardenlet boostrap and cert rotation code

* Vendor sigs.k8s.io/controller-runtime@v0.7.0

* Remove pkg/client/kubernetes/utils in favor of manager.Options.ClientDisableCacheFor

* Remove extensions/pkg/event in favor of simplified event.GenericEvent struct

* Replace runtime.Object with client.{Object,ObjectList}

* Adapt OperationAnnotationWrapper to client.Object

* Change ClientMap signatures to use contexts

* Adapt controllers to new reconciler.Reconcile interface

Adapt extension controllers to new reconciler.Reconcile interface

* Adapt event log helpers to new event type

* Adapt extensions mappers

* Adapt extensions webhooks to admissionv1

* Adapt seed admission webhooks

* Migrate seed-admission-controller to controller-runtime handlers

* Adapt and streamline signal handling

* Try to fix k-scheduler stuff under third_party

* Fix case sensitivity of gnostic dependency

* Fix bug in controller worker creation

* Address some first comments from rfranzke

* Address some first comments from timuthy

* Migrate shoot extension webhook handler to controller-runtime handler

* Make gardenlet bootstrap code fallback to certificates/v1beta1

* Address some first comments from ialidzhikov

* Pin sigs.k8s.io/structured-merge-diff/v3 back to fork

* Typo in test

* Deduplicate client-go warnings in all components
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/open-source Open Source (community, enablement, contributions, conferences, CNCF, etc.) related area/quality Output qualification (tests, checks, scans, automation in general, etc.) related kind/task General task size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants