Skip to content

Commit

Permalink
Merge pull request #8514 from Luap99/revert-8410-fix-multiple-networks
Browse files Browse the repository at this point in the history
Revert "Allow multiple --network flags for podman run/create"
  • Loading branch information
openshift-merge-robot authored Nov 30, 2020
2 parents f24812a + f3402c7 commit 1613921
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 71 deletions.
34 changes: 15 additions & 19 deletions cmd/podman/common/netflags.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ func DefineNetFlags(cmd *cobra.Command) {
_ = cmd.RegisterFlagCompletionFunc(macAddressFlagName, completion.AutocompleteNone)

networkFlagName := "network"
netFlags.StringArray(
networkFlagName, []string{containerConfig.NetNS()},
netFlags.String(
networkFlagName, containerConfig.NetNS(),
"Connect a container to a network",
)
_ = cmd.RegisterFlagCompletionFunc(networkFlagName, AutocompleteNetworks)
Expand Down Expand Up @@ -194,29 +194,25 @@ func NetFlagsToNetOptions(cmd *cobra.Command) (*entities.NetOptions, error) {
}

if cmd.Flags().Changed("network") {
networks, err := cmd.Flags().GetStringArray("network")
network, err := cmd.Flags().GetString("network")
if err != nil {
return nil, err
}
for i, network := range networks {
parts := strings.SplitN(network, ":", 2)

ns, cniNets, err := specgen.ParseNetworkNamespace(network)
if err != nil {
return nil, err
}
if i > 0 && (len(cniNets) == 0 || len(opts.CNINetworks) == 0) {
return nil, errors.Errorf("network conflict between type %s and %s", opts.Network.NSMode, ns.NSMode)
}
parts := strings.SplitN(network, ":", 2)

if len(parts) > 1 {
opts.NetworkOptions = make(map[string][]string)
opts.NetworkOptions[parts[0]] = strings.Split(parts[1], ",")
cniNets = nil
}
opts.Network = ns
opts.CNINetworks = append(opts.CNINetworks, cniNets...)
ns, cniNets, err := specgen.ParseNetworkNamespace(network)
if err != nil {
return nil, err
}

if len(parts) > 1 {
opts.NetworkOptions = make(map[string][]string)
opts.NetworkOptions[parts[0]] = strings.Split(parts[1], ",")
cniNets = nil
}
opts.Network = ns
opts.CNINetworks = cniNets
}

aliases, err := cmd.Flags().GetStringSlice("network-alias")
Expand Down
28 changes: 27 additions & 1 deletion cmd/podman/pods/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,33 @@ func create(cmd *cobra.Command, args []string) error {
if err != nil {
return err
}

createOptions.Net.Network = specgen.Namespace{}
if cmd.Flag("network").Changed {
netInput, err := cmd.Flags().GetString("network")
if err != nil {
return err
}
parts := strings.SplitN(netInput, ":", 2)

n := specgen.Namespace{}
switch {
case netInput == "bridge":
n.NSMode = specgen.Bridge
case netInput == "host":
n.NSMode = specgen.Host
case netInput == "slirp4netns", strings.HasPrefix(netInput, "slirp4netns:"):
n.NSMode = specgen.Slirp
if len(parts) > 1 {
createOptions.Net.NetworkOptions = make(map[string][]string)
createOptions.Net.NetworkOptions[parts[0]] = strings.Split(parts[1], ",")
}
default:
// Container and NS mode are presently unsupported
n.NSMode = specgen.Bridge
createOptions.Net.CNINetworks = strings.Split(netInput, ",")
}
createOptions.Net.Network = n
}
if len(createOptions.Net.PublishPorts) > 0 {
if !createOptions.Infra {
return errors.Errorf("you must have an infra container to publish port bindings to the host")
Expand Down
2 changes: 1 addition & 1 deletion docs/source/markdown/podman-create.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ Valid _mode_ values are:
- **none**: no networking;
- **container:**_id_: reuse another container's network stack;
- **host**: use the Podman host network stack. Note: the host mode gives the container full access to local system services such as D-bus and is therefore considered insecure;
- **cni-network**: connect to a user-defined network, multiple networks should be comma-separated or they can be specified with multiple uses of the **--network** option;
- _network-id_: connect to a user-defined network, multiple networks should be comma separated;
- **ns:**_path_: path to a network namespace to join;
- **private**: create a new namespace for the container (default)
- **slirp4netns[:OPTIONS,...]**: use **slirp4netns**(1) to create a user network stack. This is the default for rootless containers. It is possible to specify these additional options:
Expand Down
2 changes: 1 addition & 1 deletion docs/source/markdown/podman-run.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ Valid _mode_ values are:
- **none**: no networking;
- **container:**_id_: reuse another container's network stack;
- **host**: use the Podman host network stack. Note: the host mode gives the container full access to local system services such as D-bus and is therefore considered insecure;
- **cni-network**: connect to a user-defined network, multiple networks should be comma-separated or they can be specified with multiple uses of the **--network** option;
- _network-id_: connect to a user-defined network, multiple networks should be comma separated;
- **ns:**_path_: path to a network namespace to join;
- **private**: create a new namespace for the container (default)
- **slirp4netns[:OPTIONS,...]**: use **slirp4netns**(1) to create a user network stack. This is the default for rootless containers. It is possible to specify these additional options:
Expand Down
6 changes: 6 additions & 0 deletions pkg/specgen/namespaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,10 +278,16 @@ func ParseNetworkNamespace(ns string) (Namespace, []string, error) {
toReturn.NSMode = Private
case strings.HasPrefix(ns, "ns:"):
split := strings.SplitN(ns, ":", 2)
if len(split) != 2 {
return toReturn, nil, errors.Errorf("must provide a path to a namespace when specifying ns:")
}
toReturn.NSMode = Path
toReturn.Value = split[1]
case strings.HasPrefix(ns, "container:"):
split := strings.SplitN(ns, ":", 2)
if len(split) != 2 {
return toReturn, nil, errors.Errorf("must provide name or ID or a container when specifying container:")
}
toReturn.NSMode = FromContainer
toReturn.Value = split[1]
default:
Expand Down
20 changes: 0 additions & 20 deletions test/e2e/pod_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (

"github.com/containers/podman/v2/pkg/rootless"
. "github.com/containers/podman/v2/test/utils"
"github.com/containers/storage/pkg/stringid"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)
Expand Down Expand Up @@ -477,23 +476,4 @@ entrypoint ["/fromimage"]
Expect(status3.ExitCode()).To(Equal(0))
Expect(strings.Contains(status3.OutputToString(), "Degraded")).To(BeTrue())
})

It("podman create pod invalid network config", func() {
net1 := "n1" + stringid.GenerateNonCryptoID()
session := podmanTest.Podman([]string{"network", "create", net1})
session.WaitWithDefaultTimeout()
defer podmanTest.removeCNINetwork(net1)
Expect(session.ExitCode()).To(BeZero())

session = podmanTest.Podman([]string{"pod", "create", "--network", "host", "--network", net1})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(125))
Expect(session.ErrorToString()).To(ContainSubstring("host"))
Expect(session.ErrorToString()).To(ContainSubstring("bridge"))

session = podmanTest.Podman([]string{"pod", "create", "--network", "container:abc"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(125))
Expect(session.ErrorToString()).To(ContainSubstring("pods presently do not support network mode container"))
})
})
29 changes: 0 additions & 29 deletions test/e2e/run_networking_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -665,33 +665,4 @@ var _ = Describe("Podman run networking", func() {
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(BeZero())
})

It("podman run with multiple networks", func() {
net1 := "n1" + stringid.GenerateNonCryptoID()
session := podmanTest.Podman([]string{"network", "create", net1})
session.WaitWithDefaultTimeout()
defer podmanTest.removeCNINetwork(net1)
Expect(session.ExitCode()).To(BeZero())

net2 := "n2" + stringid.GenerateNonCryptoID()
session = podmanTest.Podman([]string{"network", "create", net2})
session.WaitWithDefaultTimeout()
defer podmanTest.removeCNINetwork(net2)
Expect(session.ExitCode()).To(BeZero())

run := podmanTest.Podman([]string{"run", "--network", net1, "--network", net2, ALPINE, "ip", "-o", "-4", "addr"})
run.WaitWithDefaultTimeout()
Expect(run.ExitCode()).To(BeZero())
Expect(len(run.OutputToStringArray())).To(Equal(3))
Expect(run.OutputToString()).To(ContainSubstring("lo"))
Expect(run.OutputToString()).To(ContainSubstring("eth0"))
Expect(run.OutputToString()).To(ContainSubstring("eth1"))

//invalid config network host and cni should fail
run = podmanTest.Podman([]string{"run", "--network", "host", "--network", net2, ALPINE, "ip", "-o", "-4", "addr"})
run.WaitWithDefaultTimeout()
Expect(run.ExitCode()).To(Equal(125))
Expect(run.ErrorToString()).To(ContainSubstring("host"))
Expect(run.ErrorToString()).To(ContainSubstring("bridge"))
})
})

0 comments on commit 1613921

Please sign in to comment.