From bddd3f5b5fb384ba2618a3ba7d9dbc96ff17253e Mon Sep 17 00:00:00 2001 From: Ygal Blum Date: Wed, 14 Dec 2022 18:26:58 +0200 Subject: [PATCH] Network Create: Add --ignore flag to support idempotent script Add --ignore flag to the command line Add a new parameter to the NetworkCreate interface in pkg/domain for CreateOptions Add a new API Network CreateWithOptions in pkg/bindings Remote API - Add a query parameter to set the ignore flag Kube - use the IgnoreIfExists flag when creating the default network instead of handling the failure Add e2e tests Update man page for podman-network-create Signed-off-by: Ygal Blum --- cmd/podman/networks/create.go | 8 ++++- .../markdown/podman-network-create.1.md | 4 +++ pkg/api/handlers/compat/networks.go | 2 +- pkg/api/handlers/libpod/networks.go | 11 ++++++- pkg/bindings/network/network.go | 16 ++++++++- pkg/bindings/network/types.go | 9 +++++ .../network/types_extracreate_options.go | 33 +++++++++++++++++++ pkg/bindings/test/networks_test.go | 15 ++++++++- pkg/domain/entities/engine_container.go | 2 +- pkg/domain/entities/network.go | 2 ++ pkg/domain/infra/abi/network.go | 4 +-- pkg/domain/infra/abi/play.go | 8 +++-- pkg/domain/infra/tunnel/network.go | 8 +++-- test/e2e/network_create_test.go | 29 ++++++++++++++++ 14 files changed, 139 insertions(+), 12 deletions(-) create mode 100644 pkg/bindings/network/types_extracreate_options.go diff --git a/cmd/podman/networks/create.go b/cmd/podman/networks/create.go index 42c342df2c..f580ce1134 100644 --- a/cmd/podman/networks/create.go +++ b/cmd/podman/networks/create.go @@ -78,6 +78,8 @@ func networkCreateFlags(cmd *cobra.Command) { _ = cmd.RegisterFlagCompletionFunc(subnetFlagName, completion.AutocompleteNone) flags.BoolVar(&networkCreateOptions.DisableDNS, "disable-dns", false, "disable dns plugin") + + flags.BoolVar(&networkCreateOptions.IgnoreIfExists, "ignore", false, "Don't fail if network already exists") } func init() { registry.Commands = append(registry.Commands, registry.CliCommand{ @@ -165,7 +167,11 @@ func networkCreate(cmd *cobra.Command, args []string) error { return errors.New("cannot set gateway or range without subnet") } - response, err := registry.ContainerEngine().NetworkCreate(registry.Context(), network) + extraCreateOptions := types.NetworkCreateOptions{ + IgnoreIfExists: networkCreateOptions.IgnoreIfExists, + } + + response, err := registry.ContainerEngine().NetworkCreate(registry.Context(), network, &extraCreateOptions) if err != nil { return err } diff --git a/docs/source/markdown/podman-network-create.1.md b/docs/source/markdown/podman-network-create.1.md index 72cec874a8..21ae2cfe41 100644 --- a/docs/source/markdown/podman-network-create.1.md +++ b/docs/source/markdown/podman-network-create.1.md @@ -39,6 +39,10 @@ Define a gateway for the subnet. If you want to provide a gateway address, you m *subnet* option. Can be specified multiple times. The argument order of the **--subnet**, **--gateway** and **--ip-range** options must match. +#### **--ignore** +Ignore the create request if a network with the same name already exists instead of failing. +Note, trying to create a network with an existing name and different parameters, will not change the configuration of the existing one + #### **--internal** Restrict external access of this network. Note when using this option, the dnsname plugin will be diff --git a/pkg/api/handlers/compat/networks.go b/pkg/api/handlers/compat/networks.go index 078e75ed39..704af4b0e4 100644 --- a/pkg/api/handlers/compat/networks.go +++ b/pkg/api/handlers/compat/networks.go @@ -278,7 +278,7 @@ func CreateNetwork(w http.ResponseWriter, r *http.Request) { } ic := abi.ContainerEngine{Libpod: runtime} - newNetwork, err := ic.NetworkCreate(r.Context(), network) + newNetwork, err := ic.NetworkCreate(r.Context(), network, nil) if err != nil { utils.InternalServerError(w, err) return diff --git a/pkg/api/handlers/libpod/networks.go b/pkg/api/handlers/libpod/networks.go index 7169a6d445..f52006c381 100644 --- a/pkg/api/handlers/libpod/networks.go +++ b/pkg/api/handlers/libpod/networks.go @@ -31,8 +31,17 @@ func CreateNetwork(w http.ResponseWriter, r *http.Request) { return } + query := struct { + IgnoreIfExists bool `schema:"ignoreIfExists"` + }{} + decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder) + 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 + } + ic := abi.ContainerEngine{Libpod: runtime} - report, err := ic.NetworkCreate(r.Context(), network) + report, err := ic.NetworkCreate(r.Context(), network, &types.NetworkCreateOptions{IgnoreIfExists: query.IgnoreIfExists}) if err != nil { if errors.Is(err, types.ErrNetworkExists) { utils.Error(w, http.StatusConflict, err) diff --git a/pkg/bindings/network/network.go b/pkg/bindings/network/network.go index 83641f6771..412f934abc 100644 --- a/pkg/bindings/network/network.go +++ b/pkg/bindings/network/network.go @@ -3,6 +3,7 @@ package network import ( "context" "net/http" + "net/url" "strings" "github.com/containers/common/libnetwork/types" @@ -13,11 +14,24 @@ import ( // Create makes a new CNI network configuration func Create(ctx context.Context, network *types.Network) (types.Network, error) { + return CreateWithOptions(ctx, network, nil) +} + +func CreateWithOptions(ctx context.Context, network *types.Network, extraCreateOptions *ExtraCreateOptions) (types.Network, error) { var report types.Network conn, err := bindings.GetClient(ctx) if err != nil { return report, err } + + var params url.Values + if extraCreateOptions != nil { + params, err = extraCreateOptions.ToParams() + if err != nil { + return report, err + } + } + // create empty network if the caller did not provide one if network == nil { network = &types.Network{} @@ -27,7 +41,7 @@ func Create(ctx context.Context, network *types.Network) (types.Network, error) return report, err } reader := strings.NewReader(networkConfig) - response, err := conn.DoRequest(ctx, reader, http.MethodPost, "/networks/create", nil, nil) + response, err := conn.DoRequest(ctx, reader, http.MethodPost, "/networks/create", params, nil) if err != nil { return report, err } diff --git a/pkg/bindings/network/types.go b/pkg/bindings/network/types.go index a3d69426c2..c205a1d8f8 100644 --- a/pkg/bindings/network/types.go +++ b/pkg/bindings/network/types.go @@ -84,3 +84,12 @@ type PruneOptions struct { // specific on choosing Filters map[string][]string } + +// ExtraCreateOptions are optional additional configuration flags for creating Networks +// that are not part of the network configuration +// +//go:generate go run ../generator/generator.go ExtraCreateOptions +type ExtraCreateOptions struct { + // IgnoreIfExists if true, do not fail if the network already exists + IgnoreIfExists *bool `schema:"ignoreIfExists"` +} diff --git a/pkg/bindings/network/types_extracreate_options.go b/pkg/bindings/network/types_extracreate_options.go new file mode 100644 index 0000000000..f12adf6bcf --- /dev/null +++ b/pkg/bindings/network/types_extracreate_options.go @@ -0,0 +1,33 @@ +// Code generated by go generate; DO NOT EDIT. +package network + +import ( + "net/url" + + "github.com/containers/podman/v4/pkg/bindings/internal/util" +) + +// Changed returns true if named field has been set +func (o *ExtraCreateOptions) Changed(fieldName string) bool { + return util.Changed(o, fieldName) +} + +// ToParams formats struct fields to be passed to API service +func (o *ExtraCreateOptions) ToParams() (url.Values, error) { + return util.ToParams(o) +} + +// WithIgnoreIfExists set field IgnoreIfExists to given value +func (o *ExtraCreateOptions) WithIgnoreIfExists(value bool) *ExtraCreateOptions { + o.IgnoreIfExists = &value + return o +} + +// GetIgnoreIfExists returns value of field IgnoreIfExists +func (o *ExtraCreateOptions) GetIgnoreIfExists() bool { + if o.IgnoreIfExists == nil { + var z bool + return z + } + return *o.IgnoreIfExists +} diff --git a/pkg/bindings/test/networks_test.go b/pkg/bindings/test/networks_test.go index 22d4ea7300..737bc4b78b 100644 --- a/pkg/bindings/test/networks_test.go +++ b/pkg/bindings/test/networks_test.go @@ -101,11 +101,24 @@ var _ = Describe("Podman networks", func() { Expect(err).ToNot(HaveOccurred()) Expect(report.Name).To(Equal(name)) - // create network with same name should 500 + // create network with same name should 409 _, err = network.Create(connText, &net) Expect(err).To(HaveOccurred()) code, _ := bindings.CheckResponseCode(err) Expect(code).To(BeNumerically("==", http.StatusConflict)) + + // create network with same name and ignore false should 409 + options := new(network.ExtraCreateOptions).WithIgnoreIfExists(false) + _, err = network.CreateWithOptions(connText, &net, options) + Expect(err).To(HaveOccurred()) + code, _ = bindings.CheckResponseCode(err) + Expect(code).To(BeNumerically("==", http.StatusConflict)) + + // create network with same name and ignore true succeed + options = new(network.ExtraCreateOptions).WithIgnoreIfExists(true) + report, err = network.CreateWithOptions(connText, &net, options) + Expect(err).ToNot(HaveOccurred()) + Expect(report.Name).To(Equal(name)) }) It("inspect network", func() { diff --git a/pkg/domain/entities/engine_container.go b/pkg/domain/entities/engine_container.go index 44c4f2e4b3..be1f85e2b9 100644 --- a/pkg/domain/entities/engine_container.go +++ b/pkg/domain/entities/engine_container.go @@ -63,7 +63,7 @@ type ContainerEngine interface { //nolint:interfacebloat Info(ctx context.Context) (*define.Info, error) KubeApply(ctx context.Context, body io.Reader, opts ApplyOptions) error NetworkConnect(ctx context.Context, networkname string, options NetworkConnectOptions) error - NetworkCreate(ctx context.Context, network types.Network) (*types.Network, error) + NetworkCreate(ctx context.Context, network types.Network, createOptions *types.NetworkCreateOptions) (*types.Network, error) NetworkDisconnect(ctx context.Context, networkname string, options NetworkDisconnectOptions) error NetworkExists(ctx context.Context, networkname string) (*BoolReport, error) NetworkInspect(ctx context.Context, namesOrIds []string, options InspectOptions) ([]types.Network, []error, error) diff --git a/pkg/domain/entities/network.go b/pkg/domain/entities/network.go index 9e59953c67..03bbc950d8 100644 --- a/pkg/domain/entities/network.go +++ b/pkg/domain/entities/network.go @@ -52,6 +52,8 @@ type NetworkCreateOptions struct { IPv6 bool // Mapping of driver options and values. Options map[string]string + // IgnoreIfExists if true, do not fail if the network already exists + IgnoreIfExists bool } // NetworkCreateReport describes a created network for the cli diff --git a/pkg/domain/infra/abi/network.go b/pkg/domain/infra/abi/network.go index 11aa83fe0d..7913ecc964 100644 --- a/pkg/domain/infra/abi/network.go +++ b/pkg/domain/infra/abi/network.go @@ -138,12 +138,12 @@ func (ic *ContainerEngine) NetworkRm(ctx context.Context, namesOrIds []string, o return reports, nil } -func (ic *ContainerEngine) NetworkCreate(ctx context.Context, network types.Network) (*types.Network, error) { +func (ic *ContainerEngine) NetworkCreate(ctx context.Context, network types.Network, createOptions *types.NetworkCreateOptions) (*types.Network, error) { // TODO (5.0): Stop accepting "pasta" as value here if util.StringInSlice(network.Name, []string{"none", "host", "bridge", "private", "slirp4netns", "container", "ns"}) { return nil, fmt.Errorf("cannot create network with name %q because it conflicts with a valid network mode", network.Name) } - network, err := ic.Libpod.Network().NetworkCreate(network, nil) + network, err := ic.Libpod.Network().NetworkCreate(network, createOptions) if err != nil { return nil, err } diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index e73bf6614d..52b0149fbf 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -122,12 +122,16 @@ func (ic *ContainerEngine) PlayKube(ctx context.Context, body io.Reader, options // when no network options are specified, create a common network for all the pods if len(options.Networks) == 0 { _, err := ic.NetworkCreate( - ctx, nettypes.Network{ + ctx, + nettypes.Network{ Name: kubeDefaultNetwork, DNSEnabled: true, }, + &nettypes.NetworkCreateOptions{ + IgnoreIfExists: true, + }, ) - if err != nil && !errors.Is(err, nettypes.ErrNetworkExists) { + if err != nil { return nil, err } } diff --git a/pkg/domain/infra/tunnel/network.go b/pkg/domain/infra/tunnel/network.go index 6e27b8e56d..2c3ce57213 100644 --- a/pkg/domain/infra/tunnel/network.go +++ b/pkg/domain/infra/tunnel/network.go @@ -66,8 +66,12 @@ func (ic *ContainerEngine) NetworkRm(ctx context.Context, namesOrIds []string, o return reports, nil } -func (ic *ContainerEngine) NetworkCreate(ctx context.Context, net types.Network) (*types.Network, error) { - net, err := network.Create(ic.ClientCtx, &net) +func (ic *ContainerEngine) NetworkCreate(ctx context.Context, net types.Network, createOptions *types.NetworkCreateOptions) (*types.Network, error) { + options := new(network.ExtraCreateOptions) + if createOptions != nil { + options = options.WithIgnoreIfExists(createOptions.IgnoreIfExists) + } + net, err := network.CreateWithOptions(ic.ClientCtx, &net, options) if err != nil { return nil, err } diff --git a/test/e2e/network_create_test.go b/test/e2e/network_create_test.go index df9937ba4b..de1feae23c 100644 --- a/test/e2e/network_create_test.go +++ b/test/e2e/network_create_test.go @@ -452,4 +452,33 @@ var _ = Describe("Podman network create", func() { Expect(nc).To(Exit(125)) Expect(nc.ErrorToString()).To(Equal("Error: cannot set more ranges than subnets")) }) + + It("podman network create same name - fail", func() { + name := "same-name-" + stringid.GenerateRandomID() + networkCreateCommand := []string{"network", "create", name} + nc := podmanTest.Podman(networkCreateCommand) + nc.WaitWithDefaultTimeout() + defer podmanTest.removeNetwork(name) + Expect(nc).To(Exit(0)) + Expect(nc.OutputToString()).To(Equal(name)) + + nc = podmanTest.Podman(networkCreateCommand) + nc.WaitWithDefaultTimeout() + Expect(nc).To(Exit(125)) + }) + + It("podman network create same name - succeed with ignore", func() { + name := "same-name-" + stringid.GenerateRandomID() + networkCreateCommand := []string{"network", "create", "--ignore", name} + nc := podmanTest.Podman(networkCreateCommand) + nc.WaitWithDefaultTimeout() + defer podmanTest.removeNetwork(name) + Expect(nc).To(Exit(0)) + Expect(nc.OutputToString()).To(Equal(name)) + + nc = podmanTest.Podman(networkCreateCommand) + nc.WaitWithDefaultTimeout() + Expect(nc).To(Exit(0)) + Expect(nc.OutputToString()).To(Equal(name)) + }) })