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

[GEP 31] Introduce API changes for supporting InPlaceUpdate #962

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

acumino
Copy link
Member

@acumino acumino commented Jan 15, 2025

What this PR does / why we need it:
This PR introduces the necessary API changes to enable in-place updates for nodes, as outlined in gardener/gardener#10219.

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

Special notes for your reviewer:

Release note:

NONE

@gardener-robot gardener-robot added kind/api-change API change with impact on API users needs/second-opinion Needs second review by someone else needs/review Needs review size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) labels Jan 15, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 15, 2025
@acumino
Copy link
Member Author

acumino commented Jan 15, 2025

/invite @unmarshall @rishabh-11

@acumino acumino marked this pull request as ready for review January 16, 2025 19:32
@acumino acumino requested a review from a team as a code owner January 16, 2025 19:32
@acumino
Copy link
Member Author

acumino commented Jan 22, 2025

/invite @elankath @aaronfern

@acumino acumino force-pushed the inplace-updates/api branch from 69a6063 to 7fd974d Compare January 31, 2025 11:42
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 31, 2025
@acumino acumino requested a review from unmarshall February 6, 2025 11:20
acumino and others added 4 commits February 6, 2025 19:11
Co-authored-by: Shafeeque E S <[email protected]>
Co-authored-by: Ashish Ranjan Yadav  <[email protected]>
Co-authored-by: Shafeeque E S <[email protected]>
Co-authored-by: Ashish Ranjan Yadav  <[email protected]>
Co-authored-by: Shafeeque E S <[email protected]>
Co-authored-by: Ashish Ranjan Yadav  <[email protected]>
Co-authored-by: Shafeeque E S <[email protected]>
Co-authored-by: Ashish Ranjan Yadav  <[email protected]>
@acumino acumino force-pushed the inplace-updates/api branch from 7fd974d to 03e077f Compare February 6, 2025 13:41
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 6, 2025
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 12, 2025
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 12, 2025
Co-Authored-By: Shafeeque E S <[email protected]>
Co-Authored-By: Ashish Ranjan Yadav <[email protected]>
@acumino acumino force-pushed the inplace-updates/api branch from 6520185 to 63ca186 Compare February 12, 2025 04:32
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 12, 2025
if spec.Strategy.Type == machine.InPlaceUpdateMachineDeploymentStrategyType {
if spec.Strategy.InPlaceUpdate == nil {
allErrs = append(allErrs, field.Required(fldPath.Child("strategy.inPlaceUpdate"), "InPlaceUpdate parameter cannot be nil for in-place update strategy"))
} else {
Copy link
Contributor

@aaronfern aaronfern Feb 12, 2025

Choose a reason for hiding this comment

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

Do we not want some validation for spec.Strategy.InPlaceUpdate.OrchestrationType? From what I understood it should only take values Auto or Manual

Comment on lines +23 to +44
const (
// GroupName is the group name use in this package
GroupName = "machine.sapcloud.io"

// AnnotationKeyMachineUpdateFailedReason is the annotation key for the machine indicating the reason for the machine update failure.
AnnotationKeyMachineUpdateFailedReason = "node.machine.sapcloud.io/update-failed-reason"

// LabelKeyMachineCandidateForUpdate is the label key for the machine indicating the machine will undergo an update.
LabelKeyMachineCandidateForUpdate = "node.machine.sapcloud.io/candidate-for-update"
// LabelKeyMachineSelectedForUpdate is the label key for the machine indicating the machine is selected for update.
LabelKeyMachineSelectedForUpdate = "node.machine.sapcloud.io/selected-for-update"
// LabelKeyMachineReadyForUpdate is the label key for the machine indicating the machine is ready for update after the node drain.
LabelKeyMachineReadyForUpdate = "node.machine.sapcloud.io/ready-for-update"
// LabelKeyMachineDrainSuccessful is the label key for the machine indicating the node drain was successful.
LabelKeyMachineDrainSuccessful = "node.machine.sapcloud.io/drain-successful"
// LabelKeyMachineUpdateSuccessful is the label key for the machine indicating the machine update was successful.
LabelKeyMachineUpdateSuccessful = "node.machine.sapcloud.io/update-successful"
// LabelKeyMachineUpdateFailed is the label key for the machine indicating the machine update failed.
LabelKeyMachineUpdateFailed = "node.machine.sapcloud.io/update-failed"
// LabelKeyMachineSetSkipUpdate is the label key for the machine indicating the machine set update should be skipped.
LabelKeyMachineSetSkipUpdate = "node.machine.sapcloud.io/machine-set-skip-update"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure this is the right file for these constants?
To me machine_types.go seems more apt, or even a new file constants.go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/api-change API change with impact on API users needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/review Needs review needs/second-opinion Needs second review by someone else size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants