Merge pull request #9739 from bprashanth/sno

Fix kubectl stop rc with sequence numbers

pkg/registry/controller/etcd/etcd_test.go

@@ -280,6 +280,55 @@ func TestEtcdControllerValidatesNamespaceOnUpdate(t *testing.T) {
 	}
 }
 
+func TestGenerationNumber(t *testing.T) {
+	storage, _ := newStorage(t)
+	modifiedSno := validController
+	modifiedSno.Generation = 100
+	modifiedSno.Status.ObservedGeneration = 10
+	ctx := api.NewDefaultContext()
+	rc, err := createController(storage, modifiedSno, t)
+	ctrl, err := storage.Get(ctx, rc.Name)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	controller, _ := ctrl.(*api.ReplicationController)
+
+	// Generation initialization
+	if controller.Generation != 1 && controller.Status.ObservedGeneration != 0 {
+		t.Fatalf("Unexpected generation number %v, status generation %v", controller.Generation, controller.Status.ObservedGeneration)
+	}
+
+	// Updates to spec should increment the generation number
+	controller.Spec.Replicas += 1
+	storage.Update(ctx, controller)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	ctrl, err = storage.Get(ctx, rc.Name)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	controller, _ = ctrl.(*api.ReplicationController)
+	if controller.Generation != 2 || controller.Status.ObservedGeneration != 0 {
+		t.Fatalf("Unexpected generation, spec: %v, status: %v", controller.Generation, controller.Status.ObservedGeneration)
+	}
+
+	// Updates to status should not increment either spec or status generation numbers
+	controller.Status.Replicas += 1
+	storage.Update(ctx, controller)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	ctrl, err = storage.Get(ctx, rc.Name)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	controller, _ = ctrl.(*api.ReplicationController)
+	if controller.Generation != 2 || controller.Status.ObservedGeneration != 0 {
+		t.Fatalf("Unexpected generation number, spec: %v, status: %v", controller.Generation, controller.Status.ObservedGeneration)
+	}
+}
+
 // TestEtcdGetControllerDifferentNamespace ensures same-name controllers in different namespaces do not clash
 func TestEtcdGetControllerDifferentNamespace(t *testing.T) {
 	storage, fakeClient := newStorage(t)

pkg/registry/controller/rest.go

@@ -18,6 +18,7 @@ package controller
 
 import (
 	"fmt"
+	"reflect"
 	"strconv"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -47,6 +48,9 @@ func (rcStrategy) NamespaceScoped() bool {
 func (rcStrategy) PrepareForCreate(obj runtime.Object) {
 	controller := obj.(*api.ReplicationController)
 	controller.Status = api.ReplicationControllerStatus{}
+
+	controller.Generation = 1
+	controller.Status.ObservedGeneration = 0
 }
 
 // PrepareForUpdate clears fields that are not allowed to be set by end users on update.
@@ -55,6 +59,23 @@ func (rcStrategy) PrepareForUpdate(obj, old runtime.Object) {
 	//newController := obj.(*api.ReplicationController)
 	//oldController := old.(*api.ReplicationController)
 	//newController.Status = oldController.Status
+	newController := obj.(*api.ReplicationController)
+	oldController := old.(*api.ReplicationController)
+
+	// Any changes to the spec increment the generation number, any changes to the
+	// status should reflect the generation number of the corresponding object. We push
+	// the burden of managing the status onto the clients because we can't (in general)
+	// know here what version of spec the writer of the status has seen. It may seem like
+	// we can at first -- since obj contains spec -- but in the future we will probably make
+	// status its own object, and even if we don't, writes may be the result of a
+	// read-update-write loop, so the contents of spec may not actually be the spec that
+	// the controller has *seen*.
+	//
+	// TODO: Any changes to a part of the object that represents desired state (labels,
+	// annotations etc) should also increment the generation.
+	if !reflect.DeepEqual(oldController.Spec, newController.Spec) {
+		newController.Generation = oldController.Generation + 1
+	}
 }
 
 // Validate validates a new replication controller.