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

Several improvements for the resources.gardener.cloud/preserve-{replicas,resources} option for managed resources #5131

Merged
merged 5 commits into from
Dec 14, 2021

Conversation

rfranzke
Copy link
Member

@rfranzke rfranzke commented Dec 7, 2021

How to categorize this PR?

/area scalability
/kind enhancement bug
/merge squash

What this PR does / why we need it:
This PR

  • adds missing documentation for the resources.gardener.cloud/preserve-{replicas,resources} options for managed resources.
  • adds missing unit tests for these options.
  • fixes a bug which prevented the resources.gardener.cloud/preserve-resources option from working properly for StatefulSets.
  • adds support for the resources.gardener.cloud/preserve-resources option for Jobs, CronJobs, and DaemonSets.

Special notes for your reviewer:
/cc @ScheererJ @DockToFuture

Release note:

The `resources.gardener.cloud/preserve-resources` annotation does now work properly for `StatefulSet`s.
Support for the `resources.gardener.cloud/preserve-resources` annotation was added for `Job`s, `CronJob`s, and `DaemonSet`s.

@rfranzke rfranzke requested a review from a team as a code owner December 7, 2021 17:02
@gardener-robot gardener-robot added kind/api-change API change with impact on API users needs/second-opinion area/scalability Scalability related kind/bug Bug kind/enhancement Enhancement, improvement, extension size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 7, 2021
ScheererJ
ScheererJ previously approved these changes Dec 8, 2021
Copy link
Contributor

@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.

/lgtm

@gardener-robot
Copy link

@ScheererJ Command /lgtm is not available to you but only to a Maintainer, Member.

acumino
acumino previously approved these changes Dec 8, 2021
ScheererJ
ScheererJ previously approved these changes Dec 9, 2021
Copy link
Contributor

@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.

/lgtm

@gardener-robot
Copy link

@ScheererJ Command /lgtm is not available to you but only to a Maintainer, Member.

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.

Only one uncertainty from my side. In addition, I noticed that spec.selector is immutable for Job objects (ref). Does it make sense to fix it in this PR as well?

pkg/resourcemanager/controller/managedresource/merger.go Outdated Show resolved Hide resolved
@rfranzke rfranzke dismissed stale reviews from ScheererJ and acumino via 9c5b444 December 10, 2021 12:29
@gardener-robot gardener-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 10, 2021
@gardener-robot gardener-robot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 10, 2021
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

@timuthy
Copy link
Contributor

timuthy commented Dec 14, 2021

@rfranzke feel free to merge since you are the release responsible but a milestone has not been created yet (just to be sure).

@rfranzke rfranzke merged commit 4b77dc8 into gardener:master Dec 14, 2021
@rfranzke rfranzke deleted the enh/grm-preserve-resources branch December 14, 2021 15:12
rfranzke added a commit that referenced this pull request Jan 13, 2022
…for the `resources.gardener.cloud/preserve-{replicas,resources}` option for managed resources (#5264)

* Add missing documentation for `preserve-{resources,replicas}` options

* Fix `mergeStatefulSet`, add unit tests for `preserveResources` option

* Implement `preserve-{replicas,resources}` for missing workload APIs

* Address PR review feedback

* Address PR review feedback
krgostev pushed a commit to krgostev/gardener that referenced this pull request Apr 21, 2022
…icas,resources}` option for managed resources (gardener#5131)

* Add missing documentation for `preserve-{resources,replicas}` options

* Fix `mergeStatefulSet`, add unit tests for `preserveResources` option

* Implement `preserve-{replicas,resources}` for missing workload APIs

* Address PR review feedback

* Address PR review feedback
krgostev pushed a commit to krgostev/gardener that referenced this pull request Jul 5, 2022
…icas,resources}` option for managed resources (gardener#5131)

* Add missing documentation for `preserve-{resources,replicas}` options

* Fix `mergeStatefulSet`, add unit tests for `preserveResources` option

* Implement `preserve-{replicas,resources}` for missing workload APIs

* Address PR review feedback

* Address PR review feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/scalability Scalability related kind/api-change API change with impact on API users kind/bug Bug kind/enhancement Enhancement, improvement, extension size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants