diff --git a/pkg/api/conversion.go b/pkg/api/conversion.go index 05323e9e262a9..e6f937103186c 100644 --- a/pkg/api/conversion.go +++ b/pkg/api/conversion.go @@ -61,5 +61,20 @@ func init() { } out.ResourceVersion = in.ResourceVersion return nil - }) + }, + + // Convert Pod to BoundPod + func(in *Pod, out *BoundPod, s conversion.Scope) error { + if err := s.Convert(&in.DesiredState.Manifest, out, 0); err != nil { + return err + } + // Only copy a subset of fields, and override manifest attributes with the pod + // metadata + out.UID = in.UID + out.ID = in.ID + out.Namespace = in.Namespace + out.CreationTimestamp = in.CreationTimestamp + return nil + }, + ) } diff --git a/pkg/constraint/constraint.go b/pkg/constraint/constraint.go index 0e08162792f3e..8acc8c073d86e 100644 --- a/pkg/constraint/constraint.go +++ b/pkg/constraint/constraint.go @@ -20,8 +20,8 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" ) -// Allowed returns true if manifests is a collection of manifests +// Allowed returns true if pods is a collection of bound pods // which can run without conflict on a single minion. -func Allowed(manifests []api.ContainerManifest) bool { - return !PortsConflict(manifests) +func Allowed(pods []api.BoundPod) bool { + return !PortsConflict(pods) } diff --git a/pkg/constraint/constraint_test.go b/pkg/constraint/constraint_test.go index 4d168551fc77f..1677d935df526 100644 --- a/pkg/constraint/constraint_test.go +++ b/pkg/constraint/constraint_test.go @@ -30,27 +30,27 @@ func containerWithHostPorts(ports ...int) api.Container { return c } -func manifestWithContainers(containers ...api.Container) api.ContainerManifest { - m := api.ContainerManifest{} +func podWithContainers(containers ...api.Container) api.BoundPod { + m := api.BoundPod{} for _, c := range containers { - m.Containers = append(m.Containers, c) + m.Spec.Containers = append(m.Spec.Containers, c) } return m } func TestAllowed(t *testing.T) { table := []struct { - allowed bool - manifests []api.ContainerManifest + allowed bool + pods []api.BoundPod }{ { allowed: true, - manifests: []api.ContainerManifest{ - manifestWithContainers( + pods: []api.BoundPod{ + podWithContainers( containerWithHostPorts(1, 2, 3), containerWithHostPorts(4, 5, 6), ), - manifestWithContainers( + podWithContainers( containerWithHostPorts(7, 8, 9), containerWithHostPorts(10, 11, 12), ), @@ -58,12 +58,12 @@ func TestAllowed(t *testing.T) { }, { allowed: true, - manifests: []api.ContainerManifest{ - manifestWithContainers( + pods: []api.BoundPod{ + podWithContainers( containerWithHostPorts(0, 0), containerWithHostPorts(0, 0), ), - manifestWithContainers( + podWithContainers( containerWithHostPorts(0, 0), containerWithHostPorts(0, 0), ), @@ -71,19 +71,19 @@ func TestAllowed(t *testing.T) { }, { allowed: false, - manifests: []api.ContainerManifest{ - manifestWithContainers( + pods: []api.BoundPod{ + podWithContainers( containerWithHostPorts(3, 3), ), }, }, { allowed: false, - manifests: []api.ContainerManifest{ - manifestWithContainers( + pods: []api.BoundPod{ + podWithContainers( containerWithHostPorts(6), ), - manifestWithContainers( + podWithContainers( containerWithHostPorts(6), ), }, @@ -91,8 +91,8 @@ func TestAllowed(t *testing.T) { } for _, item := range table { - if e, a := item.allowed, Allowed(item.manifests); e != a { - t.Errorf("Expected %v, got %v: \n%v\v", e, a, item.manifests) + if e, a := item.allowed, Allowed(item.pods); e != a { + t.Errorf("Expected %v, got %v: \n%v\v", e, a, item.pods) } } } diff --git a/pkg/constraint/ports.go b/pkg/constraint/ports.go index 92622469a2c87..f5c95f5fb2d42 100644 --- a/pkg/constraint/ports.go +++ b/pkg/constraint/ports.go @@ -22,10 +22,10 @@ import ( // PortsConflict returns true iff two containers attempt to expose // the same host port. -func PortsConflict(manifests []api.ContainerManifest) bool { +func PortsConflict(pods []api.BoundPod) bool { hostPorts := map[int]struct{}{} - for _, manifest := range manifests { - for _, container := range manifest.Containers { + for _, pod := range pods { + for _, container := range pod.Spec.Containers { for _, port := range container.Ports { if port.HostPort == 0 { continue diff --git a/pkg/master/master.go b/pkg/master/master.go index 0d53423ff8956..8f7a48146f379 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -83,15 +83,15 @@ func NewEtcdHelper(client tools.EtcdGetSet, version string) (helper tools.EtcdHe func New(c *Config) *Master { minionRegistry := makeMinionRegistry(c) serviceRegistry := etcd.NewRegistry(c.EtcdHelper, nil) - manifestFactory := &pod.BasicManifestFactory{ + boundPodFactory := &pod.BasicBoundPodFactory{ ServiceRegistry: serviceRegistry, } m := &Master{ - podRegistry: etcd.NewRegistry(c.EtcdHelper, manifestFactory), + podRegistry: etcd.NewRegistry(c.EtcdHelper, boundPodFactory), controllerRegistry: etcd.NewRegistry(c.EtcdHelper, nil), serviceRegistry: serviceRegistry, endpointRegistry: etcd.NewRegistry(c.EtcdHelper, nil), - bindingRegistry: etcd.NewRegistry(c.EtcdHelper, manifestFactory), + bindingRegistry: etcd.NewRegistry(c.EtcdHelper, boundPodFactory), minionRegistry: minionRegistry, client: c.Client, } diff --git a/pkg/registry/etcd/etcd.go b/pkg/registry/etcd/etcd.go index 36a5f8feb6278..62ca9e9aee7ce 100644 --- a/pkg/registry/etcd/etcd.go +++ b/pkg/registry/etcd/etcd.go @@ -38,15 +38,15 @@ import ( // Registry implements PodRegistry, ControllerRegistry, ServiceRegistry and MinionRegistry, backed by etcd. type Registry struct { tools.EtcdHelper - manifestFactory pod.ManifestFactory + boundPodFactory pod.BoundPodFactory } // NewRegistry creates an etcd registry. -func NewRegistry(helper tools.EtcdHelper, manifestFactory pod.ManifestFactory) *Registry { +func NewRegistry(helper tools.EtcdHelper, boundPodFactory pod.BoundPodFactory) *Registry { registry := &Registry{ EtcdHelper: helper, } - registry.manifestFactory = manifestFactory + registry.boundPodFactory = boundPodFactory return registry } @@ -174,18 +174,18 @@ func (r *Registry) assignPod(podID string, machine string) error { return err } // TODO: move this to a watch/rectification loop. - manifest, err := r.manifestFactory.MakeManifest(machine, *finalPod) + pod, err := r.boundPodFactory.MakeBoundPod(machine, finalPod) if err != nil { return err } contKey := makeContainerKey(machine) - err = r.AtomicUpdate(contKey, &api.ContainerManifestList{}, func(in runtime.Object) (runtime.Object, error) { - manifests := *in.(*api.ContainerManifestList) - manifests.Items = append(manifests.Items, manifest) - if !constraint.Allowed(manifests.Items) { + 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) { return nil, fmt.Errorf("The assignment would cause a constraint violation") } - return &manifests, nil + return &pods, nil }) if err != nil { // Put the pod's host back the way it was. This is a terrible hack that @@ -222,13 +222,13 @@ func (r *Registry) DeletePod(ctx api.Context, podID string) error { } // Next, remove the pod from the machine atomically. contKey := makeContainerKey(machine) - return r.AtomicUpdate(contKey, &api.ContainerManifestList{}, func(in runtime.Object) (runtime.Object, error) { - manifests := in.(*api.ContainerManifestList) - newManifests := make([]api.ContainerManifest, 0, len(manifests.Items)) + return r.AtomicUpdate(contKey, &api.BoundPods{}, func(in runtime.Object) (runtime.Object, error) { + pods := in.(*api.BoundPods) + newPods := make([]api.BoundPod, 0, len(pods.Items)) found := false - for _, manifest := range manifests.Items { - if manifest.ID != podID { - newManifests = append(newManifests, manifest) + for _, pod := range pods.Items { + if pod.ID != podID { + newPods = append(newPods, pod) } else { found = true } @@ -237,10 +237,10 @@ func (r *Registry) DeletePod(ctx api.Context, podID string) error { // This really shouldn't happen, it indicates something is broken, and likely // there is a lost pod somewhere. // However it is "deleted" so log it and move on - glog.Warningf("Couldn't find: %s in %#v", podID, manifests) + glog.Warningf("Couldn't find: %s in %#v", podID, pods) } - manifests.Items = newManifests - return manifests, nil + pods.Items = newPods + return pods, nil }) } diff --git a/pkg/registry/etcd/etcd_test.go b/pkg/registry/etcd/etcd_test.go index 8a02c921e4696..62ae74d8de448 100644 --- a/pkg/registry/etcd/etcd_test.go +++ b/pkg/registry/etcd/etcd_test.go @@ -35,7 +35,7 @@ import ( func NewTestEtcdRegistry(client tools.EtcdClient) *Registry { registry := NewRegistry(tools.EtcdHelper{client, latest.Codec, tools.RuntimeVersionAdapter{latest.ResourceVersioner}}, - &pod.BasicManifestFactory{ + &pod.BasicBoundPodFactory{ ServiceRegistry: ®istrytest.ServiceRegistry{}, }) return registry @@ -117,7 +117,7 @@ func TestEtcdCreatePod(t *testing.T) { }, E: tools.EtcdErrorNotFound, } - fakeClient.Set("/registry/hosts/machine/kubelet", runtime.EncodeOrDie(latest.Codec, &api.ContainerManifestList{}), 0) + fakeClient.Set("/registry/hosts/machine/kubelet", runtime.EncodeOrDie(latest.Codec, &api.BoundPods{}), 0) registry := NewTestEtcdRegistry(fakeClient) err := registry.CreatePod(ctx, &api.Pod{ TypeMeta: api.TypeMeta{ @@ -156,15 +156,15 @@ func TestEtcdCreatePod(t *testing.T) { if pod.ID != "foo" { t.Errorf("Unexpected pod: %#v %s", pod, resp.Node.Value) } - var manifests api.ContainerManifestList + var boundPods api.BoundPods resp, err = fakeClient.Get("/registry/hosts/machine/kubelet", false, false) if err != nil { t.Errorf("unexpected error: %v", err) } - err = latest.Codec.DecodeInto([]byte(resp.Node.Value), &manifests) - if len(manifests.Items) != 1 || manifests.Items[0].ID != "foo" { - t.Errorf("Unexpected manifest list: %#v", manifests) + err = latest.Codec.DecodeInto([]byte(resp.Node.Value), &boundPods) + if len(boundPods.Items) != 1 || boundPods.Items[0].ID != "foo" { + t.Errorf("Unexpected boundPod list: %#v", boundPods) } } @@ -286,15 +286,15 @@ func TestEtcdCreatePodWithContainersNotFound(t *testing.T) { if pod.ID != "foo" { t.Errorf("Unexpected pod: %#v %s", pod, resp.Node.Value) } - var manifests api.ContainerManifestList + var boundPods api.BoundPods resp, err = fakeClient.Get("/registry/hosts/machine/kubelet", false, false) if err != nil { t.Errorf("unexpected error: %v", err) } - err = latest.Codec.DecodeInto([]byte(resp.Node.Value), &manifests) - if len(manifests.Items) != 1 || manifests.Items[0].ID != "foo" { - t.Errorf("Unexpected manifest list: %#v", manifests) + err = latest.Codec.DecodeInto([]byte(resp.Node.Value), &boundPods) + if len(boundPods.Items) != 1 || boundPods.Items[0].ID != "foo" { + t.Errorf("Unexpected boundPod list: %#v", boundPods) } } @@ -308,9 +308,9 @@ func TestEtcdCreatePodWithExistingContainers(t *testing.T) { }, E: tools.EtcdErrorNotFound, } - fakeClient.Set("/registry/hosts/machine/kubelet", runtime.EncodeOrDie(latest.Codec, &api.ContainerManifestList{ - Items: []api.ContainerManifest{ - {ID: "bar"}, + fakeClient.Set("/registry/hosts/machine/kubelet", runtime.EncodeOrDie(latest.Codec, &api.BoundPods{ + Items: []api.BoundPod{ + {TypeMeta: api.TypeMeta{ID: "bar"}}, }, }), 0) registry := NewTestEtcdRegistry(fakeClient) @@ -352,15 +352,15 @@ func TestEtcdCreatePodWithExistingContainers(t *testing.T) { if pod.ID != "foo" { t.Errorf("Unexpected pod: %#v %s", pod, resp.Node.Value) } - var manifests api.ContainerManifestList + var boundPods api.BoundPods resp, err = fakeClient.Get("/registry/hosts/machine/kubelet", false, false) if err != nil { t.Errorf("unexpected error: %v", err) } - err = latest.Codec.DecodeInto([]byte(resp.Node.Value), &manifests) - if len(manifests.Items) != 2 || manifests.Items[1].ID != "foo" { - t.Errorf("Unexpected manifest list: %#v", manifests) + err = latest.Codec.DecodeInto([]byte(resp.Node.Value), &boundPods) + if len(boundPods.Items) != 2 || boundPods.Items[1].ID != "foo" { + t.Errorf("Unexpected boundPod list: %#v", boundPods) } } @@ -374,9 +374,9 @@ func TestEtcdDeletePod(t *testing.T) { TypeMeta: api.TypeMeta{ID: "foo"}, DesiredState: api.PodState{Host: "machine"}, }), 0) - fakeClient.Set("/registry/hosts/machine/kubelet", runtime.EncodeOrDie(latest.Codec, &api.ContainerManifestList{ - Items: []api.ContainerManifest{ - {ID: "foo"}, + fakeClient.Set("/registry/hosts/machine/kubelet", runtime.EncodeOrDie(latest.Codec, &api.BoundPods{ + Items: []api.BoundPod{ + {TypeMeta: api.TypeMeta{ID: "foo"}}, }, }), 0) registry := NewTestEtcdRegistry(fakeClient) @@ -394,9 +394,9 @@ func TestEtcdDeletePod(t *testing.T) { if err != nil { t.Fatalf("Unexpected error %v", err) } - var manifests api.ContainerManifestList - latest.Codec.DecodeInto([]byte(response.Node.Value), &manifests) - if len(manifests.Items) != 0 { + var boundPods api.BoundPods + latest.Codec.DecodeInto([]byte(response.Node.Value), &boundPods) + if len(boundPods.Items) != 0 { t.Errorf("Unexpected container set: %s, expected empty", response.Node.Value) } } @@ -411,10 +411,10 @@ func TestEtcdDeletePodMultipleContainers(t *testing.T) { TypeMeta: api.TypeMeta{ID: "foo"}, DesiredState: api.PodState{Host: "machine"}, }), 0) - fakeClient.Set("/registry/hosts/machine/kubelet", runtime.EncodeOrDie(latest.Codec, &api.ContainerManifestList{ - Items: []api.ContainerManifest{ - {ID: "foo"}, - {ID: "bar"}, + fakeClient.Set("/registry/hosts/machine/kubelet", runtime.EncodeOrDie(latest.Codec, &api.BoundPods{ + Items: []api.BoundPod{ + {TypeMeta: api.TypeMeta{ID: "foo"}}, + {TypeMeta: api.TypeMeta{ID: "bar"}}, }, }), 0) registry := NewTestEtcdRegistry(fakeClient) @@ -433,13 +433,13 @@ func TestEtcdDeletePodMultipleContainers(t *testing.T) { if err != nil { t.Fatalf("Unexpected error %v", err) } - var manifests api.ContainerManifestList - latest.Codec.DecodeInto([]byte(response.Node.Value), &manifests) - if len(manifests.Items) != 1 { - t.Fatalf("Unexpected manifest set: %#v, expected empty", manifests) + var boundPods api.BoundPods + latest.Codec.DecodeInto([]byte(response.Node.Value), &boundPods) + if len(boundPods.Items) != 1 { + t.Fatalf("Unexpected boundPod set: %#v, expected empty", boundPods) } - if manifests.Items[0].ID != "bar" { - t.Errorf("Deleted wrong manifest: %#v", manifests) + if boundPods.Items[0].ID != "bar" { + t.Errorf("Deleted wrong boundPod: %#v", boundPods) } } diff --git a/pkg/registry/pod/manifest_factory.go b/pkg/registry/pod/boundpodfactory.go similarity index 67% rename from pkg/registry/pod/manifest_factory.go rename to pkg/registry/pod/boundpodfactory.go index db34efa43e069..00030acac5875 100644 --- a/pkg/registry/pod/manifest_factory.go +++ b/pkg/registry/pod/boundpodfactory.go @@ -21,24 +21,27 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/service" ) -type ManifestFactory interface { +type BoundPodFactory interface { // Make a container object for a given pod, given the machine that the pod is running on. - MakeManifest(machine string, pod api.Pod) (api.ContainerManifest, error) + MakeBoundPod(machine string, pod *api.Pod) (*api.BoundPod, error) } -type BasicManifestFactory struct { +type BasicBoundPodFactory struct { // TODO: this should really point at the API rather than a registry ServiceRegistry service.Registry } -func (b *BasicManifestFactory) MakeManifest(machine string, pod api.Pod) (api.ContainerManifest, error) { +func (b *BasicBoundPodFactory) MakeBoundPod(machine string, pod *api.Pod) (*api.BoundPod, error) { envVars, err := service.GetServiceEnvironmentVariables(api.NewContext(), b.ServiceRegistry, machine) if err != nil { - return api.ContainerManifest{}, err + return nil, err } - for ix, container := range pod.DesiredState.Manifest.Containers { - pod.DesiredState.Manifest.ID = pod.ID - pod.DesiredState.Manifest.Containers[ix].Env = append(container.Env, envVars...) + boundPod := &api.BoundPod{} + if err := api.Scheme.Convert(pod, boundPod); err != nil { + return nil, err } - return pod.DesiredState.Manifest, nil + for ix, container := range boundPod.Spec.Containers { + boundPod.Spec.Containers[ix].Env = append(container.Env, envVars...) + } + return boundPod, nil } diff --git a/pkg/registry/pod/manifest_factory_test.go b/pkg/registry/pod/boundpodfactory_test.go similarity index 79% rename from pkg/registry/pod/manifest_factory_test.go rename to pkg/registry/pod/boundpodfactory_test.go index af1ccec3a80bb..5f2daac20ddb5 100644 --- a/pkg/registry/pod/manifest_factory_test.go +++ b/pkg/registry/pod/boundpodfactory_test.go @@ -25,13 +25,13 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) -func TestMakeManifestNoServices(t *testing.T) { +func TestMakeBoundPodNoServices(t *testing.T) { registry := registrytest.ServiceRegistry{} - factory := &BasicManifestFactory{ + factory := &BasicBoundPodFactory{ ServiceRegistry: ®istry, } - manifest, err := factory.MakeManifest("machine", api.Pod{ + pod, err := factory.MakeBoundPod("machine", &api.Pod{ TypeMeta: api.TypeMeta{ID: "foobar"}, DesiredState: api.PodState{ Manifest: api.ContainerManifest{ @@ -44,21 +44,21 @@ func TestMakeManifestNoServices(t *testing.T) { }, }) if err != nil { - t.Errorf("unexpected error: %v", err) + t.Fatalf("unexpected error: %v", err) } - container := manifest.Containers[0] + container := pod.Spec.Containers[0] if len(container.Env) != 1 || container.Env[0].Name != "SERVICE_HOST" || container.Env[0].Value != "machine" { - t.Errorf("Expected one env vars, got: %#v", manifest) + t.Errorf("Expected one env vars, got: %#v", pod) } - if manifest.ID != "foobar" { - t.Errorf("Failed to assign ID to manifest: %#v", manifest.ID) + if pod.ID != "foobar" { + t.Errorf("Failed to assign ID to pod: %#v", pod.ID) } } -func TestMakeManifestServices(t *testing.T) { +func TestMakeBoundPodServices(t *testing.T) { registry := registrytest.ServiceRegistry{ List: api.ServiceList{ Items: []api.Service{ @@ -73,11 +73,11 @@ func TestMakeManifestServices(t *testing.T) { }, }, } - factory := &BasicManifestFactory{ + factory := &BasicBoundPodFactory{ ServiceRegistry: ®istry, } - manifest, err := factory.MakeManifest("machine", api.Pod{ + pod, err := factory.MakeBoundPod("machine", &api.Pod{ DesiredState: api.PodState{ Manifest: api.ContainerManifest{ Containers: []api.Container{ @@ -89,10 +89,10 @@ func TestMakeManifestServices(t *testing.T) { }, }) if err != nil { - t.Errorf("unexpected error: %v", err) + t.Fatalf("unexpected error: %v", err) } - container := manifest.Containers[0] + container := pod.Spec.Containers[0] envs := []api.EnvVar{ { Name: "TEST_SERVICE_HOST", @@ -128,8 +128,7 @@ func TestMakeManifestServices(t *testing.T) { }, } if len(container.Env) != len(envs) { - t.Errorf("Expected %d env vars, got %d: %#v", len(envs), len(container.Env), manifest) - return + t.Fatalf("Expected %d env vars, got %d: %#v", len(envs), len(container.Env), pod) } for ix := range container.Env { if !reflect.DeepEqual(envs[ix], container.Env[ix]) { @@ -138,7 +137,7 @@ func TestMakeManifestServices(t *testing.T) { } } -func TestMakeManifestServicesExistingEnvVar(t *testing.T) { +func TestMakeBoundPodServicesExistingEnvVar(t *testing.T) { registry := registrytest.ServiceRegistry{ List: api.ServiceList{ Items: []api.Service{ @@ -153,11 +152,11 @@ func TestMakeManifestServicesExistingEnvVar(t *testing.T) { }, }, } - factory := &BasicManifestFactory{ + factory := &BasicBoundPodFactory{ ServiceRegistry: ®istry, } - manifest, err := factory.MakeManifest("machine", api.Pod{ + pod, err := factory.MakeBoundPod("machine", &api.Pod{ DesiredState: api.PodState{ Manifest: api.ContainerManifest{ Containers: []api.Container{ @@ -174,10 +173,10 @@ func TestMakeManifestServicesExistingEnvVar(t *testing.T) { }, }) if err != nil { - t.Errorf("unexpected error: %v", err) + t.Fatalf("unexpected error: %v", err) } - container := manifest.Containers[0] + container := pod.Spec.Containers[0] envs := []api.EnvVar{ { @@ -218,8 +217,7 @@ func TestMakeManifestServicesExistingEnvVar(t *testing.T) { }, } if len(container.Env) != len(envs) { - t.Errorf("Expected %d env vars, got: %#v", len(envs), manifest) - return + t.Fatalf("Expected %d env vars, got: %#v", len(envs), pod) } for ix := range container.Env { if !reflect.DeepEqual(envs[ix], container.Env[ix]) {