Merge pull request kubernetes#134345 from yuanwang04/restart-pod

Implement RestartAllContainers

Author: k8s-ci-robot

pkg/api/pod/util.go

@@ -428,6 +428,8 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po
 		OldPodViolatesLegacyMatchLabelKeysValidation:        false,
 		AllowContainerRestartPolicyRules:                    utilfeature.DefaultFeatureGate.Enabled(features.ContainerRestartRules),
 		AllowUserNamespacesWithVolumeDevices:                false,
+		// This also allows restart rules on sidecar containers.
+		AllowRestartAllContainers: utilfeature.DefaultFeatureGate.Enabled(features.RestartAllContainersOnContainerExits),
 	}
 
 	// If old spec uses relaxed validation or enabled the RelaxedEnvironmentVariableValidation feature gate,
@@ -476,6 +478,7 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po
 		opts.AllowSidecarResizePolicy = opts.AllowSidecarResizePolicy || hasRestartableInitContainerResizePolicy(oldPodSpec)
 
 		opts.AllowContainerRestartPolicyRules = opts.AllowContainerRestartPolicyRules || containerRestartRulesInUse(oldPodSpec)
+		opts.AllowRestartAllContainers = opts.AllowRestartAllContainers || restartAllContainersActionInUse(oldPodSpec)
 
 		// If old spec has userns and volume devices (doesn't work), we still allow
 		// modifications to it.
@@ -1826,3 +1829,28 @@ func workloadRefInUse(podSpec *api.PodSpec) bool {
 
 	return podSpec.WorkloadRef != nil
 }
+
+func restartAllContainersActionInUse(oldPodSpec *api.PodSpec) bool {
+	if oldPodSpec == nil {
+		return false
+	}
+	for _, c := range oldPodSpec.Containers {
+		for _, rule := range c.RestartPolicyRules {
+			if rule.Action == api.ContainerRestartRuleActionRestartAllContainers {
+				return true
+			}
+		}
+	}
+	for _, c := range oldPodSpec.InitContainers {
+		for _, rule := range c.RestartPolicyRules {
+			if rule.Action == api.ContainerRestartRuleActionRestartAllContainers {
+				return true
+			}
+		}
+		// This feature also allows sidecar containers to have rules.
+		if c.RestartPolicy != nil && *c.RestartPolicy == api.ContainerRestartPolicyAlways && len(c.RestartPolicyRules) > 0 {
+			return true
+		}
+	}
+	return false
+}

pkg/api/pod/util_test.go

@@ -6511,3 +6511,83 @@ func TestDisabledWorkload(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateRestartAllContainersOption(t *testing.T) {
+	policyAlways := api.ContainerRestartPolicyAlways
+	testCases := []struct {
+		name           string
+		oldPodSpec     *api.PodSpec
+		featureEnabled bool
+		want           bool
+	}{
+		{
+			name:           "feature enabled",
+			featureEnabled: true,
+			want:           true,
+		},
+		{
+			name:           "feature disabled",
+			featureEnabled: false,
+			want:           false,
+		},
+		{
+			name: "old pod spec has container without action",
+			oldPodSpec: &api.PodSpec{
+				Containers: []api.Container{{
+					Name: "container",
+				}},
+			},
+			featureEnabled: false,
+			want:           false,
+		},
+		{
+			name: "old pod spec has container with action",
+			oldPodSpec: &api.PodSpec{
+				Containers: []api.Container{{
+					Name: "container",
+					RestartPolicyRules: []api.ContainerRestartRule{{
+						Action: api.ContainerRestartRuleActionRestartAllContainers,
+						ExitCodes: &api.ContainerRestartRuleOnExitCodes{
+							Operator: api.ContainerRestartRuleOnExitCodesOpIn,
+							Values:   []int32{42},
+						},
+					}},
+				}},
+			},
+			featureEnabled: false,
+			want:           true,
+		},
+		{
+			name: "old pod spec has sidecar containers with rules",
+			oldPodSpec: &api.PodSpec{
+				InitContainers: []api.Container{{
+					RestartPolicy: &policyAlways,
+					RestartPolicyRules: []api.ContainerRestartRule{{
+						Action: api.ContainerRestartRuleActionRestartAllContainers,
+						ExitCodes: &api.ContainerRestartRuleOnExitCodes{
+							Operator: api.ContainerRestartRuleOnExitCodesOpIn,
+							Values:   []int32{42},
+						},
+					}},
+				}},
+			},
+			featureEnabled: false,
+			want:           true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			featuregatetesting.SetFeatureGatesDuringTest(t, utilfeature.DefaultFeatureGate, featuregatetesting.FeatureOverrides{
+				features.ContainerRestartRules:                tc.featureEnabled,
+				features.NodeDeclaredFeatures:                 tc.featureEnabled,
+				features.RestartAllContainersOnContainerExits: tc.featureEnabled,
+			})
+			// The new pod doesn't impact the outcome.
+			gotOptions := GetValidationOptionsFromPodSpecAndMeta(nil, tc.oldPodSpec, nil, nil)
+			if tc.want != gotOptions.AllowRestartAllContainers {
+				t.Errorf("unexpected diff, want: %v, got: %v", tc.want, gotOptions.AllowRestartAllContainers)
+			}
+		})
+	}
+}

pkg/api/v1/pod/util.go

@@ -418,11 +418,16 @@ func IsContainerRestartable(pod v1.PodSpec, container v1.Container) bool {
 // level policy are specified, pod-level restart policy is used.
 func ContainerShouldRestart(container v1.Container, pod v1.PodSpec, exitCode int32) bool {
 	if container.RestartPolicy != nil {
-		rule, ok := findMatchingContainerRestartRule(container, exitCode)
+		rule, ok := FindMatchingContainerRestartRule(container, exitCode)
 		if ok {
 			switch rule.Action {
 			case v1.ContainerRestartRuleActionRestart:
 				return true
+			case v1.ContainerRestartRuleActionRestartAllContainers:
+				if utilfeature.DefaultFeatureGate.Enabled(features.RestartAllContainersOnContainerExits) {
+					return true
+				}
+				// If feature is not enabled, fallback to container-level policy.
 			default:
 				// Do nothing, fallback to container-level restart policy.
 			}
@@ -454,10 +459,10 @@ func ContainerShouldRestart(container v1.Container, pod v1.PodSpec, exitCode int
 	}
 }
 
-// findMatchingContainerRestartRule returns a rule and true if the exitCode matched
+// FindMatchingContainerRestartRule returns a rule and true if the exitCode matched
 // one of the restart rules for the given container. Returns and empty rule and
 // false if no rules matched.
-func findMatchingContainerRestartRule(container v1.Container, exitCode int32) (rule v1.ContainerRestartRule, found bool) {
+func FindMatchingContainerRestartRule(container v1.Container, exitCode int32) (rule v1.ContainerRestartRule, found bool) {
 	for _, rule := range container.RestartPolicyRules {
 		if rule.ExitCodes != nil {
 			exitCodeMatched := false
@@ -483,6 +488,30 @@ func findMatchingContainerRestartRule(container v1.Container, exitCode int32) (r
 	return v1.ContainerRestartRule{}, false
 }
 
+// AllContainersCouldRestart returns true if all containers could be restarted
+// for the given pod. This is true if any container has a RestartAllContainers
+// action.
+func AllContainersCouldRestart(pod *v1.PodSpec) bool {
+	if pod == nil {
+		return false
+	}
+	for _, container := range pod.InitContainers {
+		for _, rule := range container.RestartPolicyRules {
+			if rule.Action == v1.ContainerRestartRuleActionRestartAllContainers {
+				return true
+			}
+		}
+	}
+	for _, container := range pod.Containers {
+		for _, rule := range container.RestartPolicyRules {
+			if rule.Action == v1.ContainerRestartRuleActionRestartAllContainers {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // CalculatePodStatusObservedGeneration calculates the observedGeneration for the pod status.
 // This is used to track the generation of the pod that was observed by the kubelet.
 // The observedGeneration is set to the pod's generation when the feature gate

pkg/api/v1/pod/util_test.go

@@ -28,6 +28,9 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	featuregatetesting "k8s.io/component-base/featuregate/testing"
+	"k8s.io/kubernetes/pkg/features"
 )
 
 func TestVisitContainers(t *testing.T) {
@@ -1182,6 +1185,7 @@ func TestContainerHasRestartablePolicy(t *testing.T) {
 		name      string
 		container v1.Container
 		podSpec   v1.PodSpec
+		podStatus v1.PodStatus
 		exitCode  int32
 		expected  bool
 	}{
@@ -1219,6 +1223,23 @@ func TestContainerHasRestartablePolicy(t *testing.T) {
 			exitCode: 99,
 			expected: true,
 		},
+		{
+			name: "Rule: 'In' operator matches with 'RestartAllContainers' action",
+			container: v1.Container{
+				RestartPolicy: &containerRestartPolicyNever,
+				RestartPolicyRules: []v1.ContainerRestartRule{
+					{
+						Action: v1.ContainerRestartRuleActionRestartAllContainers,
+						ExitCodes: &v1.ContainerRestartRuleOnExitCodes{
+							Operator: v1.ContainerRestartRuleOnExitCodesOpIn,
+							Values:   []int32{42, 50, 60},
+						},
+					},
+				},
+			},
+			exitCode: 42,
+			expected: true,
+		},
 		{
 			name: "Rule: 'In' operator does not match, should fall back to container policy",
 			container: v1.Container{
@@ -1311,6 +1332,11 @@ func TestContainerHasRestartablePolicy(t *testing.T) {
 			expected:  true,
 		},
 	}
+	featuregatetesting.SetFeatureGatesDuringTest(t, utilfeature.DefaultFeatureGate, featuregatetesting.FeatureOverrides{
+		features.ContainerRestartRules:                true,
+		features.NodeDeclaredFeatures:                 true,
+		features.RestartAllContainersOnContainerExits: true,
+	})
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -1325,6 +1351,90 @@ func TestContainerHasRestartablePolicy(t *testing.T) {
 	}
 }
 
+func TestAllContainersCouldRestart(t *testing.T) {
+	restartPolicyAlways := v1.ContainerRestartPolicyAlways
+
+	testCases := []struct {
+		name     string
+		podSpec  *v1.PodSpec
+		expected bool
+	}{
+		{
+			name: "Pod with rules in init containers",
+			podSpec: &v1.PodSpec{
+				InitContainers: []v1.Container{
+					{
+						RestartPolicyRules: []v1.ContainerRestartRule{
+							{
+								Action: v1.ContainerRestartRuleActionRestartAllContainers,
+								ExitCodes: &v1.ContainerRestartRuleOnExitCodes{
+									Operator: v1.ContainerRestartRuleOnExitCodesOpIn,
+									Values:   []int32{1},
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "Pod with rules in sidecar containers",
+			podSpec: &v1.PodSpec{
+				InitContainers: []v1.Container{
+					{
+						RestartPolicy: &restartPolicyAlways,
+						RestartPolicyRules: []v1.ContainerRestartRule{
+							{
+								Action: v1.ContainerRestartRuleActionRestartAllContainers,
+								ExitCodes: &v1.ContainerRestartRuleOnExitCodes{
+									Operator: v1.ContainerRestartRuleOnExitCodesOpIn,
+									Values:   []int32{1},
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "Pod with rules in regular containers",
+			podSpec: &v1.PodSpec{
+				Containers: []v1.Container{
+					{
+						RestartPolicy: &restartPolicyAlways,
+						RestartPolicyRules: []v1.ContainerRestartRule{
+							{
+								Action: v1.ContainerRestartRuleActionRestartAllContainers,
+								ExitCodes: &v1.ContainerRestartRuleOnExitCodes{
+									Operator: v1.ContainerRestartRuleOnExitCodesOpIn,
+									Values:   []int32{1},
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name:     "Pod without rules",
+			podSpec:  &v1.PodSpec{},
+			expected: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := AllContainersCouldRestart(tc.podSpec)
+			if got != tc.expected {
+				t.Errorf("AllContainersRestarting() = %v, want %v", got, tc.expected)
+			}
+		})
+	}
+}
+
 func TestFindMatchingContainerRestartRule(t *testing.T) {
 	ruleIn := v1.ContainerRestartRule{
 		Action: v1.ContainerRestartRuleActionRestart,
@@ -1403,7 +1513,7 @@ func TestFindMatchingContainerRestartRule(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			rule, found := findMatchingContainerRestartRule(tc.container, tc.exitCode)
+			rule, found := FindMatchingContainerRestartRule(tc.container, tc.exitCode)
 			if found != tc.expectedFound {
 				t.Errorf("FindMatchingContainerRestartRule() found = %v, want %v", found, tc.expectedFound)
 			}

pkg/apis/core/types.go

@@ -3154,6 +3154,8 @@ const (
 	// If both PodResizePending and PodResizeInProgress are set, it means that a new resize was
 	// requested in the middle of a previous pod resize that is still in progress.
 	PodResizeInProgress PodConditionType = "PodResizeInProgress"
+	// AllContainersRestarting indicates that all containers of the pod is being restarted.
+	AllContainersRestarting PodConditionType = "AllContainersRestarting"
 )
 
 // PodCondition represents pod's condition
@@ -3241,9 +3243,15 @@ type ContainerRestartRule struct {
 // container exits.
 type ContainerRestartRuleAction string
 
-// The only valid action is Restart.
+// These are valid restart rule actions.
 const (
+	// The container will be restarted if the rule matches. Only valid on normal init container and
+	// regular containers. Not valid on sidecar containers and ephemeral containers.
 	ContainerRestartRuleActionRestart ContainerRestartRuleAction = "Restart"
+	// All containers (except ephemeral containers) inside the pod will be terminated and restarted.
+	// Valid on normal init container, sidecar containers, and regular containers. Not valid on
+	// ephemeral containers.
+	ContainerRestartRuleActionRestartAllContainers ContainerRestartRuleAction = "RestartAllContainers"
 )
 
 // ContainerRestartRuleOnExitCodes describes the condition