Merge pull request kubernetes#116429 from SergeyKanzhelev/sidecar

Add SidecarContainers feature

Author: k8s-ci-robot

api/openapi-spec/swagger.json

@@ -1179,6 +1179,10 @@
             "default": {},
             "description": "Compute Resources required by this container. Cannot be updated. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/"
           },
+          "restartPolicy": {
+            "description": "RestartPolicy defines the restart behavior of individual containers in a pod. This field may only be set for init containers, and the only allowed value is \"Always\". For non-init containers or when this field is not specified, the restart behavior is defined by the Pod's restart policy and the container type. Setting the RestartPolicy as \"Always\" for the init container will have the following effect: this init container will be continually restarted on exit until all regular containers have terminated. Once all regular containers have completed, all init containers with restartPolicy \"Always\" will be shut down. This lifecycle differs from normal init containers and is often referred to as a \"sidecar\" container. Although this init container still starts in the init container sequence, it does not wait for the container to complete before proceeding to the next init container. Instead, the next init container starts immediately after this init container is started, or after any startupProbe has successfully completed.",
+            "type": "string"
+          },
           "securityContext": {
             "allOf": [
               {
@@ -2029,6 +2033,10 @@
             "default": {},
             "description": "Resources are not allowed for ephemeral containers. Ephemeral containers use spare resources already allocated to the pod."
           },
+          "restartPolicy": {
+            "description": "Restart policy for the container to manage the restart behavior of each container within a pod. This may only be set for init containers. You cannot set this field on ephemeral containers.",
+            "type": "string"
+          },
           "securityContext": {
             "allOf": [
               {

api/openapi-spec/v3/api__v1_openapi.json

@@ -1802,6 +1802,10 @@
             "default": {},
             "description": "Compute Resources required by this container. Cannot be updated. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/"
           },
+          "restartPolicy": {
+            "description": "RestartPolicy defines the restart behavior of individual containers in a pod. This field may only be set for init containers, and the only allowed value is \"Always\". For non-init containers or when this field is not specified, the restart behavior is defined by the Pod's restart policy and the container type. Setting the RestartPolicy as \"Always\" for the init container will have the following effect: this init container will be continually restarted on exit until all regular containers have terminated. Once all regular containers have completed, all init containers with restartPolicy \"Always\" will be shut down. This lifecycle differs from normal init containers and is often referred to as a \"sidecar\" container. Although this init container still starts in the init container sequence, it does not wait for the container to complete before proceeding to the next init container. Instead, the next init container starts immediately after this init container is started, or after any startupProbe has successfully completed.",
+            "type": "string"
+          },
           "securityContext": {
             "allOf": [
               {
@@ -2236,6 +2240,10 @@
             "default": {},
             "description": "Resources are not allowed for ephemeral containers. Ephemeral containers use spare resources already allocated to the pod."
           },
+          "restartPolicy": {
+            "description": "Restart policy for the container to manage the restart behavior of each container within a pod. This may only be set for init containers. You cannot set this field on ephemeral containers.",
+            "type": "string"
+          },
           "securityContext": {
             "allOf": [
               {

api/openapi-spec/v3/apis__apps__v1_openapi.json

@@ -1093,6 +1093,10 @@
             "default": {},
             "description": "Compute Resources required by this container. Cannot be updated. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/"
           },
+          "restartPolicy": {
+            "description": "RestartPolicy defines the restart behavior of individual containers in a pod. This field may only be set for init containers, and the only allowed value is \"Always\". For non-init containers or when this field is not specified, the restart behavior is defined by the Pod's restart policy and the container type. Setting the RestartPolicy as \"Always\" for the init container will have the following effect: this init container will be continually restarted on exit until all regular containers have terminated. Once all regular containers have completed, all init containers with restartPolicy \"Always\" will be shut down. This lifecycle differs from normal init containers and is often referred to as a \"sidecar\" container. Although this init container still starts in the init container sequence, it does not wait for the container to complete before proceeding to the next init container. Instead, the next init container starts immediately after this init container is started, or after any startupProbe has successfully completed.",
+            "type": "string"
+          },
           "securityContext": {
             "allOf": [
               {
@@ -1527,6 +1531,10 @@
             "default": {},
             "description": "Resources are not allowed for ephemeral containers. Ephemeral containers use spare resources already allocated to the pod."
           },
+          "restartPolicy": {
+            "description": "Restart policy for the container to manage the restart behavior of each container within a pod. This may only be set for init containers. You cannot set this field on ephemeral containers.",
+            "type": "string"
+          },
           "securityContext": {
             "allOf": [
               {

api/openapi-spec/v3/apis__batch__v1_openapi.json

@@ -510,6 +510,14 @@ func dropDisabledFields(
 			podSpec.EphemeralContainers[i].ResizePolicy = nil
 		}
 	}
+
+	if !utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) && !restartableInitContainersInUse(oldPodSpec) {
+		// Drop the RestartPolicy field of init containers.
+		for i := range podSpec.InitContainers {
+			podSpec.InitContainers[i].RestartPolicy = nil
+		}
+		// For other types of containers, validateContainers will handle them.
+	}
 }
 
 // dropDisabledPodStatusFields removes disabled fields from the pod status
@@ -778,6 +786,23 @@ func schedulingGatesInUse(podSpec *api.PodSpec) bool {
 	return len(podSpec.SchedulingGates) != 0
 }
 
+// restartableInitContainersInUse returns true if the pod spec is non-nil and
+// it has any init container with ContainerRestartPolicyAlways.
+func restartableInitContainersInUse(podSpec *api.PodSpec) bool {
+	if podSpec == nil {
+		return false
+	}
+	var inUse bool
+	VisitContainers(podSpec, InitContainers, func(c *api.Container, containerType ContainerType) bool {
+		if c.RestartPolicy != nil && *c.RestartPolicy == api.ContainerRestartPolicyAlways {
+			inUse = true
+			return false
+		}
+		return true
+	})
+	return inUse
+}
+
 func hasInvalidLabelValueInAffinitySelector(spec *api.PodSpec) bool {
 	if spec.Affinity != nil {
 		if spec.Affinity.PodAffinity != nil {

pkg/api/pod/util.go

@@ -2052,6 +2052,109 @@ func TestDropInPlacePodVerticalScaling(t *testing.T) {
 	}
 }
 
+func TestDropSidecarContainers(t *testing.T) {
+	containerRestartPolicyAlways := api.ContainerRestartPolicyAlways
+
+	podWithSidecarContainers := func() *api.Pod {
+		return &api.Pod{
+			Spec: api.PodSpec{
+				InitContainers: []api.Container{
+					{
+						Name:          "c1",
+						Image:         "image",
+						RestartPolicy: &containerRestartPolicyAlways,
+					},
+				},
+			},
+		}
+	}
+
+	podWithoutSidecarContainers := func() *api.Pod {
+		return &api.Pod{
+			Spec: api.PodSpec{
+				InitContainers: []api.Container{
+					{
+						Name:  "c1",
+						Image: "image",
+					},
+				},
+			},
+		}
+	}
+
+	podInfo := []struct {
+		description         string
+		hasSidecarContainer bool
+		pod                 func() *api.Pod
+	}{
+		{
+			description:         "has a sidecar container",
+			hasSidecarContainer: true,
+			pod:                 podWithSidecarContainers,
+		},
+		{
+			description:         "does not have a sidecar container",
+			hasSidecarContainer: false,
+			pod:                 podWithoutSidecarContainers,
+		},
+		{
+			description:         "is nil",
+			hasSidecarContainer: false,
+			pod:                 func() *api.Pod { return nil },
+		},
+	}
+
+	for _, enabled := range []bool{true, false} {
+		for _, oldPodInfo := range podInfo {
+			for _, newPodInfo := range podInfo {
+				oldPodHasSidecarContainer, oldPod := oldPodInfo.hasSidecarContainer, oldPodInfo.pod()
+				newPodHasSidecarContainer, newPod := newPodInfo.hasSidecarContainer, newPodInfo.pod()
+				if newPod == nil {
+					continue
+				}
+
+				t.Run(fmt.Sprintf("feature enabled=%v, old pod %v, new pod %v", enabled, oldPodInfo.description, newPodInfo.description), func(t *testing.T) {
+					defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SidecarContainers, enabled)()
+
+					var oldPodSpec *api.PodSpec
+					if oldPod != nil {
+						oldPodSpec = &oldPod.Spec
+					}
+					dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil)
+
+					// old pod should never be changed
+					if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) {
+						t.Errorf("old pod changed: %v", cmp.Diff(oldPod, oldPodInfo.pod()))
+					}
+
+					switch {
+					case enabled || oldPodHasSidecarContainer:
+						// new pod shouldn't change if feature enabled or if old pod has
+						// any sidecar container
+						if !reflect.DeepEqual(newPod, newPodInfo.pod()) {
+							t.Errorf("new pod changed: %v", cmp.Diff(newPod, newPodInfo.pod()))
+						}
+					case newPodHasSidecarContainer:
+						// new pod should be changed
+						if reflect.DeepEqual(newPod, newPodInfo.pod()) {
+							t.Errorf("new pod was not changed")
+						}
+						// new pod should not have any sidecar container
+						if !reflect.DeepEqual(newPod, podWithoutSidecarContainers()) {
+							t.Errorf("new pod has a sidecar container: %v", cmp.Diff(newPod, podWithoutSidecarContainers()))
+						}
+					default:
+						// new pod should not need to be changed
+						if !reflect.DeepEqual(newPod, newPodInfo.pod()) {
+							t.Errorf("new pod changed: %v", cmp.Diff(newPod, newPodInfo.pod()))
+						}
+					}
+				})
+			}
+		}
+	}
+}
+
 func TestMarkPodProposedForResize(t *testing.T) {
 	testCases := []struct {
 		desc        string

pkg/api/pod/util_test.go

@@ -65,7 +65,7 @@ func PodRequests(pod *v1.Pod, opts PodResourcesOptions) v1.ResourceList {
 			cs, found := containerStatuses[container.Name]
 			if found {
 				if pod.Status.Resize == v1.PodResizeStatusInfeasible {
-					containerReqs = cs.AllocatedResources
+					containerReqs = cs.AllocatedResources.DeepCopy()
 				} else {
 					containerReqs = max(container.Resources.Requests, cs.AllocatedResources)
 				}
@@ -83,20 +83,44 @@ func PodRequests(pod *v1.Pod, opts PodResourcesOptions) v1.ResourceList {
 		addResourceList(reqs, containerReqs)
 	}
 
+	restartableInitContainerReqs := v1.ResourceList{}
+	initContainerReqs := v1.ResourceList{}
 	// init containers define the minimum of any resource
 	// Note: In-place resize is not allowed for InitContainers, so no need to check for ResizeStatus value
+	//
+	// Let's say `InitContainerUse(i)` is the resource requirements when the i-th
+	// init container is initializing, then
+	// `InitContainerUse(i) = sum(Resources of restartable init containers with index < i) + Resources of i-th init container`.
+	//
+	// See https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/753-sidecar-containers#exposing-pod-resource-requirements for the detail.
 	for _, container := range pod.Spec.InitContainers {
 		containerReqs := container.Resources.Requests
 		if len(opts.NonMissingContainerRequests) > 0 {
 			containerReqs = applyNonMissing(containerReqs, opts.NonMissingContainerRequests)
 		}
 
+		if container.RestartPolicy != nil && *container.RestartPolicy == v1.ContainerRestartPolicyAlways {
+			// and add them to the resulting cumulative container requests
+			addResourceList(reqs, containerReqs)
+
+			// track our cumulative restartable init container resources
+			addResourceList(restartableInitContainerReqs, containerReqs)
+			containerReqs = restartableInitContainerReqs
+		} else {
+			tmp := v1.ResourceList{}
+			addResourceList(tmp, containerReqs)
+			addResourceList(tmp, restartableInitContainerReqs)
+			containerReqs = tmp
+		}
+
 		if opts.ContainerFn != nil {
 			opts.ContainerFn(containerReqs, podutil.InitContainers)
 		}
-		maxResourceList(reqs, containerReqs)
+		maxResourceList(initContainerReqs, containerReqs)
 	}
 
+	maxResourceList(reqs, initContainerReqs)
+
 	// Add overhead for running a pod to the sum of requests if requested:
 	if !opts.ExcludeOverhead && pod.Spec.Overhead != nil {
 		addResourceList(reqs, pod.Spec.Overhead)
@@ -135,14 +159,40 @@ func PodLimits(pod *v1.Pod, opts PodResourcesOptions) v1.ResourceList {
 		}
 		addResourceList(limits, container.Resources.Limits)
 	}
+
+	restartableInitContainerLimits := v1.ResourceList{}
+	initContainerLimits := v1.ResourceList{}
 	// init containers define the minimum of any resource
+	//
+	// Let's say `InitContainerUse(i)` is the resource requirements when the i-th
+	// init container is initializing, then
+	// `InitContainerUse(i) = sum(Resources of restartable init containers with index < i) + Resources of i-th init container`.
+	//
+	// See https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/753-sidecar-containers#exposing-pod-resource-requirements for the detail.
 	for _, container := range pod.Spec.InitContainers {
+		containerLimits := container.Resources.Limits
+		// Is the init container marked as a restartable init container?
+		if container.RestartPolicy != nil && *container.RestartPolicy == v1.ContainerRestartPolicyAlways {
+			addResourceList(limits, containerLimits)
+
+			// track our cumulative restartable init container resources
+			addResourceList(restartableInitContainerLimits, containerLimits)
+			containerLimits = restartableInitContainerLimits
+		} else {
+			tmp := v1.ResourceList{}
+			addResourceList(tmp, containerLimits)
+			addResourceList(tmp, restartableInitContainerLimits)
+			containerLimits = tmp
+		}
+
 		if opts.ContainerFn != nil {
-			opts.ContainerFn(container.Resources.Limits, podutil.InitContainers)
+			opts.ContainerFn(containerLimits, podutil.InitContainers)
 		}
-		maxResourceList(limits, container.Resources.Limits)
+		maxResourceList(initContainerLimits, containerLimits)
 	}
 
+	maxResourceList(limits, initContainerLimits)
+
 	// Add overhead to non-zero limits if requested:
 	if !opts.ExcludeOverhead && pod.Spec.Overhead != nil {
 		for name, quantity := range pod.Spec.Overhead {