-
Notifications
You must be signed in to change notification settings - Fork 461
Add Tenant Update Zones api #257
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
Conversation
swagger.yml
Outdated
zones: | ||
type: array | ||
items: | ||
$ref: "#/definitions/zone" |
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.
not a blocker but its a good practice to have an empty line at the end of the file because of file processing https://stackoverflow.com/questions/729692/why-should-text-files-end-with-a-newline
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.
done
Since the Tenant's zones is an array, a PUT operation was done where all zone elements on the Tenant are replaced by the defined ones on the request.
7efeff5
to
7a5feb0
Compare
} | ||
|
||
// set the zones if they are provided | ||
var newZoneArray []operator.Zone |
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.
This implies that if the put operation is missing a zone it would be deleted from the array right? Wouldn't it be better to only update the zones named in the request array?
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, there can be multiple zones with the same name
// set the zones if they are provided | ||
var newZoneArray []operator.Zone | ||
for _, zone := range zonesReq { | ||
zone, err := parseTenantZoneRequest(zone, minInst.Spec.Metadata.Annotations) |
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.
parseTenantZoneRequest
would allow to change fields that shouldn't be changed after zone creation, such as PVC storage Class or size, number of servers, etc, we should be careful not to change them since that could break havok on the tenant, can we just grab the fields that are safe to change? like memory, node selector, affinity and tolerations?
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.
since it is possible to have two zones with the same name, it would be difficult to modify the correct one, also it would make it messy in the sense that we would need to loop to find which to edit.
Since the Tenant's zones is an array, a PUT operation was done where
all zone elements on the Tenant are replaced by the defined ones on the request.
Api to update zones is done on:
PUT
/namespaces/{namespace}/tenants/{tenant}/zones
Payload can be E.g.:
Response will be in the format of
models.Tenant
(the one gotten on/namespaces/{namespace}/tenants/{tenant}
)