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

Introduce .spec.pools[].userDataSecretRef to prevent inlining the entire user data #9722

Merged
merged 5 commits into from
May 10, 2024
Merged

Introduce .spec.pools[].userDataSecretRef to prevent inlining the entire user data #9722

merged 5 commits into from
May 10, 2024

Conversation

rfranzke
Copy link
Member

@rfranzke rfranzke commented May 8, 2024 •
edited
Loading

How to categorize this PR?

/area scalability
/kind enhancement

What this PR does / why we need it:
Creating a Shoot with > 80 worker pools leads to exceeds the etcd's max-request-bytes limit of 1.5MiB due to the Worker resource. The culprit here is the .spec.pools[].userData field which inlines the user data for all pools. This can reduce the size by ~85% - 90%.

With this PR, the extensions.gardener.cloud/v1alpha1.Worker resource now has a new .spec.pools[].userDataSecretRef field which references a Secret containing the actual user data. the .spec.pools[].userData field is deprecated and will be removed in a future version. Worker extensions should fetch the user data from the secret and can use the extensions/pkg/controller/worker.FetchUserData helper function for it.

Which issue(s) this PR fixes:
Part of #9545

Special notes for your reviewer:
/cc @ScheererJ @rickardsjp @hown3d

Release note:

The `extensions.gardener.cloud/v1alpha1.Worker` resource now has a new `.spec.pools[].userDataSecretRef` field which references a `Secret` containing the actual user data. the `.spec.pools[].userData` field is deprecated and will be removed in a future version. `Worker` extensions should fetch the user data from the secret and can use the `extensions/pkg/controller/worker.FetchUserData` helper function for it.

@gardener-prow gardener-prow bot requested a review from ScheererJ May 8, 2024 09:45
@gardener-prow gardener-prow bot added the area/scalability Scalability related label May 8, 2024
@gardener-prow gardener-prow bot requested a review from rickardsjp May 8, 2024 09:45
@gardener-prow gardener-prow bot added the kind/enhancement Enhancement, improvement, extension label May 8, 2024
@gardener-prow Gardener Prow
Copy link
Contributor

gardener-prow bot commented May 8, 2024

@rfranzke: GitHub didn't allow me to request PR reviews from the following users: hown3d.

Note that only gardener members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

How to categorize this PR?

/area scalability
/kind enhancement

What this PR does / why we need it:
Creating a Shoot with > 80 worker pools leads to exceeds the etcd's max-request-bytes limit of 1.5MiB due to the Worker resource. The culprit here is the .spec.pools[].userData field which inlines the user data for all pools.

With this PR, the extensions.gardener.cloud/v1alpha1.Worker resource now has a new .spec.pools[].userDataSecretRef field which references a Secret containing the actual user data. the .spec.pools[].userData field is deprecated and will be removed in a future version. Worker extensions should fetch the user data from the secret and can use the extensions/pkg/controller/worker.FetchUserData helper function for it.

Which issue(s) this PR fixes:
Part of #9545

Special notes for your reviewer:
/cc @ScheererJ @rickardsjp @hown3d

Release note:

The `extensions.gardener.cloud/v1alpha1.Worker` resource now has a new `.spec.pools[].userDataSecretRef` field which references a `Secret` containing the actual user data. the `.spec.pools[].userData` field is deprecated and will be removed in a future version. `Worker` extensions should fetch the user data from the secret and can use the `extensions/pkg/controller/worker.FetchUserData` helper function for it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@gardener-prow gardener-prow bot added the cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. label May 8, 2024
@rfranzke rfranzke mentioned this pull request May 8, 2024
Closed
8 tasks
@gardener-prow gardener-prow bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 8, 2024
kon-angelo
kon-angelo approved these changes May 8, 2024
View reviewed changes
Copy link
Contributor

@kon-angelo kon-angelo left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label May 8, 2024
@gardener-prow Gardener Prow
Copy link
Contributor

gardener-prow bot commented May 8, 2024

LGTM label has been added.

Git tree hash: dfbb3ecd77a3952e0a899f0f774f86a648892ad7

@hown3d
Copy link

hown3d commented May 8, 2024

Looks good, thanks for the implementation!

@rfranzke
Copy link
Member Author

/approve

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 10, 2024
ScheererJ
ScheererJ approved these changes May 10, 2024
View reviewed changes
Copy link
Member

@ScheererJ ScheererJ left a comment

Choose a reason for hiding this comment

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

Thanks for the great scalability improvement allowing hundreds of worker pools :-) .

/lgtm
/approve

@gardener-prow Gardener Prow
Copy link
Contributor

gardener-prow bot commented May 10, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kon-angelo, rfranzke, ScheererJ

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gardener-prow gardener-prow bot merged commit e13c99a into gardener:master May 10, 2024
18 checks passed
@rfranzke rfranzke deleted the user-data-ref branch May 10, 2024 09:19
This was referenced May 22, 2024
Merged
Merged
Merged
Merged
Merged
@rfranzke rfranzke mentioned this pull request May 22, 2024
Merged
@vlerenc vlerenc mentioned this pull request Jun 10, 2024
Open
rfranzke added a commit to rfranzke/gardener that referenced this pull request Jul 29, 2024
@rfranzke
Drop deprecated .spec.pools[].userData from `extensions.gardener.cl…
425ddab 
…oud/v1alpha1.Worker` API

(part of gardener#9722, released with `v1.95.0`)
@rfranzke rfranzke mentioned this pull request Jul 29, 2024
Merged
rfranzke added a commit to rfranzke/gardener that referenced this pull request Jul 31, 2024
@rfranzke
Drop deprecated .spec.pools[].userData from `extensions.gardener.cl…
33c2143 
…oud/v1alpha1.Worker` API

(part of gardener#9722, released with `v1.95.0`)
gardener-prow bot pushed a commit that referenced this pull request Jul 31, 2024
@rfranzke
🧹 Cleanup some TODOs (#10220)
db06a3a 
* Drop deletion of deprecated `allow-to-shoot-networks` `NetworkPolicy`

(part of #9752, released with `v1.96.0`)

* Drop fetching extension observability configs with deprecated/legacy method

(part of #9695, released with `v1.95.0`)

* Drop Prometheus/Alertmanager migration coding

(part of #9695, released with `v1.95.0`)

* Drop deprecated `.spec.pools[].userData` from `extensions.gardener.cloud/v1alpha1.Worker` API

(part of #9722, released with `v1.95.0`)

* Drop OSC hash migration `Secret` creation

(part of #9846, released with `v1.97.0`)

* Drop OSC hash assertion from upgrade tests

(part of #9865, released with `v1.98.0`)

* Drop removal code of `HVPA` resources

(part of #9698, released with `v1.95.0`)

* Address PR review feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ScheererJ ScheererJ ScheererJ approved these changes

@kon-angelo kon-angelo kon-angelo approved these changes

@rickardsjp rickardsjp Awaiting requested review from rickardsjp

Assignees

@ScheererJ ScheererJ

@kon-angelo kon-angelo

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/scalability Scalability related cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. 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.
4 participants
@rfranzke @hown3d @ScheererJ @kon-angelo