Skip to content

[GEP-31] Introduce separate GNA Key and Worker Pool Hash calculation for in-Place Update Strategy #12178

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
Merged
merged 6 commits into from
May 28, 2025
Merged

[GEP-31] Introduce separate GNA Key and Worker Pool Hash calculation for in-Place Update Strategy #12178

merged 6 commits into from
May 28, 2025

Conversation

acumino
Copy link
Member

@acumino acumino commented May 23, 2025 •
edited
Loading

How to categorize this PR?

/area delivery open-source scalability
/kind enhancement

What this PR does / why we need it:
To avoid the complexity of worker pool hash migration during in-place updates, this PR introduces separate functions for calculating the GNA key and the worker pool hash specifically for worker pools using the in-place update strategy.

  • The GNA secret name, calculated for a worker pool using the in-place update strategy, must remain constant throughout the lifecycle of the worker pool. Changing the secret name would break the Gardener Node Agent, as it would no longer be able to access the required secret.

  • Extensions should avoid including any fields in the additional data that are currently not allowed to change without triggering a node rollout but may become safe to change in the future. Instead, the extension should enforce validation to prevent modification of such fields.

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

Special notes for your reviewer:
/cc @rfranzke @shafeeqes @ary1992
Successor of #12118

Release note:

`WorkerPoolHash` now includes a new parameter `additionalDataInPlace` to support hash calculation for worker pools using the `InPlace` update strategy.

@gardener-prow gardener-prow bot requested review from ary1992, rfranzke and shafeeqes May 23, 2025 11:27
@gardener-prow gardener-prow bot added area/delivery Delivery related area/open-source Open Source (community, enablement, contributions, conferences, CNCF, etc.) related area/scalability Scalability related kind/enhancement Enhancement, improvement, extension cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 23, 2025
@acumino acumino mentioned this pull request May 23, 2025
Closed
@acumino acumino changed the title Introduce separate GNA Key and Worker Pool Hash calculation for in-Place Update Strategy [GEP-31] Introduce separate GNA Key and Worker Pool Hash calculation for in-Place Update Strategy May 23, 2025
@acumino acumino mentioned this pull request May 23, 2025
Closed
40 tasks
shafeeqes
shafeeqes reviewed May 26, 2025
View reviewed changes
@acumino acumino force-pushed the sep-hash-inplace branch from 8a330dc to fecd71f Compare May 26, 2025 04:53
@acumino
Copy link
Member Author

acumino commented May 26, 2025

/retest-required

1 similar comment
@acumino
Copy link
Member Author

acumino commented May 26, 2025

/retest-required

@acumino acumino force-pushed the sep-hash-inplace branch from 98b41ec to fceffd0 Compare May 26, 2025 08:44
@acumino
Copy link
Member Author

acumino commented May 26, 2025 •
edited
Loading

Fixed failing operator-seed test in #12183
/hold

@acumino acumino added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 26, 2025
@acumino acumino force-pushed the sep-hash-inplace branch from fceffd0 to c9e1058 Compare May 26, 2025 12:50
@acumino
Copy link
Member Author

acumino commented May 26, 2025

/unhold

@gardener-prow gardener-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 26, 2025
shafeeqes
shafeeqes reviewed May 27, 2025
View reviewed changes
Copy link
Contributor

@shafeeqes shafeeqes 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 27, 2025
@gardener-prow Gardener Prow
Copy link
Contributor

gardener-prow bot commented May 27, 2025

LGTM label has been added.

Git tree hash: 8af85db02dfb27c308ce4306b471ab595ce3822c

rfranzke
rfranzke reviewed May 27, 2025
View reviewed changes
@acumino acumino force-pushed the sep-hash-inplace branch from c9e1058 to ef7ba5a Compare May 27, 2025 11:14
@gardener-prow gardener-prow bot removed the lgtm Indicates that a PR is ready to be merged. label May 27, 2025
@gardener-prow gardener-prow bot requested a review from shafeeqes May 27, 2025 11:14
@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label May 27, 2025
@gardener-prow Gardener Prow
Copy link
Contributor

gardener-prow bot commented May 27, 2025

LGTM label has been added.

Git tree hash: dfa46391ad505bd4b59a88e7ba4297a54faec3e8

@gardener-prow Gardener Prow
Copy link
Contributor

gardener-prow bot commented May 27, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rfranzke

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 27, 2025
@acumino acumino added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels May 27, 2025
@timuthy
Copy link
Member

timuthy commented May 27, 2025

/test pull-gardener-e2e-kind-operator-seed

@gardener-prow gardener-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 27, 2025
acumino and others added 6 commits May 28, 2025 07:00
@acumino @shafeeqes @ary1992
Use separate function for calculating WorkerPoolHash for InPlace work…
a847560 
…er pool

Co-authored-by: Shafeeque E S <shafeeque.e.s@sap.com>
Co-authored-by: Ashish Ranjan Yadav  <ashish.ranjan.yadav@sap.com>
@acumino @shafeeqes @ary1992
Use static GNA secret name for in-place worker pools
ecad0dd 
Co-authored-by: Shafeeque E S <shafeeque.e.s@sap.com>
Co-authored-by: Ashish Ranjan Yadav  <ashish.ranjan.yadav@sap.com>
@acumino
Address Review
3d40634
@acumino
Address Review
79aacd1
@acumino
Address Review
c27084b
@acumino
Rebase
41f1442
@acumino acumino force-pushed the sep-hash-inplace branch from 6cffd79 to 41f1442 Compare May 28, 2025 01:32
@gardener-prow gardener-prow bot removed the lgtm Indicates that a PR is ready to be merged. label May 28, 2025
@gardener-prow gardener-prow bot requested a review from rfranzke May 28, 2025 01:32
@gardener-prow gardener-prow bot added cla: no Indicates the PR's author has not signed the cla-assistant.io CLA. cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. 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 May 28, 2025
@shafeeqes
Copy link
Contributor

/lgtm

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

gardener-prow bot commented May 28, 2025

LGTM label has been added.

Git tree hash: 7ce772a0860617bd29fbd1caaee346630a955d59

@shafeeqes
Copy link
Contributor

/retest

@gardener-prow Gardener Prow
Copy link
Contributor

gardener-prow bot commented May 28, 2025 •
edited
Loading

@acumino: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-gardener-apidiff 41f1442 link false /test pull-gardener-apidiff

Full PR test history. Your PR dashboard. Command help for this repository.
Please help us cut down on flakes by linking this test failure to an open flake report or filing a new flake report if you can't find an existing one. Also see our testing guideline for how to avoid and hunt flakes.

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. I understand the commands that are listed here.

@gardener-prow gardener-prow bot merged commit b517e10 into gardener:master May 28, 2025
18 of 19 checks passed
@acumino acumino deleted the sep-hash-inplace branch May 28, 2025 04:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ary1992 ary1992 Awaiting requested review from ary1992

@shafeeqes shafeeqes Awaiting requested review from shafeeqes

@rfranzke rfranzke Awaiting requested review from rfranzke

Assignees

@rfranzke rfranzke

@shafeeqes shafeeqes

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/delivery Delivery related area/open-source Open Source (community, enablement, contributions, conferences, CNCF, etc.) related 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/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@acumino @timuthy @shafeeqes @rfranzke