From fa1ba17bc16fd3e4be650f436784ed7cef2f159f Mon Sep 17 00:00:00 2001 From: Urvashi Mohnani Date: Thu, 13 Apr 2023 13:45:28 -0400 Subject: [PATCH] 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 --- libpod/kube.go | 54 +++++++++++++++---------- pkg/domain/infra/abi/play.go | 11 +++-- test/e2e/generate_kube_test.go | 73 +++++++++++++++++++++++++++++++--- test/e2e/play_kube_test.go | 2 +- 4 files changed, 108 insertions(+), 32 deletions(-) diff --git a/libpod/kube.go b/libpod/kube.go index 11fb8da856..093760de72 100644 --- a/libpod/kube.go +++ b/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 - // so set it at here - 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 - } - } + // Set the pod's restart policy + pod.Spec.RestartPolicy = getPodRestartPolicy(p.config.RestartPolicy) 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 diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index dfcdb0ce70..ce602b5e74 100644 --- a/pkg/domain/infra/abi/play.go +++ b/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(), diff --git a/test/e2e/generate_kube_test.go b/test/e2e/generate_kube_test.go index ee7339c7e9..12ae1ff340 100644 --- a/test/e2e/generate_kube_test.go +++ b/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)) diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index dc2e07555f..b3d5ffe18f 100644 --- a/test/e2e/play_kube_test.go +++ b/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]))