feat(validation): add webhook validations for CVC replica scale #1621
Conversation
Signed-off-by: mittachaitu <sai.chaithanya@mayadata.io>
mittachaitu
requested review from
prateekpandey14,
sonasingh46 and
vishnuitta
Signed-off-by: mittachaitu <sai.chaithanya@mayadata.io>
Signed-off-by: mittachaitu <sai.chaithanya@mayadata.io>
mittachaitu
added this to Pre-commits and Designs - Due: Feb 29th 2020
in 1.8 Release Tracker - Due Mar 15th.
Signed-off-by: mittachaitu <sai.chaithanya@mayadata.io>
prateekpandey14
changed the title
mittachaitu
moved this from Pre-commits and Designs - Due: Feb 29th 2020
to RC1 - Due: Mar 5 2020
in 1.8 Release Tracker - Due Mar 15th.
pkg/webhook/cvc.go
Outdated
| @@ -0,0 +1,199 @@ | |||
| /* | |||
| Copyright 2019 The OpenEBS Authors. | |||
pkg/webhook/webhook.go
Outdated
| if req.Operation == v1beta1.Update { | ||
| return wh.validateCVCUpdateRequest(req, getCVCObject) | ||
| } | ||
| klog.V(2).Info("Admission wehbook for CVC module not " + |
There was a problem hiding this comment.
change verbose level 4
Signed-off-by: mittachaitu <sai.chaithanya@mayadata.io>
| SetAllowed(). | ||
| WithResultAsSuccess(http.StatusAccepted).AR | ||
| var cvcNewObj apis.CStorVolumeClaim | ||
| err := json.Unmarshal(req.Object.Raw, &cvcNewObj) |
There was a problem hiding this comment.
for this to work, upgrade of CVC should have been done?
There was a problem hiding this comment.
Yes!! The user/Controller made an update call on CVC.
pkg/webhook/cvc.go
Outdated
| // on CVC status. | ||
| func validateStatusPoolList(cvcOldObj, cvcNewObj *apis.CStorVolumeClaim) error { | ||
| replicaPoolNames := []string{} | ||
| if len(cvcOldObj.Spec.Policy.ReplicaPoolInfo) == 0 && |
There was a problem hiding this comment.
can we add validation that status should be Bound to allow any scaling operation to avoid this complex logic?
There was a problem hiding this comment.
No this is particularly to verify the content of status with the spec (#1613 (comment) to incorporate this kind of cases). IMO we shouldn't validate the status(because the status will be updated only by the controllers).
This is out of scope in this PR. It will be incorporated into newer API changes of CVC. |
Signed-off-by: mittachaitu <sai.chaithanya@mayadata.io>
pkg/webhook/cvc.go
Outdated
| // If CVC Status is not bound then reject | ||
| if cvcOldObj.Status.Phase != apis.CStorVolumeClaimPhaseBound { | ||
| // If controller is updating pool info then new CVC will be in bound state | ||
| if cvcNewObj.Status.Phase != apis.CStorVolumeClaimPhaseBound && |
There was a problem hiding this comment.
you can remove this check
There was a problem hiding this comment.
We can't remove this check if we remove then update from CVC controller filling [pool info]
(https://github.com/openebs/maya/blob/3c21a6b2d0779c80fc43767cdc3dd15cd7ae07e2/cmd/cvc-operator/controller/cvc_controller.go#L322) will fail.
pkg/webhook/cvc.go
Outdated
| @@ -130,6 +131,23 @@ func validatePoolListChanges(cvcOldObj, cvcNewObj *apis.CStorVolumeClaim) error | |||
| ) | |||
| } | |||
| } | |||
| // Reject the request if someone perform scaling when CVC is not in Bound | |||
There was a problem hiding this comment.
move this new changes to top of function
Signed-off-by: mittachaitu <sai.chaithanya@mayadata.io>
Signed-off-by: mittachaitu sai.chaithanya@mayadata.io
What this PR does / why we need it:
This PR adds validations for CVC update requests and also handles upgrade changes for the admission server controller(CVC scaling feature PR #1613).
This PR Covers Below Validations and Test Cases For CVC Update:
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
Checklist:
documentationtagbreaking-changestagrequires-upgradetag