From 1c8196a9ac2717c17cce4af8bfd2751a19f0d0f7 Mon Sep 17 00:00:00 2001 From: Ygal Blum Date: Sun, 6 Nov 2022 10:11:06 +0200 Subject: [PATCH] kube play: update the handling of PersistentVolumeClaim Up - do not fail if volume already exists, use the existing one Down - allow the user to remove the volume by passing --force Add tests Update the documentation Signed-off-by: Ygal Blum --- cmd/podman/kube/down.go | 20 +++++- cmd/podman/kube/play.go | 32 +++++++-- docs/source/markdown/podman-kube-down.1.md | 17 +++-- docs/source/markdown/podman-kube-play.1.md.in | 5 ++ pkg/api/handlers/libpod/kube.go | 15 +++- pkg/api/server/register_kube.go | 6 ++ pkg/bindings/kube/kube.go | 13 ++-- pkg/bindings/kube/types.go | 10 +++ pkg/bindings/kube/types_down_options.go | 33 +++++++++ pkg/bindings/kube/types_play_options.go | 15 ++++ pkg/bindings/play/play.go | 8 +-- pkg/domain/entities/play.go | 12 +++- pkg/domain/infra/abi/play.go | 19 ++++- pkg/domain/infra/tunnel/kube.go | 4 +- test/e2e/play_kube_test.go | 70 ++++++++++++++++++- 15 files changed, 248 insertions(+), 31 deletions(-) create mode 100644 pkg/bindings/kube/types_down_options.go diff --git a/cmd/podman/kube/down.go b/cmd/podman/kube/down.go index 792c80499044..6904df488bc3 100644 --- a/cmd/podman/kube/down.go +++ b/cmd/podman/kube/down.go @@ -3,16 +3,22 @@ package kube import ( "github.com/containers/podman/v4/cmd/podman/common" "github.com/containers/podman/v4/cmd/podman/registry" + "github.com/containers/podman/v4/cmd/podman/utils" + "github.com/containers/podman/v4/pkg/domain/entities" "github.com/spf13/cobra" ) +type downKubeOptions struct { + Force bool +} + var ( downDescription = `Reads in a structured file of Kubernetes YAML. Removes pods that have been based on the Kubernetes kind described in the YAML.` downCmd = &cobra.Command{ - Use: "down KUBEFILE|-", + Use: "down [options] KUBEFILE|-", Short: "Remove pods based on Kubernetes YAML.", Long: downDescription, RunE: down, @@ -22,6 +28,8 @@ var ( cat nginx.yml | podman kube down - podman kube down https://example.com/nginx.yml`, } + + downOptions = downKubeOptions{} ) func init() { @@ -29,6 +37,14 @@ func init() { Command: downCmd, Parent: kubeCmd, }) + downFlags(downCmd) +} + +func downFlags(cmd *cobra.Command) { + flags := cmd.Flags() + flags.SetNormalizeFunc(utils.AliasFlags) + + flags.BoolVar(&downOptions.Force, "force", false, "remove volumes") } func down(cmd *cobra.Command, args []string) error { @@ -36,5 +52,5 @@ func down(cmd *cobra.Command, args []string) error { if err != nil { return err } - return teardown(reader) + return teardown(reader, entities.PlayKubeDownOptions{Force: downOptions.Force}) } diff --git a/cmd/podman/kube/play.go b/cmd/podman/kube/play.go index 1163a6ff6e15..e9c2c9234c3b 100644 --- a/cmd/podman/kube/play.go +++ b/cmd/podman/kube/play.go @@ -138,6 +138,7 @@ func playFlags(cmd *cobra.Command) { flags.BoolVarP(&playOptions.Quiet, "quiet", "q", false, "Suppress output information when pulling images") flags.BoolVar(&playOptions.TLSVerifyCLI, "tls-verify", true, "Require HTTPS and verify certificates when contacting registries") flags.BoolVar(&playOptions.StartCLI, "start", true, "Start the pod after creating it") + flags.BoolVar(&playOptions.Force, "force", false, "Remove volumes as part of --down") authfileFlagName := "authfile" flags.StringVar(&playOptions.Authfile, authfileFlagName, auth.GetDefaultAuthFile(), "Path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override") @@ -242,17 +243,21 @@ func play(cmd *cobra.Command, args []string) error { playOptions.StaticMACs = append(playOptions.StaticMACs, m) } + if playOptions.Force && !playOptions.Down { + return errors.New("--force may be specified only with --down") + } + reader, err := readerFromArg(args[0]) if err != nil { return err } if playOptions.Down { - return teardown(reader) + return teardown(reader, entities.PlayKubeDownOptions{Force: playOptions.Force}) } if playOptions.Replace { - if err := teardown(reader); err != nil && !errorhandling.Contains(err, define.ErrNoSuchPod) { + if err := teardown(reader, entities.PlayKubeDownOptions{Force: playOptions.Force}); err != nil && !errorhandling.Contains(err, define.ErrNoSuchPod) { return err } if _, err := reader.Seek(0, 0); err != nil { @@ -302,13 +307,13 @@ func readerFromArg(fileName string) (*bytes.Reader, error) { return bytes.NewReader(data), nil } -func teardown(body io.Reader) error { +func teardown(body io.Reader, options entities.PlayKubeDownOptions) error { var ( podStopErrors utils.OutputErrors podRmErrors utils.OutputErrors + volRmErrors utils.OutputErrors ) - options := new(entities.PlayKubeDownOptions) - reports, err := registry.ContainerEngine().PlayKubeDown(registry.GetContext(), body, *options) + reports, err := registry.ContainerEngine().PlayKubeDown(registry.GetContext(), body, options) if err != nil { return err } @@ -338,7 +343,22 @@ func teardown(body io.Reader) error { } } - return podRmErrors.PrintErrors() + lastPodRmError := podRmErrors.PrintErrors() + if lastPodRmError != nil { + fmt.Fprintf(os.Stderr, "Error: %s\n", lastPodRmError) + } + + // Output rm'd volumes + fmt.Println("Volumes removed:") + for _, removed := range reports.VolumeRmReport { + if removed.Err == nil { + fmt.Println(removed.Id) + } else { + volRmErrors = append(volRmErrors, removed.Err) + } + } + + return volRmErrors.PrintErrors() } func kubeplay(body io.Reader) error { diff --git a/docs/source/markdown/podman-kube-down.1.md b/docs/source/markdown/podman-kube-down.1.md index 898188d2be57..9982340bc508 100644 --- a/docs/source/markdown/podman-kube-down.1.md +++ b/docs/source/markdown/podman-kube-down.1.md @@ -4,14 +4,21 @@ podman-kube-down - Remove containers and pods based on Kubernetes YAML ## SYNOPSIS -**podman kube down** *file.yml|-|https://website.io/file.yml* +**podman kube down** [*options*] *file.yml|-|https://website.io/file.yml* ## DESCRIPTION **podman kube down** reads a specified Kubernetes YAML file, tearing down pods that were created by the `podman kube play` command via the same Kubernetes YAML -file. Any volumes that were created by the previous `podman kube play` command remain intact. If the YAML file is specified as `-`, `podman kube down` reads the -YAML from stdin. The input can also be a URL that points to a YAML file such as https://podman.io/demo.yml. `podman kube down` will then teardown the pods and -containers created by `podman kube play` via the same Kubernetes YAML from the URL. However, `podman kube down` will not work with a URL if the YAML file the URL -points to has been changed or altered since the creation of the pods and containers using `podman kube play`. +file. Any volumes that were created by the previous `podman kube play` command remain intact unless the `--force` options is used. If the YAML file is +specified as `-`, `podman kube down` reads the YAML from stdin. The input can also be a URL that points to a YAML file such as https://podman.io/demo.yml. +`podman kube down` will then teardown the pods and containers created by `podman kube play` via the same Kubernetes YAML from the URL. However, +`podman kube down` will not work with a URL if the YAML file the URL points to has been changed or altered since the creation of the pods and containers using +`podman kube play`. + +## OPTIONS + +#### **--force** + +Tear down the volumes linked to the PersistentVolumeClaims as part --down ## EXAMPLES diff --git a/docs/source/markdown/podman-kube-play.1.md.in b/docs/source/markdown/podman-kube-play.1.md.in index 6f4f45ef8979..73c765679b7c 100644 --- a/docs/source/markdown/podman-kube-play.1.md.in +++ b/docs/source/markdown/podman-kube-play.1.md.in @@ -138,6 +138,10 @@ Use *path* as the build context directory for each image. Requires --build optio @@option creds +#### **--force** + +Tear down the volumes linked to the PersistentVolumeClaims as part of --down + #### **--help**, **-h** Print usage statement @@ -185,6 +189,7 @@ Start the pod after creating it, set to false to only create it. @@option tls-verify @@option userns.container + ## EXAMPLES Recreate the pod and containers as described in a file called `demo.yml` diff --git a/pkg/api/handlers/libpod/kube.go b/pkg/api/handlers/libpod/kube.go index e845aeaf5894..4715286020ed 100644 --- a/pkg/api/handlers/libpod/kube.go +++ b/pkg/api/handlers/libpod/kube.go @@ -112,9 +112,20 @@ func KubePlay(w http.ResponseWriter, r *http.Request) { func KubePlayDown(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime) + decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder) + query := struct { + Force bool `schema:"force"` + }{ + Force: false, + } + + if err := decoder.Decode(&query, r.URL.Query()); err != nil { + utils.Error(w, http.StatusBadRequest, fmt.Errorf("failed to parse parameters for %s: %w", r.URL.String(), err)) + return + } + containerEngine := abi.ContainerEngine{Libpod: runtime} - options := new(entities.PlayKubeDownOptions) - report, err := containerEngine.PlayKubeDown(r.Context(), r.Body, *options) + report, err := containerEngine.PlayKubeDown(r.Context(), r.Body, entities.PlayKubeDownOptions{Force: query.Force}) _ = r.Body.Close() if err != nil { utils.Error(w, http.StatusInternalServerError, fmt.Errorf("tearing down YAML file: %w", err)) diff --git a/pkg/api/server/register_kube.go b/pkg/api/server/register_kube.go index 850a1eba2a93..963bfdb5af1c 100644 --- a/pkg/api/server/register_kube.go +++ b/pkg/api/server/register_kube.go @@ -69,6 +69,12 @@ func (s *APIServer) registerKubeHandlers(r *mux.Router) error { // - pods // summary: Remove pods from kube play // description: Tears down pods defined in a YAML file + // parameters: + // - in: query + // name: force + // type: boolean + // default: false + // description: Remove volumes. // produces: // - application/json // responses: diff --git a/pkg/bindings/kube/kube.go b/pkg/bindings/kube/kube.go index 91bab3f858bb..731a35ed0033 100644 --- a/pkg/bindings/kube/kube.go +++ b/pkg/bindings/kube/kube.go @@ -67,7 +67,7 @@ func PlayWithBody(ctx context.Context, body io.Reader, options *PlayOptions) (*e return &report, nil } -func Down(ctx context.Context, path string) (*entities.KubePlayReport, error) { +func Down(ctx context.Context, path string, options DownOptions) (*entities.KubePlayReport, error) { f, err := os.Open(path) if err != nil { return nil, err @@ -78,17 +78,22 @@ func Down(ctx context.Context, path string) (*entities.KubePlayReport, error) { } }() - return DownWithBody(ctx, f) + return DownWithBody(ctx, f, options) } -func DownWithBody(ctx context.Context, body io.Reader) (*entities.KubePlayReport, error) { +func DownWithBody(ctx context.Context, body io.Reader, options DownOptions) (*entities.KubePlayReport, error) { var report entities.KubePlayReport conn, err := bindings.GetClient(ctx) if err != nil { return nil, err } - response, err := conn.DoRequest(ctx, body, http.MethodDelete, "/play/kube", nil, nil) + params, err := options.ToParams() + if err != nil { + return nil, err + } + + response, err := conn.DoRequest(ctx, body, http.MethodDelete, "/play/kube", params, nil) if err != nil { return nil, err } diff --git a/pkg/bindings/kube/types.go b/pkg/bindings/kube/types.go index 979eadebf3f0..c41a578a58a7 100644 --- a/pkg/bindings/kube/types.go +++ b/pkg/bindings/kube/types.go @@ -46,6 +46,8 @@ type PlayOptions struct { Start *bool // Userns - define the user namespace to use. Userns *string + // Force - remove volumes on --down + Force *bool } // ApplyOptions are optional options for applying kube YAML files to a k8s cluster @@ -63,3 +65,11 @@ type ApplyOptions struct { // Service - creates a service for the container being deployed. Service *bool } + +// DownOptions are optional options for tearing down kube YAML files to a k8s cluster +// +//go:generate go run ../generator/generator.go DownOptions +type DownOptions struct { + // Force - remove volumes on --down + Force *bool +} diff --git a/pkg/bindings/kube/types_down_options.go b/pkg/bindings/kube/types_down_options.go new file mode 100644 index 000000000000..99ce3abe4db8 --- /dev/null +++ b/pkg/bindings/kube/types_down_options.go @@ -0,0 +1,33 @@ +// Code generated by go generate; DO NOT EDIT. +package kube + +import ( + "net/url" + + "github.com/containers/podman/v4/pkg/bindings/internal/util" +) + +// Changed returns true if named field has been set +func (o *DownOptions) Changed(fieldName string) bool { + return util.Changed(o, fieldName) +} + +// ToParams formats struct fields to be passed to API service +func (o *DownOptions) ToParams() (url.Values, error) { + return util.ToParams(o) +} + +// WithForce set field Force to given value +func (o *DownOptions) WithForce(value bool) *DownOptions { + o.Force = &value + return o +} + +// GetForce returns value of field Force +func (o *DownOptions) GetForce() bool { + if o.Force == nil { + var z bool + return z + } + return *o.Force +} diff --git a/pkg/bindings/kube/types_play_options.go b/pkg/bindings/kube/types_play_options.go index cdc2e9dd854e..6db3241577c2 100644 --- a/pkg/bindings/kube/types_play_options.go +++ b/pkg/bindings/kube/types_play_options.go @@ -287,3 +287,18 @@ func (o *PlayOptions) GetUserns() string { } return *o.Userns } + +// WithForce set field Force to given value +func (o *PlayOptions) WithForce(value bool) *PlayOptions { + o.Force = &value + return o +} + +// GetForce returns value of field Force +func (o *PlayOptions) GetForce() bool { + if o.Force == nil { + var z bool + return z + } + return *o.Force +} diff --git a/pkg/bindings/play/play.go b/pkg/bindings/play/play.go index d5d649135390..803349b88408 100644 --- a/pkg/bindings/play/play.go +++ b/pkg/bindings/play/play.go @@ -18,10 +18,10 @@ func KubeWithBody(ctx context.Context, body io.Reader, options *KubeOptions) (*e return kube.PlayWithBody(ctx, body, options) } -func Down(ctx context.Context, path string) (*entities.PlayKubeReport, error) { - return kube.Down(ctx, path) +func Down(ctx context.Context, path string, options kube.DownOptions) (*entities.PlayKubeReport, error) { + return kube.Down(ctx, path, options) } -func DownWithBody(ctx context.Context, body io.Reader) (*entities.PlayKubeReport, error) { - return kube.DownWithBody(ctx, body) +func DownWithBody(ctx context.Context, body io.Reader, options kube.DownOptions) (*entities.PlayKubeReport, error) { + return kube.DownWithBody(ctx, body, options) } diff --git a/pkg/domain/entities/play.go b/pkg/domain/entities/play.go index 86bd47917010..a74fdbef4d88 100644 --- a/pkg/domain/entities/play.go +++ b/pkg/domain/entities/play.go @@ -60,6 +60,8 @@ type PlayKubeOptions struct { Userns string // IsRemote - was the request triggered by running podman-remote IsRemote bool + // Force - remove volumes on --down + Force bool } // PlayKubePod represents a single pod and associated containers created by play kube @@ -96,12 +98,16 @@ type PlayKubeReport struct { type KubePlayReport = PlayKubeReport // PlayKubeDownOptions are options for tearing down pods -type PlayKubeDownOptions struct{} +type PlayKubeDownOptions struct { + // Force - remove volumes if passed + Force bool +} // PlayKubeDownReport contains the results of tearing down play kube type PlayKubeTeardown struct { - StopReport []*PodStopReport - RmReport []*PodRmReport + StopReport []*PodStopReport + RmReport []*PodRmReport + VolumeRmReport []*VolumeRmReport } type PlaySecret struct { diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index 4080dcfebb75..e6c6d3ada6ae 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -869,6 +869,7 @@ func (ic *ContainerEngine) playKubePVC(ctx context.Context, pvcYAML *v1.Persiste volOptions := []libpod.VolumeCreateOption{ libpod.WithVolumeName(name), libpod.WithVolumeLabels(pvcYAML.Labels), + libpod.WithVolumeIgnoreIfExist(), } // Get pvc annotations and create remaining podman volume options if available. @@ -1110,9 +1111,10 @@ func getBuildFile(imageName string, cwd string) (string, error) { return "", err } -func (ic *ContainerEngine) PlayKubeDown(ctx context.Context, body io.Reader, _ entities.PlayKubeDownOptions) (*entities.PlayKubeReport, error) { +func (ic *ContainerEngine) PlayKubeDown(ctx context.Context, body io.Reader, options entities.PlayKubeDownOptions) (*entities.PlayKubeReport, error) { var ( - podNames []string + podNames []string + volumeNames []string ) reports := new(entities.PlayKubeReport) @@ -1162,6 +1164,12 @@ func (ic *ContainerEngine) PlayKubeDown(ctx context.Context, body io.Reader, _ e podName := fmt.Sprintf("%s-pod-%d", deploymentName, i) podNames = append(podNames, podName) } + case "PersistentVolumeClaim": + var pvcYAML v1.PersistentVolumeClaim + if err := yaml.Unmarshal(document, &pvcYAML); err != nil { + return nil, fmt.Errorf("unable to read YAML as Kube PersistentVolumeClaim: %w", err) + } + volumeNames = append(volumeNames, pvcYAML.Name) default: continue } @@ -1178,6 +1186,13 @@ func (ic *ContainerEngine) PlayKubeDown(ctx context.Context, body io.Reader, _ e return nil, err } + if options.Force { + reports.VolumeRmReport, err = ic.VolumeRm(ctx, volumeNames, entities.VolumeRmOptions{}) + if err != nil { + return nil, err + } + } + return reports, nil } diff --git a/pkg/domain/infra/tunnel/kube.go b/pkg/domain/infra/tunnel/kube.go index 22be7eea738c..7942833dcb59 100644 --- a/pkg/domain/infra/tunnel/kube.go +++ b/pkg/domain/infra/tunnel/kube.go @@ -75,8 +75,8 @@ func (ic *ContainerEngine) PlayKube(ctx context.Context, body io.Reader, opts en return play.KubeWithBody(ic.ClientCtx, body, options) } -func (ic *ContainerEngine) PlayKubeDown(ctx context.Context, body io.Reader, _ entities.PlayKubeDownOptions) (*entities.PlayKubeReport, error) { - return play.DownWithBody(ic.ClientCtx, body) +func (ic *ContainerEngine) PlayKubeDown(ctx context.Context, body io.Reader, options entities.PlayKubeDownOptions) (*entities.PlayKubeReport, error) { + return play.DownWithBody(ic.ClientCtx, body, kube.DownOptions{Force: &options.Force}) } func (ic *ContainerEngine) KubeApply(ctx context.Context, body io.Reader, opts entities.ApplyOptions) error { diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index a556675aea84..557526191dc2 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -3420,7 +3420,7 @@ invalid kube kind Expect(teardown).Should(Exit(125)) }) - It("podman play kube teardown with volume", func() { + It("podman play kube teardown with volume without force delete", func() { volName := RandomString(12) volDevice := "tmpfs" @@ -3452,6 +3452,74 @@ invalid kube kind Expect(exists).To(Exit(0)) }) + It("podman play kube teardown with volume force delete", func() { + + volName := RandomString(12) + volDevice := "tmpfs" + volType := "tmpfs" + volOpts := "nodev,noexec" + + pvc := getPVC(withPVCName(volName), + withPVCAnnotations(util.VolumeDeviceAnnotation, volDevice), + withPVCAnnotations(util.VolumeTypeAnnotation, volType), + withPVCAnnotations(util.VolumeMountOptsAnnotation, volOpts)) + err = generateKubeYaml("persistentVolumeClaim", pvc, kubeYaml) + Expect(err).To(BeNil()) + + kube := podmanTest.Podman([]string{"play", "kube", kubeYaml}) + kube.WaitWithDefaultTimeout() + Expect(kube).Should(Exit(0)) + + exists := podmanTest.Podman([]string{"volume", "exists", volName}) + exists.WaitWithDefaultTimeout() + Expect(exists).To(Exit(0)) + + teardown := podmanTest.Podman([]string{"play", "kube", "--down", "--force", kubeYaml}) + teardown.WaitWithDefaultTimeout() + Expect(teardown).To(Exit(0)) + + // volume should not be deleted on teardown + exists = podmanTest.Podman([]string{"volume", "exists", volName}) + exists.WaitWithDefaultTimeout() + Expect(exists).To(Exit(1)) + }) + + It("podman play kube after teardown with volume reuse", func() { + + volName := RandomString(12) + volDevice := "tmpfs" + volType := "tmpfs" + volOpts := "nodev,noexec" + + pvc := getPVC(withPVCName(volName), + withPVCAnnotations(util.VolumeDeviceAnnotation, volDevice), + withPVCAnnotations(util.VolumeTypeAnnotation, volType), + withPVCAnnotations(util.VolumeMountOptsAnnotation, volOpts)) + err = generateKubeYaml("persistentVolumeClaim", pvc, kubeYaml) + Expect(err).To(BeNil()) + + kube := podmanTest.Podman([]string{"play", "kube", kubeYaml}) + kube.WaitWithDefaultTimeout() + Expect(kube).Should(Exit(0)) + + exists := podmanTest.Podman([]string{"volume", "exists", volName}) + exists.WaitWithDefaultTimeout() + Expect(exists).To(Exit(0)) + + teardown := podmanTest.Podman([]string{"play", "kube", "--down", kubeYaml}) + teardown.WaitWithDefaultTimeout() + Expect(teardown).To(Exit(0)) + + // volume should not be deleted on teardown + exists = podmanTest.Podman([]string{"volume", "exists", volName}) + exists.WaitWithDefaultTimeout() + Expect(exists).To(Exit(0)) + + restart := podmanTest.Podman([]string{"play", "kube", kubeYaml}) + restart.WaitWithDefaultTimeout() + Expect(restart).To(Exit(0)) + }) + It("podman play kube use network mode from config", func() { confPath, err := filepath.Abs("config/containers-netns2.conf") Expect(err).ToNot(HaveOccurred())