Address review comments

This commit is contained in:
Brad Hoekstra 2018-09-21 20:06:32 -04:00
parent c4ec40eca8
commit 42da186b62
15 changed files with 27 additions and 31 deletions

View File

@ -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,

View File

@ -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"},

View File

@ -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,

View File

@ -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,

View File

@ -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,

View File

@ -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
}
},

View File

@ -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 {

View File

@ -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
}
}

View File

@ -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,

View File

@ -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

View File

@ -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: "",

View File

@ -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{},

View File

@ -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

View File

@ -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
}

View File

@ -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