github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-08 11:38:15 +00:00
Code Issues Projects Releases Wiki Activity

add some detailed message for dup container ports(steal from thockin)

Browse Source
This commit is contained in:
Paco Xu 2023-07-12 16:39:03 +08:00
parent 7a80d7c839
commit df0d51d3b3
2 changed files with 74 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
pkg/api/pod/warnings.go
View File

@ -255,30 +255,38 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta
return true return true
}) })
type portBlock struct {
field *field.Path
port api.ContainerPort
}
// Accumulate ports across all containers // Accumulate ports across all containers
allPorts := map[string]portBlock{} allPorts := map[string][]portBlock{}
pods.VisitContainersWithPath(podSpec, fieldPath.Child("spec"), func(c *api.Container, fldPath *field.Path) bool { pods.VisitContainersWithPath(podSpec, fieldPath.Child("spec"), func(c *api.Container, fldPath *field.Path) bool {
for i, port := range c.Ports { for i, port := range c.Ports {
k := fmt.Sprintf("%d/%s", port.ContainerPort, port.Protocol) if port.HostIP != "" && port.HostPort == 0 {
if other, found := allPorts[k]; found { warnings = append(warnings, fmt.Sprintf("%s: hostIP set without hostPort: %+v",
// Someone else has this protcol+port, but it still might not be a conflict. fldPath.Child("ports").Index(i), port))
conflict := false }
k := fmt.Sprintf("%d/%s", port.ContainerPort, port.Protocol)
// Exact matches are obvious. if others, found := allPorts[k]; found {
if port.HostIP == other.port.HostIP && port.HostPort == other.port.HostPort { // Someone else has this protcol+port, but it still might not be a conflict.
conflict = true for _, other := range others {
} if port.HostIP == other.port.HostIP && port.HostPort == other.port.HostPort {
// Unspecified fields are more subtle. // Exactly-equal is obvious. Validation should already filter for this except when these are unspecified.
if port.HostPort == 0 || other.port.HostPort == 0 || port.HostIP == "" || other.port.HostIP == "" { warnings = append(warnings, fmt.Sprintf("%s: duplicate port definition with %s", fldPath.Child("ports").Index(i), other.field))
conflict = true } else if port.HostPort == 0 || other.port.HostPort == 0 {
} // HostPort = 0 is redundant with any other value, which is odd but not really dangerous. HostIP doesn't matter here.
if conflict { warnings = append(warnings, fmt.Sprintf("%s: overlapping port definition with %s", fldPath.Child("ports").Index(i), other.field))
warnings = append(warnings, fmt.Sprintf("%s: duplicate container port %d/%s, conflicts with %s", } else if a, b := port.HostIP == "", other.port.HostIP == ""; port.HostPort == other.port.HostPort && ((a || b) && !(a && b)) {
fldPath.Child("ports").Index(i), port.ContainerPort, port.Protocol, other.field)) // If the HostPorts are the same and either HostIP is not specified while the other is not, the behavior is undefined.
continue warnings = append(warnings, fmt.Sprintf("%s: dangerously ambiguous port definition with %s", fldPath.Child("ports").Index(i), other.field))
} }
}
allPorts[k] = append(allPorts[k], portBlock{field: fldPath.Child("ports").Index(i), port: port})
} else {
allPorts[k] = []portBlock{{field: fldPath.Child("ports").Index(i), port: port}}
} }
allPorts[k] = portBlock{field: fldPath, port: port}
} }
return true return true
}) })
@ -302,11 +310,6 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta
return warnings return warnings
} }
type portBlock struct {
field *field.Path
port api.ContainerPort
}
func warningsForPodAffinityTerms(terms []api.PodAffinityTerm, fieldPath *field.Path) []string { func warningsForPodAffinityTerms(terms []api.PodAffinityTerm, fieldPath *field.Path) []string {
var warnings []string var warnings []string
for i, t := range terms { for i, t := range terms {

72
pkg/api/pod/warnings_test.go
View File

@ -659,9 +659,7 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
}}, }},
expected: []string{ expected: []string{},
`spec.containers[0].ports[1]: duplicate container port 80/TCP, conflicts with spec.containers[0]`,
},
}, },
{ {
name: "one container, two ports, same protocol, port and hostPort, different hostIP", name: "one container, two ports, same protocol, port and hostPort, different hostIP",
@ -725,9 +723,7 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
}}, }},
expected: []string{ expected: []string{},
`spec.containers[1].ports[0]: duplicate container port 80/TCP, conflicts with spec.containers[0]`,
},
}, },
{ {
name: "two containers, one port each, same protocol, port and hostPort, different hostIP", name: "two containers, one port each, same protocol, port and hostPort, different hostIP",
@ -758,7 +754,7 @@ func TestWarnings(t *testing.T) {
}}, }},
}}, }},
expected: []string{ expected: []string{
`spec.containers[0].ports[1]: duplicate container port 80/UDP, conflicts with spec.containers[0]`, `spec.containers[0].ports[1]: duplicate port definition with spec.containers[0].ports[0]`,
}, },
}, },
{ {
@ -773,7 +769,7 @@ func TestWarnings(t *testing.T) {
}}, }},
}}, }},
expected: []string{ expected: []string{
`spec.containers[0].ports[1]: duplicate container port 80/UDP, conflicts with spec.containers[0]`, `spec.containers[0].ports[1]: duplicate port definition with spec.containers[0].ports[0]`,
}, },
}, },
{ {
@ -788,7 +784,21 @@ func TestWarnings(t *testing.T) {
}}, }},
}}, }},
expected: []string{ expected: []string{
`spec.containers[0].ports[1]: duplicate container port 80/UDP, conflicts with spec.containers[0]`, `spec.containers[0].ports[1]: duplicate port definition with spec.containers[0].ports[0]`,
},
},
{
name: "one container port hostIP set without host port set",
template: &api.PodTemplateSpec{Spec: api.PodSpec{
Containers: []api.Container{{
Name: "foo",
Ports: []api.ContainerPort{
{ContainerPort: 80, Protocol: api.ProtocolUDP, HostIP: "10.0.0.1"},
},
}},
}},
expected: []string{
`spec.containers[0].ports[0]: hostIP set without hostPort: {Name: HostPort:0 ContainerPort:80 Protocol:UDP HostIP:10.0.0.1}`,
}, },
}, },
{ {
@ -803,22 +813,25 @@ func TestWarnings(t *testing.T) {
}}, }},
}}, }},
expected: []string{ expected: []string{
`spec.containers[0].ports[1]: duplicate container port 80/UDP, conflicts with spec.containers[0]`, `spec.containers[0].ports[1]: overlapping port definition with spec.containers[0].ports[0]`,
`spec.containers[0].ports[1]: hostIP set without hostPort: {Name: HostPort:0 ContainerPort:80 Protocol:UDP HostIP:10.0.0.1}`,
}, },
}, },
{ {
name: "duplicate container ports without one host IP set and one with", name: "duplicate container ports without one host IP set and two with",
template: &api.PodTemplateSpec{Spec: api.PodSpec{ template: &api.PodTemplateSpec{Spec: api.PodSpec{
Containers: []api.Container{{ Containers: []api.Container{{
Name: "foo", Name: "foo",
Ports: []api.ContainerPort{ Ports: []api.ContainerPort{
{ContainerPort: 80, HostPort: 80, Protocol: api.ProtocolUDP},
{ContainerPort: 80, HostPort: 80, Protocol: api.ProtocolUDP, HostIP: "10.0.0.1"}, {ContainerPort: 80, HostPort: 80, Protocol: api.ProtocolUDP, HostIP: "10.0.0.1"},
{ContainerPort: 80, HostPort: 80, Protocol: api.ProtocolUDP},
{ContainerPort: 80, HostPort: 80, Protocol: api.ProtocolUDP, HostIP: "10.0.0.2"},
}, },
}}, }},
}}, }},
expected: []string{ expected: []string{
`spec.containers[0].ports[1]: duplicate container port 80/UDP, conflicts with spec.containers[0]`, `spec.containers[0].ports[1]: dangerously ambiguous port definition with spec.containers[0].ports[0]`,
`spec.containers[0].ports[2]: dangerously ambiguous port definition with spec.containers[0].ports[1]`,
}, },
}, },
{ {
@ -833,7 +846,7 @@ func TestWarnings(t *testing.T) {
}}, }},
}}, }},
expected: []string{ expected: []string{
`spec.containers[0].ports[1]: duplicate container port 80/UDP, conflicts with spec.containers[0]`, `spec.containers[0].ports[1]: dangerously ambiguous port definition with spec.containers[0].ports[0]`,
}, },
}, },
{ {
@ -852,7 +865,7 @@ func TestWarnings(t *testing.T) {
}}, }},
}}, }},
expected: []string{ expected: []string{
`spec.containers[1].ports[0]: duplicate container port 80/UDP, conflicts with spec.containers[0]`, `spec.containers[1].ports[0]: duplicate port definition with spec.containers[0].ports[0]`,
}, },
}, },
{ {
@ -871,7 +884,7 @@ func TestWarnings(t *testing.T) {
}}, }},
}}, }},
expected: []string{ expected: []string{
`spec.containers[1].ports[0]: duplicate container port 80/UDP, conflicts with spec.containers[0]`, `spec.containers[1].ports[0]: duplicate port definition with spec.containers[0].ports[0]`,
}, },
}, },
{ {
@ -890,7 +903,7 @@ func TestWarnings(t *testing.T) {
}}, }},
}}, }},
expected: []string{ expected: []string{
`spec.containers[1].ports[0]: duplicate container port 80/UDP, conflicts with spec.containers[0]`, `spec.containers[1].ports[0]: duplicate port definition with spec.containers[0].ports[0]`,
}, },
}, },
{ {
@ -909,7 +922,8 @@ func TestWarnings(t *testing.T) {
}}, }},
}}, }},
expected: []string{ expected: []string{
`spec.containers[1].ports[0]: duplicate container port 80/UDP, conflicts with spec.containers[0]`, `spec.containers[1].ports[0]: overlapping port definition with spec.containers[0].ports[0]`,
`spec.containers[1].ports[0]: hostIP set without hostPort: {Name: HostPort:0 ContainerPort:80 Protocol:UDP HostIP:10.0.0.1}`,
}, },
}, },
{ {
@ -928,7 +942,7 @@ func TestWarnings(t *testing.T) {
}}, }},
}}, }},
expected: []string{ expected: []string{
`spec.containers[1].ports[0]: duplicate container port 80/UDP, conflicts with spec.containers[0]`, `spec.containers[1].ports[0]: dangerously ambiguous port definition with spec.containers[0].ports[0]`,
}, },
}, },
{ {
@ -947,7 +961,7 @@ func TestWarnings(t *testing.T) {
}}, }},
}}, }},
expected: []string{ expected: []string{
`spec.containers[1].ports[0]: duplicate container port 80/UDP, conflicts with spec.containers[0]`, `spec.containers[1].ports[0]: dangerously ambiguous port definition with spec.containers[0].ports[0]`,
}, },
}, },
{ {
@ -971,9 +985,12 @@ func TestWarnings(t *testing.T) {
}}, }},
}}, }},
expected: []string{ expected: []string{
`spec.containers[0].ports[2]: duplicate container port 80/UDP, conflicts with spec.containers[0]`, `spec.containers[0].ports[2]: duplicate port definition with spec.containers[0].ports[0]`,
`spec.containers[1].ports[0]: duplicate container port 80/UDP, conflicts with spec.containers[0]`, `spec.containers[1].ports[0]: duplicate port definition with spec.containers[0].ports[0]`,
`spec.containers[1].ports[1]: duplicate container port 80/UDP, conflicts with spec.containers[0]`, `spec.containers[1].ports[0]: duplicate port definition with spec.containers[0].ports[2]`,
`spec.containers[1].ports[1]: duplicate port definition with spec.containers[0].ports[0]`,
`spec.containers[1].ports[1]: duplicate port definition with spec.containers[0].ports[2]`,
`spec.containers[1].ports[1]: duplicate port definition with spec.containers[1].ports[0]`,
}, },
}, },
{ {
@ -1015,9 +1032,12 @@ func TestWarnings(t *testing.T) {
}}, }},
}}, }},
expected: []string{ expected: []string{
`spec.containers[0].ports[2]: duplicate container port 80/TCP, conflicts with spec.containers[0]`, `spec.containers[0].ports[2]: duplicate port definition with spec.containers[0].ports[0]`,
`spec.containers[1].ports[0]: duplicate container port 80/TCP, conflicts with spec.containers[0]`, `spec.containers[1].ports[0]: duplicate port definition with spec.containers[0].ports[0]`,
`spec.containers[1].ports[1]: duplicate container port 80/TCP, conflicts with spec.containers[0]`, `spec.containers[1].ports[0]: duplicate port definition with spec.containers[0].ports[2]`,
`spec.containers[1].ports[1]: duplicate port definition with spec.containers[0].ports[0]`,
`spec.containers[1].ports[1]: duplicate port definition with spec.containers[0].ports[2]`,
`spec.containers[1].ports[1]: duplicate port definition with spec.containers[1].ports[0]`,
}, },
}, },
} }