Update kube gen & play to use pod restart policy

Podman kube generate now uses the pod's restart policy
when generating the kube yaml. If generating from containers
only, use the restart policy of the first non-init container.
Podman kube play applies the pod restart policy from the yaml
file to the pod. The containers within a pod inherit this restart
policy.

Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>

libpod/kube.go

@@ -97,23 +97,8 @@ func (p *Pod) GenerateForKube(ctx context.Context, getService bool) (*v1.Pod, []
 	}
 	pod.Spec.HostAliases = extraHost
 
-	// vendor/k8s.io/api/core/v1/types.go: v1.Container cannot save restartPolicy
+	// Set the pod's restart policy
-	// so set it at here
+	pod.Spec.RestartPolicy = getPodRestartPolicy(p.config.RestartPolicy)
-	for _, ctr := range allContainers {
-		if !ctr.IsInfra() {
-			switch ctr.config.RestartPolicy {
-			case define.RestartPolicyAlways:
-				pod.Spec.RestartPolicy = v1.RestartPolicyAlways
-			case define.RestartPolicyOnFailure:
-				pod.Spec.RestartPolicy = v1.RestartPolicyOnFailure
-			case define.RestartPolicyNo:
-				pod.Spec.RestartPolicy = v1.RestartPolicyNever
-			default: // some pod create from cmdline, such as "", so set it to "" as k8s automatically defaults to always
-				pod.Spec.RestartPolicy = ""
-			}
-			break
-		}
-	}
 
 	if p.SharesPID() {
 		// unfortunately, go doesn't have a nice way to specify a pointer to a bool
@@ -136,7 +121,7 @@ func (p *Pod) getInfraContainer() (*Container, error) {
 // kind YAML.
 func GenerateForKubeDeployment(ctx context.Context, pod *YAMLPod, options entities.GenerateKubeOptions) (*YAMLDeployment, error) {
 	// Restart policy for Deployments can only be set to Always
-	if options.Type == define.K8sKindDeployment && !(pod.Spec.RestartPolicy == "" || pod.Spec.RestartPolicy == define.RestartPolicyAlways) {
+	if options.Type == define.K8sKindDeployment && !(pod.Spec.RestartPolicy == "" || pod.Spec.RestartPolicy == v1.RestartPolicyAlways) {
 		return nil, fmt.Errorf("k8s Deployments can only have restartPolicy set to Always")
 	}
 
@@ -599,6 +584,7 @@ func simplePodWithV1Containers(ctx context.Context, ctrs []*Container, getServic
 	kubeAnnotations := make(map[string]string)
 	ctrNames := make([]string, 0, len(ctrs))
 	var hostname string
+	var restartPolicy *string
 	for _, ctr := range ctrs {
 		ctrNames = append(ctrNames, removeUnderscores(ctr.Name()))
 		for k, v := range ctr.config.Spec.Annotations {
@@ -623,6 +609,11 @@ func simplePodWithV1Containers(ctx context.Context, ctrs []*Container, getServic
 			}
 		}
 
+		// Use the restart policy of the first non-init container
+		if !isInit && restartPolicy == nil {
+			restartPolicy = &ctr.config.RestartPolicy
+		}
+
 		if ctr.config.Spec.Process != nil {
 			var ulimitArr []string
 			defaultUlimits := util.DefaultContainerConfig().Ulimits()
@@ -700,7 +691,7 @@ func simplePodWithV1Containers(ctx context.Context, ctrs []*Container, getServic
 		podName += "-pod"
 	}
 
-	return newPodObject(
+	pod := newPodObject(
 		podName,
 		kubeAnnotations,
 		kubeInitCtrs,
@@ -709,7 +700,30 @@ func simplePodWithV1Containers(ctx context.Context, ctrs []*Container, getServic
 		&podDNS,
 		hostNetwork,
 		hostUsers,
-		hostname), nil
+		hostname)
+
+	// Set the pod's restart policy
+	policy := ""
+	if restartPolicy != nil {
+		policy = *restartPolicy
+	}
+	pod.Spec.RestartPolicy = getPodRestartPolicy(policy)
+
+	return pod, nil
+}
+
+// getPodRestartPolicy returns the pod restart policy to be set in the generated kube yaml
+func getPodRestartPolicy(policy string) v1.RestartPolicy {
+	switch policy {
+	case define.RestartPolicyNo:
+		return v1.RestartPolicyNever
+	case define.RestartPolicyAlways:
+		return v1.RestartPolicyAlways
+	case define.RestartPolicyOnFailure:
+		return v1.RestartPolicyOnFailure
+	default: // some pod/ctr create from cmdline, such as "" - set it to "" and let k8s handle the defaults
+		return ""
+	}
 }
 
 // containerToV1Container converts information we know about a libpod container

pkg/domain/infra/abi/play.go

@@ -592,16 +592,16 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
 		return nil, nil, err
 	}
 
-	var ctrRestartPolicy string
+	// Set the restart policy from the kube yaml at the pod level in podman
 	switch podYAML.Spec.RestartPolicy {
 	case v1.RestartPolicyAlways:
-		ctrRestartPolicy = define.RestartPolicyAlways
+		podSpec.PodSpecGen.RestartPolicy = define.RestartPolicyAlways
 	case v1.RestartPolicyOnFailure:
-		ctrRestartPolicy = define.RestartPolicyOnFailure
+		podSpec.PodSpecGen.RestartPolicy = define.RestartPolicyOnFailure
 	case v1.RestartPolicyNever:
-		ctrRestartPolicy = define.RestartPolicyNo
+		podSpec.PodSpecGen.RestartPolicy = define.RestartPolicyNo
 	default: // Default to Always
-		ctrRestartPolicy = define.RestartPolicyAlways
+		podSpec.PodSpecGen.RestartPolicy = define.RestartPolicyAlways
 	}
 
 	if podOpt.Infra {
@@ -775,7 +775,6 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
 			PodName:            podName,
 			PodSecurityContext: podYAML.Spec.SecurityContext,
 			ReadOnly:           readOnly,
-			RestartPolicy:      ctrRestartPolicy,
 			SeccompPaths:       seccompPaths,
 			SecretsManager:     secretsManager,
 			UserNSIsHost:       p.Userns.IsHost(),

test/e2e/generate_kube_test.go

@@ -528,18 +528,21 @@ var _ = Describe("Podman kube generate", func() {
 		Expect(err).ToNot(HaveOccurred())
 	})
 
-	It("podman generate kube on pod with restartPolicy", func() {
+	It("podman generate kube on pod with restartPolicy set for container in a pod", func() {
+		//TODO: v5.0 - change/remove test once we block --restart on container when it is in a pod
 		// podName,  set,  expect
 		testSli := [][]string{
 			{"testPod1", "", ""}, // some pod create from cmdline, so set it to an empty string and let k8s default it to Always
 			{"testPod2", "always", "Always"},
 			{"testPod3", "on-failure", "OnFailure"},
 			{"testPod4", "no", "Never"},
+			{"testPod5", "never", "Never"},
 		}
 
 		for k, v := range testSli {
 			podName := v[0]
-			podSession := podmanTest.Podman([]string{"pod", "create", "--name", podName})
+			// Need to set --restart during pod creation as gen kube only picks up the pod's restart policy
+			podSession := podmanTest.Podman([]string{"pod", "create", "--restart", v[1], "--name", podName})
 			podSession.WaitWithDefaultTimeout()
 			Expect(podSession).Should(Exit(0))
 
@@ -561,6 +564,67 @@ var _ = Describe("Podman kube generate", func() {
 		}
 	})
 
+	It("podman generate kube on pod with restartPolicy", func() {
+		// podName,  set,  expect
+		testSli := [][]string{
+			{"testPod1", "", ""},
+			{"testPod2", "always", "Always"},
+			{"testPod3", "on-failure", "OnFailure"},
+			{"testPod4", "no", "Never"},
+			{"testPod5", "never", "Never"},
+		}
+
+		for k, v := range testSli {
+			podName := v[0]
+			podSession := podmanTest.Podman([]string{"pod", "create", "--restart", v[1], podName})
+			podSession.WaitWithDefaultTimeout()
+			Expect(podSession).Should(Exit(0))
+
+			ctrName := "ctr" + strconv.Itoa(k)
+			ctr1Session := podmanTest.Podman([]string{"create", "--name", ctrName, "--pod", podName, ALPINE, "top"})
+			ctr1Session.WaitWithDefaultTimeout()
+			Expect(ctr1Session).Should(Exit(0))
+
+			kube := podmanTest.Podman([]string{"generate", "kube", podName})
+			kube.WaitWithDefaultTimeout()
+			Expect(kube).Should(Exit(0))
+
+			pod := new(v1.Pod)
+			err := yaml.Unmarshal(kube.Out.Contents(), pod)
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(string(pod.Spec.RestartPolicy)).To(Equal(v[2]))
+		}
+	})
+
+	It("podman generate kube on ctr with restartPolicy", func() {
+		// podName,  set,  expect
+		testSli := [][]string{
+			{"", ""}, // some ctr created from cmdline, set it to "" and let k8s default it to Always
+			{"always", "Always"},
+			{"on-failure", "OnFailure"},
+			{"no", "Never"},
+			{"never", "Never"},
+		}
+
+		for k, v := range testSli {
+			ctrName := "ctr" + strconv.Itoa(k)
+			ctrSession := podmanTest.Podman([]string{"create", "--restart", v[0], "--name", ctrName, ALPINE, "top"})
+			ctrSession.WaitWithDefaultTimeout()
+			Expect(ctrSession).Should(Exit(0))
+
+			kube := podmanTest.Podman([]string{"generate", "kube", ctrName})
+			kube.WaitWithDefaultTimeout()
+			Expect(kube).Should(Exit(0))
+
+			pod := new(v1.Pod)
+			err := yaml.Unmarshal(kube.Out.Contents(), pod)
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(string(pod.Spec.RestartPolicy)).To(Equal(v[1]))
+		}
+	})
+
 	It("podman generate kube on pod with memory limit", func() {
 		SkipIfRootlessCgroupsV1("Not supported for rootless + CgroupsV1")
 		podName := "testMemoryLimit"
@@ -1465,13 +1529,12 @@ USER test1`
 	})
 
 	It("podman generate kube on pod with --type=deployment and --restart=no should fail", func() {
-		// TODO: When we add --restart for pods, fix this test to reflect that
 		podName := "test-pod"
-		session := podmanTest.Podman([]string{"pod", "create", podName})
+		session := podmanTest.Podman([]string{"pod", "create", "--restart", "no", podName})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
 
-		session = podmanTest.Podman([]string{"create", "--pod", podName, "--restart", "no", ALPINE, "top"})
+		session = podmanTest.Podman([]string{"create", "--pod", podName, ALPINE, "top"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
 

test/e2e/play_kube_test.go

@@ -2382,7 +2382,7 @@ var _ = Describe("Podman play kube", func() {
 			kube.WaitWithDefaultTimeout()
 			Expect(kube).Should(Exit(0))
 
-			inspect := podmanTest.Podman([]string{"inspect", getCtrNameInPod(pod), "--format", "{{.HostConfig.RestartPolicy.Name}}"})
+			inspect := podmanTest.Podman([]string{"inspect", pod.Name, "--format", "{{.RestartPolicy}}"})
 			inspect.WaitWithDefaultTimeout()
 			Expect(inspect).Should(Exit(0))
 			Expect(inspect.OutputToString()).To(Equal(v[2]))