Merge pull request #32442 from kubernetes/revert-31111-scale_subresource

pkg/client/unversioned/testclient/fixture.go

@@ -54,22 +54,13 @@ type ObjectScheme interface {
 // Object or error that matches the requested action. For instance, list-replicationControllers
 // should attempt to return a list of replication controllers. This method delegates to the
 // ObjectRetriever interface to satisfy retrieval of lists or retrieval of single items.
+// TODO: add support for sub resources
 func ObjectReaction(o ObjectRetriever, mapper meta.RESTMapper) ReactionFunc {
 
 	return func(action Action) (bool, runtime.Object, error) {
-		var err error
-		var kind unversioned.GroupVersionKind
-
-		if action.GetSubresource() != "" {
-			kind, err = mapper.KindFor(unversioned.GroupVersionResource{Resource: action.GetSubresource()})
-			if err != nil {
-				return false, nil, fmt.Errorf("unrecognized subresource action %s/%s: %v", action.GetResource(), action.GetSubresource(), err)
-			}
-		} else {
-			kind, err = mapper.KindFor(unversioned.GroupVersionResource{Resource: action.GetResource()})
-			if err != nil {
-				return false, nil, fmt.Errorf("unrecognized action %s: %v", action.GetResource(), err)
-			}
+		kind, err := mapper.KindFor(unversioned.GroupVersionResource{Resource: action.GetResource()})
+		if err != nil {
+			return false, nil, fmt.Errorf("unrecognized action %s: %v", action.GetResource(), err)
 		}
 
 		// TODO: have mapper return a Kind for a subresource?

pkg/kubectl/scale.go

@@ -143,13 +143,13 @@ func (precondition *ScalePrecondition) ValidatePetSet(ps *apps.PetSet) error {
 	return nil
 }
 
-// ValidateScale ensures that the preconditions match.  Returns nil if they are valid, an error otherwise
-func (precondition *ScalePrecondition) ValidateScale(scale *extensions.Scale) error {
-	if precondition.Size != -1 && int(scale.Spec.Replicas) != precondition.Size {
-		return PreconditionError{"replicas", strconv.Itoa(precondition.Size), strconv.Itoa(int(scale.Spec.Replicas))}
+// ValidateReplicationController ensures that the preconditions match.  Returns nil if they are valid, an error otherwise
+func (precondition *ScalePrecondition) ValidateReplicationController(controller *api.ReplicationController) error {
+	if precondition.Size != -1 && int(controller.Spec.Replicas) != precondition.Size {
+		return PreconditionError{"replicas", strconv.Itoa(precondition.Size), strconv.Itoa(int(controller.Spec.Replicas))}
 	}
-	if len(precondition.ResourceVersion) != 0 && scale.ResourceVersion != precondition.ResourceVersion {
-		return PreconditionError{"resource version", precondition.ResourceVersion, scale.ResourceVersion}
+	if len(precondition.ResourceVersion) != 0 && controller.ResourceVersion != precondition.ResourceVersion {
+		return PreconditionError{"resource version", precondition.ResourceVersion, controller.ResourceVersion}
 	}
 	return nil
 }
@@ -161,24 +161,24 @@ type ReplicationControllerScaler struct {
 // ScaleSimple does a simple one-shot attempt at scaling. It returns the
 // resourceVersion of the replication controller if the update is successful.
 func (scaler *ReplicationControllerScaler) ScaleSimple(namespace, name string, preconditions *ScalePrecondition, newSize uint) (string, error) {
-	sc := scaler.c.Extensions().Scales(namespace)
-	scale, err := sc.Get("ReplicationController", name)
+	controller, err := scaler.c.ReplicationControllers(namespace).Get(name)
 	if err != nil {
 		return "", ScaleError{ScaleGetFailure, "Unknown", err}
 	}
 	if preconditions != nil {
-		if err := preconditions.ValidateScale(scale); err != nil {
+		if err := preconditions.ValidateReplicationController(controller); err != nil {
 			return "", err
 		}
 	}
-	scale.Spec.Replicas = int32(newSize)
-	if _, err := sc.Update("ReplicationController", scale); err != nil {
+	controller.Spec.Replicas = int32(newSize)
+	updatedRC, err := scaler.c.ReplicationControllers(namespace).Update(controller)
+	if err != nil {
 		if errors.IsConflict(err) {
-			return "", ScaleError{ScaleUpdateConflictFailure, scale.ResourceVersion, err}
+			return "", ScaleError{ScaleUpdateConflictFailure, controller.ResourceVersion, err}
 		}
-		return "", ScaleError{ScaleUpdateFailure, scale.ResourceVersion, err}
+		return "", ScaleError{ScaleUpdateFailure, controller.ResourceVersion, err}
 	}
-	return scale.ObjectMeta.ResourceVersion, nil
+	return updatedRC.ObjectMeta.ResourceVersion, nil
 }
 
 // Scale updates a ReplicationController to a new size, with optional precondition check (if preconditions is not nil),
@@ -241,6 +241,17 @@ func (scaler *ReplicationControllerScaler) Scale(namespace, name string, newSize
 	return nil
 }
 
+// ValidateReplicaSet ensures that the preconditions match.  Returns nil if they are valid, an error otherwise
+func (precondition *ScalePrecondition) ValidateReplicaSet(replicaSet *extensions.ReplicaSet) error {
+	if precondition.Size != -1 && int(replicaSet.Spec.Replicas) != precondition.Size {
+		return PreconditionError{"replicas", strconv.Itoa(precondition.Size), strconv.Itoa(int(replicaSet.Spec.Replicas))}
+	}
+	if len(precondition.ResourceVersion) != 0 && replicaSet.ResourceVersion != precondition.ResourceVersion {
+		return PreconditionError{"resource version", precondition.ResourceVersion, replicaSet.ResourceVersion}
+	}
+	return nil
+}
+
 type ReplicaSetScaler struct {
 	c client.ExtensionsInterface
 }
@@ -248,24 +259,24 @@ type ReplicaSetScaler struct {
 // ScaleSimple does a simple one-shot attempt at scaling. It returns the
 // resourceVersion of the replicaset if the update is successful.
 func (scaler *ReplicaSetScaler) ScaleSimple(namespace, name string, preconditions *ScalePrecondition, newSize uint) (string, error) {
-	sc := scaler.c.Scales(namespace)
-	scale, err := sc.Get("ReplicaSet", name)
+	rs, err := scaler.c.ReplicaSets(namespace).Get(name)
 	if err != nil {
 		return "", ScaleError{ScaleGetFailure, "Unknown", err}
 	}
 	if preconditions != nil {
-		if err := preconditions.ValidateScale(scale); err != nil {
+		if err := preconditions.ValidateReplicaSet(rs); err != nil {
 			return "", err
 		}
 	}
-	scale.Spec.Replicas = int32(newSize)
-	if _, err := sc.Update("ReplicaSet", scale); err != nil {
+	rs.Spec.Replicas = int32(newSize)
+	updatedRS, err := scaler.c.ReplicaSets(namespace).Update(rs)
+	if err != nil {
 		if errors.IsConflict(err) {
-			return "", ScaleError{ScaleUpdateConflictFailure, scale.ResourceVersion, err}
+			return "", ScaleError{ScaleUpdateConflictFailure, rs.ResourceVersion, err}
 		}
-		return "", ScaleError{ScaleUpdateFailure, scale.ResourceVersion, err}
+		return "", ScaleError{ScaleUpdateFailure, rs.ResourceVersion, err}
 	}
-	return scale.ObjectMeta.ResourceVersion, nil
+	return updatedRS.ObjectMeta.ResourceVersion, nil
 }
 
 // Scale updates a ReplicaSet to a new size, with optional precondition check (if preconditions is
@@ -422,6 +433,17 @@ func (scaler *JobScaler) Scale(namespace, name string, newSize uint, preconditio
 	return nil
 }
 
+// ValidateDeployment ensures that the preconditions match.  Returns nil if they are valid, an error otherwise.
+func (precondition *ScalePrecondition) ValidateDeployment(deployment *extensions.Deployment) error {
+	if precondition.Size != -1 && int(deployment.Spec.Replicas) != precondition.Size {
+		return PreconditionError{"replicas", strconv.Itoa(precondition.Size), strconv.Itoa(int(deployment.Spec.Replicas))}
+	}
+	if len(precondition.ResourceVersion) != 0 && deployment.ResourceVersion != precondition.ResourceVersion {
+		return PreconditionError{"resource version", precondition.ResourceVersion, deployment.ResourceVersion}
+	}
+	return nil
+}
+
 type DeploymentScaler struct {
 	c client.ExtensionsInterface
 }
@@ -430,25 +452,27 @@ type DeploymentScaler struct {
 // 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) {
-	sc := scaler.c.Scales(namespace)
-	scale, err := sc.Get("Deployment", name)
+	deployment, err := scaler.c.Deployments(namespace).Get(name)
 	if err != nil {
 		return "", ScaleError{ScaleGetFailure, "Unknown", err}
 	}
 	if preconditions != nil {
-		if err := preconditions.ValidateScale(scale); err != nil {
+		if err := preconditions.ValidateDeployment(deployment); err != nil {
 			return "", err
 		}
 	}
 
-	scale.Spec.Replicas = int32(newSize)
-	if _, err := sc.Update("Deployment", scale); err != nil {
+	// TODO(madhusudancs): Fix this when Scale group issues are resolved (see issue #18528).
+	// For now I'm falling back to regular Deployment update operation.
+	deployment.Spec.Replicas = int32(newSize)
+	updatedDeployment, err := scaler.c.Deployments(namespace).Update(deployment)
+	if err != nil {
 		if errors.IsConflict(err) {
-			return "", ScaleError{ScaleUpdateConflictFailure, scale.ResourceVersion, err}
+			return "", ScaleError{ScaleUpdateConflictFailure, deployment.ResourceVersion, err}
 		}
-		return "", ScaleError{ScaleUpdateFailure, scale.ResourceVersion, err}
+		return "", ScaleError{ScaleUpdateFailure, deployment.ResourceVersion, err}
 	}
-	return scale.ObjectMeta.ResourceVersion, nil
+	return updatedDeployment.ObjectMeta.ResourceVersion, nil
 }
 
 // Scale updates a deployment to a new size, with optional precondition check (if preconditions is not nil),

pkg/kubectl/stop_test.go

@@ -69,11 +69,6 @@ func TestReplicationControllerStop(t *testing.T) {
 						},
 					},
 				},
-				&extensions.Scale{
-					Spec: extensions.ScaleSpec{
-						Replicas: 0,
-					},
-				},
 			},
 			StopError:       nil,
 			ExpectedActions: []string{"get", "list", "get", "update", "get", "delete"},
@@ -112,11 +107,6 @@ func TestReplicationControllerStop(t *testing.T) {
 						},
 					},
 				},
-				&extensions.Scale{
-					Spec: extensions.ScaleSpec{
-						Replicas: 0,
-					},
-				},
 			},
 			StopError:       nil,
 			ExpectedActions: []string{"get", "list", "get", "update", "get", "delete"},
@@ -279,7 +269,7 @@ func TestReplicationControllerStop(t *testing.T) {
 			continue
 		}
 		for i, verb := range test.ExpectedActions {
-			if actions[i].GetResource() != "replicationcontrollers" && actions[i].GetSubresource() != "scale" {
+			if actions[i].GetResource() != "replicationcontrollers" {
 				t.Errorf("%s unexpected action: %+v, expected %s-replicationController", test.Name, actions[i], verb)
 			}
 			if actions[i].GetVerb() != verb {
@@ -325,11 +315,6 @@ func TestReplicaSetStop(t *testing.T) {
 						},
 					},
 				},
-				&extensions.Scale{
-					Spec: extensions.ScaleSpec{
-						Replicas: 0,
-					},
-				},
 			},
 			StopError:       nil,
 			ExpectedActions: []string{"get", "get", "update", "get", "get", "delete"},
@@ -371,11 +356,6 @@ func TestReplicaSetStop(t *testing.T) {
 						},
 					},
 				},
-				&extensions.Scale{
-					Spec: extensions.ScaleSpec{
-						Replicas: 0,
-					},
-				},
 			},
 			StopError:       nil,
 			ExpectedActions: []string{"get", "get", "update", "get", "get", "delete"},
@@ -399,7 +379,7 @@ func TestReplicaSetStop(t *testing.T) {
 			continue
 		}
 		for i, verb := range test.ExpectedActions {
-			if actions[i].GetResource() != "replicasets" && actions[i].GetSubresource() != "scale" {
+			if actions[i].GetResource() != "replicasets" {
 				t.Errorf("%s unexpected action: %+v, expected %s-replicaSet", test.Name, actions[i], verb)
 			}
 			if actions[i].GetVerb() != verb {
@@ -591,11 +571,6 @@ func TestDeploymentStop(t *testing.T) {
 						},
 					},
 				},
-				&extensions.Scale{
-					Spec: extensions.ScaleSpec{
-						Replicas: 0,
-					},
-				},
 			},
 			StopError: nil,
 			ExpectedActions: []string{"get:deployments", "update:deployments",
@@ -624,7 +599,7 @@ func TestDeploymentStop(t *testing.T) {
 			if actions[i].GetVerb() != action[0] {
 				t.Errorf("%s unexpected verb: %+v, expected %s", test.Name, actions[i], expAction)
 			}
-			if actions[i].GetResource() != action[1] && actions[i].GetSubresource() != "scale" {
+			if actions[i].GetResource() != action[1] {
 				t.Errorf("%s unexpected resource: %+v, expected %s", test.Name, actions[i], expAction)
 			}
 			if len(action) == 3 && actions[i].GetSubresource() != action[2] {