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

Use protobuf in gardener components #3467

Merged
merged 6 commits into from Feb 2, 2021
Merged

Use protobuf in gardener components #3467

merged 6 commits into from Feb 2, 2021

Conversation

timebertt
Copy link
Member

@timebertt timebertt commented Jan 28, 2021 •
edited

How to categorize this PR?

/area performance cost networking
/kind enhancement
/priority normal

What this PR does / why we need it:

This PR configures clients in all gardener components to use the protobuf content type wherever possible.
Earlier we used application/json everywhere (set in gardener helm charts, defaulted in component configs).

Now, the content type fields are optional in the charts as well as the component configs and are not defaulted anymore.
If left empty, gardener will configure all clients that talk to k8s / gardener APIs using protobuf, for CRDs we still use application/json as they don't support protobuf.

Which issue(s) this PR fixes:
Fixes #2836
Part of #3109

Special notes for your reviewer:
I think, I cut down json requests to the minimum amount that is possible ATM. There are still json requests in the following cases:

  • g-API posts tokenreviews/subjectaccessreviews to k-api via json as part of the delegating authn/authz mechanisms (not configurable in the API server library, IISIC)
  • some kcm controllers (maybe gc controller?) seem to talk to g-api via json sometimes, also not configurable but not to frequent
  • g-api and kcm watch the kube-system/extension-apiserver-authentication cm using json, also not configurable
  • unstructured requests to c-r clients still use json (-> therefore also the chart applier) -> usage of these requests will decrease significantly with the ongoing refactorings

/squash

Release note:

Gardener components now use the protobuf content type wherever possible when talking to the Gardener or Kubernetes APIs, if the content type fields are left empty in the respective component configs.
Operators can override this behavior by explicitly specifying `application/json` as the content type in the respective component configs.

@timebertt timebertt requested a review from a team as a code owner January 28, 2021 08:23
@gardener-robot gardener-robot added area/cost Cost related area/networking Networking related area/performance Performance (across all domains, such as control plane, networking, storage, etc.) related kind/enhancement Enhancement, improvement, extension merge/squash size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 28, 2021
@rfranzke
Copy link
Member

/assign

@timebertt timebertt mentioned this pull request Jan 28, 2021
Closed
@ialidzhikov
Copy link
Member

/assign @ialidzhikov

rfranzke
rfranzke requested changes Jan 29, 2021
View reviewed changes
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 done, only one comment

pkg/client/kubernetes/client.go Show resolved Hide resolved
timuthy
timuthy previously approved these changes Jan 29, 2021
View reviewed changes
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

ialidzhikov
ialidzhikov previously approved these changes Feb 1, 2021
View reviewed changes
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

stoyanr
stoyanr previously approved these changes Feb 1, 2021
View reviewed changes
pkg/gardenlet/apis/config/v1alpha1/defaults_test.go Outdated Show resolved Hide resolved
@stoyanr
Copy link
Contributor

stoyanr commented Feb 1, 2021

/lgtm

@timebertt timebertt dismissed stale reviews from stoyanr and ialidzhikov via 8916532 February 1, 2021 14:32
@rfranzke rfranzke merged commit 8bbd643 into gardener:master Feb 2, 2021
@timebertt timebertt deleted the enh/enable-protobuf branch February 2, 2021 06:54
@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
@timebertt
Use protobuf in gardener components (gardener#3467)
6710448 
* Don't default content type in config APIs

* Make content type optional in charts

* Configure clients to use protobuf wherever possible

* Enable protobuf also for seedmanagement

* Correct typos

* Improve gardenlet config defaults tests
krgostev pushed a commit to krgostev/gardener that referenced this pull request Jul 5, 2022
@timebertt
Use protobuf in gardener components (gardener#3467)
3890bcc 
* Don't default content type in config APIs

* Make content type optional in charts

* Configure clients to use protobuf wherever possible

* Enable protobuf also for seedmanagement

* Correct typos

* Improve gardenlet config defaults tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rfranzke rfranzke rfranzke approved these changes

@timuthy timuthy timuthy left review comments

@stoyanr stoyanr Awaiting requested review from stoyanr

@ialidzhikov ialidzhikov Awaiting requested review from ialidzhikov

Assignees

@ialidzhikov ialidzhikov

@rfranzke rfranzke

Labels
area/cost Cost related area/networking Networking related area/performance Performance (across all domains, such as control plane, networking, storage, etc.) related kind/enhancement Enhancement, improvement, extension size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use protobuf in gardener components
9 participants
@timebertt @rfranzke @ialidzhikov @stoyanr @timuthy @gardener-robot-ci-1 @gardener-robot-ci-2 @gardener-robot-ci-3 @gardener-robot