diff --git a/pkg/registry/etcd/etcd.go b/pkg/registry/etcd/etcd.go index 63422d3cda6..65befb3fc82 100644 --- a/pkg/registry/etcd/etcd.go +++ b/pkg/registry/etcd/etcd.go @@ -229,23 +229,24 @@ func (r *Registry) assignPod(ctx api.Context, podID string, machine string) erro if err != nil { return err } - // TODO: move this to a watch/rectification loop. - pod, err := r.boundPodFactory.MakeBoundPod(machine, finalPod) + boundPod, err := r.boundPodFactory.MakeBoundPod(machine, finalPod) if err != nil { return err } + // Doing the constraint check this way provides atomicity guarantees. contKey := makeContainerKey(machine) err = r.AtomicUpdate(contKey, &api.BoundPods{}, func(in runtime.Object) (runtime.Object, error) { - pods := *in.(*api.BoundPods) - pods.Items = append(pods.Items, *pod) - if !constraint.Allowed(pods.Items) { + boundPodList := in.(*api.BoundPods) + boundPodList.Items = append(boundPodList.Items, *boundPod) + if !constraint.Allowed(boundPodList.Items) { return nil, fmt.Errorf("The assignment would cause a constraint violation") } - return &pods, nil + return boundPodList, nil }) if err != nil { - // Put the pod's host back the way it was. This is a terrible hack that - // won't be needed if we convert this to a rectification loop. + // Put the pod's host back the way it was. This is a terrible hack, but + // can't really be helped, since there's not really a way to do atomic + // multi-object changes in etcd. if _, err2 := r.setPodHostTo(ctx, podID, machine, ""); err2 != nil { glog.Errorf("Stranding pod %v; couldn't clear host after previous error: %v", podID, err2) } diff --git a/pkg/registry/pod/bound_pod_factory.go b/pkg/registry/pod/bound_pod_factory.go index 00030acac58..0cbbd54c5a2 100644 --- a/pkg/registry/pod/bound_pod_factory.go +++ b/pkg/registry/pod/bound_pod_factory.go @@ -43,5 +43,9 @@ func (b *BasicBoundPodFactory) MakeBoundPod(machine string, pod *api.Pod) (*api. for ix, container := range boundPod.Spec.Containers { boundPod.Spec.Containers[ix].Env = append(container.Env, envVars...) } + // Make a dummy self link so that references to this bound pod will work. + // TODO: When kubelets get boundPods from apiserver instead of etcd, then + // the selflink should be generated there. + boundPod.SelfLink = "/api/v1beta1/boundPods/" + boundPod.Name return boundPod, nil } diff --git a/pkg/registry/pod/bound_pod_factory_test.go b/pkg/registry/pod/bound_pod_factory_test.go index ce0eb21f87c..4ce66b1a66e 100644 --- a/pkg/registry/pod/bound_pod_factory_test.go +++ b/pkg/registry/pod/bound_pod_factory_test.go @@ -54,6 +54,10 @@ func TestMakeBoundPodNoServices(t *testing.T) { if pod.Name != "foobar" { t.Errorf("Failed to assign ID to pod: %#v", pod.Name) } + + if _, err := api.GetReference(pod); err != nil { + t.Errorf("Unable to get a reference to bound pod: %v", err) + } } func TestMakeBoundPodServices(t *testing.T) {