Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle the restartPolicy for play kube and generate kube #7671

Merged
merged 1 commit into from
Sep 18, 2020
Merged

handle the restartPolicy for play kube and generate kube #7671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions libpod/kube.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,24 @@ func (p *Pod) GenerateForKube() (*v1.Pod, []v1.ServicePort, error) {
}
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 RestartPolicyAlways:
pod.Spec.RestartPolicy = v1.RestartPolicyAlways
case RestartPolicyOnFailure:
pod.Spec.RestartPolicy = v1.RestartPolicyOnFailure
case RestartPolicyNo:
pod.Spec.RestartPolicy = v1.RestartPolicyNever
default: // some pod create from cmdline, such as "", so set it to Never
pod.Spec.RestartPolicy = v1.RestartPolicyNever
}
break
}
}

if p.SharesPID() {
// unfortunately, go doesn't have a nice way to specify a pointer to a bool
b := true
Expand Down
13 changes: 13 additions & 0 deletions pkg/domain/infra/abi/play.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,18 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
return nil, err
}

var ctrRestartPolicy string
switch podYAML.Spec.RestartPolicy {
case v1.RestartPolicyAlways:
ctrRestartPolicy = libpod.RestartPolicyAlways
case v1.RestartPolicyOnFailure:
ctrRestartPolicy = libpod.RestartPolicyOnFailure
case v1.RestartPolicyNever:
ctrRestartPolicy = libpod.RestartPolicyNo
default: // Default to Always
ctrRestartPolicy = libpod.RestartPolicyAlways
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the default? I remember the upstream issue mentioning OnFailure was the default for Kube. @haircommander Would you happen to know this?

Copy link
Collaborator Author

@zhangguanzhang zhangguanzhang Sep 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

[root@k8s-m1 ~]# kubectl explain pod.spec.restartPolicy
KIND:     Pod
VERSION:  v1

FIELD:    restartPolicy <string>

DESCRIPTION:
     Restart policy for all containers within the pod. One of Always, OnFailure,
     Never. Default to Always. More info:
     https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#restart-policy

Default to Always

Copy link

@dtitov dtitov Sep 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mheon, if you are referring to my issue, then I took this info from this doc:

–restart-policy=policy

Set the systemd restart policy. The restart-policy must be one of: “no”, “on-success”, “on-failure”, “on-abnormal”, “on-watchdog”, “on-abort”, or “always”. The default policy is on-failure.

But now I see that it's a wrong doc. In Kubernetes the default restart policy is indeed Always. I have just updated the issue.

}

containers := make([]*libpod.Container, 0, len(podYAML.Spec.Containers))
for _, container := range podYAML.Spec.Containers {
pullPolicy := util.PullImageMissing
Expand Down Expand Up @@ -326,6 +338,7 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
if err != nil {
return nil, err
}
conf.RestartPolicy = ctrRestartPolicy
ctr, err := createconfig.CreateContainerFromCreateConfig(ctx, ic.Libpod, conf, pod)
if err != nil {
return nil, err
Expand Down
34 changes: 34 additions & 0 deletions test/e2e/generate_kube_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package integration
import (
"os"
"path/filepath"
"strconv"

. "github.com/containers/podman/v2/test/utils"
"github.com/ghodss/yaml"
Expand Down Expand Up @@ -201,6 +202,39 @@ var _ = Describe("Podman generate kube", func() {
// Expect(err).To(BeNil())
})

It("podman generate kube on pod with restartPolicy", func() {
// podName, set, expect
testSli := [][]string{
{"testPod1", "", "Never"}, // some pod create from cmdline, so set it to Never
{"testPod2", "always", "Always"},
{"testPod3", "on-failure", "OnFailure"},
{"testPod4", "no", "Never"},
}

for k, v := range testSli {
podName := v[0]
podSession := podmanTest.Podman([]string{"pod", "create", "--name", podName})
podSession.WaitWithDefaultTimeout()
Expect(podSession.ExitCode()).To(Equal(0))

ctrName := "ctr" + strconv.Itoa(k)
ctr1Session := podmanTest.Podman([]string{"create", "--name", ctrName, "--pod", podName,
"--restart", v[1], ALPINE, "top"})
ctr1Session.WaitWithDefaultTimeout()
Expect(ctr1Session.ExitCode()).To(Equal(0))

kube := podmanTest.Podman([]string{"generate", "kube", podName})
kube.WaitWithDefaultTimeout()
Expect(kube.ExitCode()).To(Equal(0))

pod := new(v1.Pod)
err := yaml.Unmarshal(kube.Out.Contents(), pod)
Expect(err).To(BeNil())

Expect(string(pod.Spec.RestartPolicy)).To(Equal(v[2]))
}
})

It("podman generate kube on pod with ports", func() {
podName := "test"
podSession := podmanTest.Podman([]string{"pod", "create", "--name", podName, "-p", "4000:4000", "-p", "5000:5000"})
Expand Down
68 changes: 54 additions & 14 deletions test/e2e/play_kube_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ metadata:
{{ end }}

spec:
restartPolicy: {{ .RestartPolicy }}
hostname: {{ .Hostname }}
hostAliases:
{{ range .HostAliases }}
Expand Down Expand Up @@ -165,6 +166,7 @@ spec:
{{- end }}
{{- end }}
spec:
restartPolicy: {{ .RestartPolicy }}
hostname: {{ .Hostname }}
containers:
{{ with .Ctrs }}
Expand Down Expand Up @@ -274,13 +276,14 @@ func generateDeploymentKubeYaml(deployment *Deployment, fileName string) error {

// Pod describes the options a kube yaml can be configured at pod level
type Pod struct {
Name string
Hostname string
HostAliases []HostAlias
Ctrs []*Ctr
Volumes []*Volume
Labels map[string]string
Annotations map[string]string
Name string
RestartPolicy string
Hostname string
HostAliases []HostAlias
Ctrs []*Ctr
Volumes []*Volume
Labels map[string]string
Annotations map[string]string
}

type HostAlias struct {
Expand All @@ -293,13 +296,14 @@ type HostAlias struct {
// if no containers are added, it will add the default container
func getPod(options ...podOption) *Pod {
p := Pod{
Name: defaultPodName,
Hostname: "",
HostAliases: nil,
Ctrs: make([]*Ctr, 0),
Volumes: make([]*Volume, 0),
Labels: make(map[string]string),
Annotations: make(map[string]string),
Name: defaultPodName,
RestartPolicy: "Never",
Hostname: "",
HostAliases: nil,
Ctrs: make([]*Ctr, 0),
Volumes: make([]*Volume, 0),
Labels: make(map[string]string),
Annotations: make(map[string]string),
}
for _, option := range options {
option(&p)
Expand All @@ -312,6 +316,12 @@ func getPod(options ...podOption) *Pod {

type podOption func(*Pod)

func withPodName(name string) podOption {
return func(pod *Pod) {
pod.Name = name
}
}

func withHostname(h string) podOption {
return func(pod *Pod) {
pod.Hostname = h
Expand All @@ -333,6 +343,12 @@ func withCtr(c *Ctr) podOption {
}
}

func withRestartPolicy(policy string) podOption {
return func(pod *Pod) {
pod.RestartPolicy = policy
}
}

func withLabel(k, v string) podOption {
return func(pod *Pod) {
pod.Labels[k] = v
Expand Down Expand Up @@ -649,6 +665,30 @@ var _ = Describe("Podman generate kube", func() {
Expect(inspect.OutputToString()).To(ContainSubstring(`[echo hello world]`))
})

It("podman play kube test restartPolicy", func() {
// podName, set, expect
testSli := [][]string{
{"testPod1", "", "always"}, // Default eqaul to always
{"testPod2", "Always", "always"},
{"testPod3", "OnFailure", "on-failure"},
{"testPod4", "Never", "no"},
}
for _, v := range testSli {
pod := getPod(withPodName(v[0]), withRestartPolicy(v[1]))
err := generatePodKubeYaml(pod, kubeYaml)
Expect(err).To(BeNil())

kube := podmanTest.Podman([]string{"play", "kube", kubeYaml})
kube.WaitWithDefaultTimeout()
Expect(kube.ExitCode()).To(Equal(0))

inspect := podmanTest.Podman([]string{"inspect", getCtrNameInPod(pod), "--format", "{{.HostConfig.RestartPolicy.Name}}"})
inspect.WaitWithDefaultTimeout()
Expect(inspect.ExitCode()).To(Equal(0))
Expect(inspect.OutputToString()).To(Equal(v[2]))
}
})

It("podman play kube test hostname", func() {
pod := getPod()
err := generatePodKubeYaml(pod, kubeYaml)
Expand Down