Merge pull request #5005 from jdef/persistent_bindings

Persist Bindings annotations alongside pod Host assignment

pkg/registry/pod/etcd/etcd.go

@@ -147,15 +147,15 @@ func (r *BindingREST) Create(ctx api.Context, obj runtime.Object) (out runtime.O
 	if len(binding.Target.Name) == 0 {
 		return nil, errors.NewInvalid("binding", binding.Name, errors.ValidationErrorList{errors.NewFieldRequired("to.name", binding.Target.Name)})
 	}
-	err = r.assignPod(ctx, binding.Name, binding.Target.Name)
+	err = r.assignPod(ctx, binding.Name, binding.Target.Name, binding.Annotations)
-	err = etcderr.InterpretCreateError(err, "binding", "")
 	out = &api.Status{Status: api.StatusSuccess}
 	return
 }
 
-// setPodHostTo sets the given pod's host to 'machine' iff it was previously 'oldMachine'.
+// setPodHostAndAnnotations sets the given pod's host to 'machine' iff it was previously 'oldMachine' and merges
+// the provided annotations with those of the pod.
 // Returns the current state of the pod, or an error.
-func (r *BindingREST) setPodHostTo(ctx api.Context, podID, oldMachine, machine string) (finalPod *api.Pod, err error) {
+func (r *BindingREST) setPodHostAndAnnotations(ctx api.Context, podID, oldMachine, machine string, annotations map[string]string) (finalPod *api.Pod, err error) {
 	podKey, err := r.store.KeyFunc(ctx, podID)
 	if err != nil {
 		return nil, err
@@ -170,6 +170,12 @@ func (r *BindingREST) setPodHostTo(ctx api.Context, podID, oldMachine, machine s
 		}
 		pod.Spec.Host = machine
 		pod.Status.Host = machine
+		if pod.Annotations == nil {
+			pod.Annotations = make(map[string]string)
+		}
+		for k, v := range annotations {
+			pod.Annotations[k] = v
+		}
 		finalPod = pod
 		return pod, 0, nil
 	})
@@ -177,12 +183,15 @@ func (r *BindingREST) setPodHostTo(ctx api.Context, podID, oldMachine, machine s
 }
 
 // assignPod assigns the given pod to the given machine.
-func (r *BindingREST) assignPod(ctx api.Context, podID string, machine string) error {
+func (r *BindingREST) assignPod(ctx api.Context, podID string, machine string, annotations map[string]string) (err error) {
-	_, err := r.setPodHostTo(ctx, podID, "", machine)
+	if _, err = r.setPodHostAndAnnotations(ctx, podID, "", machine, annotations); err != nil {
-	if err != nil {
+		err = etcderr.InterpretGetError(err, "pod", podID)
-		return err
+		err = etcderr.InterpretUpdateError(err, "pod", podID)
+		if _, ok := err.(*errors.StatusError); !ok {
+			err = errors.NewConflict("binding", podID, err)
+		}
 	}
-	return err
+	return
 }
 
 type podLifecycle struct{}

pkg/registry/pod/etcd/etcd_test.go

@@ -894,8 +894,12 @@ func TestEtcdCreateWithContainersNotFound(t *testing.T) {
 
 	// Suddenly, a wild scheduler appears:
 	_, err = bindingRegistry.Create(ctx, &api.Binding{
-		ObjectMeta: api.ObjectMeta{Namespace: api.NamespaceDefault, Name: "foo"},
+		ObjectMeta: api.ObjectMeta{
-		Target:     api.ObjectReference{Name: "machine"},
+			Namespace:   api.NamespaceDefault,
+			Name:        "foo",
+			Annotations: map[string]string{"label1": "value1"},
+		},
+		Target: api.ObjectReference{Name: "machine"},
 	})
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -914,6 +918,46 @@ func TestEtcdCreateWithContainersNotFound(t *testing.T) {
 	if pod.Name != "foo" {
 		t.Errorf("Unexpected pod: %#v %s", pod, resp.Node.Value)
 	}
+	if !(pod.Annotations != nil && pod.Annotations["label1"] == "value1") {
+		t.Fatalf("Pod annotations don't match the expected: %v", pod.Annotations)
+	}
+}
+
+func TestEtcdCreateWithConflict(t *testing.T) {
+	registry, bindingRegistry, _, fakeClient, _ := newStorage(t)
+	ctx := api.NewDefaultContext()
+	fakeClient.TestIndex = true
+	key, _ := registry.store.KeyFunc(ctx, "foo")
+	fakeClient.Data[key] = tools.EtcdResponseWithError{
+		R: &etcd.Response{
+			Node: nil,
+		},
+		E: tools.EtcdErrorNotFound,
+	}
+
+	_, err := registry.Create(ctx, validNewPod())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Suddenly, a wild scheduler appears:
+	binding := api.Binding{
+		ObjectMeta: api.ObjectMeta{
+			Namespace:   api.NamespaceDefault,
+			Name:        "foo",
+			Annotations: map[string]string{"label1": "value1"},
+		},
+		Target: api.ObjectReference{Name: "machine"},
+	}
+	_, err = bindingRegistry.Create(ctx, &binding)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	_, err = bindingRegistry.Create(ctx, &binding)
+	if err == nil || !errors.IsConflict(err) {
+		t.Fatalf("expected resource conflict error, not: %v", err)
+	}
 }
 
 func TestEtcdCreateWithExistingContainers(t *testing.T) {