From 734c435e01855d1da4c8b2cc2e6d3f96bd4edb72 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Fri, 21 Oct 2022 09:13:32 +0200 Subject: [PATCH 1/2] Add podman volume create --ignore This ignores the create request if the named volume already exists. It is very useful when scripting stuff. Signed-off-by: Alexander Larsson --- cmd/podman/volumes/create.go | 11 ++++++++-- .../source/markdown/podman-volume-create.1.md | 4 ++++ libpod/options.go | 11 ++++++++++ libpod/runtime_volume_common.go | 21 +++++++++++++----- libpod/volume.go | 9 ++++---- pkg/api/handlers/libpod/volumes.go | 5 +++++ pkg/domain/entities/volumes.go | 2 ++ pkg/domain/infra/abi/volumes.go | 5 +++++ test/e2e/volume_create_test.go | 22 +++++++++++++++++++ 9 files changed, 78 insertions(+), 12 deletions(-) diff --git a/cmd/podman/volumes/create.go b/cmd/podman/volumes/create.go index 0d19fab47c..69d1d97cfd 100644 --- a/cmd/podman/volumes/create.go +++ b/cmd/podman/volumes/create.go @@ -30,8 +30,9 @@ var ( var ( createOpts = entities.VolumeCreateOptions{} opts = struct { - Label []string - Opts []string + Label []string + Opts []string + Ignore bool }{} ) @@ -53,6 +54,9 @@ func init() { optFlagName := "opt" flags.StringArrayVarP(&opts.Opts, optFlagName, "o", []string{}, "Set driver specific options (default [])") _ = createCommand.RegisterFlagCompletionFunc(optFlagName, completion.AutocompleteNone) + + ignoreFlagName := "ignore" + flags.BoolVar(&opts.Ignore, ignoreFlagName, false, "Don't fail if volume already exists") } func create(cmd *cobra.Command, args []string) error { @@ -62,6 +66,9 @@ func create(cmd *cobra.Command, args []string) error { if len(args) > 0 { createOpts.Name = args[0] } + + createOpts.IgnoreIfExists = opts.Ignore + createOpts.Label, err = parse.GetAllLabels([]string{}, opts.Label) if err != nil { return fmt.Errorf("unable to process labels: %w", err) diff --git a/docs/source/markdown/podman-volume-create.1.md b/docs/source/markdown/podman-volume-create.1.md index 1e99df55a9..b3e556131b 100644 --- a/docs/source/markdown/podman-volume-create.1.md +++ b/docs/source/markdown/podman-volume-create.1.md @@ -29,6 +29,10 @@ Such plugins must be defined in the **volume_plugins** section of the **[contain Print usage statement +#### **--ignore** + +Don't fail if the named volume already exists, instead just print the name. Note that the new options are not applied to the existing volume. + #### **--label**, **-l**=*label* Set metadata for a volume (e.g., --label mykey=value). diff --git a/libpod/options.go b/libpod/options.go index 71ad3d11e5..8487e77ee1 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1551,6 +1551,17 @@ func WithCreateWorkingDir() CtrCreateOption { // Volume Creation Options +func WithVolumeIgnoreIfExist() VolumeCreateOption { + return func(volume *Volume) error { + if volume.valid { + return define.ErrVolumeFinalized + } + volume.ignoreIfExists = true + + return nil + } +} + // WithVolumeName sets the name of the volume. func WithVolumeName(name string) VolumeCreateOption { return func(volume *Volume) error { diff --git a/libpod/runtime_volume_common.go b/libpod/runtime_volume_common.go index b1de2be866..5c22580320 100644 --- a/libpod/runtime_volume_common.go +++ b/libpod/runtime_volume_common.go @@ -53,12 +53,14 @@ func (r *Runtime) newVolume(ctx context.Context, noCreatePluginVolume bool, opti volume.config.CreatedTime = time.Now() // Check if volume with given name exists. - exists, err := r.state.HasVolume(volume.config.Name) - if err != nil { - return nil, fmt.Errorf("checking if volume with name %s exists: %w", volume.config.Name, err) - } - if exists { - return nil, fmt.Errorf("volume with name %s already exists: %w", volume.config.Name, define.ErrVolumeExists) + if !volume.ignoreIfExists { + exists, err := r.state.HasVolume(volume.config.Name) + if err != nil { + return nil, fmt.Errorf("checking if volume with name %s exists: %w", volume.config.Name, err) + } + if exists { + return nil, fmt.Errorf("volume with name %s already exists: %w", volume.config.Name, define.ErrVolumeExists) + } } // Plugin can be nil if driver is local, but that's OK - superfluous @@ -209,6 +211,13 @@ func (r *Runtime) newVolume(ctx context.Context, noCreatePluginVolume bool, opti // Add the volume to state if err := r.state.AddVolume(volume); err != nil { + if volume.ignoreIfExists && errors.Is(err, define.ErrVolumeExists) { + existingVolume, err := r.state.Volume(volume.config.Name) + if err != nil { + return nil, fmt.Errorf("reading volume from state: %w", err) + } + return existingVolume, nil + } return nil, fmt.Errorf("adding volume to state: %w", err) } defer volume.newVolumeEvent(events.Create) diff --git a/libpod/volume.go b/libpod/volume.go index 2d4ea4280e..a9ac3938a4 100644 --- a/libpod/volume.go +++ b/libpod/volume.go @@ -17,10 +17,11 @@ type Volume struct { config *VolumeConfig state *VolumeState - valid bool - plugin *plugin.VolumePlugin - runtime *Runtime - lock lock.Locker + ignoreIfExists bool + valid bool + plugin *plugin.VolumePlugin + runtime *Runtime + lock lock.Locker } // VolumeConfig holds the volume's immutable configuration. diff --git a/pkg/api/handlers/libpod/volumes.go b/pkg/api/handlers/libpod/volumes.go index 5eac76f5ba..7ec85e3c04 100644 --- a/pkg/api/handlers/libpod/volumes.go +++ b/pkg/api/handlers/libpod/volumes.go @@ -70,6 +70,11 @@ func CreateVolume(w http.ResponseWriter, r *http.Request) { } volumeOptions = append(volumeOptions, parsedOptions...) } + + if input.IgnoreIfExists { + volumeOptions = append(volumeOptions, libpod.WithVolumeIgnoreIfExist()) + } + vol, err := runtime.NewVolume(r.Context(), volumeOptions...) if err != nil { utils.InternalServerError(w, err) diff --git a/pkg/domain/entities/volumes.go b/pkg/domain/entities/volumes.go index 9a06b22389..dad09e07bd 100644 --- a/pkg/domain/entities/volumes.go +++ b/pkg/domain/entities/volumes.go @@ -19,6 +19,8 @@ type VolumeCreateOptions struct { Labels map[string]string `schema:"labels"` // Mapping of driver options and values. Options map[string]string `schema:"opts"` + // Ignore existing volumes + IgnoreIfExists bool `schema:"ignoreIfExist"` } type VolumeConfigResponse struct { diff --git a/pkg/domain/infra/abi/volumes.go b/pkg/domain/infra/abi/volumes.go index bdfd4d5aad..5430f134f3 100644 --- a/pkg/domain/infra/abi/volumes.go +++ b/pkg/domain/infra/abi/volumes.go @@ -33,6 +33,11 @@ func (ic *ContainerEngine) VolumeCreate(ctx context.Context, opts entities.Volum } volumeOptions = append(volumeOptions, parsedOptions...) } + + if opts.IgnoreIfExists { + volumeOptions = append(volumeOptions, libpod.WithVolumeIgnoreIfExist()) + } + vol, err := ic.Libpod.NewVolume(ctx, volumeOptions...) if err != nil { return nil, err diff --git a/test/e2e/volume_create_test.go b/test/e2e/volume_create_test.go index 5dfa4d0fcd..148b8bf767 100644 --- a/test/e2e/volume_create_test.go +++ b/test/e2e/volume_create_test.go @@ -58,6 +58,28 @@ var _ = Describe("Podman volume create", func() { Expect(check.OutputToStringArray()).To(HaveLen(1)) }) + It("podman create volume with existing name fails", func() { + session := podmanTest.Podman([]string{"volume", "create", "myvol"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + + session = podmanTest.Podman([]string{"volume", "create", "myvol"}) + session.WaitWithDefaultTimeout() + Expect(session).To(ExitWithError()) + }) + + It("podman create volume --ignore", func() { + session := podmanTest.Podman([]string{"volume", "create", "myvol"}) + session.WaitWithDefaultTimeout() + volName := session.OutputToString() + Expect(session).Should(Exit(0)) + + session = podmanTest.Podman([]string{"volume", "create", "--ignore", "myvol"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(Equal(volName)) + }) + It("podman create and export volume", func() { if podmanTest.RemoteTest { Skip("Volume export check does not work with a remote client") From b7f05cef0bae389745f831d1328c7279e848a00f Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Fri, 21 Oct 2022 09:17:28 +0200 Subject: [PATCH 2/2] quadlet: Use the new podman create volume --ignore This way we don't have to use the `ExecCondition=podman volume exist`, which saves one process start. Signed-off-by: Alexander Larsson --- pkg/systemd/quadlet/quadlet.go | 5 +---- test/e2e/quadlet/basic.volume | 3 +-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/pkg/systemd/quadlet/quadlet.go b/pkg/systemd/quadlet/quadlet.go index b1869cca23..8bb5a30e75 100644 --- a/pkg/systemd/quadlet/quadlet.go +++ b/pkg/systemd/quadlet/quadlet.go @@ -659,11 +659,9 @@ func ConvertVolume(volume *parser.UnitFile, name string) (*parser.UnitFile, erro // Need the containers filesystem mounted to start podman service.Add(UnitGroup, "RequiresMountsFor", "%t/containers") - execCond := fmt.Sprintf("/usr/bin/bash -c \"! /usr/bin/podman volume exists %s\"", volumeName) - labels := volume.LookupAllKeyVal(VolumeGroup, "Label") - podman := NewPodmanCmdline("volume", "create") + podman := NewPodmanCmdline("volume", "create", "--ignore") var opts strings.Builder opts.WriteString("o=") @@ -696,7 +694,6 @@ func ConvertVolume(volume *parser.UnitFile, name string) (*parser.UnitFile, erro service.Setv(ServiceGroup, "Type", "oneshot", "RemainAfterExit", "yes", - "ExecCondition", execCond, // The default syslog identifier is the exec basename (podman) which isn't very useful here "SyslogIdentifier", "%N") diff --git a/test/e2e/quadlet/basic.volume b/test/e2e/quadlet/basic.volume index ce8dbbed57..3bc5213737 100644 --- a/test/e2e/quadlet/basic.volume +++ b/test/e2e/quadlet/basic.volume @@ -1,8 +1,7 @@ ## assert-key-is Unit RequiresMountsFor "%t/containers" ## assert-key-is Service Type oneshot ## assert-key-is Service RemainAfterExit yes -## assert-key-is Service ExecCondition '/usr/bin/bash -c "! /usr/bin/podman volume exists systemd-basic"' -## assert-key-is Service ExecStart "/usr/bin/podman volume create systemd-basic" +## assert-key-is Service ExecStart "/usr/bin/podman volume create --ignore systemd-basic" ## assert-key-is Service SyslogIdentifier "%N" [Volume]