diff --git a/pkg/api/pod/warnings.go b/pkg/api/pod/warnings.go index c30c99b02d3..84fb25a64d0 100644 --- a/pkg/api/pod/warnings.go +++ b/pkg/api/pod/warnings.go @@ -320,6 +320,21 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta return true }) + // Accumulate port names of containers and sidecar containers + allPortsNames := map[string]*field.Path{} + pods.VisitContainersWithPath(podSpec, fieldPath.Child("spec"), func(c *api.Container, fldPath *field.Path) bool { + for i, port := range c.Ports { + if port.Name != "" { + if other, found := allPortsNames[port.Name]; found { + warnings = append(warnings, fmt.Sprintf("%s: duplicate port name %q with %s, services and probes that select ports by name will use %s", fldPath.Child("ports").Index(i), port.Name, other, other)) + } else { + allPortsNames[port.Name] = fldPath.Child("ports").Index(i) + } + } + } + return true + }) + // warn if the terminationGracePeriodSeconds is negative. if podSpec.TerminationGracePeriodSeconds != nil && *podSpec.TerminationGracePeriodSeconds < 0 { warnings = append(warnings, fmt.Sprintf("%s: must be >= 0; negative values are invalid and will be treated as 1", fieldPath.Child("spec", "terminationGracePeriodSeconds"))) diff --git a/pkg/api/pod/warnings_test.go b/pkg/api/pod/warnings_test.go index f3f0687131d..b285a4b9f09 100644 --- a/pkg/api/pod/warnings_test.go +++ b/pkg/api/pod/warnings_test.go @@ -140,6 +140,7 @@ func BenchmarkWarnings(b *testing.B) { } func TestWarnings(t *testing.T) { + containerRestartPolicyAlways := api.ContainerRestartPolicyAlways resources := api.ResourceList{ api.ResourceCPU: resource.MustParse("100m"), api.ResourceMemory: resource.MustParse("4m"), @@ -1520,6 +1521,196 @@ func TestWarnings(t *testing.T) { `spec.containers[1].ports[1]: duplicate port definition with spec.containers[1].ports[0]`, }, }, + { + name: "create duplicate container ports name in two containers", + template: &api.PodTemplateSpec{Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "foo1", + Ports: []api.ContainerPort{ + {ContainerPort: 80, Protocol: api.ProtocolUDP, Name: "test"}, + }, + }, + { + Name: "foo", + Ports: []api.ContainerPort{ + {ContainerPort: 8090, Protocol: api.ProtocolTCP, Name: "test"}, + }, + }}, + }}, + expected: []string{ + `spec.containers[1].ports[0]: duplicate port name "test" with spec.containers[0].ports[0], services and probes that select ports by name will use spec.containers[0].ports[0]`, + }, + }, + { + name: "update duplicate container ports name in two containers", + template: &api.PodTemplateSpec{Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "foo1", + Ports: []api.ContainerPort{ + {ContainerPort: 80, Protocol: api.ProtocolUDP, Name: "test"}, + }, + }, + { + Name: "foo", + Ports: []api.ContainerPort{ + {ContainerPort: 8090, Protocol: api.ProtocolUDP, Name: "test1"}, + {ContainerPort: 8092, Protocol: api.ProtocolUDP, Name: "test"}, + }, + }}, + }}, + oldTemplate: &api.PodTemplateSpec{Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "foo1", + Ports: []api.ContainerPort{ + {ContainerPort: 80, Protocol: api.ProtocolUDP, Name: "test"}, + }, + }, + { + Name: "foo", + Ports: []api.ContainerPort{ + {ContainerPort: 8090, Protocol: api.ProtocolUDP, Name: "test1"}, + }, + }}, + }}, + expected: []string{ + `spec.containers[1].ports[1]: duplicate port name "test" with spec.containers[0].ports[0], services and probes that select ports by name will use spec.containers[0].ports[0]`, + }, + }, + { + name: "create duplicate container ports name in two sidecar containers", + template: &api.PodTemplateSpec{Spec: api.PodSpec{ + InitContainers: []api.Container{ + { + RestartPolicy: &containerRestartPolicyAlways, + Name: "foo1", + Ports: []api.ContainerPort{ + {ContainerPort: 80, Protocol: api.ProtocolUDP, Name: "test"}, + }, + }, + { + Name: "foo", + RestartPolicy: &containerRestartPolicyAlways, + Ports: []api.ContainerPort{ + {ContainerPort: 8090, Protocol: api.ProtocolTCP, Name: "test"}, + }, + }}, + }}, + expected: []string{ + `spec.initContainers[1].ports[0]: duplicate port name "test" with spec.initContainers[0].ports[0], services and probes that select ports by name will use spec.initContainers[0].ports[0]`, + }, + }, + { + name: "update duplicate container ports name in two sidecar containers", + template: &api.PodTemplateSpec{Spec: api.PodSpec{ + InitContainers: []api.Container{ + { + Name: "foo1", + RestartPolicy: &containerRestartPolicyAlways, + Ports: []api.ContainerPort{ + {ContainerPort: 80, Protocol: api.ProtocolUDP, Name: "test"}, + }, + }, + { + Name: "foo", + RestartPolicy: &containerRestartPolicyAlways, + Ports: []api.ContainerPort{ + {ContainerPort: 8090, Protocol: api.ProtocolUDP, Name: "test1"}, + {ContainerPort: 8091, Protocol: api.ProtocolUDP, Name: "test"}, + }, + }}, + }}, + oldTemplate: &api.PodTemplateSpec{Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "foo1", + RestartPolicy: &containerRestartPolicyAlways, + Ports: []api.ContainerPort{ + {ContainerPort: 80, Protocol: api.ProtocolUDP, Name: "test"}, + }, + }, + { + Name: "foo", + RestartPolicy: &containerRestartPolicyAlways, + Ports: []api.ContainerPort{ + {ContainerPort: 8090, Protocol: api.ProtocolUDP, Name: "test1"}, + }, + }}, + }}, + expected: []string{ + `spec.initContainers[1].ports[1]: duplicate port name "test" with spec.initContainers[0].ports[0], services and probes that select ports by name will use spec.initContainers[0].ports[0]`, + }, + }, + { + name: "create duplicate container ports name in containers and sidecar containers", + template: &api.PodTemplateSpec{Spec: api.PodSpec{ + InitContainers: []api.Container{ + { + RestartPolicy: &containerRestartPolicyAlways, + Name: "foo1", + Ports: []api.ContainerPort{ + {ContainerPort: 80, Protocol: api.ProtocolUDP, Name: "test"}, + }, + }, + }, + Containers: []api.Container{ + { + Name: "foo", + Ports: []api.ContainerPort{ + {ContainerPort: 8090, Protocol: api.ProtocolTCP, Name: "test"}, + }, + }}, + }}, + expected: []string{ + `spec.containers[0].ports[0]: duplicate port name "test" with spec.initContainers[0].ports[0], services and probes that select ports by name will use spec.initContainers[0].ports[0]`, + }, + }, + { + name: "update duplicate container ports name in containers and sidecar containers", + template: &api.PodTemplateSpec{Spec: api.PodSpec{ + InitContainers: []api.Container{ + { + Name: "foo1", + RestartPolicy: &containerRestartPolicyAlways, + Ports: []api.ContainerPort{ + {ContainerPort: 80, Protocol: api.ProtocolUDP, Name: "test"}, + }, + }, + }, + Containers: []api.Container{ + { + Name: "foo", + Ports: []api.ContainerPort{ + {ContainerPort: 8090, Protocol: api.ProtocolUDP, Name: "test1"}, + {ContainerPort: 8092, Protocol: api.ProtocolUDP, Name: "test"}, + }, + }, + }, + }}, + oldTemplate: &api.PodTemplateSpec{Spec: api.PodSpec{ + InitContainers: []api.Container{ + { + Name: "foo1", + RestartPolicy: &containerRestartPolicyAlways, + Ports: []api.ContainerPort{ + {ContainerPort: 80, Protocol: api.ProtocolUDP, Name: "test"}, + }, + }, + }, + Containers: []api.Container{ + { + Name: "foo", + Ports: []api.ContainerPort{ + {ContainerPort: 8090, Protocol: api.ProtocolUDP, Name: "test1"}, + }, + }}, + }}, + expected: []string{ + `spec.containers[0].ports[1]: duplicate port name "test" with spec.initContainers[0].ports[0], services and probes that select ports by name will use spec.initContainers[0].ports[0]`, + }, + }, } for _, tc := range testcases {