-
Notifications
You must be signed in to change notification settings - Fork 264
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
Allow tolerationSeconds to be empty on Zone tolerations Requests #238
Conversation
Since toleration seconds can be empty, we were forcing it to be an integer defaulting to 0 which was creating a toleration with value 0 when value should have been nil.
be treated as 0 (evict immediately) by the system. | ||
type: object | ||
required: | ||
- seconds |
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 thought this was allowed to be nil :S
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.
it is, that's the main purpose of this PR, zoneTolerationSeconds is not required, but if zoneTolerationSeconds is defined, then seconds is required.
@@ -1233,12 +1226,18 @@ func parseTenantZoneRequest(zoneParams *models.Zone, annotations map[string]stri | |||
// parse tolerations | |||
tolerations := []corev1.Toleration{} | |||
for _, elem := range zoneParams.Tolerations { | |||
var tolerationSeconds *int64 | |||
if elem.TolerationSeconds != nil { |
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.
won't the swagger API reject this?
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.
no because TolerationSeconds it is not required, the required one is seconds
on TolerationSeconds, kind of weird because both are pointers but swagger doesn't validate this.
Since toleration seconds can be empty, we were forcing it to be an integer defaulting to 0 which
was creating a toleration with value 0 when value should have been nil.
Request now changes to:
If tolerationSeconds exists it should be assigned to the Tenant instance with the desired value
If tolerationSeconds is not defined it should not assign any value on the Tenant instance and leave it empty.