-
Notifications
You must be signed in to change notification settings - Fork 122
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
base: master
Are you sure you want to change the base?
Conversation
/invite @unmarshall @rishabh-11 |
/invite @elankath @aaronfern |
69a6063
to
7fd974d
Compare
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]>
7fd974d
to
03e077f
Compare
Co-Authored-By: Shafeeque E S <[email protected]> Co-Authored-By: Ashish Ranjan Yadav <[email protected]>
6520185
to
63ca186
Compare
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 { |
There was a problem hiding this comment.
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
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" | ||
) |
There was a problem hiding this comment.
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
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: