Merge pull request #57460 from dixudx/validate_initcontainer_hostport

Automatic merge from submit-queue (batch tested with PRs 62951, 57460, 63118). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

fix hostport checking for initContainers since they run in sequential order

**What this PR does / why we need it**:
Fix hostport checking for initContainers since they run in sequential order

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
partial Fixes #57443

**Special notes for your reviewer**:
/assign @CaoShuFeng @dims 

**Release note**:

```release-note
None
```
This commit is contained in:
Kubernetes Submit Queue 2018-04-25 02:01:53 -07:00 committed by GitHub
commit 4f233eb92a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 75 additions and 7 deletions

View File

@ -2506,7 +2506,7 @@ func validatePullPolicy(policy core.PullPolicy, fldPath *field.Path) field.Error
func validateInitContainers(containers, otherContainers []core.Container, deviceVolumes map[string]core.VolumeSource, fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList
if len(containers) > 0 {
allErrs = append(allErrs, validateContainers(containers, deviceVolumes, fldPath)...)
allErrs = append(allErrs, validateContainers(containers, true, deviceVolumes, fldPath)...)
}
allNames := sets.String{}
@ -2534,7 +2534,7 @@ func validateInitContainers(containers, otherContainers []core.Container, device
return allErrs
}
func validateContainers(containers []core.Container, volumes map[string]core.VolumeSource, fldPath *field.Path) field.ErrorList {
func validateContainers(containers []core.Container, isInitContainers bool, volumes map[string]core.VolumeSource, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if len(containers) == 0 {
@ -2591,8 +2591,16 @@ func validateContainers(containers []core.Container, volumes map[string]core.Vol
allErrs = append(allErrs, ValidateResourceRequirements(&ctr.Resources, idxPath.Child("resources"))...)
allErrs = append(allErrs, ValidateSecurityContext(ctr.SecurityContext, idxPath.Child("securityContext"))...)
}
// Check for colliding ports across all containers.
allErrs = append(allErrs, checkHostPortConflicts(containers, fldPath)...)
if isInitContainers {
// check initContainers one by one since they are running in sequential order.
for _, initContainer := range containers {
allErrs = append(allErrs, checkHostPortConflicts([]core.Container{initContainer}, fldPath)...)
}
} else {
// Check for colliding ports across all containers.
allErrs = append(allErrs, checkHostPortConflicts(containers, fldPath)...)
}
return allErrs
}
@ -2922,7 +2930,7 @@ func ValidatePodSpec(spec *core.PodSpec, fldPath *field.Path) field.ErrorList {
vols, vErrs := ValidateVolumes(spec.Volumes, fldPath.Child("volumes"))
allErrs = append(allErrs, vErrs...)
allErrs = append(allErrs, validateContainers(spec.Containers, vols, fldPath.Child("containers"))...)
allErrs = append(allErrs, validateContainers(spec.Containers, false, vols, fldPath.Child("containers"))...)
allErrs = append(allErrs, validateInitContainers(spec.InitContainers, spec.Containers, vols, fldPath.Child("initContainers"))...)
allErrs = append(allErrs, validateRestartPolicy(&spec.RestartPolicy, fldPath.Child("restartPolicy"))...)
allErrs = append(allErrs, validateDNSPolicy(&spec.DNSPolicy, fldPath.Child("dnsPolicy"))...)

View File

@ -5163,7 +5163,7 @@ func TestValidateContainers(t *testing.T) {
},
{Name: "abc-1234", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", SecurityContext: fakeValidSecurityContext(true)},
}
if errs := validateContainers(successCase, volumeDevices, field.NewPath("field")); len(errs) != 0 {
if errs := validateContainers(successCase, false, volumeDevices, field.NewPath("field")); len(errs) != 0 {
t.Errorf("expected success: %v", errs)
}
@ -5390,7 +5390,67 @@ func TestValidateContainers(t *testing.T) {
},
}
for k, v := range errorCases {
if errs := validateContainers(v, volumeDevices, field.NewPath("field")); len(errs) == 0 {
if errs := validateContainers(v, false, volumeDevices, field.NewPath("field")); len(errs) == 0 {
t.Errorf("expected failure for %s", k)
}
}
}
func TestValidateInitContainers(t *testing.T) {
volumeDevices := make(map[string]core.VolumeSource)
capabilities.SetForTests(capabilities.Capabilities{
AllowPrivileged: true,
})
successCase := []core.Container{
{
Name: "container-1-same-host-port-different-protocol",
Image: "image",
Ports: []core.ContainerPort{
{ContainerPort: 80, HostPort: 80, Protocol: "TCP"},
{ContainerPort: 80, HostPort: 80, Protocol: "UDP"},
},
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File",
},
{
Name: "container-2-same-host-port-different-protocol",
Image: "image",
Ports: []core.ContainerPort{
{ContainerPort: 80, HostPort: 80, Protocol: "TCP"},
{ContainerPort: 80, HostPort: 80, Protocol: "UDP"},
},
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File",
},
}
if errs := validateContainers(successCase, true, volumeDevices, field.NewPath("field")); len(errs) != 0 {
t.Errorf("expected success: %v", errs)
}
capabilities.SetForTests(capabilities.Capabilities{
AllowPrivileged: false,
})
errorCases := map[string][]core.Container{
"duplicate ports": {
{
Name: "abc",
Image: "image",
Ports: []core.ContainerPort{
{
ContainerPort: 8080, HostPort: 8080, Protocol: "TCP",
},
{
ContainerPort: 8080, HostPort: 8080, Protocol: "TCP",
},
},
ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File",
},
},
}
for k, v := range errorCases {
if errs := validateContainers(v, true, volumeDevices, field.NewPath("field")); len(errs) == 0 {
t.Errorf("expected failure for %s", k)
}
}