add status.labelSelector field to unitedDeployment to support scale sub-resource #1314
Conversation
…ub-resource Signed-off-by: liuzhenwei <dui_zhang@163.com>
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #1314 +/- ##
==========================================
- Coverage 48.59% 48.50% -0.09%
==========================================
Files 151 151
Lines 21042 21210 +168
==========================================
+ Hits 10225 10288 +63
- Misses 9700 9793 +93
- Partials 1117 1129 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 10 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
This is a romantic PR id |
| @@ -231,6 +233,14 @@ func (r *ReconcileUnitedDeployment) Reconcile(_ context.Context, request reconci | |||
| return reconcile.Result{}, err | |||
| } | |||
|
|
|||
| selector, err := util.ValidatedLabelSelectorAsSelector(instance.Spec.Selector) | |||
There was a problem hiding this comment.
just using ValidatedLabelSelectorAsSelector is not enough to identify some label selector errors, you can call LabelSelectorAsSelector and check if there is any error. Similar code is in the cloneset validation validateCloneSetSpec
There was a problem hiding this comment.
Do you mean validate UnitedDeploymentSpec? https://github.com/openkruise/kruise/blob/9e74506ca800877f5bf63723efa33521a3533c46/pkg/webhook/uniteddeployment/validating/uniteddeployment_validation.go#LL52C60-L52C60
There was a problem hiding this comment.
yes, it seems the uniteddeployment webhook already validate the selector, is it necessary to validate again the controller
There was a problem hiding this comment.
yes, it seems the uniteddeployment webhook already validate the selector, is it necessary to validate again the controller
Unnecessary, but we need converts the LabelSelector api type into internalSelector, i think.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: zmberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…ub-resource (openkruise#1314) Signed-off-by: liuzhenwei <dui_zhang@163.com>
…ub-resource (openkruise#1314) Signed-off-by: liuzhenwei <dui_zhang@163.com>
Ⅰ. Describe what this PR does
Add status.labelSelector field to unitedDeployment to support scale sub-resource
Ⅱ. Does this pull request fix one issue?
fixes #1308
Ⅲ. Describe how to verify it
Ⅳ. Special notes for reviews