kubectl scale implementation for core workload controllers

[foxish]

This issue covers the proposal for the implementation of kubectl scale for the core controllers.
This is relevant to StatefulSet, ReplicaSet and Deployment.

Currently, kubectl uses GET/PUT on the resource to update scale, and should instead use the scale endpoint.
The scale endpoint exists in RS and Deployment, and will be added to SS as well shortly.

In order to keep compatibility with older server versions, the proposed change to the kubectl scale implementation is as follows:

1. Use the discovery API to check for the presence of a scale sub-resource
2. If scale sub-resource is present, use it for updating the number of replicas
3. If it is absent, fall back to the existing method of GET/PUT on the resource

cc @liggitt @kubernetes/sig-cli-misc @kubernetes/sig-apps-misc

[liggitt]

1. Use the discovery API to check for the presence of a scale sub-resource
2. If scale sub-resource is present, use it for updating the number of replicas

This should allow kubectl scale to work against arbitrary resources if they include a /scale subresource, right?

[foxish]

Currently, we have ScaleInterface being implemented by each group's client. So, extending it to arbitrary types would need a new generic ScaleInterface.
From what I understand, we'd need such a Scale type and interface to exist in metav1 if we want to implement a generic interface of this nature.

[crimsonfaith91]

+1 for generic Scale interface.

[smarterclayton]

There have been longer discussions of this in the past, but there are two factors at play: * no matter what, the schema version of Scale as exposed by the sub resource needs to be locked in time and follow the versioning rules of its group (no breaking changes) * we believe that the content negotiation mechanism that was part of 1.7 is the appropriate way to allow generic clients to request a known Scale type ( Proposal: Alternate API representations for resource <kubernetes/community#123>) What we haven't done is settle on where generic types like Scale should go - a "server" group (is there only one Scale interface in all of Kube?), or a net new group per use case. The former is easier, but could lead to bloat in the core Kube API and suffers from the rate limit imposed on the fundamental API. The latter is more complicated.

…

On Mon, Jul 24, 2017 at 8:15 PM, Jun Xiang Tee ***@***.***> wrote: +1 for generic Scale interface. — You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub <#49504 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p5yEDYiDxX8Hkgvnaxugx9eB7hGrks5sRTOZgaJpZM4Ohaz4> .

[smarterclayton]

I lean slightly towards adding "Scale" to another group (like "autoscaling" or a net new empty group) On Tue, Jul 25, 2017 at 1:00 AM, Clayton Coleman <[email protected]> wrote:

…

There have been longer discussions of this in the past, but there are two factors at play: * no matter what, the schema version of Scale as exposed by the sub resource needs to be locked in time and follow the versioning rules of its group (no breaking changes) * we believe that the content negotiation mechanism that was part of 1.7 is the appropriate way to allow generic clients to request a known Scale type ( Proposal: Alternate API representations for resource <kubernetes/community#123>) What we haven't done is settle on where generic types like Scale should go - a "server" group (is there only one Scale interface in all of Kube?), or a net new group per use case. The former is easier, but could lead to bloat in the core Kube API and suffers from the rate limit imposed on the fundamental API. The latter is more complicated. On Mon, Jul 24, 2017 at 8:15 PM, Jun Xiang Tee ***@***.***> wrote: > +1 for generic Scale interface. > > — > You are receiving this because you are on a team that was mentioned. > Reply to this email directly, view it on GitHub > <#49504 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABG_p5yEDYiDxX8Hkgvnaxugx9eB7hGrks5sRTOZgaJpZM4Ohaz4> > . >

[mwielgus]

cc: @DirectXMan12

[DirectXMan12]

I think scale should live in autoscaling. I look at Scale like an interface, and groups like packages in this case -- it makes sense to me for the "interfaces" required for autoscaling to live in the autoscaling "package" with the "concrete types" (HPA).

[DirectXMan12]

I lean slightly towards adding "Scale" to another group (like "autoscaling" or a net new empty group)

For future reference, there's already a copy of scale there ;-).

[adohe-zz]

and scale for jobs here.

[sttts]

I think scale should live in autoscaling

Doesn't sound right. Autoscaling is only one way of scaling an object.

[DirectXMan12]

sure, but the scale subresource is mainly used for autoscaling -- that's mainly why it exists so "automated" processes can scale different resources without having specific knowledge of them.

[nikhita]

What must be done here? The proposal for subresources for CustomResources (kubernetes/community#913) is blocked on where a unified Scale object should live.

cc @deads2k @liggitt @enisoc

[sttts]

@DirectXMan12 any update on polymorphic scale?

[kow3ns]

I think there is still open discussion on placement. I would propose that we should have one representation of scale for serving and stateful workloads. This should live in the apps/v1 group version and should have the existing signature.

// ScaleSpec describes the attributes of a scale subresource
type ScaleSpec struct {
	// desired number of instances for the scaled object.
	// +optional
	Replicas int32 `json:"replicas,omitempty" protobuf:"varint,1,opt,name=replicas"`
}

// ScaleStatus represents the current status of a scale subresource.
type ScaleStatus struct {
	// actual number of observed instances of the scaled object.
	Replicas int32 `json:"replicas" protobuf:"varint,1,opt,name=replicas"`

	// label query over pods that should match the replicas count. More info: http://kubernetes.io/docs/user-guide/labels#label-selectors
	// +optional
	Selector map[string]string `json:"selector,omitempty" protobuf:"bytes,2,rep,name=selector"`

	// label selector for pods that should match the replicas count. This is a serializated
	// version of both map-based and more expressive set-based selectors. This is done to
	// avoid introspection in the clients. The string will be in the same format as the
	// query-param syntax. If the target type only supports map-based selectors, both this
	// field and map-based selector field are populated.
	// More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors
	// +optional
	TargetSelector string `json:"targetSelector,omitempty" protobuf:"bytes,3,opt,name=targetSelector"`
}

// Scale represents a scaling request for a resource.
type Scale struct {
	metav1.TypeMeta `json:",inline"`
	// Standard object metadata; More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata.
	// +optional
	metav1.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`

	// defines the behavior of the scale. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#spec-and-status.
	// +optional
	Spec ScaleSpec `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"`

	// current status of the scale. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#spec-and-status. Read-only.
	// +optional
	Status ScaleStatus `json:"status,omitempty" protobuf:"bytes,3,opt,name=status"`
}

We moved everything that is a workload (DaemonSet, Deployment, ReplicaSet, and StatefulSet) to apps, and for workloads that have a notion of scale (Deployment ,ReplicaSet, and StatefulSet), they all have an implemented scale resource. I would argue that anything that is a scalable serving or stateful workload should use the same interface.

There has also been a proposal for scalable batch workloads. Scaling a Job has a different meaning than scaling a serving or stateful workload. When you scale a serving or stateful workload, you are definitively requesting or releasing capacity (mem, compute, or storage), in general in response to QPS demands. When you scale a batch workload (as represented by Job), you are actually asking for potentially more capacity, to increase parallelism, and to thereby decrease the time to completion. Depending on the number of already completed Tasks, scaling a Job may not actually require more capacity. There is a proposed API for this type of scaling and it looks something like this.

type ScaleSpec struct {
	// Specifies the maximum desired number of pods the job should
	// run at any given time. The actual number of pods running in steady state will
	// be less than this number when ((.spec.completions - .status.successful) < .spec.parallelism),
	// i.e. when the work left to do is less than max parallelism.
	// +optional
	Parallelism int32
}

// represents the current status of a scale subresource.
type ScaleStatus struct {
	// Specifies the desired number of successfully finished pods the
	// job should be run with.  Setting to nil means that the success of any
	// pod signals the success of all pods, and allows parallelism to have any positive
	// value.  Setting to 1 means that parallelism is limited to 1 and the success of that
	// pod signals the success of the job.
	// +optional
	Completions int32

	// The number of actively running pods.
	// +optional
	Active int32

       // The current max parallelism of the batch resource.
       Parallelism int32

	// label query over pods that should match the completions + active count.
	// More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors
	// +optional
	Selector *metav1.LabelSelector
}

// represents a scaling request for a resource.
type Scale struct {
	metav1.TypeMeta
	// Standard object metadata; More info: http://releases.k8s.io/HEAD/docs/devel/api-conventions.md#metadata.
	// +optional
	metav1.ObjectMeta

	// defines the behavior of the scale. More info: http://releases.k8s.io/HEAD/docs/devel/api-conventions.md#spec-and-status.
	// +optional
	Spec ScaleSpec

	// current status of the scale. More info: http://releases.k8s.io/HEAD/docs/devel/api-conventions.md#spec-and-status. Read-only.
	// +optional
	Status ScaleStatus
}

As the status of the scale of batch Job considers the number of complete Jobs, the number of active Jobs, and the max parallelism, and as the spec is really changing the parallelism and not requesting a fixed capacity increase or decrease, imo, it warrants having a different signature for the Scale sub resource. I'd argue this sub resource should live in batch/v1.

[smarterclayton]

Note that not all workloads or batch apis will live in batch or apps, so it's not enough to define a sub resource there. If we create multiple Scale resource subtypes then Scale is not generic anymore. And I don't think that we have clearly articulated why that is superior.

…

On Wed, Oct 4, 2017 at 6:30 PM, Kenneth Owens ***@***.***> wrote: I think there is still open discussion on placement. I would propose that we should have one representation of scale for serving and stateful workloads. This should live in the apps/v1 group version and should have the existing signature. // ScaleSpec describes the attributes of a scale subresourcetype ScaleSpec struct { // desired number of instances for the scaled object. // +optional Replicas int32 `json:"replicas,omitempty" protobuf:"varint,1,opt,name=replicas"` } // ScaleStatus represents the current status of a scale subresource.type ScaleStatus struct { // actual number of observed instances of the scaled object. Replicas int32 `json:"replicas" protobuf:"varint,1,opt,name=replicas"` // label query over pods that should match the replicas count. More info: http://kubernetes.io/docs/user-guide/labels#label-selectors // +optional Selector map[string]string `json:"selector,omitempty" protobuf:"bytes,2,rep,name=selector"` // label selector for pods that should match the replicas count. This is a serializated // version of both map-based and more expressive set-based selectors. This is done to // avoid introspection in the clients. The string will be in the same format as the // query-param syntax. If the target type only supports map-based selectors, both this // field and map-based selector field are populated. // More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors // +optional TargetSelector string `json:"targetSelector,omitempty" protobuf:"bytes,3,opt,name=targetSelector"` } // Scale represents a scaling request for a resource.type Scale struct { metav1.TypeMeta `json:",inline"` // Standard object metadata; More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata. // +optional metav1.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` // defines the behavior of the scale. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#spec-and-status. // +optional Spec ScaleSpec `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"` // current status of the scale. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#spec-and-status. Read-only. // +optional Status ScaleStatus `json:"status,omitempty" protobuf:"bytes,3,opt,name=status"` } We moved everything that is a workload (DaemonSet, Deployment, ReplicaSet, and StatefulSet) to apps, and for workloads that have a notion of scale (Deployment ,ReplicaSet, and StatefulSet), they all have an implemented scale resource. I would argue that anything that is a scalable serving or stateful workload should use the same interface. There has also been a proposal for scalable batch workloads. Scaling a Job has a different meaning than scaling a serving or stateful workload. When you scale a serving or stateful workload, you are definitively requesting or releasing capacity (mem, compute, or storage), in general in response to QPS demands. When you scale a batch workload (as represented by Job), you are actually asking for potentially more capacity, to increase parallelism, and to thereby decrease the time to completion. Depending on the number of already completed Tasks, scaling a Job may not actually require more capacity. There is a proposed API for this type of scaling and it looks something like this. type ScaleSpec struct { // Specifies the maximum desired number of pods the job should // run at any given time. The actual number of pods running in steady state will // be less than this number when ((.spec.completions - .status.successful) < .spec.parallelism), // i.e. when the work left to do is less than max parallelism. // +optional Parallelism int32 } // represents the current status of a scale subresource.type ScaleStatus struct { // Specifies the desired number of successfully finished pods the // job should be run with. Setting to nil means that the success of any // pod signals the success of all pods, and allows parallelism to have any positive // value. Setting to 1 means that parallelism is limited to 1 and the success of that // pod signals the success of the job. // +optional Completions int32 // The number of actively running pods. // +optional Active int32 // The current max parallelism of the batch resource. Parallelism int32 // label query over pods that should match the completions + active count. // More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors // +optional Selector *metav1.LabelSelector } // represents a scaling request for a resource.type Scale struct { metav1.TypeMeta // Standard object metadata; More info: http://releases.k8s.io/HEAD/docs/devel/api-conventions.md#metadata. // +optional metav1.ObjectMeta // defines the behavior of the scale. More info: http://releases.k8s.io/HEAD/docs/devel/api-conventions.md#spec-and-status. // +optional Spec ScaleSpec // current status of the scale. More info: http://releases.k8s.io/HEAD/docs/devel/api-conventions.md#spec-and-status. Read-only. // +optional Status ScaleStatus } As the status of the scale of batch Job considers the number of complete Jobs, the number of active Jobs, and the max parallelism, and as the spec is really changing the parallelism and not requesting a fixed capacity increase or decrease, imo, it warrants having a different signature for the Scale sub resource. I'd argue this sub resource should live in batch/v1. — You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub <#49504 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p7sWVi9ZeUv48Sb_wJ_Ia189zbSaks5spAcMgaJpZM4Ohaz4> .

[kow3ns]

If we go with a generic implementation over specialized implementations (within reason), aren't we just aggregating the bloat into a single location and causing confusion?

1. If I want to extend the existing scale to support completions in its status. I have to add a field that now applies to serving workloads, and probably doesn't make sense.
2. The existing replicas field doesn't really communicate what scaling a Job does. Increasing parallelism may or may not increase the number of Pods.

Perhaps apps and batch might not be the right home, but if we are moving toward and architecture where the nucleus is the core and most workloads are in the applications layer, apps might be a good place.

I think, as it stands now, we could have a general implementation for any controller that launches Pods with a required restart policy of always, and another for any controller that launches Pods that will eventually complete as successful or failed. I'm not sure I see a benefit to attempting to unify them when the semantics are so different.

[bgrant0607]

This is a dupe of #29698

[bgrant0607]

Do we feel like the scale subresource has worked well for autoscaling, at least?

As with conditions (#7856), I am starting to feel that subresources are overly complicated and cumbersome just for polymorphism. I am now in favor of duck typing, which has been extremely successful for metadata. We could scale anything with a .status.replicas field. I'm in favor of creating a convention for selectors in status, also: #3676.

I also want to do this for client-side resource manipulation, anyway. If we use subresources for kubectl scale I'll also want a generickubectl set replicas that works based on the field path alone.

#29698 (comment)

.status.replicas doesn't work for Job, but that's acceptable IMO. Job scaling requires policies that take into account at least the amount of queued work and the deadline. Why do we need a Scale flavor for Job? Do we expect to have more than one Job-like API?

With respect to the Scale subresource type:

@DirectXMan12 accurately explained why Scale was put into the autoscaling group. Is there a concrete mechanical problem with that? Why can't every controller that implements Scale use its own copy? When encoded, all copies will be structurally indistinguishable. Go typing is not helping us here, but I don't understand why we care. The type is very small.

[kow3ns]

@bgrant0607 If we don't do a generic scale sub resource, we will need to find another mechanism to integrate pod disruption budgets and custom resource definitions. Currently PDBs will work based on the replicas of known controller objects only.

[bgrant0607]

@kow3ns By "generic", you just mean structurally equivalent?

[enisoc]

We could scale anything with a .status.replicas field.

The duck typing philosophy sounds interesting to me, although it puts more pressure on fixing some related issues. One of the reasons we moved to /scale for some controllers was because it helped paper over version skew problems with get/roundtrip/update on the main resource. We need to fix that anyway though.

[bgrant0607]

Yes, we need to fix patch/merge anyway.

We need to figure out how to deal with protobuf tags. Honestly, proto's tagging scheme is not a great fit for us given that we have API versioning and discovery via OpenAPI.

Maybe we can paper over it using OpenAPI. I also wonder if we could autogenerate the tags by hashing the json paths.

We could also use extra OpenAPI / JSON schema attributes for APIs that deviated from the convention -- so convention over configuration, but with configuration as a backup.

[bgrant0607]

Don't the existing workload APIs support the version of scale from the autoscaling API group?

We have a way to independently version the Scale API, so I'm fine with using the one from the autoscaling group until there is an obviously better answer.

[kow3ns]

Currently apps uses the extensions representation internally with each group version having a separate, versioned sub resource.
While we may one day scale things other than the kinds that we have decided to move to apps, we don't currently. In particular HPA works for Replication Controllers and Deployments, but, with the addition of custom metrics, it may be possible to get it to work well for StatefulSets. Also, we would like to use the sub resource to implement kubectl scale so that we stop round tripping resources and stripping data due to version skew between kubectl and the API server.

[liggitt]

Don't the existing workload APIs support the version of scale from the autoscaling API group?

replicationcontrollers/scale uses autoscaling Scale:

$ kubectl get --raw /api/v1 | jq . | grep --color scale -A 5
      "name": "replicationcontrollers/scale",
      "singularName": "",
      "namespaced": true,
      "group": "autoscaling",
      "version": "v1",
      "kind": "Scale",

all the rest (in extensions/v1beta1, apps/v1beta1, and apps/v1beta2 API groups) advertise extensions/v1beta1 Scale as their scale subresource type:

$ kubectl get --raw /apis/extensions/v1beta1 | jq . | grep --color scale -A 5
      "name": "deployments/scale",
      "singularName": "",
      "namespaced": true,
      "group": "extensions",
      "version": "v1beta1",
      "kind": "Scale",
--
      "name": "replicasets/scale",
      "singularName": "",
      "namespaced": true,
      "group": "extensions",
      "version": "v1beta1",
      "kind": "Scale",
--
      "name": "replicationcontrollers/scale",
      "singularName": "",
      "namespaced": true,
      "kind": "Scale",

$ kubectl get --raw /apis/apps/v1beta1 | jq . | grep --color scale -A 5
      "name": "deployments/scale",
      "singularName": "",
      "namespaced": true,
      "group": "extensions",
      "version": "v1beta1",
      "kind": "Scale",
--
      "name": "statefulsets/scale",
      "singularName": "",
      "namespaced": true,
      "group": "extensions",
      "version": "v1beta1",
      "kind": "Scale",

$ kubectl get --raw /apis/apps/v1beta2 | jq . | grep --color scale -A 5
      "name": "deployments/scale",
      "singularName": "",
      "namespaced": true,
      "group": "extensions",
      "version": "v1beta1",
      "kind": "Scale",
--
      "name": "replicasets/scale",
      "singularName": "",
      "namespaced": true,
      "group": "extensions",
      "version": "v1beta1",
      "kind": "Scale",
--
      "name": "statefulsets/scale",
      "singularName": "",
      "namespaced": true,
      "group": "extensions",
      "version": "v1beta1",
      "kind": "Scale",

however, they return content that does not match the version advertised in discovery

[kow3ns]

@liggitt is correct. While each group version defines a scale sub-resource, the rest implementation wires in the extension scale sub-resource.

[kow3ns]

more context here

[bgrant0607]

Well, extensions is just wrong, for any new API.

[liggitt]

Well, extensions is just wrong, for any new API.

Agree. Scale in autoscaling/v1 is the closest thing we have to a uniform scale object (is supported by replication controllers, and is defined by the API group related to scaling, not related to a particular workload). It makes sense to me to coalesce around that as the common scale subresource kind

[DirectXMan12]

While each group version defines a scale sub-resource,

Eeeek. That needs to be removed before going stable. We shouldn't have a very on scale in every external API version. It kind-of defeats the point.

[kow3ns]

SGTM

[smarterclayton]

The other reason for subresources was so that autoscalers couldn't mutate other fields on the object. Re: proto tags we have an idl, we could in theory identify some tags but in general the structure of json and proto diverges on a few types.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

[nikhita]

/remove-lifecycle stale

[kow3ns]

/close