Better error handling when scaling a rc

pkg/kubectl/scale.go

@@ -22,6 +22,7 @@ import (
 	"time"
 
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/errors"
 	client "k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/util/wait"
 )
@@ -52,6 +53,7 @@ type ControllerScaleErrorType int
 const (
 	ControllerScaleGetFailure ControllerScaleErrorType = iota
 	ControllerScaleUpdateFailure
+	ControllerScaleUpdateInvalidFailure
 )
 
 // A ControllerScaleError is returned when a scale request passes
@@ -118,6 +120,10 @@ func ScaleCondition(r Scaler, precondition *ScalePrecondition, namespace, name s
 		case nil:
 			return true, nil
 		case ControllerScaleError:
+			// if it's invalid we shouldn't keep waiting
+			if e.FailureType == ControllerScaleUpdateInvalidFailure {
+				return false, err
+			}
 			if e.FailureType == ControllerScaleUpdateFailure {
 				return false, nil
 			}
@@ -139,6 +145,9 @@ func (scaler *ReplicationControllerScaler) ScaleSimple(namespace, name string, p
 	controller.Spec.Replicas = int(newSize)
 	// TODO: do retry on 409 errors here?
 	if _, err := scaler.c.UpdateReplicationController(namespace, controller); err != nil {
+		if errors.IsInvalid(err) {
+			return "", ControllerScaleError{ControllerScaleUpdateInvalidFailure, controller.ResourceVersion, err}
+		}
 		return "", ControllerScaleError{ControllerScaleUpdateFailure, controller.ResourceVersion, err}
 	}
 	// TODO: do a better job of printing objects here.

pkg/kubectl/scale_test.go

@@ -21,6 +21,7 @@ import (
 	"testing"
 
 	"k8s.io/kubernetes/pkg/api"
+	kerrors "k8s.io/kubernetes/pkg/api/errors"
 	client "k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/client/unversioned/testclient"
 )
@@ -41,6 +42,22 @@ func (c *ErrorReplicationControllerClient) ReplicationControllers(namespace stri
 	return &ErrorReplicationControllers{testclient.FakeReplicationControllers{Fake: &c.Fake, Namespace: namespace}}
 }
 
+type InvalidReplicationControllers struct {
+	testclient.FakeReplicationControllers
+}
+
+func (c *InvalidReplicationControllers) Update(controller *api.ReplicationController) (*api.ReplicationController, error) {
+	return nil, kerrors.NewInvalid(controller.Kind, controller.Name, nil)
+}
+
+type InvalidReplicationControllerClient struct {
+	testclient.Fake
+}
+
+func (c *InvalidReplicationControllerClient) ReplicationControllers(namespace string) client.ReplicationControllerInterface {
+	return &InvalidReplicationControllers{testclient.FakeReplicationControllers{Fake: &c.Fake, Namespace: namespace}}
+}
+
 func TestReplicationControllerScaleRetry(t *testing.T) {
 	fake := &ErrorReplicationControllerClient{Fake: testclient.Fake{}}
 	scaler := ReplicationControllerScaler{NewScalerClient(fake)}
@@ -51,7 +68,7 @@ func TestReplicationControllerScaleRetry(t *testing.T) {
 
 	scaleFunc := ScaleCondition(&scaler, &preconditions, namespace, name, count)
 	pass, err := scaleFunc()
-	if pass != false {
+	if pass {
 		t.Errorf("Expected an update failure to return pass = false, got pass = %v", pass)
 	}
 	if err != nil {
@@ -65,6 +82,25 @@ func TestReplicationControllerScaleRetry(t *testing.T) {
 	}
 }
 
+func TestReplicationControllerScaleInvalid(t *testing.T) {
+	fake := &InvalidReplicationControllerClient{Fake: testclient.Fake{}}
+	scaler := ReplicationControllerScaler{NewScalerClient(fake)}
+	preconditions := ScalePrecondition{-1, ""}
+	count := uint(3)
+	name := "foo"
+	namespace := "default"
+
+	scaleFunc := ScaleCondition(&scaler, &preconditions, namespace, name, count)
+	pass, err := scaleFunc()
+	if pass {
+		t.Errorf("Expected an update failure to return pass = false, got pass = %v", pass)
+	}
+	e, ok := err.(ControllerScaleError)
+	if err == nil || !ok || e.FailureType != ControllerScaleUpdateInvalidFailure {
+		t.Errorf("Expected error on invalid update failure, got %v", err)
+	}
+}
+
 func TestReplicationControllerScale(t *testing.T) {
 	fake := &testclient.Fake{}
 	scaler := ReplicationControllerScaler{NewScalerClient(fake)}