From 42da186b629b7edaaeada9acfdaa0b610e0e842f Mon Sep 17 00:00:00 2001 From: Brad Hoekstra Date: Fri, 21 Sep 2018 20:06:32 -0400 Subject: [PATCH] Address review comments --- pkg/api/testing/pod_specs.go | 4 ++-- pkg/api/testing/serialization_test.go | 2 +- pkg/apis/apps/v1/defaults_test.go | 6 +++--- pkg/apis/apps/v1beta1/defaults_test.go | 2 +- pkg/apis/apps/v1beta2/defaults_test.go | 6 +++--- pkg/apis/core/fuzzer/fuzzer.go | 3 ++- pkg/apis/core/types.go | 5 ----- pkg/apis/core/v1/defaults.go | 3 +-- pkg/apis/extensions/v1beta1/defaults_test.go | 4 ++-- pkg/kubectl/cmd/util/helpers_test.go | 3 ++- pkg/kubelet/config/common_test.go | 4 ++-- pkg/kubelet/config/file_linux_test.go | 2 +- pkg/kubelet/config/http_test.go | 2 +- pkg/kubelet/kubelet_pods.go | 6 +++--- pkg/registry/core/pod/storage/storage_test.go | 6 +++--- 15 files changed, 27 insertions(+), 31 deletions(-) diff --git a/pkg/api/testing/pod_specs.go b/pkg/api/testing/pod_specs.go index c76d47fdeae..9c3d0dcb7bf 100644 --- a/pkg/api/testing/pod_specs.go +++ b/pkg/api/testing/pod_specs.go @@ -24,7 +24,7 @@ import ( // DeepEqualSafePodSpec returns a PodSpec which is ready to be used with apiequality.Semantic.DeepEqual func DeepEqualSafePodSpec() api.PodSpec { grace := int64(30) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks return api.PodSpec{ RestartPolicy: api.RestartPolicyAlways, DNSPolicy: api.DNSClusterFirst, @@ -38,7 +38,7 @@ func DeepEqualSafePodSpec() api.PodSpec { // V1DeepEqualSafePodSpec returns a PodSpec which is ready to be used with apiequality.Semantic.DeepEqual func V1DeepEqualSafePodSpec() v1.PodSpec { grace := int64(30) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks return v1.PodSpec{ RestartPolicy: v1.RestartPolicyAlways, DNSPolicy: v1.DNSClusterFirst, diff --git a/pkg/api/testing/serialization_test.go b/pkg/api/testing/serialization_test.go index 652e47ac8c1..2e4a3cd8ff2 100644 --- a/pkg/api/testing/serialization_test.go +++ b/pkg/api/testing/serialization_test.go @@ -214,7 +214,7 @@ func TestRoundTripTypes(t *testing.T) { // decoded without information loss or mutation. func TestEncodePtr(t *testing.T) { grace := int64(30) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks pod := &api.Pod{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"name": "foo"}, diff --git a/pkg/apis/apps/v1/defaults_test.go b/pkg/apis/apps/v1/defaults_test.go index 6a07b92b171..04322523272 100644 --- a/pkg/apis/apps/v1/defaults_test.go +++ b/pkg/apis/apps/v1/defaults_test.go @@ -39,7 +39,7 @@ func TestSetDefaultDaemonSetSpec(t *testing.T) { defaultLabels := map[string]string{"foo": "bar"} maxUnavailable := intstr.FromInt(1) period := int64(v1.DefaultTerminationGracePeriodSeconds) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks defaultTemplate := v1.PodTemplateSpec{ Spec: v1.PodSpec{ DNSPolicy: v1.DNSClusterFirst, @@ -178,7 +178,7 @@ func TestSetDefaultStatefulSet(t *testing.T) { var defaultReplicas int32 = 1 period := int64(v1.DefaultTerminationGracePeriodSeconds) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks defaultTemplate := v1.PodTemplateSpec{ Spec: v1.PodSpec{ DNSPolicy: v1.DNSClusterFirst, @@ -291,7 +291,7 @@ func TestSetDefaultDeployment(t *testing.T) { defaultIntOrString := intstr.FromString("25%") differentIntOrString := intstr.FromInt(5) period := int64(v1.DefaultTerminationGracePeriodSeconds) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks defaultTemplate := v1.PodTemplateSpec{ Spec: v1.PodSpec{ DNSPolicy: v1.DNSClusterFirst, diff --git a/pkg/apis/apps/v1beta1/defaults_test.go b/pkg/apis/apps/v1beta1/defaults_test.go index 9cf5e1287a6..210339607ca 100644 --- a/pkg/apis/apps/v1beta1/defaults_test.go +++ b/pkg/apis/apps/v1beta1/defaults_test.go @@ -38,7 +38,7 @@ func TestSetDefaultDeployment(t *testing.T) { defaultIntOrString := intstr.FromString("25%") differentIntOrString := intstr.FromInt(5) period := int64(v1.DefaultTerminationGracePeriodSeconds) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks defaultTemplate := v1.PodTemplateSpec{ Spec: v1.PodSpec{ DNSPolicy: v1.DNSClusterFirst, diff --git a/pkg/apis/apps/v1beta2/defaults_test.go b/pkg/apis/apps/v1beta2/defaults_test.go index 0a61138d725..4258f20d896 100644 --- a/pkg/apis/apps/v1beta2/defaults_test.go +++ b/pkg/apis/apps/v1beta2/defaults_test.go @@ -39,7 +39,7 @@ func TestSetDefaultDaemonSetSpec(t *testing.T) { defaultLabels := map[string]string{"foo": "bar"} maxUnavailable := intstr.FromInt(1) period := int64(v1.DefaultTerminationGracePeriodSeconds) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks defaultTemplate := v1.PodTemplateSpec{ Spec: v1.PodSpec{ DNSPolicy: v1.DNSClusterFirst, @@ -178,7 +178,7 @@ func TestSetDefaultStatefulSet(t *testing.T) { var defaultReplicas int32 = 1 period := int64(v1.DefaultTerminationGracePeriodSeconds) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks defaultTemplate := v1.PodTemplateSpec{ Spec: v1.PodSpec{ DNSPolicy: v1.DNSClusterFirst, @@ -291,7 +291,7 @@ func TestSetDefaultDeployment(t *testing.T) { defaultIntOrString := intstr.FromString("25%") differentIntOrString := intstr.FromInt(5) period := int64(v1.DefaultTerminationGracePeriodSeconds) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks defaultTemplate := v1.PodTemplateSpec{ Spec: v1.PodSpec{ DNSPolicy: v1.DNSClusterFirst, diff --git a/pkg/apis/core/fuzzer/fuzzer.go b/pkg/apis/core/fuzzer/fuzzer.go index a5899cf4d1f..cc18ed395d8 100644 --- a/pkg/apis/core/fuzzer/fuzzer.go +++ b/pkg/apis/core/fuzzer/fuzzer.go @@ -23,6 +23,7 @@ import ( fuzz "github.com/google/gofuzz" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -85,7 +86,7 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} { s.SchedulerName = core.DefaultSchedulerName } if s.EnableServiceLinks == nil { - enableServiceLinks := core.DefaultEnableServiceLinks + enableServiceLinks := corev1.DefaultEnableServiceLinks s.EnableServiceLinks = &enableServiceLinks } }, diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index d13429a0df1..72c8a966127 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -2604,11 +2604,6 @@ type PodSpec struct { EnableServiceLinks *bool } -const ( - // The default value for enableServiceLinks attribute. - DefaultEnableServiceLinks = true -) - // HostAlias holds the mapping between IP and hostnames that will be injected as an entry in the // pod's hosts file. type HostAlias struct { diff --git a/pkg/apis/core/v1/defaults.go b/pkg/apis/core/v1/defaults.go index d95036911b5..3edf9170017 100644 --- a/pkg/apis/core/v1/defaults.go +++ b/pkg/apis/core/v1/defaults.go @@ -23,7 +23,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" utilfeature "k8s.io/apiserver/pkg/util/feature" - "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/util/parsers" utilpointer "k8s.io/utils/pointer" @@ -184,7 +183,7 @@ func SetDefaults_PodSpec(obj *v1.PodSpec) { obj.SchedulerName = v1.DefaultSchedulerName } if obj.EnableServiceLinks == nil { - enableServiceLinks := core.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks obj.EnableServiceLinks = &enableServiceLinks } } diff --git a/pkg/apis/extensions/v1beta1/defaults_test.go b/pkg/apis/extensions/v1beta1/defaults_test.go index 6073813255b..fa9b1b6166e 100644 --- a/pkg/apis/extensions/v1beta1/defaults_test.go +++ b/pkg/apis/extensions/v1beta1/defaults_test.go @@ -40,7 +40,7 @@ import ( func TestSetDefaultDaemonSetSpec(t *testing.T) { defaultLabels := map[string]string{"foo": "bar"} period := int64(v1.DefaultTerminationGracePeriodSeconds) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks defaultTemplate := v1.PodTemplateSpec{ Spec: v1.PodSpec{ DNSPolicy: v1.DNSClusterFirst, @@ -167,7 +167,7 @@ func TestSetDefaultDeployment(t *testing.T) { defaultIntOrString := intstr.FromInt(1) differentIntOrString := intstr.FromInt(5) period := int64(v1.DefaultTerminationGracePeriodSeconds) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks defaultTemplate := v1.PodTemplateSpec{ Spec: v1.PodSpec{ DNSPolicy: v1.DNSClusterFirst, diff --git a/pkg/kubectl/cmd/util/helpers_test.go b/pkg/kubectl/cmd/util/helpers_test.go index f1774d68a01..c4ae341915c 100644 --- a/pkg/kubectl/cmd/util/helpers_test.go +++ b/pkg/kubectl/cmd/util/helpers_test.go @@ -24,6 +24,7 @@ import ( "syscall" "testing" + "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -39,7 +40,7 @@ import ( func TestMerge(t *testing.T) { grace := int64(30) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks tests := []struct { obj runtime.Object fragment string diff --git a/pkg/kubelet/config/common_test.go b/pkg/kubelet/config/common_test.go index 3c980f4f40d..709fdcf3315 100644 --- a/pkg/kubelet/config/common_test.go +++ b/pkg/kubelet/config/common_test.go @@ -37,7 +37,7 @@ func noDefault(*core.Pod) error { return nil } func TestDecodeSinglePod(t *testing.T) { grace := int64(30) - enableServiceLinks := core.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks pod := &v1.Pod{ TypeMeta: metav1.TypeMeta{ APIVersion: "", @@ -101,7 +101,7 @@ func TestDecodeSinglePod(t *testing.T) { func TestDecodePodList(t *testing.T) { grace := int64(30) - enableServiceLinks := core.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks pod := &v1.Pod{ TypeMeta: metav1.TypeMeta{ APIVersion: "", diff --git a/pkg/kubelet/config/file_linux_test.go b/pkg/kubelet/config/file_linux_test.go index 1d718a8c7b4..4c26b0ed819 100644 --- a/pkg/kubelet/config/file_linux_test.go +++ b/pkg/kubelet/config/file_linux_test.go @@ -140,7 +140,7 @@ type testCase struct { func getTestCases(hostname types.NodeName) []*testCase { grace := int64(30) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks return []*testCase{ { lock: &sync.Mutex{}, diff --git a/pkg/kubelet/config/http_test.go b/pkg/kubelet/config/http_test.go index c061fcd2533..3b7f00b0be6 100644 --- a/pkg/kubelet/config/http_test.go +++ b/pkg/kubelet/config/http_test.go @@ -129,7 +129,7 @@ func TestExtractPodsFromHTTP(t *testing.T) { nodeName := "different-value" grace := int64(30) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks var testCases = []struct { desc string pods runtime.Object diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index d77c945712c..dadad8d9c1d 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -488,7 +488,7 @@ var masterServices = sets.NewString("kubernetes") // getServiceEnvVarMap makes a map[string]string of env vars for services a // pod in namespace ns should see. -func (kl *Kubelet) getServiceEnvVarMap(ns string, enableServiceLinks *bool) (map[string]string, error) { +func (kl *Kubelet) getServiceEnvVarMap(ns string, enableServiceLinks bool) (map[string]string, error) { var ( serviceMap = make(map[string]*v1.Service) m = make(map[string]string) @@ -522,7 +522,7 @@ func (kl *Kubelet) getServiceEnvVarMap(ns string, enableServiceLinks *bool) (map if _, exists := serviceMap[serviceName]; !exists { serviceMap[serviceName] = service } - } else if service.Namespace == ns && *enableServiceLinks { + } else if service.Namespace == ns && enableServiceLinks { serviceMap[serviceName] = service } } @@ -550,7 +550,7 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *v1.Pod, container *v1.Container // To avoid this users can: (1) wait between starting a service and starting; or (2) detect // missing service env var and exit and be restarted; or (3) use DNS instead of env vars // and keep trying to resolve the DNS name of the service (recommended). - serviceEnv, err := kl.getServiceEnvVarMap(pod.Namespace, pod.Spec.EnableServiceLinks) + serviceEnv, err := kl.getServiceEnvVarMap(pod.Namespace, *pod.Spec.EnableServiceLinks) if err != nil { return result, err } diff --git a/pkg/registry/core/pod/storage/storage_test.go b/pkg/registry/core/pod/storage/storage_test.go index 2c23e8c0b90..c806fe6441c 100644 --- a/pkg/registry/core/pod/storage/storage_test.go +++ b/pkg/registry/core/pod/storage/storage_test.go @@ -61,7 +61,7 @@ func newStorage(t *testing.T) (*REST, *BindingREST, *StatusREST, *etcdtesting.Et func validNewPod() *api.Pod { grace := int64(30) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks return &api.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "foo", @@ -834,7 +834,7 @@ func TestEtcdUpdateScheduled(t *testing.T) { } grace := int64(30) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks podIn := api.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "foo", @@ -936,7 +936,7 @@ func TestEtcdUpdateStatus(t *testing.T) { expected := podStart expected.ResourceVersion = "2" grace := int64(30) - enableServiceLinks := api.DefaultEnableServiceLinks + enableServiceLinks := v1.DefaultEnableServiceLinks expected.Spec.TerminationGracePeriodSeconds = &grace expected.Spec.RestartPolicy = api.RestartPolicyAlways expected.Spec.DNSPolicy = api.DNSClusterFirst