Merge pull request #79176 from verb/debug-iterate-containers

Add helpers for iterating containers in a pod

pkg/api/pod/util.go

@@ -26,6 +26,28 @@ import (
 	"k8s.io/kubernetes/pkg/security/apparmor"
 )
 
+// ContainerVisitor is called with each container spec, and returns true
+// if visiting should continue.
+type ContainerVisitor func(container *api.Container) (shouldContinue bool)
+
+// VisitContainers invokes the visitor function with a pointer to the container
+// spec of every container in the given pod spec. If visitor returns false,
+// visiting is short-circuited. VisitContainers returns true if visiting completes,
+// false if visiting was short-circuited.
+func VisitContainers(podSpec *api.PodSpec, visitor ContainerVisitor) bool {
+	for i := range podSpec.InitContainers {
+		if !visitor(&podSpec.InitContainers[i]) {
+			return false
+		}
+	}
+	for i := range podSpec.Containers {
+		if !visitor(&podSpec.Containers[i]) {
+			return false
+		}
+	}
+	return true
+}
+
 // Visitor is called with each object name, and returns true if visiting should continue
 type Visitor func(name string) (shouldContinue bool)
 
@@ -39,16 +61,9 @@ func VisitPodSecretNames(pod *api.Pod, visitor Visitor) bool {
 			return false
 		}
 	}
-	for i := range pod.Spec.InitContainers {
+	VisitContainers(&pod.Spec, func(c *api.Container) bool {
-		if !visitContainerSecretNames(&pod.Spec.InitContainers[i], visitor) {
+		return visitContainerSecretNames(c, visitor)
-			return false
+	})
-		}
-	}
-	for i := range pod.Spec.Containers {
-		if !visitContainerSecretNames(&pod.Spec.Containers[i], visitor) {
-			return false
-		}
-	}
 	var source *api.VolumeSource
 	for i := range pod.Spec.Volumes {
 		source = &pod.Spec.Volumes[i].VolumeSource
@@ -129,16 +144,9 @@ func visitContainerSecretNames(container *api.Container, visitor Visitor) bool {
 // Transitive references (e.g. pod -> pvc -> pv -> secret) are not visited.
 // Returns true if visiting completed, false if visiting was short-circuited.
 func VisitPodConfigmapNames(pod *api.Pod, visitor Visitor) bool {
-	for i := range pod.Spec.InitContainers {
+	VisitContainers(&pod.Spec, func(c *api.Container) bool {
-		if !visitContainerConfigmapNames(&pod.Spec.InitContainers[i], visitor) {
+		return visitContainerConfigmapNames(c, visitor)
-			return false
+	})
-		}
-	}
-	for i := range pod.Spec.Containers {
-		if !visitContainerConfigmapNames(&pod.Spec.Containers[i], visitor) {
-			return false
-		}
-	}
 	var source *api.VolumeSource
 	for i := range pod.Spec.Volumes {
 		source = &pod.Spec.Volumes[i].VolumeSource
@@ -331,30 +339,22 @@ func dropDisabledFields(
 
 	if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpath) && !subpathInUse(oldPodSpec) {
 		// drop subpath from the pod if the feature is disabled and the old spec did not specify subpaths
-		for i := range podSpec.Containers {
+		VisitContainers(podSpec, func(c *api.Container) bool {
-			for j := range podSpec.Containers[i].VolumeMounts {
+			for i := range c.VolumeMounts {
-				podSpec.Containers[i].VolumeMounts[j].SubPath = ""
+				c.VolumeMounts[i].SubPath = ""
 			}
-		}
+			return true
-		for i := range podSpec.InitContainers {
+		})
-			for j := range podSpec.InitContainers[i].VolumeMounts {
-				podSpec.InitContainers[i].VolumeMounts[j].SubPath = ""
-			}
-		}
 	}
 
 	if (!utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpath) || !utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpathEnvExpansion)) && !subpathExprInUse(oldPodSpec) {
 		// drop subpath env expansion from the pod if either of the subpath features is disabled and the old spec did not specify subpath env expansion
-		for i := range podSpec.Containers {
+		VisitContainers(podSpec, func(c *api.Container) bool {
-			for j := range podSpec.Containers[i].VolumeMounts {
+			for i := range c.VolumeMounts {
-				podSpec.Containers[i].VolumeMounts[j].SubPathExpr = ""
+				c.VolumeMounts[i].SubPathExpr = ""
 			}
-		}
+			return true
-		for i := range podSpec.InitContainers {
+		})
-			for j := range podSpec.InitContainers[i].VolumeMounts {
-				podSpec.InitContainers[i].VolumeMounts[j].SubPathExpr = ""
-			}
-		}
 	}
 
 	dropDisabledVolumeDevicesFields(podSpec, oldPodSpec)
@@ -392,16 +392,12 @@ func dropDisabledRunAsGroupField(podSpec, oldPodSpec *api.PodSpec) {
 		if podSpec.SecurityContext != nil {
 			podSpec.SecurityContext.RunAsGroup = nil
 		}
-		for i := range podSpec.Containers {
+		VisitContainers(podSpec, func(c *api.Container) bool {
-			if podSpec.Containers[i].SecurityContext != nil {
+			if c.SecurityContext != nil {
-				podSpec.Containers[i].SecurityContext.RunAsGroup = nil
+				c.SecurityContext.RunAsGroup = nil
 			}
-		}
+			return true
-		for i := range podSpec.InitContainers {
+		})
-			if podSpec.InitContainers[i].SecurityContext != nil {
-				podSpec.InitContainers[i].SecurityContext.RunAsGroup = nil
-			}
-		}
 	}
 }
 
@@ -443,26 +439,15 @@ func dropDisabledGMSAFieldsFromContainers(containers []api.Container) {
 func dropDisabledProcMountField(podSpec, oldPodSpec *api.PodSpec) {
 	if !utilfeature.DefaultFeatureGate.Enabled(features.ProcMountType) && !procMountInUse(oldPodSpec) {
 		defaultProcMount := api.DefaultProcMount
-		for i := range podSpec.Containers {
+		VisitContainers(podSpec, func(c *api.Container) bool {
-			if podSpec.Containers[i].SecurityContext != nil {
+			if c.SecurityContext != nil && c.SecurityContext.ProcMount != nil {
-				if podSpec.Containers[i].SecurityContext.ProcMount != nil {
+				// The ProcMount field was improperly forced to non-nil in 1.12.
-					// The ProcMount field was improperly forced to non-nil in 1.12.
+				// If the feature is disabled, and the existing object is not using any non-default values, and the ProcMount field is present in the incoming object, force to the default value.
-					// If the feature is disabled, and the existing object is not using any non-default values, and the ProcMount field is present in the incoming object, force to the default value.
+				// Note: we cannot force the field to nil when the feature is disabled because it causes a diff against previously persisted data.
-					// Note: we cannot force the field to nil when the feature is disabled because it causes a diff against previously persisted data.
+				c.SecurityContext.ProcMount = &defaultProcMount
-					podSpec.Containers[i].SecurityContext.ProcMount = &defaultProcMount
-				}
 			}
-		}
+			return true
-		for i := range podSpec.InitContainers {
+		})
-			if podSpec.InitContainers[i].SecurityContext != nil {
-				if podSpec.InitContainers[i].SecurityContext.ProcMount != nil {
-					// The ProcMount field was improperly forced to non-nil in 1.12.
-					// If the feature is disabled, and the existing object is not using any non-default values, and the ProcMount field is present in the incoming object, force to the default value.
-					// Note: we cannot force the field to nil when the feature is disabled because it causes a diff against previously persisted data.
-					podSpec.InitContainers[i].SecurityContext.ProcMount = &defaultProcMount
-				}
-			}
-		}
 	}
 }
 
@@ -470,12 +455,10 @@ func dropDisabledProcMountField(podSpec, oldPodSpec *api.PodSpec) {
 // This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a VolumeDevice
 func dropDisabledVolumeDevicesFields(podSpec, oldPodSpec *api.PodSpec) {
 	if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) && !volumeDevicesInUse(oldPodSpec) {
-		for i := range podSpec.Containers {
+		VisitContainers(podSpec, func(c *api.Container) bool {
-			podSpec.Containers[i].VolumeDevices = nil
+			c.VolumeDevices = nil
-		}
+			return true
-		for i := range podSpec.InitContainers {
+		})
-			podSpec.InitContainers[i].VolumeDevices = nil
-		}
 	}
 }
 
@@ -494,21 +477,19 @@ func subpathInUse(podSpec *api.PodSpec) bool {
 	if podSpec == nil {
 		return false
 	}
-	for i := range podSpec.Containers {
+
-		for j := range podSpec.Containers[i].VolumeMounts {
+	var inUse bool
-			if len(podSpec.Containers[i].VolumeMounts[j].SubPath) > 0 {
+	VisitContainers(podSpec, func(c *api.Container) bool {
-				return true
+		for i := range c.VolumeMounts {
+			if len(c.VolumeMounts[i].SubPath) > 0 {
+				inUse = true
+				return false
 			}
 		}
-	}
+		return true
-	for i := range podSpec.InitContainers {
+	})
-		for j := range podSpec.InitContainers[i].VolumeMounts {
+
-			if len(podSpec.InitContainers[i].VolumeMounts[j].SubPath) > 0 {
+	return inUse
-				return true
-			}
-		}
-	}
-	return false
 }
 
 // runtimeClassInUse returns true if the pod spec is non-nil and has a RuntimeClassName set
@@ -539,25 +520,20 @@ func procMountInUse(podSpec *api.PodSpec) bool {
 	if podSpec == nil {
 		return false
 	}
-	for i := range podSpec.Containers {
+
-		if podSpec.Containers[i].SecurityContext != nil {
+	var inUse bool
-			if podSpec.Containers[i].SecurityContext.ProcMount != nil {
+	VisitContainers(podSpec, func(c *api.Container) bool {
-				if *podSpec.Containers[i].SecurityContext.ProcMount != api.DefaultProcMount {
+		if c.SecurityContext == nil || c.SecurityContext.ProcMount == nil {
-					return true
+			return true
-				}
-			}
 		}
-	}
+		if *c.SecurityContext.ProcMount != api.DefaultProcMount {
-	for i := range podSpec.InitContainers {
+			inUse = true
-		if podSpec.InitContainers[i].SecurityContext != nil {
+			return false
-			if podSpec.InitContainers[i].SecurityContext.ProcMount != nil {
-				if *podSpec.InitContainers[i].SecurityContext.ProcMount != api.DefaultProcMount {
-					return true
-				}
-			}
 		}
-	}
+		return true
-	return false
+	})
+
+	return inUse
 }
 
 // appArmorInUse returns true if the pod has apparmor related information
@@ -638,17 +614,17 @@ func volumeDevicesInUse(podSpec *api.PodSpec) bool {
 	if podSpec == nil {
 		return false
 	}
-	for i := range podSpec.Containers {
+
-		if podSpec.Containers[i].VolumeDevices != nil {
+	var inUse bool
-			return true
+	VisitContainers(podSpec, func(c *api.Container) bool {
+		if c.VolumeDevices != nil {
+			inUse = true
+			return false
 		}
-	}
+		return true
-	for i := range podSpec.InitContainers {
+	})
-		if podSpec.InitContainers[i].VolumeDevices != nil {
+
-			return true
+	return inUse
-		}
-	}
-	return false
 }
 
 // runAsGroupInUse returns true if the pod spec is non-nil and has a SecurityContext's RunAsGroup field set
@@ -660,17 +636,17 @@ func runAsGroupInUse(podSpec *api.PodSpec) bool {
 	if podSpec.SecurityContext != nil && podSpec.SecurityContext.RunAsGroup != nil {
 		return true
 	}
-	for i := range podSpec.Containers {
+
-		if podSpec.Containers[i].SecurityContext != nil && podSpec.Containers[i].SecurityContext.RunAsGroup != nil {
+	var inUse bool
-			return true
+	VisitContainers(podSpec, func(c *api.Container) bool {
+		if c.SecurityContext != nil && c.SecurityContext.RunAsGroup != nil {
+			inUse = true
+			return false
 		}
-	}
+		return true
-	for i := range podSpec.InitContainers {
+	})
-		if podSpec.InitContainers[i].SecurityContext != nil && podSpec.InitContainers[i].SecurityContext.RunAsGroup != nil {
+
-			return true
+	return inUse
-		}
-	}
-	return false
 }
 
 // gMSAFieldsInUse returns true if the pod spec is non-nil and has one of any
@@ -716,21 +692,19 @@ func subpathExprInUse(podSpec *api.PodSpec) bool {
 	if podSpec == nil {
 		return false
 	}
-	for i := range podSpec.Containers {
+
-		for j := range podSpec.Containers[i].VolumeMounts {
+	var inUse bool
-			if len(podSpec.Containers[i].VolumeMounts[j].SubPathExpr) > 0 {
+	VisitContainers(podSpec, func(c *api.Container) bool {
-				return true
+		for i := range c.VolumeMounts {
+			if len(c.VolumeMounts[i].SubPathExpr) > 0 {
+				inUse = true
+				return false
 			}
 		}
-	}
+		return true
-	for i := range podSpec.InitContainers {
+	})
-		for j := range podSpec.InitContainers[i].VolumeMounts {
+
-			if len(podSpec.InitContainers[i].VolumeMounts[j].SubPathExpr) > 0 {
+	return inUse
-				return true
-			}
-		}
-	}
-	return false
 }
 
 // csiInUse returns true if any pod's spec include inline CSI volumes.

pkg/api/pod/util_test.go

@@ -34,6 +34,92 @@ import (
 	"k8s.io/kubernetes/pkg/security/apparmor"
 )
 
+func TestVisitContainers(t *testing.T) {
+	testCases := []struct {
+		description string
+		haveSpec    *api.PodSpec
+		wantNames   []string
+	}{
+		{
+			"empty podspec",
+			&api.PodSpec{},
+			[]string{},
+		},
+		{
+			"regular containers",
+			&api.PodSpec{
+				Containers: []api.Container{
+					{Name: "c1"},
+					{Name: "c2"},
+				},
+			},
+			[]string{"c1", "c2"},
+		},
+		{
+			"init containers",
+			&api.PodSpec{
+				InitContainers: []api.Container{
+					{Name: "i1"},
+					{Name: "i2"},
+				},
+			},
+			[]string{"i1", "i2"},
+		},
+		{
+			"regular and init containers",
+			&api.PodSpec{
+				Containers: []api.Container{
+					{Name: "c1"},
+					{Name: "c2"},
+				},
+				InitContainers: []api.Container{
+					{Name: "i1"},
+					{Name: "i2"},
+				},
+			},
+			[]string{"i1", "i2", "c1", "c2"},
+		},
+		{
+			"dropping fields",
+			&api.PodSpec{
+				Containers: []api.Container{
+					{Name: "c1"},
+					{Name: "c2", SecurityContext: &api.SecurityContext{}},
+				},
+				InitContainers: []api.Container{
+					{Name: "i1"},
+					{Name: "i2", SecurityContext: &api.SecurityContext{}},
+				},
+			},
+			[]string{"i1", "i2", "c1", "c2"},
+		},
+	}
+
+	for _, tc := range testCases {
+		gotNames := []string{}
+		VisitContainers(tc.haveSpec, func(c *api.Container) bool {
+			gotNames = append(gotNames, c.Name)
+			if c.SecurityContext != nil {
+				c.SecurityContext = nil
+			}
+			return true
+		})
+		if !reflect.DeepEqual(gotNames, tc.wantNames) {
+			t.Errorf("VisitContainers() for test case %q visited containers %q, wanted to visit %q", tc.description, gotNames, tc.wantNames)
+		}
+		for _, c := range tc.haveSpec.Containers {
+			if c.SecurityContext != nil {
+				t.Errorf("VisitContainers() for test case %q: got SecurityContext %#v for container %v, wanted nil", tc.description, c.SecurityContext, c.Name)
+			}
+		}
+		for _, c := range tc.haveSpec.InitContainers {
+			if c.SecurityContext != nil {
+				t.Errorf("VisitContainers() for test case %q: got SecurityContext %#v for init container %v, wanted nil", tc.description, c.SecurityContext, c.Name)
+			}
+		}
+	}
+}
+
 func TestPodSecrets(t *testing.T) {
 	// Stub containing all possible secret references in a pod.
 	// The names of the referenced secrets match struct paths detected by reflection.

pkg/api/v1/pod/util.go

@@ -48,6 +48,28 @@ func FindPort(pod *v1.Pod, svcPort *v1.ServicePort) (int, error) {
 	return 0, fmt.Errorf("no suitable port for manifest: %s", pod.UID)
 }
 
+// ContainerVisitor is called with each container spec, and returns true
+// if visiting should continue.
+type ContainerVisitor func(container *v1.Container) (shouldContinue bool)
+
+// VisitContainers invokes the visitor function with a pointer to the container
+// spec of every container in the given pod spec. If visitor returns false,
+// visiting is short-circuited. VisitContainers returns true if visiting completes,
+// false if visiting was short-circuited.
+func VisitContainers(podSpec *v1.PodSpec, visitor ContainerVisitor) bool {
+	for i := range podSpec.InitContainers {
+		if !visitor(&podSpec.InitContainers[i]) {
+			return false
+		}
+	}
+	for i := range podSpec.Containers {
+		if !visitor(&podSpec.Containers[i]) {
+			return false
+		}
+	}
+	return true
+}
+
 // Visitor is called with each object name, and returns true if visiting should continue
 type Visitor func(name string) (shouldContinue bool)
 
@@ -61,16 +83,9 @@ func VisitPodSecretNames(pod *v1.Pod, visitor Visitor) bool {
 			return false
 		}
 	}
-	for i := range pod.Spec.InitContainers {
+	VisitContainers(&pod.Spec, func(c *v1.Container) bool {
-		if !visitContainerSecretNames(&pod.Spec.InitContainers[i], visitor) {
+		return visitContainerSecretNames(c, visitor)
-			return false
+	})
-		}
-	}
-	for i := range pod.Spec.Containers {
-		if !visitContainerSecretNames(&pod.Spec.Containers[i], visitor) {
-			return false
-		}
-	}
 	var source *v1.VolumeSource
 
 	for i := range pod.Spec.Volumes {
@@ -152,16 +167,9 @@ func visitContainerSecretNames(container *v1.Container, visitor Visitor) bool {
 // Transitive references (e.g. pod -> pvc -> pv -> secret) are not visited.
 // Returns true if visiting completed, false if visiting was short-circuited.
 func VisitPodConfigmapNames(pod *v1.Pod, visitor Visitor) bool {
-	for i := range pod.Spec.InitContainers {
+	VisitContainers(&pod.Spec, func(c *v1.Container) bool {
-		if !visitContainerConfigmapNames(&pod.Spec.InitContainers[i], visitor) {
+		return visitContainerConfigmapNames(c, visitor)
-			return false
+	})
-		}
-	}
-	for i := range pod.Spec.Containers {
-		if !visitContainerConfigmapNames(&pod.Spec.Containers[i], visitor) {
-			return false
-		}
-	}
 	var source *v1.VolumeSource
 	for i := range pod.Spec.Volumes {
 		source = &pod.Spec.Volumes[i].VolumeSource

pkg/api/v1/pod/util_test.go

@@ -198,6 +198,92 @@ func TestFindPort(t *testing.T) {
 	}
 }
 
+func TestVisitContainers(t *testing.T) {
+	testCases := []struct {
+		description string
+		haveSpec    *v1.PodSpec
+		wantNames   []string
+	}{
+		{
+			"empty podspec",
+			&v1.PodSpec{},
+			[]string{},
+		},
+		{
+			"regular containers",
+			&v1.PodSpec{
+				Containers: []v1.Container{
+					{Name: "c1"},
+					{Name: "c2"},
+				},
+			},
+			[]string{"c1", "c2"},
+		},
+		{
+			"init containers",
+			&v1.PodSpec{
+				InitContainers: []v1.Container{
+					{Name: "i1"},
+					{Name: "i2"},
+				},
+			},
+			[]string{"i1", "i2"},
+		},
+		{
+			"regular and init containers",
+			&v1.PodSpec{
+				Containers: []v1.Container{
+					{Name: "c1"},
+					{Name: "c2"},
+				},
+				InitContainers: []v1.Container{
+					{Name: "i1"},
+					{Name: "i2"},
+				},
+			},
+			[]string{"i1", "i2", "c1", "c2"},
+		},
+		{
+			"dropping fields",
+			&v1.PodSpec{
+				Containers: []v1.Container{
+					{Name: "c1"},
+					{Name: "c2", SecurityContext: &v1.SecurityContext{}},
+				},
+				InitContainers: []v1.Container{
+					{Name: "i1"},
+					{Name: "i2", SecurityContext: &v1.SecurityContext{}},
+				},
+			},
+			[]string{"i1", "i2", "c1", "c2"},
+		},
+	}
+
+	for _, tc := range testCases {
+		gotNames := []string{}
+		VisitContainers(tc.haveSpec, func(c *v1.Container) bool {
+			gotNames = append(gotNames, c.Name)
+			if c.SecurityContext != nil {
+				c.SecurityContext = nil
+			}
+			return true
+		})
+		if !reflect.DeepEqual(gotNames, tc.wantNames) {
+			t.Errorf("VisitContainers() for test case %q visited containers %q, wanted to visit %q", tc.description, gotNames, tc.wantNames)
+		}
+		for _, c := range tc.haveSpec.Containers {
+			if c.SecurityContext != nil {
+				t.Errorf("VisitContainers() for test case %q: got SecurityContext %#v for container %v, wanted nil", tc.description, c.SecurityContext, c.Name)
+			}
+		}
+		for _, c := range tc.haveSpec.InitContainers {
+			if c.SecurityContext != nil {
+				t.Errorf("VisitContainers() for test case %q: got SecurityContext %#v for init container %v, wanted nil", tc.description, c.SecurityContext, c.Name)
+			}
+		}
+	}
+}
+
 func TestPodSecrets(t *testing.T) {
 	// Stub containing all possible secret references in a pod.
 	// The names of the referenced secrets match struct paths detected by reflection.

pkg/apis/core/pods/BUILD

@@ -5,13 +5,21 @@ go_library(
     srcs = ["helpers.go"],
     importpath = "k8s.io/kubernetes/pkg/apis/core/pods",
     visibility = ["//visibility:public"],
-    deps = ["//pkg/fieldpath:go_default_library"],
+    deps = [
+        "//pkg/apis/core:go_default_library",
+        "//pkg/fieldpath:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
+    ],
 )
 
 go_test(
     name = "go_default_test",
     srcs = ["helpers_test.go"],
     embed = [":go_default_library"],
+    deps = [
+        "//pkg/apis/core:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
+    ],
 )
 
 filegroup(

pkg/apis/core/pods/helpers.go

@@ -19,9 +19,27 @@ package pods
 import (
 	"fmt"
 
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/fieldpath"
 )
 
+// ContainerVisitorWithPath is called with each container and the field.Path to that container
+type ContainerVisitorWithPath func(container *api.Container, path *field.Path)
+
+// VisitContainersWithPath invokes the visitor function with a pointer to the spec
+// of every container in the given pod spec and the field.Path to that container.
+func VisitContainersWithPath(podSpec *api.PodSpec, visitor ContainerVisitorWithPath) {
+	path := field.NewPath("spec", "initContainers")
+	for i := range podSpec.InitContainers {
+		visitor(&podSpec.InitContainers[i], path.Index(i))
+	}
+	path = field.NewPath("spec", "containers")
+	for i := range podSpec.Containers {
+		visitor(&podSpec.Containers[i], path.Index(i))
+	}
+}
+
 // ConvertDownwardAPIFieldLabel converts the specified downward API field label
 // and its value in the pod of the specified version to the internal version,
 // and returns the converted label and value. This function returns an error if

pkg/apis/core/pods/helpers_test.go

@@ -17,9 +17,71 @@ limitations under the License.
 package pods
 
 import (
+	"reflect"
 	"testing"
+
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	api "k8s.io/kubernetes/pkg/apis/core"
 )
 
+func TestVisitContainersWithPath(t *testing.T) {
+	testCases := []struct {
+		description string
+		haveSpec    *api.PodSpec
+		wantNames   []string
+	}{
+		{
+			"empty podspec",
+			&api.PodSpec{},
+			[]string{},
+		},
+		{
+			"regular containers",
+			&api.PodSpec{
+				Containers: []api.Container{
+					{Name: "c1"},
+					{Name: "c2"},
+				},
+			},
+			[]string{"spec.containers[0]", "spec.containers[1]"},
+		},
+		{
+			"init containers",
+			&api.PodSpec{
+				InitContainers: []api.Container{
+					{Name: "i1"},
+					{Name: "i2"},
+				},
+			},
+			[]string{"spec.initContainers[0]", "spec.initContainers[1]"},
+		},
+		{
+			"regular and init containers",
+			&api.PodSpec{
+				Containers: []api.Container{
+					{Name: "c1"},
+					{Name: "c2"},
+				},
+				InitContainers: []api.Container{
+					{Name: "i1"},
+					{Name: "i2"},
+				},
+			},
+			[]string{"spec.initContainers[0]", "spec.initContainers[1]", "spec.containers[0]", "spec.containers[1]"},
+		},
+	}
+
+	for _, tc := range testCases {
+		gotNames := []string{}
+		VisitContainersWithPath(tc.haveSpec, func(c *api.Container, p *field.Path) {
+			gotNames = append(gotNames, p.String())
+		})
+		if !reflect.DeepEqual(gotNames, tc.wantNames) {
+			t.Errorf("VisitContainersWithPath() for test case %q visited containers %q, wanted to visit %q", tc.description, gotNames, tc.wantNames)
+		}
+	}
+}
+
 func TestConvertDownwardAPIFieldLabel(t *testing.T) {
 	testCases := []struct {
 		version       string

pkg/apis/core/validation/validation.go

@@ -3434,17 +3434,13 @@ func ValidateAppArmorPodAnnotations(annotations map[string]string, spec *core.Po
 }
 
 func podSpecHasContainer(spec *core.PodSpec, containerName string) bool {
-	for _, c := range spec.InitContainers {
+	var hasContainer bool
+	podshelper.VisitContainersWithPath(spec, func(c *core.Container, _ *field.Path) {
 		if c.Name == containerName {
-			return true
+			hasContainer = true
 		}
-	}
+	})
-	for _, c := range spec.Containers {
+	return hasContainer
-		if c.Name == containerName {
-			return true
-		}
-	}
-	return false
 }
 
 const (

pkg/registry/core/pod/strategy.go

@@ -361,17 +361,15 @@ func LogLocation(
 }
 
 func podHasContainerWithName(pod *api.Pod, containerName string) bool {
-	for _, c := range pod.Spec.Containers {
+	var hasContainer bool
+	podutil.VisitContainers(&pod.Spec, func(c *api.Container) bool {
 		if c.Name == containerName {
-			return true
+			hasContainer = true
+			return false
 		}
-	}
+		return true
-	for _, c := range pod.Spec.InitContainers {
+	})
-		if c.Name == containerName {
+	return hasContainer
-			return true
-		}
-	}
-	return false
 }
 
 func streamParams(params url.Values, opts runtime.Object) error {

pkg/security/apparmor/BUILD

@@ -15,6 +15,7 @@ go_library(
     ],
     importpath = "k8s.io/kubernetes/pkg/security/apparmor",
     deps = [
+        "//pkg/api/v1/pod:go_default_library",
         "//pkg/features:go_default_library",
         "//pkg/kubelet/types:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",

pkg/security/apparmor/validate.go

@@ -27,6 +27,7 @@ import (
 
 	"k8s.io/api/core/v1"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	podutil "k8s.io/kubernetes/pkg/api/v1/pod"
 	"k8s.io/kubernetes/pkg/features"
 	kubetypes "k8s.io/kubernetes/pkg/kubelet/types"
 	utilpath "k8s.io/utils/path"
@@ -76,18 +77,16 @@ func (v *validator) Validate(pod *v1.Pod) error {
 		return fmt.Errorf("could not read loaded profiles: %v", err)
 	}
 
-	for _, container := range pod.Spec.InitContainers {
+	var retErr error
-		if err := validateProfile(GetProfileName(pod, container.Name), loadedProfiles); err != nil {
+	podutil.VisitContainers(&pod.Spec, func(container *v1.Container) bool {
-			return err
+		retErr = validateProfile(GetProfileName(pod, container.Name), loadedProfiles)
+		if retErr != nil {
+			return false
 		}
-	}
+		return true
-	for _, container := range pod.Spec.Containers {
+	})
-		if err := validateProfile(GetProfileName(pod, container.Name), loadedProfiles); err != nil {
-			return err
-		}
-	}
 
-	return nil
+	return retErr
 }
 
 func (v *validator) ValidateHost() error {

pkg/security/podsecuritypolicy/BUILD

@@ -16,7 +16,9 @@ go_library(
     ],
     importpath = "k8s.io/kubernetes/pkg/security/podsecuritypolicy",
     deps = [
+        "//pkg/api/pod:go_default_library",
         "//pkg/apis/core:go_default_library",
+        "//pkg/apis/core/pods:go_default_library",
         "//pkg/features:go_default_library",
         "//pkg/security/podsecuritypolicy/apparmor:go_default_library",
         "//pkg/security/podsecuritypolicy/capabilities:go_default_library",

pkg/security/podsecuritypolicy/provider.go

@@ -24,7 +24,9 @@ import (
 	policy "k8s.io/api/policy/v1beta1"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	podutil "k8s.io/kubernetes/pkg/api/pod"
 	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/apis/core/pods"
 	"k8s.io/kubernetes/pkg/features"
 	psputil "k8s.io/kubernetes/pkg/security/podsecuritypolicy/util"
 	"k8s.io/kubernetes/pkg/securitycontext"
@@ -108,19 +110,16 @@ func (s *simpleProvider) MutatePod(pod *api.Pod) error {
 		pod.Spec.RuntimeClassName = s.psp.Spec.RuntimeClass.DefaultRuntimeClassName
 	}
 
-	for i := range pod.Spec.InitContainers {
+	var retErr error
-		if err := s.mutateContainer(pod, &pod.Spec.InitContainers[i]); err != nil {
+	podutil.VisitContainers(&pod.Spec, func(c *api.Container) bool {
-			return err
+		retErr = s.mutateContainer(pod, c)
+		if retErr != nil {
+			return false
 		}
-	}
+		return true
+	})
 
-	for i := range pod.Spec.Containers {
+	return retErr
-		if err := s.mutateContainer(pod, &pod.Spec.Containers[i]); err != nil {
-			return err
-		}
-	}
-
-	return nil
 }
 
 // mutateContainer sets the default values of the required but not filled fields.
@@ -239,15 +238,9 @@ func (s *simpleProvider) ValidatePod(pod *api.Pod) field.ErrorList {
 		allErrs = append(allErrs, validateRuntimeClassName(pod.Spec.RuntimeClassName, s.psp.Spec.RuntimeClass.AllowedRuntimeClassNames)...)
 	}
 
-	fldPath := field.NewPath("spec", "initContainers")
+	pods.VisitContainersWithPath(&pod.Spec, func(c *api.Container, p *field.Path) {
-	for i := range pod.Spec.InitContainers {
+		allErrs = append(allErrs, s.validateContainer(pod, c, p)...)
-		allErrs = append(allErrs, s.validateContainer(pod, &pod.Spec.InitContainers[i], fldPath.Index(i))...)
+	})
-	}
-
-	fldPath = field.NewPath("spec", "containers")
-	for i := range pod.Spec.Containers {
-		allErrs = append(allErrs, s.validateContainer(pod, &pod.Spec.Containers[i], fldPath.Index(i))...)
-	}
 
 	return allErrs
 }
@@ -281,26 +274,13 @@ func (s *simpleProvider) validatePodVolumes(pod *api.Pod) field.ErrorList {
 						fmt.Sprintf("is not allowed to be used")))
 				} else if mustBeReadOnly {
 					// Ensure all the VolumeMounts that use this volume are read-only
-					for i, c := range pod.Spec.InitContainers {
+					pods.VisitContainersWithPath(&pod.Spec, func(c *api.Container, p *field.Path) {
-						for j, cv := range c.VolumeMounts {
+						for i, cv := range c.VolumeMounts {
 							if cv.Name == v.Name && !cv.ReadOnly {
-								allErrs = append(allErrs, field.Invalid(
+								allErrs = append(allErrs, field.Invalid(p.Child("volumeMounts").Index(i).Child("readOnly"), cv.ReadOnly, "must be read-only"))
-									field.NewPath("spec", "initContainers").Index(i).Child("volumeMounts").Index(j).Child("readOnly"),
-									cv.ReadOnly, "must be read-only"),
-								)
 							}
 						}
-					}
+					})
-					for i, c := range pod.Spec.Containers {
-						for j, cv := range c.VolumeMounts {
-							if cv.Name == v.Name && !cv.ReadOnly {
-								allErrs = append(allErrs, field.Invalid(
-									field.NewPath("spec", "containers").Index(i).Child("volumeMounts").Index(j).Child("readOnly"),
-									cv.ReadOnly, "must be read-only"),
-								)
-							}
-						}
-					}
 				}
 
 			case policy.FlexVolume:

pkg/volume/util/BUILD

@@ -20,6 +20,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/api/legacyscheme:go_default_library",
+        "//pkg/api/v1/pod:go_default_library",
         "//pkg/apis/core/v1/helper:go_default_library",
         "//pkg/features:go_default_library",
         "//pkg/util/mount:go_default_library",

pkg/volume/util/nested_volumes.go

@@ -24,6 +24,7 @@ import (
 	"strings"
 
 	"k8s.io/api/core/v1"
+	podutil "k8s.io/kubernetes/pkg/api/v1/pod"
 )
 
 // getNestedMountpoints returns a list of mountpoint directories that should be created
@@ -70,16 +71,19 @@ func getNestedMountpoints(name, baseDir string, pod v1.Pod) ([]string, error) {
 		}
 		return nil
 	}
-	for _, container := range pod.Spec.InitContainers {
+
-		if err := checkContainer(&container); err != nil {
+	var retErr error
-			return nil, err
+	podutil.VisitContainers(&pod.Spec, func(c *v1.Container) bool {
-		}
+		retErr = checkContainer(c)
-	}
+		if retErr != nil {
-	for _, container := range pod.Spec.Containers {
+			return false
-		if err := checkContainer(&container); err != nil {
-			return nil, err
 		}
+		return true
+	})
+	if retErr != nil {
+		return nil, retErr
 	}
+
 	return retval, nil
 }
 

plugin/pkg/admission/alwayspullimages/BUILD

@@ -12,6 +12,7 @@ go_library(
     importpath = "k8s.io/kubernetes/plugin/pkg/admission/alwayspullimages",
     deps = [
         "//pkg/apis/core:go_default_library",
+        "//pkg/apis/core/pods:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",

plugin/pkg/admission/alwayspullimages/admission.go

@@ -31,6 +31,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/admission"
 	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/apis/core/pods"
 )
 
 // PluginName indicates name of admission plugin.
@@ -63,13 +64,9 @@ func (a *AlwaysPullImages) Admit(attributes admission.Attributes, o admission.Ob
 		return apierrors.NewBadRequest("Resource was marked with kind Pod but was unable to be converted")
 	}
 
-	for i := range pod.Spec.InitContainers {
+	pods.VisitContainersWithPath(&pod.Spec, func(c *api.Container, _ *field.Path) {
-		pod.Spec.InitContainers[i].ImagePullPolicy = api.PullAlways
+		c.ImagePullPolicy = api.PullAlways
-	}
+	})
-
-	for i := range pod.Spec.Containers {
-		pod.Spec.Containers[i].ImagePullPolicy = api.PullAlways
-	}
 
 	return nil
 }
@@ -85,23 +82,17 @@ func (*AlwaysPullImages) Validate(attributes admission.Attributes, o admission.O
 		return apierrors.NewBadRequest("Resource was marked with kind Pod but was unable to be converted")
 	}
 
-	for i := range pod.Spec.InitContainers {
+	var allErrs []error
-		if pod.Spec.InitContainers[i].ImagePullPolicy != api.PullAlways {
+	pods.VisitContainersWithPath(&pod.Spec, func(c *api.Container, p *field.Path) {
-			return admission.NewForbidden(attributes,
+		if c.ImagePullPolicy != api.PullAlways {
-				field.NotSupported(field.NewPath("spec", "initContainers").Index(i).Child("imagePullPolicy"),
+			allErrs = append(allErrs, admission.NewForbidden(attributes,
-					pod.Spec.InitContainers[i].ImagePullPolicy, []string{string(api.PullAlways)},
+				field.NotSupported(p.Child("imagePullPolicy"), c.ImagePullPolicy, []string{string(api.PullAlways)}),
-				),
+			))
-			)
-		}
-	}
-	for i := range pod.Spec.Containers {
-		if pod.Spec.Containers[i].ImagePullPolicy != api.PullAlways {
-			return admission.NewForbidden(attributes,
-				field.NotSupported(field.NewPath("spec", "containers").Index(i).Child("imagePullPolicy"),
-					pod.Spec.Containers[i].ImagePullPolicy, []string{string(api.PullAlways)},
-				),
-			)
 		}
+	})
+	if len(allErrs) > 0 {
+		// TODO: consider using utilerrors.NewAggregate(allErrs)
+		return allErrs[0]
 	}
 
 	return nil

plugin/pkg/admission/exec/BUILD

@@ -11,6 +11,7 @@ go_library(
     srcs = ["admission.go"],
     importpath = "k8s.io/kubernetes/plugin/pkg/admission/exec",
     deps = [
+        "//pkg/api/v1/pod:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",

plugin/pkg/admission/exec/admission.go

@@ -26,6 +26,7 @@ import (
 	genericadmissioninitializer "k8s.io/apiserver/pkg/admission/initializer"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/klog"
+	podutil "k8s.io/kubernetes/pkg/api/v1/pod"
 )
 
 const (
@@ -146,21 +147,16 @@ func (d *DenyExec) Validate(a admission.Attributes, o admission.ObjectInterfaces
 
 // isPrivileged will return true a pod has any privileged containers
 func isPrivileged(pod *corev1.Pod) bool {
-	for _, c := range pod.Spec.InitContainers {
+	var privileged bool
+	podutil.VisitContainers(&pod.Spec, func(c *corev1.Container) bool {
 		if c.SecurityContext == nil || c.SecurityContext.Privileged == nil {
-			continue
-		}
-		if *c.SecurityContext.Privileged {
 			return true
 		}
-	}
-	for _, c := range pod.Spec.Containers {
-		if c.SecurityContext == nil || c.SecurityContext.Privileged == nil {
-			continue
-		}
 		if *c.SecurityContext.Privileged {
-			return true
+			privileged = true
+			return false
 		}
-	}
+		return true
-	return false
+	})
+	return privileged
 }

plugin/pkg/admission/podpreset/BUILD

@@ -34,12 +34,14 @@ go_library(
     importpath = "k8s.io/kubernetes/plugin/pkg/admission/podpreset",
     deps = [
         "//pkg/apis/core:go_default_library",
+        "//pkg/apis/core/pods:go_default_library",
         "//pkg/apis/core/v1:go_default_library",
         "//staging/src/k8s.io/api/settings/v1alpha1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission/initializer:go_default_library",
         "//staging/src/k8s.io/client-go/informers:go_default_library",

plugin/pkg/admission/podpreset/admission.go

@@ -29,12 +29,14 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/admission"
 	genericadmissioninitializer "k8s.io/apiserver/pkg/admission/initializer"
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	settingsv1alpha1listers "k8s.io/client-go/listers/settings/v1alpha1"
 	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/apis/core/pods"
 	apiscorev1 "k8s.io/kubernetes/pkg/apis/core/v1"
 )
 
@@ -183,16 +185,11 @@ func safeToApplyPodPresetsOnPod(pod *api.Pod, podPresets []*settingsv1alpha1.Pod
 	if _, err := mergeVolumes(pod.Spec.Volumes, podPresets); err != nil {
 		errs = append(errs, err)
 	}
-	for _, ctr := range pod.Spec.Containers {
+	pods.VisitContainersWithPath(&pod.Spec, func(c *api.Container, _ *field.Path) {
-		if err := safeToApplyPodPresetsOnContainer(&ctr, podPresets); err != nil {
+		if err := safeToApplyPodPresetsOnContainer(c, podPresets); err != nil {
 			errs = append(errs, err)
 		}
-	}
+	})
-	for _, iCtr := range pod.Spec.InitContainers {
-		if err := safeToApplyPodPresetsOnContainer(&iCtr, podPresets); err != nil {
-			errs = append(errs, err)
-		}
-	}
 
 	return utilerrors.NewAggregate(errs)
 }

staging/publishing/import-restrictions.yaml

@@ -2,6 +2,7 @@
   allowedImports:
   - k8s.io/apimachinery
   - k8s.io/apiserver/pkg/util/feature
+  - k8s.io/component-base/featuregate/testing
   - k8s.io/kubernetes/pkg/apis/core
   - k8s.io/kubernetes/pkg/features
   - k8s.io/kubernetes/pkg/fieldpath