Deployment scaler to use the scale subresource #51093
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: foxish Assign the PR to them by writing Associated issue: 29698 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
k8s-github-robot
added
size/L
|
/test pull-kubernetes-e2e-kops-aws |
k8s-github-robot
added
release-note
| @@ -61,7 +61,7 @@ func ScalerFor(kind schema.GroupKind, c internalclientset.Interface) (Scaler, er | |||
| case apps.Kind("StatefulSet"): | |||
| return &StatefulSetScaler{c.Apps()}, nil | |||
| case extensions.Kind("Deployment"), apps.Kind("Deployment"): | |||
| return &DeploymentScaler{c.Extensions()}, nil | |||
| return &DeploymentScaler{c.Extensions(), c.Extensions()}, nil | |||
| } | ||
| return nil | ||
| } | ||
|
|
||
| type DeploymentScaler struct { | ||
| c extensionsclient.DeploymentsGetter | ||
| d extensionsclient.ScalesGetter | ||
| } | ||
|
|
||
| // ScaleSimple is responsible for updating a deployment's desired replicas | ||
| // count. It returns the resourceVersion of the deployment if the update is | ||
| // successful. | ||
| func (scaler *DeploymentScaler) ScaleSimple(namespace, name string, preconditions *ScalePrecondition, newSize uint) (string, error) { |
There was a problem hiding this comment.
Do we need to keep both ScaleSimple() and Scale()?
| deployment.Spec.Replicas = int32(newSize) | ||
| updatedDeployment, err := scaler.c.Deployments(namespace).Update(deployment) | ||
| scale.Spec.Replicas = int32(newSize) | ||
| updatedDeployment, err := scaler.d.Scales(namespace).Update("deployment", scale) | ||
| if err != nil { | ||
| if errors.IsConflict(err) { |
There was a problem hiding this comment.
When using scale to update, it should never have conflict.
Is it correct? @caesarxuchao
|
xref: #32523 (comment) |
|
@foxish Do we plan to scale all workload to use scale subresource? |
| @@ -69,6 +69,9 @@ func ObjectReaction(tracker ObjectTracker) ReactionFunc { | |||
| return func(action Action) (bool, runtime.Object, error) { | |||
| ns := action.GetNamespace() | |||
| gvr := action.GetResource() | |||
| if action.GetSubresource() == "scale" { | |||
| gvr = schema.GroupVersionResource{Group: "extensions", Version: "", Resource: "scales"} | |||
There was a problem hiding this comment.
This isn't right. The group cannot be assumed to be extensions. See RCs and stateful sets as examples.
|
This PR hasn't been active in 90 days. Closing this PR. Please reopen if you would like to work towards merging this change, if/when the PR is ready for the next round of review. You can add 'keep-open' label to prevent this from happening again, or add a comment to keep it open another 90 days |
What this PR does / why we need it: This PR sets the deployment scaler to use the scale subresource.
Partially fixes #29698
Special notes for your reviewer:
Release note:
cc @kubernetes/sig-cli-pr-reviews