-
Notifications
You must be signed in to change notification settings - Fork 22
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
KIM Integration - shoot name and networking for basic plans #951
KIM Integration - shoot name and networking for basic plans #951
Conversation
Add one of following labels |
simple logic for failure tolerance, to be validated TODO added failure tolerance set assertion for tolerance networking set asserting Networking
2c11fd8
to
7698d6b
Compare
|
||
var failureTolerance *gardener.HighAvailability | ||
if s.config.ControlPlaneFailureTolerance == string(gardener.FailureToleranceTypeZone) { | ||
failureTolerance = &gardener.HighAvailability{ |
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.
Can we shorten this method and only set "Type" field in the if block?
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.
Which form is easier to read is highly subjective.
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.
In the next commit
} | ||
|
||
return imv1.Networking{ | ||
Pods: DefaultIfParamNotSet(networking.DefaultPodsCIDR, networkingParams.PodsCidr), |
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.
I don't get you question.
|
||
failureType := gardener.FailureToleranceTypeZone | ||
if s.config.ControlPlaneFailureTolerance != string(gardener.FailureToleranceTypeZone) { | ||
failureType = gardener.FailureToleranceTypeNode |
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.
Can we change name to toleranceType?
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.
Pushed.
Description
Changes proposed in this pull request:
Related issue(s)
#905