Allow override of maximum node count per shoot from `CloudProfile` in `NamespacedCloudProfile` #11647

LucaBernstein · 2025-03-12T05:56:20Z

How to categorize this PR?

/area usability robustness
/kind enhancement

What this PR does / why we need it:
With this PR the limits defined in a CloudProfile can be overridden in a NamespacedCloudProfile to make the defined limits more strict.
Shoot owners must ensure that their currently running shoots and their workers deployed comply to the defined limits.

See also previous PR for CloudProfiles: #11279

Which issue(s) this PR fixes:

Special notes for your reviewer:
/cc @timuthy @timebertt

Release note:

`NamespacedCloudProfile.spec.limits.maxNodesTotal` can be used to override the limit of the maximum number of nodes a shoot can have during runtime, as defined in the parent `CloudProfile`. See [the documentation](https://github.com/gardener/gardener/blob/v1.115.0/docs/usage/shoot/shoot_limits.md) for more details.

marc1404

/lgtm

Thanks for the well-structured PR and tests!
I have a few minor suggestions but nothing that would prevent merging your changes :)

pkg/controllermanager/controller/namespacedcloudprofile/reconciler_test.go

pkg/apis/core/validation/cloudprofile.go

plugin/pkg/global/resourcereferencemanager/admission.go

pkg/utils/numbers_test.go

gardener-prow · 2025-03-12T09:49:32Z

LGTM label has been added.

Git tree hash: fb8a930da29c012d8e4e41cf085ff1a53e58709c

timuthy · 2025-03-12T11:45:13Z

/assign

tobschli

Thank you very much :)

docs/usage/shoot/shoot_limits.md

marc1404

/lgtm

Thanks for addressing my feedback :)

gardener-prow · 2025-03-13T09:32:43Z

LGTM label has been added.

Git tree hash: 75fc4058f506396c1f27db3757c1b074c8c2b072

timuthy

Nice! Thank you for the well structured PR, it was a blast to review it 😃

timuthy · 2025-03-14T10:16:40Z

plugin/pkg/global/resourcereferencemanager/admission.go

@@ -585,6 +588,9 @@ func (r *ReferenceManager) Admit(ctx context.Context, a admission.Attributes, _
 								channel <- fmt.Errorf("unable to delete Machine image version '%s/%s' from CloudProfile %q - version is still in use by shoot '%s/%s' by worker %q", worker.Machine.Image.Name, *worker.Machine.Image.Version, cloudProfile.Name, shoot.Namespace, shoot.Name, worker.Name)
 							}
 						}
+						if hasDecreasedLimits {
+							validateShootWorkerLimits(channel, shoot, cloudProfile.Spec.Limits)


Shouldn't this validation be executed whenever the limit was changed? This would cover the case that a CloudProfile with a low value was created, and a then increased value is validated appropriately.

In addition, hasDecreasedLimits is a bit contradicting to the core validation where decreasing directly is forbidden (only through add).

I agree that the variable name is misleading. Would you agree to wasLimitAdded?
The validation only needs to be executed if a limit has been introduced, not if it is increased or removed again.

I would suggest the same renaming also for the NamespacedCloudProfile validation.

PTAL at 5f06be0.

plugin/pkg/namespacedcloudprofile/validator/admission.go

test/integration/controllermanager/namespacedcloudprofile/namespacedcloudprofile_test.go

…Profile controller

Tests are duplicated from CloudProfile validation.

Amendment to previous PR with ResourceReferenceManager check for already existing Shoots not passing the already existing Shoot validation (if they were to be newly created afterwards).

… and updates Do not allow higher limit values than in parent CloudProfile on creation and update.

…lidation logic

marc1404

/lgtm

gardener-prow · 2025-03-18T08:24:15Z

LGTM label has been added.

Git tree hash: a79a509099401bb298bc2e2d8db38f53eb591248

gardener-prow · 2025-03-18T08:24:18Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: marc1404

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:

~~OWNERS~~ [marc1404]

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

gardener-prow bot requested review from timebertt and timuthy March 12, 2025 05:56

LucaBernstein force-pushed the nscpfl-limits branch from f9d15c0 to 4aad202 Compare March 12, 2025 08:18

marc1404 reviewed Mar 12, 2025

View reviewed changes

gardener-prow bot assigned marc1404 Mar 12, 2025

gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Mar 12, 2025

gardener-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 12, 2025

gardener-prow bot requested a review from marc1404 March 12, 2025 10:31

LucaBernstein force-pushed the nscpfl-limits branch from fabe318 to 1842a9e Compare March 12, 2025 11:33

gardener-prow bot assigned timuthy Mar 12, 2025

tobschli reviewed Mar 12, 2025

View reviewed changes

docs/usage/shoot/shoot_limits.md Show resolved Hide resolved

LucaBernstein force-pushed the nscpfl-limits branch from 2dcca28 to 4adbece Compare March 13, 2025 06:31

marc1404 reviewed Mar 13, 2025

View reviewed changes

gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Mar 13, 2025

timuthy reviewed Mar 14, 2025

View reviewed changes

gardener-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 14, 2025

gardener-prow bot requested review from marc1404 and timuthy March 14, 2025 14:31

LucaBernstein force-pushed the nscpfl-limits branch from 5f06be0 to f84cdae Compare March 14, 2025 14:59

gardener-prow bot added cla: no Indicates the PR's author has not signed the cla-assistant.io CLA. and removed cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. cla: no Indicates the PR's author has not signed the cla-assistant.io CLA. labels Mar 14, 2025

gardener-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 17, 2025

LucaBernstein added 14 commits March 17, 2025 12:43

docs: extend shoot docs and add example value comment

d304237

feat: add limits field to NamespacedCloudProfile types

27e5e76

chore: make generate

a405387

feat: implement limits.maxNodesTotal merging logic in NamespacedCloud…

6b027bd

…Profile controller

refactor: extract IsDecreasedMaxTotalNodes func

908edb0

fix: correct order of args

e718bd3

feat: add core validation for limits

b544d7d

Tests are duplicated from CloudProfile validation.

feat: add limits validation for CloudProfile updates

11a3424

Amendment to previous PR with ResourceReferenceManager check for already existing Shoots not passing the already existing Shoot validation (if they were to be newly created afterwards).

feat: add limits validation for NamespacedCloudProfile updates

e47b772

test: add limit tests and refactor tests in general

8c35188

refactor: move MinGreaterThanZero function to utils package

e55e80b

Address review comments

84b739c

feat: implement limits validation for NamespacedCloudProfile creation…

42b938a

… and updates Do not allow higher limit values than in parent CloudProfile on creation and update.

refactor: address review and rename variable for clarity in limits va…

fe6393a

…lidation logic

LucaBernstein force-pushed the nscpfl-limits branch from f84cdae to fe6393a Compare March 17, 2025 11:49

gardener-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2025

gardener-prow bot requested review from marc1404 and timuthy March 17, 2025 11:49

gardener-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 17, 2025

marc1404 approved these changes Mar 18, 2025

View reviewed changes

gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Mar 18, 2025

gardener-prow bot merged commit ec7922b into gardener:master Mar 18, 2025
19 checks passed

LucaBernstein mentioned this pull request Mar 27, 2025

Add ability to increase maxNodesTotal limit in NamespacedCloudProfile #11760

Closed

Kostov6 mentioned this pull request Mar 27, 2025

[Flaky Test] NamespacedCloudProfile flakes in case should not update the NamespacedCloudProfile status with an increased limit #11761

Closed

LucaBernstein mentioned this pull request Apr 2, 2025

Add ability to increase maxNodesTotal limit in NamespacedCloudProfile #11796

Merged

LucaBernstein deleted the nscpfl-limits branch June 23, 2025 12:24

Allow override of maximum node count per shoot from CloudProfile in NamespacedCloudProfile #11647

Allow override of maximum node count per shoot from CloudProfile in NamespacedCloudProfile #11647

Uh oh!

Conversation

LucaBernstein commented Mar 12, 2025 • edited by ialidzhikov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marc1404 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gardener-prow bot commented Mar 12, 2025

Uh oh!

timuthy commented Mar 12, 2025

Uh oh!

tobschli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marc1404 left a comment

Choose a reason for hiding this comment

Uh oh!

gardener-prow bot commented Mar 13, 2025

Uh oh!

timuthy left a comment

Choose a reason for hiding this comment

Uh oh!

timuthy Mar 14, 2025

Choose a reason for hiding this comment

Uh oh!

LucaBernstein Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LucaBernstein Mar 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marc1404 left a comment

Choose a reason for hiding this comment

Uh oh!

gardener-prow bot commented Mar 18, 2025

Uh oh!

gardener-prow bot commented Mar 18, 2025

Uh oh!

Uh oh!

Uh oh!

Allow override of maximum node count per shoot from `CloudProfile` in `NamespacedCloudProfile` #11647

Allow override of maximum node count per shoot from `CloudProfile` in `NamespacedCloudProfile` #11647

LucaBernstein commented Mar 12, 2025 •

edited by ialidzhikov

Loading

LucaBernstein Mar 14, 2025 •

edited

Loading