From 5b4569697f610b2868edf5e0ee8d2a6cca6d60d5 Mon Sep 17 00:00:00 2001 From: Eric Tune Date: Wed, 11 Feb 2015 15:40:54 -0800 Subject: [PATCH] Stop putting env vars into BoundPods. They will still show up in etcd. They never were available through the API. A subsequent PR(s) will rip out all BoundPods code. Working in small increments. This PR will cause users on lagging cloud providers to not get env vars in their pods if they update to this code. They have already been warned via email. Removed unit tests of BasicBoundPodFactory. There is adequate coverage in pkg/kubelet/kubelet_test.go. --- pkg/master/master.go | 11 +- pkg/registry/etcd/etcd_test.go | 5 +- pkg/registry/pod/bound_pod_factory.go | 50 +-- pkg/registry/pod/bound_pod_factory_test.go | 401 +-------------------- 4 files changed, 6 insertions(+), 461 deletions(-) diff --git a/pkg/master/master.go b/pkg/master/master.go index c86ef744b10..18bfd4d89fc 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -253,12 +253,7 @@ func setDefaults(c *Config) { // any unhandled paths to "Handler". func New(c *Config) *Master { setDefaults(c) - minionRegistry := etcd.NewRegistry(c.EtcdHelper, nil) - serviceRegistry := etcd.NewRegistry(c.EtcdHelper, nil) - boundPodFactory := &pod.BasicBoundPodFactory{ - ServiceRegistry: serviceRegistry, - MasterServiceNamespace: c.MasterServiceNamespace, - } + boundPodFactory := &pod.BasicBoundPodFactory{} if c.KubeletClient == nil { glog.Fatalf("master.New() called with config.KubeletClient == nil") } @@ -277,12 +272,12 @@ func New(c *Config) *Master { m := &Master{ podRegistry: etcd.NewRegistry(c.EtcdHelper, boundPodFactory), controllerRegistry: etcd.NewRegistry(c.EtcdHelper, nil), - serviceRegistry: serviceRegistry, + serviceRegistry: etcd.NewRegistry(c.EtcdHelper, nil), endpointRegistry: etcd.NewRegistry(c.EtcdHelper, nil), bindingRegistry: etcd.NewRegistry(c.EtcdHelper, boundPodFactory), eventRegistry: event.NewEtcdRegistry(c.EtcdHelper, uint64(c.EventTTL.Seconds())), namespaceRegistry: namespace.NewEtcdRegistry(c.EtcdHelper), - minionRegistry: minionRegistry, + minionRegistry: etcd.NewRegistry(c.EtcdHelper, nil), limitRangeRegistry: limitrange.NewEtcdRegistry(c.EtcdHelper), resourceQuotaRegistry: resourcequota.NewEtcdRegistry(c.EtcdHelper), client: c.Client, diff --git a/pkg/registry/etcd/etcd_test.go b/pkg/registry/etcd/etcd_test.go index ef13c2dfd13..50626de197c 100644 --- a/pkg/registry/etcd/etcd_test.go +++ b/pkg/registry/etcd/etcd_test.go @@ -27,7 +27,6 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/pod" - "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/registrytest" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/tools" @@ -36,9 +35,7 @@ import ( func NewTestEtcdRegistry(client tools.EtcdClient) *Registry { registry := NewRegistry(tools.EtcdHelper{client, latest.Codec, tools.RuntimeVersionAdapter{latest.ResourceVersioner}}, - &pod.BasicBoundPodFactory{ - ServiceRegistry: ®istrytest.ServiceRegistry{}, - }) + &pod.BasicBoundPodFactory{}) return registry } diff --git a/pkg/registry/pod/bound_pod_factory.go b/pkg/registry/pod/bound_pod_factory.go index 2664aaa4438..d421d1def6e 100644 --- a/pkg/registry/pod/bound_pod_factory.go +++ b/pkg/registry/pod/bound_pod_factory.go @@ -18,9 +18,6 @@ package pod import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/envvars" - "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/service" - "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) type BoundPodFactory interface { @@ -28,58 +25,13 @@ type BoundPodFactory interface { MakeBoundPod(machine string, pod *api.Pod) (*api.BoundPod, error) } -type BasicBoundPodFactory struct { - // TODO: this should really point at the API rather than a registry - ServiceRegistry service.Registry - MasterServiceNamespace string -} - -var masterServiceNames = util.NewStringSet("kubernetes", "kubernetes-ro") - -// getServiceEnvironmentVariables populates a list of environment variables that are used -// in the container environment to get access to services. -func (b *BasicBoundPodFactory) getServiceEnvironmentVariables(ctx api.Context, registry service.Registry, machine string) ([]api.EnvVar, error) { - var result []api.EnvVar - servicesInNs, err := registry.ListServices(ctx) - if err != nil { - return result, err - } - - masterServices, err := registry.ListServices(api.WithNamespace(api.NewContext(), b.MasterServiceNamespace)) - if err != nil { - return result, err - } - - projection := map[string]api.Service{} - services := []api.Service{} - for _, service := range masterServices.Items { - if masterServiceNames.Has(service.Name) { - projection[service.Name] = service - } - } - for _, service := range servicesInNs.Items { - projection[service.Name] = service - } - for _, service := range projection { - services = append(services, service) - } - - return envvars.FromServices(&api.ServiceList{Items: services}), nil -} +type BasicBoundPodFactory struct{} func (b *BasicBoundPodFactory) MakeBoundPod(machine string, pod *api.Pod) (*api.BoundPod, error) { - envVars, err := b.getServiceEnvironmentVariables(api.WithNamespace(api.NewContext(), pod.Namespace), b.ServiceRegistry, machine) - if err != nil { - return nil, err - } - boundPod := &api.BoundPod{} if err := api.Scheme.Convert(pod, boundPod); err != nil { return nil, err } - 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. 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 84323bd16a2..68336e0bb15 100644 --- a/pkg/registry/pod/bound_pod_factory_test.go +++ b/pkg/registry/pod/bound_pod_factory_test.go @@ -17,19 +17,13 @@ limitations under the License. package pod import ( - "reflect" "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/registrytest" ) func TestMakeBoundPodNoServices(t *testing.T) { - registry := registrytest.ServiceRegistry{} - factory := &BasicBoundPodFactory{ - ServiceRegistry: ®istry, - MasterServiceNamespace: api.NamespaceDefault, - } + factory := &BasicBoundPodFactory{} pod, err := factory.MakeBoundPod("machine", &api.Pod{ ObjectMeta: api.ObjectMeta{Name: "foobar"}, @@ -57,396 +51,3 @@ func TestMakeBoundPodNoServices(t *testing.T) { t.Errorf("Unable to get a reference to bound pod: %v", err) } } - -func TestMakeBoundPodServices(t *testing.T) { - registry := registrytest.ServiceRegistry{ - List: api.ServiceList{ - Items: []api.Service{ - { - ObjectMeta: api.ObjectMeta{Name: "test", Namespace: "test"}, - Spec: api.ServiceSpec{ - Port: 8080, - PortalIP: "1.2.3.4", - }, - }, - }, - }, - } - factory := &BasicBoundPodFactory{ - ServiceRegistry: ®istry, - MasterServiceNamespace: api.NamespaceDefault, - } - - pod, err := factory.MakeBoundPod("machine", &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "foobar", Namespace: "test"}, - Spec: api.PodSpec{ - Containers: []api.Container{ - { - Name: "foo", - }, - }, - }, - }) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - container := pod.Spec.Containers[0] - envs := []api.EnvVar{ - { - Name: "TEST_SERVICE_HOST", - Value: "1.2.3.4", - }, - { - Name: "TEST_SERVICE_PORT", - Value: "8080", - }, - { - Name: "TEST_PORT", - Value: "tcp://1.2.3.4:8080", - }, - { - Name: "TEST_PORT_8080_TCP", - Value: "tcp://1.2.3.4:8080", - }, - { - Name: "TEST_PORT_8080_TCP_PROTO", - Value: "tcp", - }, - { - Name: "TEST_PORT_8080_TCP_PORT", - Value: "8080", - }, - { - Name: "TEST_PORT_8080_TCP_ADDR", - Value: "1.2.3.4", - }, - } - if len(container.Env) != len(envs) { - 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]) { - t.Errorf("expected %#v, got %#v", envs[ix], container.Env[ix]) - } - } -} - -func TestMakeBoundPodServicesExistingEnvVar(t *testing.T) { - registry := registrytest.ServiceRegistry{ - List: api.ServiceList{ - Items: []api.Service{ - { - ObjectMeta: api.ObjectMeta{Name: "test", Namespace: "test"}, - Spec: api.ServiceSpec{ - Port: 8080, - PortalIP: "1.2.3.4", - }, - }, - }, - }, - } - factory := &BasicBoundPodFactory{ - ServiceRegistry: ®istry, - MasterServiceNamespace: api.NamespaceDefault, - } - - pod, err := factory.MakeBoundPod("machine", &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "foobar", Namespace: "test"}, - Spec: api.PodSpec{ - Containers: []api.Container{ - { - Env: []api.EnvVar{ - { - Name: "foo", - Value: "bar", - }, - }, - }, - }, - }, - }) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - container := pod.Spec.Containers[0] - - envs := []api.EnvVar{ - { - Name: "foo", - Value: "bar", - }, - { - Name: "TEST_SERVICE_HOST", - Value: "1.2.3.4", - }, - { - Name: "TEST_SERVICE_PORT", - Value: "8080", - }, - { - Name: "TEST_PORT", - Value: "tcp://1.2.3.4:8080", - }, - { - Name: "TEST_PORT_8080_TCP", - Value: "tcp://1.2.3.4:8080", - }, - { - Name: "TEST_PORT_8080_TCP_PROTO", - Value: "tcp", - }, - { - Name: "TEST_PORT_8080_TCP_PORT", - Value: "8080", - }, - { - Name: "TEST_PORT_8080_TCP_ADDR", - Value: "1.2.3.4", - }, - } - if len(container.Env) != len(envs) { - t.Fatalf("Expected %d env vars, got: %#v", len(envs), pod) - } - for ix := range container.Env { - if !reflect.DeepEqual(envs[ix], container.Env[ix]) { - t.Errorf("expected %#v, got %#v", envs[ix], container.Env[ix]) - } - } -} - -func TestMakeBoundPodOnlyVisibleServices(t *testing.T) { - registry := registrytest.ServiceRegistry{ - List: api.ServiceList{ - Items: []api.Service{ - { - ObjectMeta: api.ObjectMeta{Name: "test", Namespace: api.NamespaceDefault}, - Spec: api.ServiceSpec{ - Port: 8080, - PortalIP: "1.2.3.4", - }, - }, - { - ObjectMeta: api.ObjectMeta{Name: "test", Namespace: "test"}, - Spec: api.ServiceSpec{ - Port: 8081, - PortalIP: "1.2.3.5", - }, - }, - { - ObjectMeta: api.ObjectMeta{Name: "test3", Namespace: "test"}, - Spec: api.ServiceSpec{ - Port: 8083, - PortalIP: "1.2.3.7", - }, - }, - }, - }, - } - factory := &BasicBoundPodFactory{ - ServiceRegistry: ®istry, - MasterServiceNamespace: api.NamespaceDefault, - } - - pod, err := factory.MakeBoundPod("machine", &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "foobar", Namespace: "test"}, - Spec: api.PodSpec{ - Containers: []api.Container{ - { - Name: "foo", - }, - }, - }, - }) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - container := pod.Spec.Containers[0] - envs := map[string]string{ - "TEST_SERVICE_HOST": "1.2.3.5", - "TEST_SERVICE_PORT": "8081", - "TEST_PORT": "tcp://1.2.3.5:8081", - "TEST_PORT_8081_TCP": "tcp://1.2.3.5:8081", - "TEST_PORT_8081_TCP_PROTO": "tcp", - "TEST_PORT_8081_TCP_PORT": "8081", - "TEST_PORT_8081_TCP_ADDR": "1.2.3.5", - "TEST3_SERVICE_HOST": "1.2.3.7", - "TEST3_SERVICE_PORT": "8083", - "TEST3_PORT": "tcp://1.2.3.7:8083", - "TEST3_PORT_8083_TCP": "tcp://1.2.3.7:8083", - "TEST3_PORT_8083_TCP_PROTO": "tcp", - "TEST3_PORT_8083_TCP_PORT": "8083", - "TEST3_PORT_8083_TCP_ADDR": "1.2.3.7", - } - - if len(container.Env) != len(envs) { - t.Fatalf("Expected %d env vars, got %d: %#v", len(envs), len(container.Env), pod) - } - for _, env := range container.Env { - expectedValue := envs[env.Name] - if expectedValue != env.Value { - t.Errorf("expected env %v value %v, got %v", env.Name, expectedValue, env.Value) - } - } -} - -func TestMakeBoundPodMasterServices(t *testing.T) { - registry := registrytest.ServiceRegistry{ - List: api.ServiceList{ - Items: []api.Service{ - { - ObjectMeta: api.ObjectMeta{Name: "kubernetes", Namespace: api.NamespaceDefault}, - Spec: api.ServiceSpec{ - Port: 8080, - PortalIP: "1.2.3.4", - }, - }, - { - ObjectMeta: api.ObjectMeta{Name: "test", Namespace: "test"}, - Spec: api.ServiceSpec{ - Port: 8081, - PortalIP: "1.2.3.5", - }, - }, - { - ObjectMeta: api.ObjectMeta{Name: "test3", Namespace: "test"}, - Spec: api.ServiceSpec{ - Port: 8083, - PortalIP: "1.2.3.7", - }, - }, - }, - }, - } - factory := &BasicBoundPodFactory{ - ServiceRegistry: ®istry, - MasterServiceNamespace: api.NamespaceDefault, - } - - pod, err := factory.MakeBoundPod("machine", &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "foobar", Namespace: "test"}, - Spec: api.PodSpec{ - Containers: []api.Container{ - { - Name: "foo", - }, - }, - }, - }) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - container := pod.Spec.Containers[0] - envs := map[string]string{ - "TEST_SERVICE_HOST": "1.2.3.5", - "TEST_SERVICE_PORT": "8081", - "TEST_PORT": "tcp://1.2.3.5:8081", - "TEST_PORT_8081_TCP": "tcp://1.2.3.5:8081", - "TEST_PORT_8081_TCP_PROTO": "tcp", - "TEST_PORT_8081_TCP_PORT": "8081", - "TEST_PORT_8081_TCP_ADDR": "1.2.3.5", - "TEST3_SERVICE_HOST": "1.2.3.7", - "TEST3_SERVICE_PORT": "8083", - "TEST3_PORT": "tcp://1.2.3.7:8083", - "TEST3_PORT_8083_TCP": "tcp://1.2.3.7:8083", - "TEST3_PORT_8083_TCP_PROTO": "tcp", - "TEST3_PORT_8083_TCP_PORT": "8083", - "TEST3_PORT_8083_TCP_ADDR": "1.2.3.7", - "KUBERNETES_SERVICE_HOST": "1.2.3.4", - "KUBERNETES_SERVICE_PORT": "8080", - "KUBERNETES_PORT": "tcp://1.2.3.4:8080", - "KUBERNETES_PORT_8080_TCP": "tcp://1.2.3.4:8080", - "KUBERNETES_PORT_8080_TCP_PROTO": "tcp", - "KUBERNETES_PORT_8080_TCP_PORT": "8080", - "KUBERNETES_PORT_8080_TCP_ADDR": "1.2.3.4", - } - - if len(container.Env) != len(envs) { - t.Fatalf("Expected %d env vars, got %d: %#v", len(envs), len(container.Env), pod) - } - for _, env := range container.Env { - expectedValue := envs[env.Name] - if expectedValue != env.Value { - t.Errorf("expected env %v value %v, got %v", env.Name, expectedValue, env.Value) - } - } -} - -func TestMakeBoundPodMasterServiceInNs(t *testing.T) { - registry := registrytest.ServiceRegistry{ - List: api.ServiceList{ - Items: []api.Service{ - { - ObjectMeta: api.ObjectMeta{Name: "kubernetes", Namespace: api.NamespaceDefault}, - Spec: api.ServiceSpec{ - Port: 8080, - PortalIP: "1.2.3.4", - }, - }, - { - ObjectMeta: api.ObjectMeta{Name: "test", Namespace: "test"}, - Spec: api.ServiceSpec{ - Port: 8081, - PortalIP: "1.2.3.5", - }, - }, - { - ObjectMeta: api.ObjectMeta{Name: "kubernetes", Namespace: "test"}, - Spec: api.ServiceSpec{ - Port: 8083, - PortalIP: "1.2.3.7", - }, - }, - }, - }, - } - factory := &BasicBoundPodFactory{ - ServiceRegistry: ®istry, - MasterServiceNamespace: api.NamespaceDefault, - } - - pod, err := factory.MakeBoundPod("machine", &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "foobar", Namespace: "test"}, - Spec: api.PodSpec{ - Containers: []api.Container{ - { - Name: "foo", - }, - }, - }, - }) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - container := pod.Spec.Containers[0] - envs := map[string]string{ - "TEST_SERVICE_HOST": "1.2.3.5", - "TEST_SERVICE_PORT": "8081", - "TEST_PORT": "tcp://1.2.3.5:8081", - "TEST_PORT_8081_TCP": "tcp://1.2.3.5:8081", - "TEST_PORT_8081_TCP_PROTO": "tcp", - "TEST_PORT_8081_TCP_PORT": "8081", - "TEST_PORT_8081_TCP_ADDR": "1.2.3.5", - "KUBERNETES_SERVICE_HOST": "1.2.3.7", - "KUBERNETES_SERVICE_PORT": "8083", - "KUBERNETES_PORT": "tcp://1.2.3.7:8083", - "KUBERNETES_PORT_8083_TCP": "tcp://1.2.3.7:8083", - "KUBERNETES_PORT_8083_TCP_PROTO": "tcp", - "KUBERNETES_PORT_8083_TCP_PORT": "8083", - "KUBERNETES_PORT_8083_TCP_ADDR": "1.2.3.7", - } - - if len(container.Env) != len(envs) { - t.Fatalf("Expected %d env vars, got %d: %#v", len(envs), len(container.Env), pod) - } - for _, env := range container.Env { - expectedValue := envs[env.Name] - if expectedValue != env.Value { - t.Errorf("expected env %v value %v, got %v", env.Name, expectedValue, env.Value) - } - } -}