From 5490be67b3620c7a13319f4a18519ab691c20bef Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 7 Dec 2021 19:04:13 +0100 Subject: [PATCH 01/10] network db rewrite: migrate existing settings The new network db structure stores everything in the networks bucket. Previously some network settings were not written the the network bucket and only stored in the container config. Instead of the old format which used the container ID as value in the networks buckets we now use the PerNetworkoptions struct there. To migrate existing users we use the state.GetNetworks() function. If it fails to read the new format it will automatically migrate the old config format to the new one. This is allows a flawless migration path. Signed-off-by: Paul Holzinger --- libpod/boltdb_state.go | 172 ++++++++++++++++++----- libpod/container.go | 57 +++----- libpod/container_internal_linux.go | 42 ++---- libpod/networking_linux.go | 215 ++++++++--------------------- libpod/state.go | 6 +- 5 files changed, 226 insertions(+), 266 deletions(-) diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index 1242a8d6ba..d322ccc539 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -2,11 +2,14 @@ package libpod import ( "bytes" + "fmt" + "net" "os" "strings" "sync" "github.com/containers/podman/v3/libpod/define" + "github.com/containers/podman/v3/libpod/network/types" jsoniter "github.com/json-iterator/go" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -971,7 +974,7 @@ func (s *BoltState) AllContainers() ([]*Container, error) { } // GetNetworks returns the CNI networks this container is a part of. -func (s *BoltState) GetNetworks(ctr *Container) ([]string, error) { +func (s *BoltState) GetNetworks(ctr *Container) (map[string]types.PerNetworkOptions, error) { if !s.valid { return nil, define.ErrDBClosed } @@ -984,6 +987,11 @@ func (s *BoltState) GetNetworks(ctr *Container) ([]string, error) { return nil, errors.Wrapf(define.ErrNSMismatch, "container %s is in namespace %q, does not match our namespace %q", ctr.ID(), ctr.config.Namespace, s.namespace) } + // if the network mode is not bridge return no networks + if !ctr.config.NetMode.IsBridge() { + return nil, nil + } + ctrID := []byte(ctr.ID()) db, err := s.getDBCon() @@ -992,7 +1000,9 @@ func (s *BoltState) GetNetworks(ctr *Container) ([]string, error) { } defer s.deferredCloseDBCon(db) - networks := []string{} + networks := make(map[string]types.PerNetworkOptions) + + var convertDB bool err = db.View(func(tx *bolt.Tx) error { ctrBucket, err := getCtrBucket(tx) @@ -1008,17 +1018,131 @@ func (s *BoltState) GetNetworks(ctr *Container) ([]string, error) { ctrNetworkBkt := dbCtr.Bucket(networksBkt) if ctrNetworkBkt == nil { - return errors.Wrapf(define.ErrNoSuchNetwork, "container %s is not joined to any CNI networks", ctr.ID()) + // convert if needed + convertDB = true + return nil } return ctrNetworkBkt.ForEach(func(network, v []byte) error { - networks = append(networks, string(network)) + opts := types.PerNetworkOptions{} + if err := json.Unmarshal(v, &opts); err != nil { + // special case for backwards compat + // earlier version used the container id as value so we set a + // special error to indicate the we have to migrate the db + if !bytes.Equal(v, ctrID) { + return err + } + convertDB = true + } + networks[string(network)] = opts return nil }) }) if err != nil { return nil, err } + if convertDB { + err = db.Update(func(tx *bolt.Tx) error { + ctrBucket, err := getCtrBucket(tx) + if err != nil { + return err + } + + dbCtr := ctrBucket.Bucket(ctrID) + if dbCtr == nil { + ctr.valid = false + return errors.Wrapf(define.ErrNoSuchCtr, "container %s does not exist in database", ctr.ID()) + } + + var networkList []string + + ctrNetworkBkt := dbCtr.Bucket(networksBkt) + if ctrNetworkBkt == nil { + ctrNetworkBkt, err = dbCtr.CreateBucket(networksBkt) + if err != nil { + return errors.Wrapf(err, "error creating networks bucket for container %s", ctr.ID()) + } + // the container has no networks in the db lookup config and write to the db + networkList = ctr.config.Networks + // if there are no networks we have to add the default + if len(networkList) == 0 { + networkList = []string{ctr.runtime.config.Network.DefaultNetwork} + } + } else { + err = ctrNetworkBkt.ForEach(func(network, v []byte) error { + networkList = append(networkList, string(network)) + return nil + }) + if err != nil { + return err + } + } + + // the container has no networks in the db lookup config and write to the db + for i, network := range networkList { + var intName string + if ctr.state.NetInterfaceDescriptions != nil { + eth, exists := ctr.state.NetInterfaceDescriptions.getInterfaceByName(network) + if !exists { + return errors.Errorf("no network interface name for container %s on network %s", ctr.config.ID, network) + } + intName = eth + } else { + intName = fmt.Sprintf("eth%d", i) + } + getAliases := func(network string) []string { + var aliases []string + ctrAliasesBkt := dbCtr.Bucket(aliasesBkt) + if ctrAliasesBkt == nil { + return nil + } + netAliasesBkt := ctrAliasesBkt.Bucket([]byte(network)) + if netAliasesBkt == nil { + // No aliases for this specific network. + return nil + } + + // lets ignore the error here there is nothing we can do + _ = netAliasesBkt.ForEach(func(alias, v []byte) error { + aliases = append(aliases, string(alias)) + return nil + }) + // also add the short container id as alias + return aliases + } + + netOpts := &types.PerNetworkOptions{ + InterfaceName: intName, + // we have to add the short id as alias for docker compat + Aliases: append(getAliases(network), ctr.config.ID[:12]), + } + // only set the static ip/mac on the first network + if i == 0 { + if ctr.config.StaticIP != nil { + netOpts.StaticIPs = []net.IP{ctr.config.StaticIP} + } + netOpts.StaticMAC = ctr.config.StaticMAC + } + + optsBytes, err := json.Marshal(netOpts) + if err != nil { + return err + } + // insert into network map because we need to return this + networks[network] = *netOpts + + err = ctrNetworkBkt.Put([]byte(network), optsBytes) + if err != nil { + return err + } + } + return nil + }) + if err != nil { + return nil, err + } + } + return networks, nil } @@ -1173,7 +1297,7 @@ func (s *BoltState) GetAllNetworkAliases(ctr *Container) (map[string][]string, e // NetworkConnect adds the given container to the given network. If aliases are // specified, those will be added to the given network. -func (s *BoltState) NetworkConnect(ctr *Container, network string, aliases []string) error { +func (s *BoltState) NetworkConnect(ctr *Container, network string, opts types.PerNetworkOptions) error { if !s.valid { return define.ErrDBClosed } @@ -1190,6 +1314,11 @@ func (s *BoltState) NetworkConnect(ctr *Container, network string, aliases []str return errors.Wrapf(define.ErrNSMismatch, "container %s is in namespace %q, does not match our namespace %q", ctr.ID(), ctr.config.Namespace, s.namespace) } + optBytes, err := json.Marshal(opts) + if err != nil { + return errors.Wrapf(err, "error marshalling network options JSON for container %s", ctr.ID()) + } + ctrID := []byte(ctr.ID()) db, err := s.getDBCon() @@ -1210,47 +1339,20 @@ func (s *BoltState) NetworkConnect(ctr *Container, network string, aliases []str return errors.Wrapf(define.ErrNoSuchCtr, "container %s does not exist in database", ctr.ID()) } - ctrAliasesBkt, err := dbCtr.CreateBucketIfNotExists(aliasesBkt) - if err != nil { - return errors.Wrapf(err, "error creating aliases bucket for container %s", ctr.ID()) - } - ctrNetworksBkt := dbCtr.Bucket(networksBkt) if ctrNetworksBkt == nil { - ctrNetworksBkt, err = dbCtr.CreateBucket(networksBkt) - if err != nil { - return errors.Wrapf(err, "error creating networks bucket for container %s", ctr.ID()) - } - ctrNetworks := ctr.config.Networks - if len(ctrNetworks) == 0 { - ctrNetworks = []string{ctr.runtime.config.Network.DefaultNetwork} - } - // Copy in all the container's CNI networks - for _, net := range ctrNetworks { - if err := ctrNetworksBkt.Put([]byte(net), ctrID); err != nil { - return errors.Wrapf(err, "error adding container %s network %s to DB", ctr.ID(), net) - } - } + return errors.Wrapf(define.ErrNoSuchNetwork, "container %s does not have a network bucket", ctr.ID()) } netConnected := ctrNetworksBkt.Get([]byte(network)) if netConnected != nil { - return errors.Wrapf(define.ErrNetworkExists, "container %s is already connected to CNI network %q", ctr.ID(), network) + return errors.Wrapf(define.ErrNetworkExists, "container %s is already connected to network %q", ctr.ID(), network) } // Add the network - if err := ctrNetworksBkt.Put([]byte(network), ctrID); err != nil { + if err := ctrNetworksBkt.Put([]byte(network), optBytes); err != nil { return errors.Wrapf(err, "error adding container %s to network %s in DB", ctr.ID(), network) } - ctrNetAliasesBkt, err := ctrAliasesBkt.CreateBucketIfNotExists([]byte(network)) - if err != nil { - return errors.Wrapf(err, "error adding container %s network aliases bucket for network %s", ctr.ID(), network) - } - for _, alias := range aliases { - if err := ctrNetAliasesBkt.Put([]byte(alias), ctrID); err != nil { - return errors.Wrapf(err, "error adding container %s network alias %s for network %s", ctr.ID(), alias, network) - } - } return nil }) } diff --git a/libpod/container.go b/libpod/container.go index 2b74a19433..006661d5bb 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -1176,7 +1176,18 @@ func (c *Container) Networks() ([]string, bool, error) { } } - return c.networks() + networks, err := c.networks() + if err != nil { + return nil, false, err + } + + names := make([]string, 0, len(networks)) + + for name := range networks { + names = append(names, name) + } + + return names, false, nil } // NetworkMode gets the configured network mode for the container. @@ -1220,36 +1231,8 @@ func (c *Container) NetworkMode() string { } // Unlocked accessor for networks -func (c *Container) networks() ([]string, bool, error) { - networks, err := c.runtime.state.GetNetworks(c) - if err != nil && errors.Cause(err) == define.ErrNoSuchNetwork { - if len(c.config.Networks) == 0 && c.config.NetMode.IsBridge() { - return []string{c.runtime.config.Network.DefaultNetwork}, true, nil - } - return c.config.Networks, false, nil - } - - return networks, false, err -} - -// networksByNameIndex provides us with a map of container networks where key -// is network name and value is the index position -func (c *Container) networksByNameIndex() (map[string]int, error) { - networks, _, err := c.networks() - if err != nil { - return nil, err - } - networkNamesByIndex := make(map[string]int, len(networks)) - for index, name := range networks { - networkNamesByIndex[name] = index - } - return networkNamesByIndex, nil -} - -// add puts the new given CNI network name into the tracking map -// and assigns it a new integer based on the map length -func (d ContainerNetworkDescriptions) add(networkName string) { - d[networkName] = len(d) +func (c *Container) networks() (map[string]types.PerNetworkOptions, error) { + return c.runtime.state.GetNetworks(c) } // getInterfaceByName returns a formatted interface name for a given @@ -1270,9 +1253,7 @@ func (c *Container) getNetworkStatus() map[string]types.StatusBlock { return c.state.NetworkStatus } if c.state.NetworkStatusOld != nil { - // Note: NetworkStatusOld does not contain the network names so we get them extra - // Generally the order should be the same - networks, _, err := c.networks() + networks, err := c.networks() if err != nil { return nil } @@ -1280,12 +1261,16 @@ func (c *Container) getNetworkStatus() map[string]types.StatusBlock { return nil } result := make(map[string]types.StatusBlock, len(c.state.NetworkStatusOld)) - for i := range c.state.NetworkStatusOld { + i := 0 + // Note: NetworkStatusOld does not contain the network names so we get them extra + // We cannot guarantee the same order but after a state refresh it should work + for netName := range networks { status, err := cni.CNIResultToStatus(c.state.NetworkStatusOld[i]) if err != nil { return nil } - result[networks[i]] = status + result[netName] = status + i++ } c.state.NetworkStatus = result _ = c.save() diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index ef9f13f445..f4b629a83e 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -1293,23 +1293,6 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti return nil, 0, err } - // If a container is restored multiple times from an exported checkpoint with - // the help of '--import --name', the restore will fail if during 'podman run' - // a static container IP was set with '--ip'. The user can tell the restore - // process to ignore the static IP with '--ignore-static-ip' - if options.IgnoreStaticIP { - c.config.StaticIP = nil - } - - // If a container is restored multiple times from an exported checkpoint with - // the help of '--import --name', the restore will fail if during 'podman run' - // a static container MAC address was set with '--mac-address'. The user - // can tell the restore process to ignore the static MAC with - // '--ignore-static-mac' - if options.IgnoreStaticMAC { - c.config.StaticMAC = nil - } - // Read network configuration from checkpoint var netStatus map[string]types.StatusBlock _, err := metadata.ReadJSONFile(&netStatus, c.bundlePath(), metadata.NetworkStatusFile) @@ -1325,19 +1308,19 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti if err == nil && options.Name == "" && (!options.IgnoreStaticIP || !options.IgnoreStaticMAC) { // The file with the network.status does exist. Let's restore the // container with the same networks settings as during checkpointing. - aliases, err := c.GetAllNetworkAliases() + networkOpts, err := c.networks() if err != nil { return nil, 0, err } + netOpts := make(map[string]types.PerNetworkOptions, len(netStatus)) - for network, status := range netStatus { - perNetOpts := types.PerNetworkOptions{} - for name, netInt := range status.Interfaces { - perNetOpts = types.PerNetworkOptions{ - InterfaceName: name, - Aliases: aliases[network], - } - if !options.IgnoreStaticMAC { + for network, perNetOpts := range networkOpts { + // unset mac and ips before we start adding the ones from the status + perNetOpts.StaticMAC = nil + perNetOpts.StaticIPs = nil + for name, netInt := range netStatus[network].Interfaces { + perNetOpts.InterfaceName = name + if !options.IgnoreStaticIP { perNetOpts.StaticMAC = netInt.MacAddress } if !options.IgnoreStaticIP { @@ -1349,13 +1332,6 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti // For now just use the first interface to get the ips this should be good enough for most cases. break } - if perNetOpts.InterfaceName == "" { - eth, exists := c.state.NetInterfaceDescriptions.getInterfaceByName(network) - if !exists { - return nil, 0, errors.Errorf("no network interface name for container %s on network %s", c.config.ID, network) - } - perNetOpts.InterfaceName = eth - } netOpts[network] = perNetOpts } c.perNetworkOpts = netOpts diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 5fb97dd73b..3f56be855f 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -53,41 +53,6 @@ const ( persistentCNIDir = "/var/lib/cni" ) -// GetAllNetworkAliases returns all configured aliases for this container. -// It also adds the container short ID as alias to match docker. -func (c *Container) GetAllNetworkAliases() (map[string][]string, error) { - allAliases, err := c.runtime.state.GetAllNetworkAliases(c) - if err != nil { - return nil, err - } - - // get the all attached networks, we cannot use GetAllNetworkAliases() - // since it returns nil if there are no aliases - nets, _, err := c.networks() - if err != nil { - return nil, err - } - - // add container short ID as alias to match docker - for _, net := range nets { - allAliases[net] = append(allAliases[net], c.config.ID[:12]) - } - return allAliases, nil -} - -// GetNetworkAliases returns configured aliases for this network. -// It also adds the container short ID as alias to match docker. -func (c *Container) GetNetworkAliases(netName string) ([]string, error) { - aliases, err := c.runtime.state.GetNetworkAliases(c, netName) - if err != nil { - return nil, err - } - - // add container short ID as alias to match docker - aliases = append(aliases, c.config.ID[:12]) - return aliases, nil -} - // convertPortMappings will remove the HostIP part from the ports when running inside podman machine. // This is need because a HostIP of 127.0.0.1 would now allow the gvproxy forwarder to reach to open ports. // For machine the HostIP must only be used by gvproxy and never in the VM. @@ -104,53 +69,20 @@ func (c *Container) convertPortMappings() []types.PortMapping { return newPorts } -func (c *Container) getNetworkOptions() (types.NetworkOptions, error) { +func (c *Container) getNetworkOptions(networkOpts map[string]types.PerNetworkOptions) (types.NetworkOptions, error) { opts := types.NetworkOptions{ ContainerID: c.config.ID, ContainerName: getCNIPodName(c), } opts.PortMappings = c.convertPortMappings() - networks, _, err := c.networks() - if err != nil { - return opts, err - } - aliases, err := c.GetAllNetworkAliases() - if err != nil { - return opts, err - } // If the container requested special network options use this instead of the config. // This is the case for container restore or network reload. if c.perNetworkOpts != nil { opts.Networks = c.perNetworkOpts - return opts, nil - } - - // Update container map of interface descriptions - if err := c.setupNetworkDescriptions(networks); err != nil { - return opts, err - } - - nets := make(map[string]types.PerNetworkOptions, len(networks)) - for i, network := range networks { - eth, exists := c.state.NetInterfaceDescriptions.getInterfaceByName(network) - if !exists { - return opts, errors.Errorf("no network interface name for container %s on network %s", c.config.ID, network) - } - netOpts := types.PerNetworkOptions{ - InterfaceName: eth, - Aliases: aliases[network], - } - // only set the static ip/mac on the first network - if i == 0 { - if c.config.StaticIP != nil { - netOpts.StaticIPs = []net.IP{c.config.StaticIP} - } - netOpts.StaticMAC = c.config.StaticMAC - } - nets[network] = netOpts + } else { + opts.Networks = networkOpts } - opts.Networks = nets return opts, nil } @@ -697,7 +629,7 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) (status map[str if ctr.config.NetMode.IsSlirp4netns() { return nil, r.setupSlirp4netns(ctr, ctrNS) } - networks, _, err := ctr.networks() + networks, err := ctr.networks() if err != nil { return nil, err } @@ -707,7 +639,7 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) (status map[str return nil, nil } - netOpts, err := ctr.getNetworkOptions() + netOpts, err := ctr.getNetworkOptions(networks) if err != nil { return nil, err } @@ -862,13 +794,13 @@ func (r *Runtime) teardownCNI(ctr *Container) error { logrus.Debugf("Tearing down network namespace at %s for container %s", ctr.state.NetNS.Path(), ctr.ID()) - networks, _, err := ctr.networks() + networks, err := ctr.networks() if err != nil { return err } if !ctr.config.NetMode.IsSlirp4netns() && len(networks) > 0 { - netOpts, err := ctr.getNetworkOptions() + netOpts, err := ctr.getNetworkOptions(networks) if err != nil { return err } @@ -960,22 +892,17 @@ func (r *Runtime) reloadContainerNetwork(ctr *Container) (map[string]types.Statu } } - aliases, err := ctr.GetAllNetworkAliases() + networkOpts, err := ctr.networks() if err != nil { return nil, err } // Set the same network settings as before.. netStatus := ctr.getNetworkStatus() - netOpts := make(map[string]types.PerNetworkOptions, len(netStatus)) - for network, status := range netStatus { - perNetOpts := types.PerNetworkOptions{} - for name, netInt := range status.Interfaces { - perNetOpts = types.PerNetworkOptions{ - InterfaceName: name, - Aliases: aliases[network], - StaticMAC: netInt.MacAddress, - } + for network, perNetOpts := range networkOpts { + for name, netInt := range netStatus[network].Interfaces { + perNetOpts.InterfaceName = name + perNetOpts.StaticMAC = netInt.MacAddress for _, netAddress := range netInt.Subnets { perNetOpts.StaticIPs = append(perNetOpts.StaticIPs, netAddress.IPNet.IP) } @@ -983,16 +910,9 @@ func (r *Runtime) reloadContainerNetwork(ctr *Container) (map[string]types.Statu // For now just use the first interface to get the ips this should be good enough for most cases. break } - if perNetOpts.InterfaceName == "" { - eth, exists := ctr.state.NetInterfaceDescriptions.getInterfaceByName(network) - if !exists { - return nil, errors.Errorf("no network interface name for container %s on network %s", ctr.config.ID, network) - } - perNetOpts.InterfaceName = eth - } - netOpts[network] = perNetOpts + networkOpts[network] = perNetOpts } - ctr.perNetworkOpts = netOpts + ctr.perNetworkOpts = networkOpts return r.configureNetNS(ctr, ctr.state.NetNS) } @@ -1049,7 +969,7 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e settings := new(define.InspectNetworkSettings) settings.Ports = makeInspectPortBindings(c.config.PortMappings, c.config.ExposedPorts) - networks, isDefault, err := c.networks() + networks, err := c.networks() if err != nil { return nil, err } @@ -1060,14 +980,10 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e // the container joined. if len(networks) > 0 { settings.Networks = make(map[string]*define.InspectAdditionalNetwork, len(networks)) - for _, net := range networks { + for net, opts := range networks { cniNet := new(define.InspectAdditionalNetwork) cniNet.NetworkID = net - aliases, err := c.GetNetworkAliases(net) - if err != nil { - return nil, err - } - cniNet.Aliases = aliases + cniNet.Aliases = opts.Aliases settings.Networks[net] = cniNet } } @@ -1092,7 +1008,7 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e settings.Networks = make(map[string]*define.InspectAdditionalNetwork) - for _, name := range networks { + for name, opts := range networks { result := netStatus[name] addedNet := new(define.InspectAdditionalNetwork) addedNet.NetworkID = name @@ -1101,19 +1017,17 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e if err != nil { return nil, err } - - aliases, err := c.GetNetworkAliases(name) - if err != nil { - return nil, err - } - addedNet.Aliases = aliases + addedNet.Aliases = opts.Aliases addedNet.InspectBasicNetworkConfig = basicConfig settings.Networks[name] = addedNet } - if !isDefault { + // if not only the default network is connected we can return here + // otherwise we have to populate the InspectBasicNetworkConfig settings + _, isDefaultNet := networks[c.runtime.config.Network.DefaultNetwork] + if !(len(networks) == 1 && isDefaultNet) { return settings, nil } } @@ -1135,29 +1049,6 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e return settings, nil } -// setupNetworkDescriptions adds networks and eth values to the container's -// network descriptions -func (c *Container) setupNetworkDescriptions(networks []string) error { - // if the map is nil and we have networks - if c.state.NetInterfaceDescriptions == nil && len(networks) > 0 { - c.state.NetInterfaceDescriptions = make(ContainerNetworkDescriptions) - } - origLen := len(c.state.NetInterfaceDescriptions) - for _, n := range networks { - // if the network is not in the map, add it - if _, exists := c.state.NetInterfaceDescriptions[n]; !exists { - c.state.NetInterfaceDescriptions.add(n) - } - } - // if the map changed, we need to save the container state - if origLen != len(c.state.NetInterfaceDescriptions) { - if err := c.save(); err != nil { - return err - } - } - return nil -} - // resultToBasicNetworkConfig produces an InspectBasicNetworkConfig from a CNI // result func resultToBasicNetworkConfig(result types.StatusBlock) (define.InspectBasicNetworkConfig, error) { @@ -1213,7 +1104,7 @@ func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) erro c.lock.Lock() defer c.lock.Unlock() - networks, err := c.networksByNameIndex() + networks, err := c.networks() if err != nil { return err } @@ -1254,14 +1145,8 @@ func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) erro ContainerName: getCNIPodName(c), } opts.PortMappings = c.convertPortMappings() - eth, exists := c.state.NetInterfaceDescriptions.getInterfaceByName(netName) - if !exists { - return errors.Errorf("no network interface name for container %s on network %s", c.config.ID, netName) - } opts.Networks = map[string]types.PerNetworkOptions{ - netName: { - InterfaceName: eth, - }, + netName: networks[netName], } if err := c.runtime.teardownNetwork(c.state.NetNS.Path(), opts); err != nil { @@ -1294,7 +1179,7 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e c.lock.Lock() defer c.lock.Unlock() - networks, err := c.networksByNameIndex() + networks, err := c.networks() if err != nil { return err } @@ -1321,7 +1206,18 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e return errors.Wrapf(define.ErrInvalidArg, "cannot set network aliases for network %q because dns is disabled", netName) } - if err := c.runtime.state.NetworkConnect(c, netName, aliases); err != nil { + eth := getFreeInterfaceName(networks) + if eth == "" { + return errors.New("could not find free network interface name") + } + + perNetOpt := types.PerNetworkOptions{ + // always add the short id as alias to match docker + Aliases: append(aliases, c.config.ID[:12]), + InterfaceName: eth, + } + + if err := c.runtime.state.NetworkConnect(c, netName, perNetOpt); err != nil { return err } c.newNetworkEvent(events.NetworkConnect, netName) @@ -1332,30 +1228,13 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e return errors.Wrapf(define.ErrNoNetwork, "unable to connect %s to %s", nameOrID, netName) } - ctrNetworks, _, err := c.networks() - if err != nil { - return err - } - // Update network descriptions - if err := c.setupNetworkDescriptions(ctrNetworks); err != nil { - return err - } - opts := types.NetworkOptions{ ContainerID: c.config.ID, ContainerName: getCNIPodName(c), } opts.PortMappings = c.convertPortMappings() - eth, exists := c.state.NetInterfaceDescriptions.getInterfaceByName(netName) - if !exists { - return errors.Errorf("no network interface name for container %s on network %s", c.config.ID, netName) - } - aliases = append(aliases, c.config.ID[:12]) opts.Networks = map[string]types.PerNetworkOptions{ - netName: { - Aliases: aliases, - InterfaceName: eth, - }, + netName: perNetOpt, } results, err := c.runtime.setUpNetwork(c.state.NetNS.Path(), opts) @@ -1385,6 +1264,22 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e return nil } +// get a free interface name for a new network +// return an empty string if no free name was found +func getFreeInterfaceName(networks map[string]types.PerNetworkOptions) string { + ifNames := make([]string, 0, len(networks)) + for _, opts := range networks { + ifNames = append(ifNames, opts.InterfaceName) + } + for i := 0; i < 100000; i++ { + ifName := fmt.Sprintf("eth%d", i) + if !util.StringInSlice(ifName, ifNames) { + return ifName + } + } + return "" +} + // DisconnectContainerFromNetwork removes a container from its CNI network func (r *Runtime) DisconnectContainerFromNetwork(nameOrID, netName string, force bool) error { ctr, err := r.LookupContainer(nameOrID) diff --git a/libpod/state.go b/libpod/state.go index 4b711bae92..fe7b86fdbb 100644 --- a/libpod/state.go +++ b/libpod/state.go @@ -1,5 +1,7 @@ package libpod +import "github.com/containers/podman/v3/libpod/network/types" + // State is a storage backend for libpod's current state. // A State is only initialized once per instance of libpod. // As such, initialization methods for State implementations may safely assume @@ -99,14 +101,14 @@ type State interface { AllContainers() ([]*Container, error) // Get networks the container is currently connected to. - GetNetworks(ctr *Container) ([]string, error) + GetNetworks(ctr *Container) (map[string]types.PerNetworkOptions, error) // Get network aliases for the given container in the given network. GetNetworkAliases(ctr *Container, network string) ([]string, error) // Get all network aliases for the given container. GetAllNetworkAliases(ctr *Container) (map[string][]string, error) // Add the container to the given network, adding the given aliases // (if present). - NetworkConnect(ctr *Container, network string, aliases []string) error + NetworkConnect(ctr *Container, network string, opts types.PerNetworkOptions) error // Remove the container from the given network, removing all aliases for // the container in that network in the process. NetworkDisconnect(ctr *Container, network string) error From 4e8ad039cee5debcc1afe93a455cd8a25e88d16b Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 7 Dec 2021 20:43:04 +0100 Subject: [PATCH 02/10] remove unneeded return value from c.Networks() We do not need to return a extra bool. Signed-off-by: Paul Holzinger --- libpod/container.go | 8 ++++---- pkg/domain/filters/containers.go | 2 +- pkg/domain/filters/pods.go | 2 +- pkg/domain/infra/abi/network.go | 4 ++-- pkg/domain/infra/abi/pods.go | 2 +- pkg/ps/ps.go | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/libpod/container.go b/libpod/container.go index 006661d5bb..1270f21125 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -1166,19 +1166,19 @@ func (c *Container) Secrets() []*ContainerSecret { // is joining the default CNI network - the network name will be included in the // returned array of network names, but the container did not explicitly join // this network. -func (c *Container) Networks() ([]string, bool, error) { +func (c *Container) Networks() ([]string, error) { if !c.batched { c.lock.Lock() defer c.lock.Unlock() if err := c.syncContainer(); err != nil { - return nil, false, err + return nil, err } } networks, err := c.networks() if err != nil { - return nil, false, err + return nil, err } names := make([]string, 0, len(networks)) @@ -1187,7 +1187,7 @@ func (c *Container) Networks() ([]string, bool, error) { names = append(names, name) } - return names, false, nil + return names, nil } // NetworkMode gets the configured network mode for the container. diff --git a/pkg/domain/filters/containers.go b/pkg/domain/filters/containers.go index 269cd2d27d..bd5167fa5c 100644 --- a/pkg/domain/filters/containers.go +++ b/pkg/domain/filters/containers.go @@ -241,7 +241,7 @@ func GenerateContainerFilterFuncs(filter string, filterValues []string, r *libpo return false } - networks, _, err := c.Networks() + networks, err := c.Networks() // if err or no networks, quick out if err != nil || len(networks) == 0 { return false diff --git a/pkg/domain/filters/pods.go b/pkg/domain/filters/pods.go index 9a2f0a3ba8..59a6d0d780 100644 --- a/pkg/domain/filters/pods.go +++ b/pkg/domain/filters/pods.go @@ -134,7 +134,7 @@ func GeneratePodFilterFunc(filter string, filterValues []string) ( if err != nil { return false } - networks, _, err := infra.Networks() + networks, err := infra.Networks() // if err or no networks, quick out if err != nil || len(networks) == 0 { return false diff --git a/pkg/domain/infra/abi/network.go b/pkg/domain/infra/abi/network.go index ee7403ed56..49c7dc60d5 100644 --- a/pkg/domain/infra/abi/network.go +++ b/pkg/domain/infra/abi/network.go @@ -71,7 +71,7 @@ func (ic *ContainerEngine) NetworkRm(ctx context.Context, namesOrIds []string, o } // We need to iterate containers looking to see if they belong to the given network for _, c := range containers { - networks, _, err := c.Networks() + networks, err := c.Networks() // if container vanished or network does not exist, go to next container if errors.Is(err, define.ErrNoSuchNetwork) || errors.Is(err, define.ErrNoSuchCtr) { continue @@ -152,7 +152,7 @@ func (ic *ContainerEngine) NetworkPrune(ctx context.Context, options entities.Ne // containers want networksToKeep := make(map[string]bool) for _, c := range cons { - nets, _, err := c.Networks() + nets, err := c.Networks() if err != nil { return nil, err } diff --git a/pkg/domain/infra/abi/pods.go b/pkg/domain/infra/abi/pods.go index 028de9e812..c8c7b0d9e9 100644 --- a/pkg/domain/infra/abi/pods.go +++ b/pkg/domain/infra/abi/pods.go @@ -376,7 +376,7 @@ func (ic *ContainerEngine) PodPs(ctx context.Context, options entities.PodPSOpti if err != nil { return nil, err } - networks, _, err = infra.Networks() + networks, err = infra.Networks() if err != nil { return nil, err } diff --git a/pkg/ps/ps.go b/pkg/ps/ps.go index 90ad23f491..391bd54d67 100644 --- a/pkg/ps/ps.go +++ b/pkg/ps/ps.go @@ -207,7 +207,7 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities return entities.ListContainer{}, err } - networks, _, err := ctr.Networks() + networks, err := ctr.Networks() if err != nil { return entities.ListContainer{}, err } From 9ce6b64133bc37433efac391bcaf9234c3890fc5 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 7 Dec 2021 23:04:47 +0100 Subject: [PATCH 03/10] network db: add new strucutre to container create Make sure we create new containers in the db with the correct structure. Also remove some unneeded code for alias handling. We no longer need this functions. The specgen format has not been changed for now. Signed-off-by: Paul Holzinger --- libpod/boltdb_state.go | 151 +---------------------- libpod/boltdb_state_internal.go | 62 ++++------ libpod/container_config.go | 24 ++-- libpod/container_validate.go | 13 +- libpod/network/types/network.go | 3 +- libpod/options.go | 50 +------- libpod/pod_api.go | 14 ++- libpod/runtime_ctr.go | 64 ++++++---- libpod/state.go | 7 +- libpod/state_test.go | 18 +-- pkg/specgen/generate/container_create.go | 4 - pkg/specgen/generate/namespaces.go | 36 ++++-- 12 files changed, 128 insertions(+), 318 deletions(-) diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index d322ccc539..9669cf921e 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -1063,7 +1063,7 @@ func (s *BoltState) GetNetworks(ctr *Container) (map[string]types.PerNetworkOpti return errors.Wrapf(err, "error creating networks bucket for container %s", ctr.ID()) } // the container has no networks in the db lookup config and write to the db - networkList = ctr.config.Networks + networkList = ctr.config.NetworksDeprecated // if there are no networks we have to add the default if len(networkList) == 0 { networkList = []string{ctr.runtime.config.Network.DefaultNetwork} @@ -1146,155 +1146,6 @@ func (s *BoltState) GetNetworks(ctr *Container) (map[string]types.PerNetworkOpti return networks, nil } -// GetNetworkAliases retrieves the network aliases for the given container in -// the given CNI network. -func (s *BoltState) GetNetworkAliases(ctr *Container, network string) ([]string, error) { - if !s.valid { - return nil, define.ErrDBClosed - } - - if !ctr.valid { - return nil, define.ErrCtrRemoved - } - - if network == "" { - return nil, errors.Wrapf(define.ErrInvalidArg, "network names must not be empty") - } - - if s.namespace != "" && s.namespace != ctr.config.Namespace { - return nil, errors.Wrapf(define.ErrNSMismatch, "container %s is in namespace %q, does not match our namespace %q", ctr.ID(), ctr.config.Namespace, s.namespace) - } - - ctrID := []byte(ctr.ID()) - - db, err := s.getDBCon() - if err != nil { - return nil, err - } - defer s.deferredCloseDBCon(db) - - aliases := []string{} - - err = db.View(func(tx *bolt.Tx) error { - ctrBucket, err := getCtrBucket(tx) - if err != nil { - return err - } - - dbCtr := ctrBucket.Bucket(ctrID) - if dbCtr == nil { - ctr.valid = false - return errors.Wrapf(define.ErrNoSuchCtr, "container %s does not exist in database", ctr.ID()) - } - - ctrNetworkBkt := dbCtr.Bucket(networksBkt) - if ctrNetworkBkt == nil { - // No networks joined, so no aliases - return nil - } - - inNetwork := ctrNetworkBkt.Get([]byte(network)) - if inNetwork == nil { - return errors.Wrapf(define.ErrNoAliases, "container %s is not part of network %s, no aliases found", ctr.ID(), network) - } - - ctrAliasesBkt := dbCtr.Bucket(aliasesBkt) - if ctrAliasesBkt == nil { - // No aliases - return nil - } - - netAliasesBkt := ctrAliasesBkt.Bucket([]byte(network)) - if netAliasesBkt == nil { - // No aliases for this specific network. - return nil - } - - return netAliasesBkt.ForEach(func(alias, v []byte) error { - aliases = append(aliases, string(alias)) - return nil - }) - }) - if err != nil { - return nil, err - } - - return aliases, nil -} - -// GetAllNetworkAliases retrieves the network aliases for the given container in -// all CNI networks. -func (s *BoltState) GetAllNetworkAliases(ctr *Container) (map[string][]string, error) { - if !s.valid { - return nil, define.ErrDBClosed - } - - if !ctr.valid { - return nil, define.ErrCtrRemoved - } - - if s.namespace != "" && s.namespace != ctr.config.Namespace { - return nil, errors.Wrapf(define.ErrNSMismatch, "container %s is in namespace %q, does not match our namespace %q", ctr.ID(), ctr.config.Namespace, s.namespace) - } - - ctrID := []byte(ctr.ID()) - - db, err := s.getDBCon() - if err != nil { - return nil, err - } - defer s.deferredCloseDBCon(db) - - aliases := make(map[string][]string) - - err = db.View(func(tx *bolt.Tx) error { - ctrBucket, err := getCtrBucket(tx) - if err != nil { - return err - } - - dbCtr := ctrBucket.Bucket(ctrID) - if dbCtr == nil { - ctr.valid = false - return errors.Wrapf(define.ErrNoSuchCtr, "container %s does not exist in database", ctr.ID()) - } - - ctrAliasesBkt := dbCtr.Bucket(aliasesBkt) - if ctrAliasesBkt == nil { - // No aliases present - return nil - } - - ctrNetworkBkt := dbCtr.Bucket(networksBkt) - if ctrNetworkBkt == nil { - // No networks joined, so no aliases - return nil - } - - return ctrNetworkBkt.ForEach(func(network, v []byte) error { - netAliasesBkt := ctrAliasesBkt.Bucket(network) - if netAliasesBkt == nil { - return nil - } - - netAliases := []string{} - - _ = netAliasesBkt.ForEach(func(alias, v []byte) error { - netAliases = append(netAliases, string(alias)) - return nil - }) - - aliases[string(network)] = netAliases - return nil - }) - }) - if err != nil { - return nil, err - } - - return aliases, nil -} - // NetworkConnect adds the given container to the given network. If aliases are // specified, those will be added to the given network. func (s *BoltState) NetworkConnect(ctr *Container, network string, opts types.PerNetworkOptions) error { diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go index e71d82736c..5f0f6ba7de 100644 --- a/libpod/boltdb_state_internal.go +++ b/libpod/boltdb_state_internal.go @@ -563,6 +563,28 @@ func (s *BoltState) addContainer(ctr *Container, pod *Pod) error { ctrNamespace = []byte(ctr.config.Namespace) } + // make sure to marshal the network options before we get the db lock + networks := make(map[string][]byte, len(ctr.config.Networks)) + for net, opts := range ctr.config.Networks { + // Check that we don't have any empty network names + if net == "" { + return errors.Wrapf(define.ErrInvalidArg, "network names cannot be an empty string") + } + if opts.InterfaceName == "" { + return errors.Wrapf(define.ErrInvalidArg, "network interface name cannot be an empty string") + } + // always add the short id as alias for docker compat + opts.Aliases = append(opts.Aliases, ctr.config.ID[:12]) + optBytes, err := json.Marshal(opts) + if err != nil { + return errors.Wrapf(err, "error marshalling network options JSON for container %s", ctr.ID()) + } + networks[net] = optBytes + } + // Set the original value to nil. We can safe some space by not storing it in the config + // since we store it in a different mutable bucket anyway. + ctr.config.Networks = nil + db, err := s.getDBCon() if err != nil { return err @@ -646,23 +668,6 @@ func (s *BoltState) addContainer(ctr *Container, pod *Pod) error { return errors.Wrapf(err, "name \"%s\" is in use", ctr.Name()) } - allNets := make(map[string]bool) - - // Check that we don't have any empty network names - for _, net := range ctr.config.Networks { - if net == "" { - return errors.Wrapf(define.ErrInvalidArg, "network names cannot be an empty string") - } - allNets[net] = true - } - - // Each network we have aliases for, must exist in networks - for net := range ctr.config.NetworkAliases { - if !allNets[net] { - return errors.Wrapf(define.ErrNoSuchNetwork, "container %s has network aliases for network %q but is not part of that network", ctr.ID(), net) - } - } - // No overlapping containers // Add the new container to the DB if err := idsBucket.Put(ctrID, ctrName); err != nil { @@ -706,34 +711,17 @@ func (s *BoltState) addContainer(ctr *Container, pod *Pod) error { return errors.Wrapf(err, "error adding container %s netns path to DB", ctr.ID()) } } - if ctr.config.Networks != nil { + if len(networks) > 0 { ctrNetworksBkt, err := newCtrBkt.CreateBucket(networksBkt) if err != nil { return errors.Wrapf(err, "error creating networks bucket for container %s", ctr.ID()) } - for _, network := range ctr.config.Networks { - if err := ctrNetworksBkt.Put([]byte(network), ctrID); err != nil { + for network, opts := range networks { + if err := ctrNetworksBkt.Put([]byte(network), opts); err != nil { return errors.Wrapf(err, "error adding network %q to networks bucket for container %s", network, ctr.ID()) } } } - if ctr.config.NetworkAliases != nil { - ctrAliasesBkt, err := newCtrBkt.CreateBucket(aliasesBkt) - if err != nil { - return errors.Wrapf(err, "error creating network aliases bucket for container %s", ctr.ID()) - } - for net, aliases := range ctr.config.NetworkAliases { - netAliasesBkt, err := ctrAliasesBkt.CreateBucket([]byte(net)) - if err != nil { - return errors.Wrapf(err, "error creating network aliases bucket for network %q in container %s", net, ctr.ID()) - } - for _, alias := range aliases { - if err := netAliasesBkt.Put([]byte(alias), ctrID); err != nil { - return errors.Wrapf(err, "error creating network alias %q in network %q for container %s", alias, net, ctr.ID()) - } - } - } - } if _, err := newCtrBkt.CreateBucket(dependenciesBkt); err != nil { return errors.Wrapf(err, "error creating dependencies bucket for container %s", ctr.ID()) diff --git a/libpod/container_config.go b/libpod/container_config.go index 57f5b92acc..adc585fa1b 100644 --- a/libpod/container_config.go +++ b/libpod/container_config.go @@ -229,10 +229,12 @@ type ContainerNetworkConfig struct { // StaticIP is a static IP to request for the container. // This cannot be set unless CreateNetNS is set. // If not set, the container will be dynamically assigned an IP by CNI. + // Deprecated: Do no use this anymore, this is only for DB backwards compat. StaticIP net.IP `json:"staticIP"` // StaticMAC is a static MAC to request for the container. // This cannot be set unless CreateNetNS is set. // If not set, the container will be dynamically assigned a MAC by CNI. + // Deprecated: Do no use this anymore, this is only for DB backwards compat. StaticMAC types.HardwareAddr `json:"staticMAC"` // PortMappings are the ports forwarded to the container's network // namespace @@ -269,24 +271,24 @@ type ContainerNetworkConfig struct { // Hosts to add in container // Will be appended to host's host file HostAdd []string `json:"hostsAdd,omitempty"` - // Network names (CNI) to add container to. Empty to use default network. + // Network names with the network specific options. + // Please note that these can be altered at runtime. The actual list is + // stored in the DB and should be retrieved from there via c.networks() + // this value is only used for container create. + // Added in podman 4.0, previously NetworksDeprecated was used. Make + // sure to not change the json tags. + Networks map[string]types.PerNetworkOptions `json:"newNetworks,omitempty"` + // Network names to add container to. Empty to use default network. // Please note that these can be altered at runtime. The actual list is // stored in the DB and should be retrieved from there; this is only the // set of networks the container was *created* with. - Networks []string `json:"networks,omitempty"` + // Deprecated: Do no use this anymore, this is only for DB backwards compat. + // Also note that we need to keep the old json tag to decode from DB correctly + NetworksDeprecated []string `json:"networks,omitempty"` // Network mode specified for the default network. NetMode namespaces.NetworkMode `json:"networkMode,omitempty"` // NetworkOptions are additional options for each network NetworkOptions map[string][]string `json:"network_options,omitempty"` - // NetworkAliases are aliases that will be added to each network. - // These are additional names that this container can be accessed as via - // DNS when the CNI dnsname plugin is in use. - // Please note that these can be altered at runtime. As such, the actual - // list is stored in the database and should be retrieved from there; - // this is only the set of aliases the container was *created with*. - // Formatted as map of network name to aliases. All network names must - // be present in the Networks list above. - NetworkAliases map[string][]string `json:"network_alises,omitempty"` } // ContainerImageConfig is an embedded sub-config providing image configuration diff --git a/libpod/container_validate.go b/libpod/container_validate.go index 91ebe93fba..ca5ce8b2a8 100644 --- a/libpod/container_validate.go +++ b/libpod/container_validate.go @@ -74,7 +74,7 @@ func (c *Container) validate() error { // Cannot set static IP or MAC if joining >1 CNI network. if len(c.config.Networks) > 1 && (c.config.StaticIP != nil || c.config.StaticMAC != nil) { - return errors.Wrapf(define.ErrInvalidArg, "cannot set static IP or MAC address if joining more than one CNI network") + return errors.Wrapf(define.ErrInvalidArg, "cannot set static IP or MAC address if joining more than one network") } // Using image resolv.conf conflicts with various DNS settings. @@ -115,17 +115,6 @@ func (c *Container) validate() error { destinations[vol.Dest] = true } - // Check that networks and network aliases match up. - ctrNets := make(map[string]bool) - for _, net := range c.config.Networks { - ctrNets[net] = true - } - for net := range c.config.NetworkAliases { - if _, ok := ctrNets[net]; !ok { - return errors.Wrapf(define.ErrNoSuchNetwork, "container tried to set network aliases for network %s but is not connected to the network", net) - } - } - // If User in the OCI spec is set, require that c.config.User is set for // security reasons (a lot of our code relies on c.config.User). if c.config.User == "" && (c.config.Spec.Process.User.UID != 0 || c.config.Spec.Process.User.GID != 0) { diff --git a/libpod/network/types/network.go b/libpod/network/types/network.go index 105641e705..37fa114615 100644 --- a/libpod/network/types/network.go +++ b/libpod/network/types/network.go @@ -204,7 +204,8 @@ type PerNetworkOptions struct { Aliases []string `json:"aliases,omitempty"` // StaticMac for this container. Optional. StaticMAC HardwareAddr `json:"static_mac,omitempty"` - // InterfaceName for this container. Required. + // InterfaceName for this container. Required in the backend. + // Optional in the frontend. Will be filled with ethX (where X is a integer) when empty. InterfaceName string `json:"interface_name"` } diff --git a/libpod/options.go b/libpod/options.go index 8f2d5cb150..dbcc507418 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1058,7 +1058,7 @@ func WithDependencyCtrs(ctrs []*Container) CtrCreateOption { // namespace with a minimal configuration. // An optional array of port mappings can be provided. // Conflicts with WithNetNSFrom(). -func WithNetNS(portMappings []nettypes.PortMapping, exposedPorts map[uint16][]string, postConfigureNetNS bool, netmode string, networks []string) CtrCreateOption { +func WithNetNS(portMappings []nettypes.PortMapping, exposedPorts map[uint16][]string, postConfigureNetNS bool, netmode string, networks map[string]nettypes.PerNetworkOptions) CtrCreateOption { return func(ctr *Container) error { if ctr.valid { return define.ErrCtrFinalized @@ -1076,23 +1076,6 @@ func WithNetNS(portMappings []nettypes.PortMapping, exposedPorts map[uint16][]st } } -// WithStaticIP indicates that the container should request a static IP from -// the CNI plugins. -// It cannot be set unless WithNetNS has already been passed. -// Further, it cannot be set if additional CNI networks to join have been -// specified. -func WithStaticIP(ip net.IP) CtrCreateOption { - return func(ctr *Container) error { - if ctr.valid { - return define.ErrCtrFinalized - } - - ctr.config.StaticIP = ip - - return nil - } -} - // WithNetworkOptions sets additional options for the networks. func WithNetworkOptions(options map[string][]string) CtrCreateOption { return func(ctr *Container) error { @@ -1106,23 +1089,6 @@ func WithNetworkOptions(options map[string][]string) CtrCreateOption { } } -// WithStaticMAC indicates that the container should request a static MAC from -// the CNI plugins. -// It cannot be set unless WithNetNS has already been passed. -// Further, it cannot be set if additional CNI networks to join have been -// specified. -func WithStaticMAC(mac nettypes.HardwareAddr) CtrCreateOption { - return func(ctr *Container) error { - if ctr.valid { - return define.ErrCtrFinalized - } - - ctr.config.StaticMAC = mac - - return nil - } -} - // WithLogDriver sets the log driver for the container func WithLogDriver(driver string) CtrCreateOption { return func(ctr *Container) error { @@ -1572,20 +1538,6 @@ func WithCreateWorkingDir() CtrCreateOption { } } -// WithNetworkAliases sets network aliases for the container. -// Accepts a map of network name to aliases. -func WithNetworkAliases(aliases map[string][]string) CtrCreateOption { - return func(ctr *Container) error { - if ctr.valid { - return define.ErrCtrFinalized - } - - ctr.config.NetworkAliases = aliases - - return nil - } -} - // Volume Creation Options // WithVolumeName sets the name of the volume. diff --git a/libpod/pod_api.go b/libpod/pod_api.go index 80ecb690a2..95a82721eb 100644 --- a/libpod/pod_api.go +++ b/libpod/pod_api.go @@ -637,9 +637,17 @@ func (p *Pod) Inspect() (*define.InspectPodData, error) { infraConfig.HostAdd = make([]string, 0, len(infra.config.HostAdd)) infraConfig.HostAdd = append(infraConfig.HostAdd, infra.config.HostAdd...) } - if len(infra.config.ContainerNetworkConfig.Networks) > 0 { - infraConfig.Networks = make([]string, 0, len(infra.config.ContainerNetworkConfig.Networks)) - infraConfig.Networks = append(infraConfig.Networks, infra.config.ContainerNetworkConfig.Networks...) + + networks, err := infra.networks() + if err != nil { + return nil, err + } + netNames := make([]string, 0, len(networks)) + for name := range networks { + netNames = append(netNames, name) + } + if len(netNames) > 0 { + infraConfig.Networks = netNames } infraConfig.NetworkOptions = infra.config.ContainerNetworkConfig.NetworkOptions infraConfig.PortBindings = makeInspectPortBindings(infra.config.ContainerNetworkConfig.PortMappings, nil) diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 6c46eb747a..59a1fd1534 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -2,6 +2,7 @@ package libpod import ( "context" + "fmt" "os" "path" "path/filepath" @@ -13,10 +14,12 @@ import ( "github.com/containers/common/pkg/config" "github.com/containers/podman/v3/libpod/define" "github.com/containers/podman/v3/libpod/events" + "github.com/containers/podman/v3/libpod/network/types" "github.com/containers/podman/v3/libpod/shutdown" "github.com/containers/podman/v3/pkg/domain/entities/reports" "github.com/containers/podman/v3/pkg/rootless" "github.com/containers/podman/v3/pkg/specgen" + "github.com/containers/podman/v3/pkg/util" "github.com/containers/storage" "github.com/containers/storage/pkg/stringid" "github.com/docker/go-units" @@ -230,39 +233,56 @@ func (r *Runtime) newContainer(ctx context.Context, rSpec *spec.Spec, options .. func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Container, retErr error) { // normalize the networks to names - // ocicni only knows about cni names so we have to make + // the db backend only knows about network names so we have to make // sure we do not use ids internally if len(ctr.config.Networks) > 0 { - netNames := make([]string, 0, len(ctr.config.Networks)) - for _, nameOrID := range ctr.config.Networks { - netName, err := r.normalizeNetworkName(nameOrID) - if err != nil { - return nil, err + normalizeNetworks := make(map[string]types.PerNetworkOptions, len(ctr.config.Networks)) + // first get the already used interface names so we do not conflict + usedIfNames := make([]string, 0, len(ctr.config.Networks)) + for _, opts := range ctr.config.Networks { + if opts.InterfaceName != "" { + // check that no name is assigned to more than network + if util.StringInSlice(opts.InterfaceName, usedIfNames) { + return nil, errors.Errorf("network interface name %q is already assigned to another network", opts.InterfaceName) + } + usedIfNames = append(usedIfNames, opts.InterfaceName) } - netNames = append(netNames, netName) } - ctr.config.Networks = netNames - } - - // https://github.com/containers/podman/issues/11285 - // normalize the networks aliases to use network names and never ids - if len(ctr.config.NetworkAliases) > 0 { - netAliases := make(map[string][]string, len(ctr.config.NetworkAliases)) - for nameOrID, aliases := range ctr.config.NetworkAliases { + i := 0 + for nameOrID, opts := range ctr.config.Networks { netName, err := r.normalizeNetworkName(nameOrID) if err != nil { return nil, err } - network, err := r.network.NetworkInspect(netName) - if err != nil { - return nil, err + if len(opts.Aliases) > 0 { + network, err := r.network.NetworkInspect(netName) + if err != nil { + return nil, err + } + if !network.DNSEnabled { + return nil, errors.Wrapf(define.ErrInvalidArg, "cannot set network aliases for network %q because dns is disabled", netName) + } } - if !network.DNSEnabled { - return nil, errors.Wrapf(define.ErrInvalidArg, "cannot set network aliases for network %q because dns is disabled", netName) + // assign interface name if empty + if opts.InterfaceName == "" { + for i < 100000 { + ifName := fmt.Sprintf("eth%d", i) + if !util.StringInSlice(ifName, usedIfNames) { + opts.InterfaceName = ifName + usedIfNames = append(usedIfNames, ifName) + break + } + i++ + } + // if still empty we did not find a free name + if opts.InterfaceName == "" { + return nil, errors.New("failed to find free network interface name") + } } - netAliases[netName] = aliases + + normalizeNetworks[netName] = opts } - ctr.config.NetworkAliases = netAliases + ctr.config.Networks = normalizeNetworks } // Validate the container diff --git a/libpod/state.go b/libpod/state.go index fe7b86fdbb..21525107fd 100644 --- a/libpod/state.go +++ b/libpod/state.go @@ -102,12 +102,7 @@ type State interface { // Get networks the container is currently connected to. GetNetworks(ctr *Container) (map[string]types.PerNetworkOptions, error) - // Get network aliases for the given container in the given network. - GetNetworkAliases(ctr *Container, network string) ([]string, error) - // Get all network aliases for the given container. - GetAllNetworkAliases(ctr *Container) (map[string][]string, error) - // Add the container to the given network, adding the given aliases - // (if present). + // Add the container to the given network with the given options NetworkConnect(ctr *Container, network string, opts types.PerNetworkOptions) error // Remove the container from the given network, removing all aliases for // the container in that network in the process. diff --git a/libpod/state_test.go b/libpod/state_test.go index 5c3b0d7f70..b0793127de 100644 --- a/libpod/state_test.go +++ b/libpod/state_test.go @@ -1299,21 +1299,9 @@ func TestAddContainerEmptyNetworkNameErrors(t *testing.T) { testCtr, err := getTestCtr1(manager) assert.NoError(t, err) - testCtr.config.Networks = []string{""} - - err = state.AddContainer(testCtr) - assert.Error(t, err) - }) -} - -func TestAddContainerNetworkAliasesButNoMatchingNetwork(t *testing.T) { - runForAllStates(t, func(t *testing.T, state State, manager lock.Manager) { - testCtr, err := getTestCtr1(manager) - assert.NoError(t, err) - - testCtr.config.Networks = []string{"test1"} - testCtr.config.NetworkAliases = make(map[string][]string) - testCtr.config.NetworkAliases["test2"] = []string{"alias1"} + testCtr.config.Networks = map[string]types.PerNetworkOptions{ + "": {}, + } err = state.AddContainer(testCtr) assert.Error(t, err) diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index df5d2e8ffe..331c9393a4 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -160,10 +160,6 @@ func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGener } options = append(options, opts...) - if len(s.Aliases) > 0 { - options = append(options, libpod.WithNetworkAliases(s.Aliases)) - } - if containerType := s.InitContainerType; len(containerType) > 0 { options = append(options, libpod.WithInitCtrType(containerType)) } diff --git a/pkg/specgen/generate/namespaces.go b/pkg/specgen/generate/namespaces.go index 7d63fc10f0..ebdd2abd03 100644 --- a/pkg/specgen/generate/namespaces.go +++ b/pkg/specgen/generate/namespaces.go @@ -10,6 +10,7 @@ import ( "github.com/containers/common/pkg/config" "github.com/containers/podman/v3/libpod" "github.com/containers/podman/v3/libpod/define" + "github.com/containers/podman/v3/libpod/network/types" "github.com/containers/podman/v3/pkg/rootless" "github.com/containers/podman/v3/pkg/specgen" "github.com/containers/podman/v3/pkg/util" @@ -250,7 +251,7 @@ func namespaceOptions(ctx context.Context, s *specgen.SpecGenerator, rt *libpod. if s.NetNS.Value != "" { val = fmt.Sprintf("slirp4netns:%s", s.NetNS.Value) } - toReturn = append(toReturn, libpod.WithNetNS(portMappings, expose, postConfigureNetNS, val, s.CNINetworks)) + toReturn = append(toReturn, libpod.WithNetNS(portMappings, expose, postConfigureNetNS, val, nil)) case specgen.Private: fallthrough case specgen.Bridge: @@ -258,7 +259,32 @@ func namespaceOptions(ctx context.Context, s *specgen.SpecGenerator, rt *libpod. if err != nil { return nil, err } - toReturn = append(toReturn, libpod.WithNetNS(portMappings, expose, postConfigureNetNS, "bridge", s.CNINetworks)) + if len(s.CNINetworks) == 0 { + rtConfig, err := rt.GetConfigNoCopy() + if err != nil { + return nil, err + } + s.CNINetworks = append(s.CNINetworks, rtConfig.Network.DefaultNetwork) + } + networks := make(map[string]types.PerNetworkOptions, len(s.CNINetworks)) + for i, netName := range s.CNINetworks { + opts := types.PerNetworkOptions{} + opts.Aliases = s.Aliases[netName] + if i == 0 { + if s.StaticIP != nil { + opts.StaticIPs = append(opts.StaticIPs, *s.StaticIP) + } + if s.StaticIPv6 != nil { + opts.StaticIPs = append(opts.StaticIPs, *s.StaticIPv6) + } + if s.StaticMAC != nil { + opts.StaticMAC = *s.StaticMAC + } + } + networks[netName] = opts + } + + toReturn = append(toReturn, libpod.WithNetNS(portMappings, expose, postConfigureNetNS, "bridge", networks)) } if s.UseImageHosts { @@ -281,12 +307,6 @@ func namespaceOptions(ctx context.Context, s *specgen.SpecGenerator, rt *libpod. if len(s.DNSOptions) > 0 { toReturn = append(toReturn, libpod.WithDNSOption(s.DNSOptions)) } - if s.StaticIP != nil { - toReturn = append(toReturn, libpod.WithStaticIP(*s.StaticIP)) - } - if s.StaticMAC != nil { - toReturn = append(toReturn, libpod.WithStaticMAC(*s.StaticMAC)) - } if s.NetworkOptions != nil { toReturn = append(toReturn, libpod.WithNetworkOptions(s.NetworkOptions)) } From 4791595b5cb3b2350c66b0d1ba91a6a171652409 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 8 Dec 2021 15:31:49 +0100 Subject: [PATCH 04/10] network connect allow ip, ipv6 and mac address Network connect now supports setting a static ipv4, ipv6 and mac address for the container network. The options are added to the cli and api. Fixes #9883 Signed-off-by: Paul Holzinger --- cmd/podman/networks/connect.go | 33 +++++++++++- .../markdown/podman-network-connect.1.md | 16 +++++- libpod/networking_linux.go | 28 +++++----- pkg/api/handlers/compat/networks.go | 52 +++++++++++++++++-- pkg/api/handlers/libpod/networks.go | 2 +- pkg/bindings/network/network.go | 25 +++------ pkg/bindings/network/types.go | 13 ++--- pkg/bindings/network/types_connect_options.go | 33 ------------ pkg/domain/entities/network.go | 6 ++- pkg/domain/infra/abi/network.go | 2 +- pkg/domain/infra/tunnel/network.go | 3 +- test/e2e/network_connect_disconnect_test.go | 8 ++- 12 files changed, 133 insertions(+), 88 deletions(-) delete mode 100644 pkg/bindings/network/types_connect_options.go diff --git a/cmd/podman/networks/connect.go b/cmd/podman/networks/connect.go index 0d62a45df0..b0ffbfe6d4 100644 --- a/cmd/podman/networks/connect.go +++ b/cmd/podman/networks/connect.go @@ -1,9 +1,12 @@ package network import ( + "net" + "github.com/containers/common/pkg/completion" "github.com/containers/podman/v3/cmd/podman/common" "github.com/containers/podman/v3/cmd/podman/registry" + "github.com/containers/podman/v3/libpod/network/types" "github.com/containers/podman/v3/pkg/domain/entities" "github.com/spf13/cobra" ) @@ -23,13 +26,28 @@ var ( var ( networkConnectOptions entities.NetworkConnectOptions + ipv4 net.IP + ipv6 net.IP + macAddress string ) func networkConnectFlags(cmd *cobra.Command) { flags := cmd.Flags() aliasFlagName := "alias" - flags.StringSliceVar(&networkConnectOptions.Aliases, aliasFlagName, []string{}, "network scoped alias for container") + flags.StringSliceVar(&networkConnectOptions.Aliases, aliasFlagName, nil, "network scoped alias for container") _ = cmd.RegisterFlagCompletionFunc(aliasFlagName, completion.AutocompleteNone) + + ipAddressFlagName := "ip" + flags.IPVar(&ipv4, ipAddressFlagName, nil, "set a static ipv4 address for this container network") + _ = cmd.RegisterFlagCompletionFunc(ipAddressFlagName, completion.AutocompleteNone) + + ipv6AddressFlagName := "ip6" + flags.IPVar(&ipv6, ipv6AddressFlagName, nil, "set a static ipv6 address for this container network") + _ = cmd.RegisterFlagCompletionFunc(ipv6AddressFlagName, completion.AutocompleteNone) + + macAddressFlagName := "mac-address" + flags.StringVar(&macAddress, macAddressFlagName, "", "set a static mac address for this container network") + _ = cmd.RegisterFlagCompletionFunc(macAddressFlagName, completion.AutocompleteNone) } func init() { @@ -42,5 +60,18 @@ func init() { func networkConnect(cmd *cobra.Command, args []string) error { networkConnectOptions.Container = args[1] + if macAddress != "" { + mac, err := net.ParseMAC(macAddress) + if err != nil { + return err + } + networkConnectOptions.StaticMAC = types.HardwareAddr(mac) + } + for _, ip := range []net.IP{ipv4, ipv6} { + if ip != nil { + networkConnectOptions.StaticIPs = append(networkConnectOptions.StaticIPs, ip) + } + } + return registry.ContainerEngine().NetworkConnect(registry.Context(), args[0], networkConnectOptions) } diff --git a/docs/source/markdown/podman-network-connect.1.md b/docs/source/markdown/podman-network-connect.1.md index b998d4b7e1..c3eef4038e 100644 --- a/docs/source/markdown/podman-network-connect.1.md +++ b/docs/source/markdown/podman-network-connect.1.md @@ -11,12 +11,21 @@ Connects a container to a network. A container can be connected to a network by Once connected, the container can communicate with other containers in the same network. ## OPTIONS -#### **--alias** +#### **--alias**=*name* Add network-scoped alias for the container. If the network is using the `dnsname` CNI plugin, these aliases can be used for name resolution on the given network. Multiple *--alias* options may be specified as input. NOTE: A container will only have access to aliases on the first network that it joins. This is a limitation that will be removed in a later release. +#### **--ip**=*address* +Set a static ipv4 address for this container on this network. + +#### **--ip6**=*address* +Set a static ipv6 address for this container on this network. + +#### **--mac-address**=*address* +Set a static mac address for this container on this network. + ## EXAMPLE Connect a container named *web* to a network named *test* @@ -29,6 +38,11 @@ Connect a container name *web* to a network named *test* with two aliases: web1 podman network connect --alias web1 --alias web2 test web ``` +Connect a container name *web* to a network named *test* with a static ip. +``` +podman network connect --ip 10.89.1.13 test web +``` + ## SEE ALSO **[podman(1)](podman.1.md)**, **[podman-network(1)](podman-network.1.md)**, **[podman-network-disconnect(1)](podman-network-disconnect.1.md)** diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 3f56be855f..a931774f82 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -1170,7 +1170,7 @@ func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) erro } // ConnectNetwork connects a container to a given network -func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) error { +func (c *Container) NetworkConnect(nameOrID, netName string, netOpts types.PerNetworkOptions) error { // only the bridge mode supports cni networks if err := isBridgeNetMode(c.config.NetMode); err != nil { return err @@ -1202,22 +1202,20 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e if err != nil { return err } - if !network.DNSEnabled && len(aliases) > 0 { + if !network.DNSEnabled && len(netOpts.Aliases) > 0 { return errors.Wrapf(define.ErrInvalidArg, "cannot set network aliases for network %q because dns is disabled", netName) } + // always add the short id as alias for docker compat + netOpts.Aliases = append(netOpts.Aliases, c.config.ID[:12]) - eth := getFreeInterfaceName(networks) - if eth == "" { - return errors.New("could not find free network interface name") - } - - perNetOpt := types.PerNetworkOptions{ - // always add the short id as alias to match docker - Aliases: append(aliases, c.config.ID[:12]), - InterfaceName: eth, + if netOpts.InterfaceName == "" { + netOpts.InterfaceName = getFreeInterfaceName(networks) + if netOpts.InterfaceName == "" { + return errors.New("could not find free network interface name") + } } - if err := c.runtime.state.NetworkConnect(c, netName, perNetOpt); err != nil { + if err := c.runtime.state.NetworkConnect(c, netName, netOpts); err != nil { return err } c.newNetworkEvent(events.NetworkConnect, netName) @@ -1234,7 +1232,7 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e } opts.PortMappings = c.convertPortMappings() opts.Networks = map[string]types.PerNetworkOptions{ - netName: perNetOpt, + netName: netOpts, } results, err := c.runtime.setUpNetwork(c.state.NetNS.Path(), opts) @@ -1290,12 +1288,12 @@ func (r *Runtime) DisconnectContainerFromNetwork(nameOrID, netName string, force } // ConnectContainerToNetwork connects a container to a CNI network -func (r *Runtime) ConnectContainerToNetwork(nameOrID, netName string, aliases []string) error { +func (r *Runtime) ConnectContainerToNetwork(nameOrID, netName string, netOpts types.PerNetworkOptions) error { ctr, err := r.LookupContainer(nameOrID) if err != nil { return err } - return ctr.NetworkConnect(nameOrID, netName, aliases) + return ctr.NetworkConnect(nameOrID, netName, netOpts) } // normalizeNetworkName takes a network name, a partial or a full network ID and returns the network name. diff --git a/pkg/api/handlers/compat/networks.go b/pkg/api/handlers/compat/networks.go index 8aab29658b..db3af7d0b1 100644 --- a/pkg/api/handlers/compat/networks.go +++ b/pkg/api/handlers/compat/networks.go @@ -299,20 +299,66 @@ func Connect(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime) var ( - aliases []string netConnect types.NetworkConnect ) if err := json.NewDecoder(r.Body).Decode(&netConnect); err != nil { utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "Decode()")) return } + + netOpts := nettypes.PerNetworkOptions{} + name := utils.GetName(r) if netConnect.EndpointConfig != nil { if netConnect.EndpointConfig.Aliases != nil { - aliases = netConnect.EndpointConfig.Aliases + netOpts.Aliases = netConnect.EndpointConfig.Aliases + } + + // if IP address is provided + if len(netConnect.EndpointConfig.IPAddress) > 0 { + staticIP := net.ParseIP(netConnect.EndpointConfig.IPAddress) + if staticIP == nil { + utils.Error(w, "failed to parse the ip address", http.StatusInternalServerError, + errors.Errorf("failed to parse the ip address %q", netConnect.EndpointConfig.IPAddress)) + return + } + netOpts.StaticIPs = append(netOpts.StaticIPs, staticIP) + } + + if netConnect.EndpointConfig.IPAMConfig != nil { + // if IPAMConfig.IPv4Address is provided + if len(netConnect.EndpointConfig.IPAMConfig.IPv4Address) > 0 { + staticIP := net.ParseIP(netConnect.EndpointConfig.IPAMConfig.IPv4Address) + if staticIP == nil { + utils.Error(w, "failed to parse the ipv4 address", http.StatusInternalServerError, + errors.Errorf("failed to parse the ipv4 address %q", netConnect.EndpointConfig.IPAMConfig.IPv4Address)) + return + } + netOpts.StaticIPs = append(netOpts.StaticIPs, staticIP) + } + // if IPAMConfig.IPv6Address is provided + if len(netConnect.EndpointConfig.IPAMConfig.IPv6Address) > 0 { + staticIP := net.ParseIP(netConnect.EndpointConfig.IPAMConfig.IPv6Address) + if staticIP == nil { + utils.Error(w, "failed to parse the ipv6 address", http.StatusInternalServerError, + errors.Errorf("failed to parse the ipv6 address %q", netConnect.EndpointConfig.IPAMConfig.IPv6Address)) + return + } + netOpts.StaticIPs = append(netOpts.StaticIPs, staticIP) + } + } + // If MAC address is provided + if len(netConnect.EndpointConfig.MacAddress) > 0 { + staticMac, err := net.ParseMAC(netConnect.EndpointConfig.MacAddress) + if err != nil { + utils.Error(w, "failed to parse the mac address", http.StatusInternalServerError, + errors.Errorf("failed to parse the mac address %q", netConnect.EndpointConfig.IPAMConfig.IPv6Address)) + return + } + netOpts.StaticMAC = nettypes.HardwareAddr(staticMac) } } - err := runtime.ConnectContainerToNetwork(netConnect.Container, name, aliases) + err := runtime.ConnectContainerToNetwork(netConnect.Container, name, netOpts) if err != nil { if errors.Cause(err) == define.ErrNoSuchCtr { utils.ContainerNotFound(w, netConnect.Container, err) diff --git a/pkg/api/handlers/libpod/networks.go b/pkg/api/handlers/libpod/networks.go index 1f7f2e26c9..a28c3c57cd 100644 --- a/pkg/api/handlers/libpod/networks.go +++ b/pkg/api/handlers/libpod/networks.go @@ -125,7 +125,7 @@ func Connect(w http.ResponseWriter, r *http.Request) { return } name := utils.GetName(r) - err := runtime.ConnectContainerToNetwork(netConnect.Container, name, netConnect.Aliases) + err := runtime.ConnectContainerToNetwork(netConnect.Container, name, netConnect.PerNetworkOptions) if err != nil { if errors.Cause(err) == define.ErrNoSuchCtr { utils.ContainerNotFound(w, netConnect.Container, err) diff --git a/pkg/bindings/network/network.go b/pkg/bindings/network/network.go index 172598be1c..66e01a0168 100644 --- a/pkg/bindings/network/network.go +++ b/pkg/bindings/network/network.go @@ -3,7 +3,6 @@ package network import ( "context" "net/http" - "net/url" "strings" "github.com/containers/podman/v3/libpod/network/types" @@ -110,8 +109,6 @@ func Disconnect(ctx context.Context, networkName string, ContainerNameOrID strin if err != nil { return err } - // No params are used for disconnect - params := url.Values{} // Disconnect sends everything in body disconnect := struct { Container string @@ -128,7 +125,7 @@ func Disconnect(ctx context.Context, networkName string, ContainerNameOrID strin return err } stringReader := strings.NewReader(body) - response, err := conn.DoRequest(ctx, stringReader, http.MethodPost, "/networks/%s/disconnect", params, nil, networkName) + response, err := conn.DoRequest(ctx, stringReader, http.MethodPost, "/networks/%s/disconnect", nil, nil, networkName) if err != nil { return err } @@ -138,32 +135,26 @@ func Disconnect(ctx context.Context, networkName string, ContainerNameOrID strin } // Connect adds a container to a network -func Connect(ctx context.Context, networkName string, ContainerNameOrID string, options *ConnectOptions) error { +func Connect(ctx context.Context, networkName string, containerNameOrID string, options *types.PerNetworkOptions) error { if options == nil { - options = new(ConnectOptions) + options = new(types.PerNetworkOptions) } conn, err := bindings.GetClient(ctx) if err != nil { return err } - // No params are used in connect - params := url.Values{} // Connect sends everything in body - connect := struct { - Container string - Aliases []string - }{ - Container: ContainerNameOrID, - } - if aliases := options.GetAliases(); options.Changed("Aliases") { - connect.Aliases = aliases + connect := entities.NetworkConnectOptions{ + Container: containerNameOrID, + PerNetworkOptions: *options, } + body, err := jsoniter.MarshalToString(connect) if err != nil { return err } stringReader := strings.NewReader(body) - response, err := conn.DoRequest(ctx, stringReader, http.MethodPost, "/networks/%s/connect", params, nil, networkName) + response, err := conn.DoRequest(ctx, stringReader, http.MethodPost, "/networks/%s/connect", nil, nil, networkName) if err != nil { return err } diff --git a/pkg/bindings/network/types.go b/pkg/bindings/network/types.go index 8088de0610..b82c0e4385 100644 --- a/pkg/bindings/network/types.go +++ b/pkg/bindings/network/types.go @@ -1,6 +1,8 @@ package network -import "net" +import ( + "net" +) //go:generate go run ../generator/generator.go CreateOptions // CreateOptions are optional options for creating networks @@ -61,15 +63,6 @@ type DisconnectOptions struct { Force *bool } -//go:generate go run ../generator/generator.go ConnectOptions -// ConnectOptions are optional options for connecting -// containers from a network -type ConnectOptions struct { - // Aliases are names the container will be known as - // when using the dns plugin - Aliases *[]string -} - //go:generate go run ../generator/generator.go ExistsOptions // ExistsOptions are optional options for checking // if a network exists diff --git a/pkg/bindings/network/types_connect_options.go b/pkg/bindings/network/types_connect_options.go deleted file mode 100644 index b7a4659995..0000000000 --- a/pkg/bindings/network/types_connect_options.go +++ /dev/null @@ -1,33 +0,0 @@ -// Code generated by go generate; DO NOT EDIT. -package network - -import ( - "net/url" - - "github.com/containers/podman/v3/pkg/bindings/internal/util" -) - -// Changed returns true if named field has been set -func (o *ConnectOptions) Changed(fieldName string) bool { - return util.Changed(o, fieldName) -} - -// ToParams formats struct fields to be passed to API service -func (o *ConnectOptions) ToParams() (url.Values, error) { - return util.ToParams(o) -} - -// WithAliases set field Aliases to given value -func (o *ConnectOptions) WithAliases(value []string) *ConnectOptions { - o.Aliases = &value - return o -} - -// GetAliases returns value of field Aliases -func (o *ConnectOptions) GetAliases() []string { - if o.Aliases == nil { - var z []string - return z - } - return *o.Aliases -} diff --git a/pkg/domain/entities/network.go b/pkg/domain/entities/network.go index d7389a699f..34b89ae7dd 100644 --- a/pkg/domain/entities/network.go +++ b/pkg/domain/entities/network.go @@ -2,6 +2,8 @@ package entities import ( "net" + + "github.com/containers/podman/v3/libpod/network/types" ) // NetworkListOptions describes options for listing networks in cli @@ -67,8 +69,8 @@ type NetworkDisconnectOptions struct { // NetworkConnectOptions describes options for connecting // a container to a network type NetworkConnectOptions struct { - Aliases []string - Container string + Container string `json:"container"` + types.PerNetworkOptions } // NetworkPruneReport containers the name of network and an error diff --git a/pkg/domain/infra/abi/network.go b/pkg/domain/infra/abi/network.go index 49c7dc60d5..c7b12663c4 100644 --- a/pkg/domain/infra/abi/network.go +++ b/pkg/domain/infra/abi/network.go @@ -124,7 +124,7 @@ func (ic *ContainerEngine) NetworkDisconnect(ctx context.Context, networkname st } func (ic *ContainerEngine) NetworkConnect(ctx context.Context, networkname string, options entities.NetworkConnectOptions) error { - return ic.Libpod.ConnectContainerToNetwork(options.Container, networkname, options.Aliases) + return ic.Libpod.ConnectContainerToNetwork(options.Container, networkname, options.PerNetworkOptions) } // NetworkExists checks if the given network exists diff --git a/pkg/domain/infra/tunnel/network.go b/pkg/domain/infra/tunnel/network.go index 069982d307..b5050345ab 100644 --- a/pkg/domain/infra/tunnel/network.go +++ b/pkg/domain/infra/tunnel/network.go @@ -81,8 +81,7 @@ func (ic *ContainerEngine) NetworkDisconnect(ctx context.Context, networkname st // NetworkConnect removes a container from a given network func (ic *ContainerEngine) NetworkConnect(ctx context.Context, networkname string, opts entities.NetworkConnectOptions) error { - options := new(network.ConnectOptions).WithAliases(opts.Aliases) - return network.Connect(ic.ClientCtx, networkname, opts.Container, options) + return network.Connect(ic.ClientCtx, networkname, opts.Container, &opts.PerNetworkOptions) } // NetworkExists checks if the given network exists diff --git a/test/e2e/network_connect_disconnect_test.go b/test/e2e/network_connect_disconnect_test.go index 2205a12630..be94ecb2d3 100644 --- a/test/e2e/network_connect_disconnect_test.go +++ b/test/e2e/network_connect_disconnect_test.go @@ -176,12 +176,14 @@ var _ = Describe("Podman network connect and disconnect", func() { // Create a second network newNetName := "aliasTest" + stringid.GenerateNonCryptoID() - session = podmanTest.Podman([]string{"network", "create", newNetName}) + session = podmanTest.Podman([]string{"network", "create", newNetName, "--subnet", "10.11.100.0/24"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) defer podmanTest.removeCNINetwork(newNetName) - connect := podmanTest.Podman([]string{"network", "connect", newNetName, "test"}) + ip := "10.11.100.99" + mac := "44:11:44:11:44:11" + connect := podmanTest.Podman([]string{"network", "connect", "--ip", ip, "--mac-address", mac, newNetName, "test"}) connect.WaitWithDefaultTimeout() Expect(connect).Should(Exit(0)) Expect(connect.ErrorToString()).Should(Equal("")) @@ -200,6 +202,8 @@ var _ = Describe("Podman network connect and disconnect", func() { exec = podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth1"}) exec.WaitWithDefaultTimeout() Expect(exec).Should(Exit(0)) + Expect(exec.OutputToString()).Should(ContainSubstring(ip)) + Expect(exec.OutputToString()).Should(ContainSubstring(mac)) // make sure no logrus errors are shown https://github.com/containers/podman/issues/9602 rm := podmanTest.Podman([]string{"rm", "--time=0", "-f", "test"}) From 46938bbf889de590b00c9be8ea5b4fb86f363519 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 8 Dec 2021 18:19:43 +0100 Subject: [PATCH 05/10] fix incorrect swagger doc for network dis/connect The swagger api docs used the extra Body struct as part of the request which is wrong. We just want the plain type. Signed-off-by: Paul Holzinger --- pkg/api/handlers/compat/swagger.go | 10 ++++------ pkg/api/handlers/libpod/swagger.go | 6 ++++++ pkg/api/server/register_networks.go | 6 +++--- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/api/handlers/compat/swagger.go b/pkg/api/handlers/compat/swagger.go index cfbdd11547..32167d6b54 100644 --- a/pkg/api/handlers/compat/swagger.go +++ b/pkg/api/handlers/compat/swagger.go @@ -55,15 +55,13 @@ type swagCompatNetworkCreateResponse struct { } // Network disconnect -// swagger:model NetworkConnectRequest +// swagger:model NetworkCompatConnectRequest type swagCompatNetworkConnectRequest struct { - // in:body - Body struct{ types.NetworkConnect } + types.NetworkConnect } // Network disconnect -// swagger:model NetworkDisconnectRequest +// swagger:model NetworkCompatDisconnectRequest type swagCompatNetworkDisconnectRequest struct { - // in:body - Body struct{ types.NetworkDisconnect } + types.NetworkDisconnect } diff --git a/pkg/api/handlers/libpod/swagger.go b/pkg/api/handlers/libpod/swagger.go index 7ccfdd0f32..8d7058b1e6 100644 --- a/pkg/api/handlers/libpod/swagger.go +++ b/pkg/api/handlers/libpod/swagger.go @@ -133,6 +133,12 @@ type swagNetworkPruneResponse struct { Body []entities.NetworkPruneReport } +// Network connect +// swagger:model NetworkConnectRequest +type swagNetworkConnectRequest struct { + entities.NetworkConnectOptions +} + func ServeSwagger(w http.ResponseWriter, r *http.Request) { path := DefaultPodmanSwaggerSpec if p, found := os.LookupEnv("PODMAN_SWAGGER_SPEC"); found { diff --git a/pkg/api/server/register_networks.go b/pkg/api/server/register_networks.go index 641bce3337..344486299d 100644 --- a/pkg/api/server/register_networks.go +++ b/pkg/api/server/register_networks.go @@ -131,7 +131,7 @@ func (s *APIServer) registerNetworkHandlers(r *mux.Router) error { // name: create // description: attributes for connecting a container to a network // schema: - // $ref: "#/definitions/NetworkConnectRequest" + // $ref: "#/definitions/NetworkCompatConnectRequest" // responses: // 200: // description: OK @@ -159,7 +159,7 @@ func (s *APIServer) registerNetworkHandlers(r *mux.Router) error { // name: create // description: attributes for disconnecting a container from a network // schema: - // $ref: "#/definitions/NetworkDisconnectRequest" + // $ref: "#/definitions/NetworkCompatDisconnectRequest" // responses: // 200: // description: OK @@ -368,7 +368,7 @@ func (s *APIServer) registerNetworkHandlers(r *mux.Router) error { // name: create // description: attributes for disconnecting a container from a network // schema: - // $ref: "#/definitions/NetworkDisconnectRequest" + // $ref: "#/definitions/NetworkCompatDisconnectRequest" // responses: // 200: // description: OK From d072167fe2f75db9648bf1be4181b42e9b7db9a4 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 9 Dec 2021 15:59:54 +0100 Subject: [PATCH 06/10] Add new networks format to spegecen Add the new networks format to specgen. For api users cni_networks is still supported to make migration easier however the static ip and mac fields are removed. Signed-off-by: Paul Holzinger --- cmd/podman/common/create_opts.go | 70 +++++++++++++++------------- cmd/podman/common/create_test.go | 2 +- cmd/podman/common/netflags.go | 74 +++++++++++++++++++----------- libpod/options.go | 3 ++ pkg/domain/entities/pods.go | 6 +-- pkg/domain/entities/types.go | 22 ++++----- pkg/domain/infra/abi/play.go | 38 +++++++-------- pkg/specgen/container_validate.go | 17 +------ pkg/specgen/generate/namespaces.go | 38 +++++++-------- pkg/specgen/generate/pod_create.go | 18 +++----- pkg/specgen/namespaces.go | 21 +++++---- pkg/specgen/pod_validate.go | 20 +------- pkg/specgen/podspecgen.go | 35 ++++++-------- pkg/specgen/specgen.go | 24 ++++------ pkg/specgenutil/specgen.go | 18 +------- 15 files changed, 181 insertions(+), 225 deletions(-) diff --git a/cmd/podman/common/create_opts.go b/cmd/podman/common/create_opts.go index 7d6471fd4d..8f0cd1be6d 100644 --- a/cmd/podman/common/create_opts.go +++ b/cmd/podman/common/create_opts.go @@ -184,52 +184,56 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, rtc *c // network names switch { case len(cc.NetworkingConfig.EndpointsConfig) > 0: - var aliases []string - endpointsConfig := cc.NetworkingConfig.EndpointsConfig - cniNetworks := make([]string, 0, len(endpointsConfig)) + networks := make(map[string]types.PerNetworkOptions, len(endpointsConfig)) for netName, endpoint := range endpointsConfig { - cniNetworks = append(cniNetworks, netName) - - if endpoint == nil { - continue - } - if len(endpoint.Aliases) > 0 { - aliases = append(aliases, endpoint.Aliases...) - } - } + netOpts := types.PerNetworkOptions{} + if endpoint != nil { + netOpts.Aliases = endpoint.Aliases - // static IP and MAC - if len(endpointsConfig) == 1 { - for _, ep := range endpointsConfig { - if ep == nil { - continue - } // if IP address is provided - if len(ep.IPAddress) > 0 { - staticIP := net.ParseIP(ep.IPAddress) - netInfo.StaticIP = &staticIP + if len(endpoint.IPAddress) > 0 { + staticIP := net.ParseIP(endpoint.IPAddress) + if staticIP == nil { + return nil, nil, errors.Errorf("failed to parse the ip address %q", endpoint.IPAddress) + } + netOpts.StaticIPs = append(netOpts.StaticIPs, staticIP) } - // if IPAMConfig.IPv4Address is provided - if ep.IPAMConfig != nil && ep.IPAMConfig.IPv4Address != "" { - staticIP := net.ParseIP(ep.IPAMConfig.IPv4Address) - netInfo.StaticIP = &staticIP + + if endpoint.IPAMConfig != nil { + // if IPAMConfig.IPv4Address is provided + if len(endpoint.IPAMConfig.IPv4Address) > 0 { + staticIP := net.ParseIP(endpoint.IPAMConfig.IPv4Address) + if staticIP == nil { + return nil, nil, errors.Errorf("failed to parse the ipv4 address %q", endpoint.IPAMConfig.IPv4Address) + } + netOpts.StaticIPs = append(netOpts.StaticIPs, staticIP) + } + // if IPAMConfig.IPv6Address is provided + if len(endpoint.IPAMConfig.IPv6Address) > 0 { + staticIP := net.ParseIP(endpoint.IPAMConfig.IPv6Address) + if staticIP == nil { + return nil, nil, errors.Errorf("failed to parse the ipv6 address %q", endpoint.IPAMConfig.IPv6Address) + } + netOpts.StaticIPs = append(netOpts.StaticIPs, staticIP) + } } // If MAC address is provided - if len(ep.MacAddress) > 0 { - staticMac, err := net.ParseMAC(ep.MacAddress) + if len(endpoint.MacAddress) > 0 { + staticMac, err := net.ParseMAC(endpoint.MacAddress) if err != nil { - return nil, nil, err + return nil, nil, errors.Errorf("failed to parse the mac address %q", endpoint.MacAddress) } - netInfo.StaticMAC = &staticMac + netOpts.StaticMAC = types.HardwareAddr(staticMac) } - break } + + networks[netName] = netOpts } - netInfo.Aliases = aliases - netInfo.CNINetworks = cniNetworks + + netInfo.Networks = networks case len(cc.HostConfig.NetworkMode) > 0: - netInfo.CNINetworks = networks + netInfo.Networks = networks } parsedTmp := make([]string, 0, len(cc.HostConfig.Tmpfs)) diff --git a/cmd/podman/common/create_test.go b/cmd/podman/common/create_test.go index 17b47dd163..601078b610 100644 --- a/cmd/podman/common/create_test.go +++ b/cmd/podman/common/create_test.go @@ -12,7 +12,7 @@ import ( func TestPodOptions(t *testing.T) { entry := "/test1" - exampleOptions := entities.ContainerCreateOptions{CPUS: 5.5, CPUSetCPUs: "0-4", Entrypoint: &entry, Hostname: "foo", Name: "testing123", Volume: []string{"/fakeVol1", "/fakeVol2"}, Net: &entities.NetOptions{CNINetworks: []string{"FakeNetwork"}}, PID: "ns:/proc/self/ns"} + exampleOptions := entities.ContainerCreateOptions{CPUS: 5.5, CPUSetCPUs: "0-4", Entrypoint: &entry, Hostname: "foo", Name: "testing123", Volume: []string{"/fakeVol1", "/fakeVol2"}, Net: &entities.NetOptions{DNSSearch: []string{"search"}}, PID: "ns:/proc/self/ns"} podOptions := entities.PodCreateOptions{} err := common.ContainerToPodOptions(&exampleOptions, &podOptions) diff --git a/cmd/podman/common/netflags.go b/cmd/podman/common/netflags.go index d11f3c9d29..f79a9fd88a 100644 --- a/cmd/podman/common/netflags.go +++ b/cmd/podman/common/netflags.go @@ -6,6 +6,7 @@ import ( "github.com/containers/common/pkg/completion" "github.com/containers/podman/v3/cmd/podman/parse" "github.com/containers/podman/v3/libpod/define" + "github.com/containers/podman/v3/libpod/network/types" "github.com/containers/podman/v3/pkg/domain/entities" "github.com/containers/podman/v3/pkg/specgen" "github.com/containers/podman/v3/pkg/specgenutil" @@ -151,27 +152,40 @@ func NetFlagsToNetOptions(opts *entities.NetOptions, flags pflag.FlagSet, netnsF } opts.DNSSearch = dnsSearches - m, err := flags.GetString("mac-address") + inputPorts, err := flags.GetStringSlice("publish") if err != nil { return nil, err } - if len(m) > 0 { - mac, err := net.ParseMAC(m) + if len(inputPorts) > 0 { + opts.PublishPorts, err = specgenutil.CreatePortBindings(inputPorts) if err != nil { return nil, err } - opts.StaticMAC = &mac } - inputPorts, err := flags.GetStringSlice("publish") + opts.NoHosts, err = flags.GetBool("no-hosts") if err != nil { return nil, err } - if len(inputPorts) > 0 { - opts.PublishPorts, err = specgenutil.CreatePortBindings(inputPorts) + + // parse the --network value only when the flag is set or we need to use + // the netns config value, e.g. when --pod is not used + if netnsFromConfig || flags.Changed("network") { + network, err := flags.GetString("network") if err != nil { return nil, err } + + ns, networks, options, err := specgen.ParseNetworkString(network) + if err != nil { + return nil, err + } + + if len(options) > 0 { + opts.NetworkOptions = options + } + opts.Network = ns + opts.Networks = networks } ip, err := flags.GetString("ip") @@ -183,35 +197,37 @@ func NetFlagsToNetOptions(opts *entities.NetOptions, flags pflag.FlagSet, netnsF if staticIP == nil { return nil, errors.Errorf("%s is not an ip address", ip) } - if staticIP.To4() == nil { - return nil, errors.Wrapf(define.ErrInvalidArg, "%s is not an IPv4 address", ip) + if !opts.Network.IsBridge() { + return nil, errors.Wrap(define.ErrInvalidArg, "--ip can only be set when the network mode is bridge") + } + if len(opts.Networks) != 1 { + return nil, errors.Wrap(define.ErrInvalidArg, "--ip can only be set for a single network") + } + for name, netOpts := range opts.Networks { + netOpts.StaticIPs = append(netOpts.StaticIPs, staticIP) + opts.Networks[name] = netOpts } - opts.StaticIP = &staticIP } - opts.NoHosts, err = flags.GetBool("no-hosts") + m, err := flags.GetString("mac-address") if err != nil { return nil, err } - - // parse the --network value only when the flag is set or we need to use - // the netns config value, e.g. when --pod is not used - if netnsFromConfig || flags.Changed("network") { - network, err := flags.GetString("network") + if len(m) > 0 { + mac, err := net.ParseMAC(m) if err != nil { return nil, err } - - ns, cniNets, options, err := specgen.ParseNetworkString(network) - if err != nil { - return nil, err + if !opts.Network.IsBridge() { + return nil, errors.Wrap(define.ErrInvalidArg, "--mac-address can only be set when the network mode is bridge") } - - if len(options) > 0 { - opts.NetworkOptions = options + if len(opts.Networks) != 1 { + return nil, errors.Wrap(define.ErrInvalidArg, "--mac-address can only be set for a single network") + } + for name, netOpts := range opts.Networks { + netOpts.StaticMAC = types.HardwareAddr(mac) + opts.Networks[name] = netOpts } - opts.Network = ns - opts.CNINetworks = cniNets } aliases, err := flags.GetStringSlice("network-alias") @@ -219,7 +235,13 @@ func NetFlagsToNetOptions(opts *entities.NetOptions, flags pflag.FlagSet, netnsF return nil, err } if len(aliases) > 0 { - opts.Aliases = aliases + if !opts.Network.IsBridge() { + return nil, errors.Wrap(define.ErrInvalidArg, "--network-alias can only be set when the network mode is bridge") + } + for name, netOpts := range opts.Networks { + netOpts.Aliases = aliases + opts.Networks[name] = netOpts + } } return opts, err diff --git a/libpod/options.go b/libpod/options.go index dbcc507418..e6fa987a82 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1070,6 +1070,9 @@ func WithNetNS(portMappings []nettypes.PortMapping, exposedPorts map[uint16][]st ctr.config.PortMappings = portMappings ctr.config.ExposedPorts = exposedPorts + if !ctr.config.NetMode.IsBridge() && len(networks) > 0 { + return errors.New("cannot use networks when network mode is not bridge") + } ctr.config.Networks = networks return nil diff --git a/pkg/domain/entities/pods.go b/pkg/domain/entities/pods.go index b255785c25..14127e4688 100644 --- a/pkg/domain/entities/pods.go +++ b/pkg/domain/entities/pods.go @@ -7,7 +7,6 @@ import ( commonFlag "github.com/containers/common/pkg/flag" "github.com/containers/podman/v3/libpod/define" - "github.com/containers/podman/v3/libpod/network/types" "github.com/containers/podman/v3/pkg/specgen" "github.com/containers/podman/v3/pkg/util" "github.com/opencontainers/runtime-spec/specs-go" @@ -329,11 +328,8 @@ func ToPodSpecGen(s specgen.PodSpecGenerator, p *PodCreateOptions) (*specgen.Pod if p.Net != nil { s.NetNS = p.Net.Network - s.StaticIP = p.Net.StaticIP - // type cast to types.HardwareAddr - s.StaticMAC = (*types.HardwareAddr)(p.Net.StaticMAC) s.PortMappings = p.Net.PublishPorts - s.CNINetworks = p.Net.CNINetworks + s.Networks = p.Net.Networks s.NetworkOptions = p.Net.NetworkOptions if p.Net.UseImageResolvConf { s.NoManageResolvConf = true diff --git a/pkg/domain/entities/types.go b/pkg/domain/entities/types.go index e062b94428..0348c0af56 100644 --- a/pkg/domain/entities/types.go +++ b/pkg/domain/entities/types.go @@ -45,18 +45,16 @@ type NetFlags struct { // NetOptions reflect the shared network options between // pods and containers type NetOptions struct { - AddHosts []string `json:"hostadd,omitempty"` - Aliases []string `json:"network_alias,omitempty"` - CNINetworks []string `json:"cni_networks,omitempty"` - UseImageResolvConf bool `json:"no_manage_resolv_conf,omitempty"` - DNSOptions []string `json:"dns_option,omitempty"` - DNSSearch []string `json:"dns_search,omitempty"` - DNSServers []net.IP `json:"dns_server,omitempty"` - Network specgen.Namespace `json:"netns,omitempty"` - NoHosts bool `json:"no_manage_hosts,omitempty"` - PublishPorts []types.PortMapping `json:"portmappings,omitempty"` - StaticIP *net.IP `json:"static_ip,omitempty"` - StaticMAC *net.HardwareAddr `json:"static_mac,omitempty"` + AddHosts []string `json:"hostadd,omitempty"` + Aliases []string `json:"network_alias,omitempty"` + Networks map[string]types.PerNetworkOptions `json:"networks,omitempty"` + UseImageResolvConf bool `json:"no_manage_resolv_conf,omitempty"` + DNSOptions []string `json:"dns_option,omitempty"` + DNSSearch []string `json:"dns_search,omitempty"` + DNSServers []net.IP `json:"dns_server,omitempty"` + Network specgen.Namespace `json:"netns,omitempty"` + NoHosts bool `json:"no_manage_hosts,omitempty"` + PublishPorts []types.PortMapping `json:"portmappings,omitempty"` // NetworkOptions are additional options for each network NetworkOptions map[string][]string `json:"network_options,omitempty"` } diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index 4c024a3d81..b0b68567a3 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -6,7 +6,6 @@ import ( "fmt" "io" "io/ioutil" - "net" "os" "path/filepath" "strconv" @@ -190,44 +189,45 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY } } - podOpt := entities.PodCreateOptions{Infra: true, Net: &entities.NetOptions{StaticIP: &net.IP{}, StaticMAC: &net.HardwareAddr{}, NoHosts: options.NoHosts}} + podOpt := entities.PodCreateOptions{Infra: true, Net: &entities.NetOptions{NoHosts: options.NoHosts}} podOpt, err = kube.ToPodOpt(ctx, podName, podOpt, podYAML) if err != nil { return nil, err } if options.Network != "" { - ns, cniNets, netOpts, err := specgen.ParseNetworkString(options.Network) + ns, networks, netOpts, err := specgen.ParseNetworkString(options.Network) if err != nil { return nil, err } - if (ns.IsBridge() && len(cniNets) == 0) || ns.IsHost() { + if (ns.IsBridge() && len(networks) == 0) || ns.IsHost() { return nil, errors.Errorf("invalid value passed to --network: bridge or host networking must be configured in YAML") } podOpt.Net.Network = ns - if len(cniNets) > 0 { - podOpt.Net.CNINetworks = append(podOpt.Net.CNINetworks, cniNets...) + if len(networks) > 0 { + podOpt.Net.Networks = networks } if len(netOpts) > 0 { podOpt.Net.NetworkOptions = netOpts } } - if len(options.StaticIPs) > *ipIndex { - podOpt.Net.StaticIP = &options.StaticIPs[*ipIndex] - } else if len(options.StaticIPs) > 0 { - // only warn if the user has set at least one ip - logrus.Warn("No more static ips left using a random one") - } - if len(options.StaticMACs) > *ipIndex { - podOpt.Net.StaticMAC = &options.StaticMACs[*ipIndex] - } else if len(options.StaticIPs) > 0 { - // only warn if the user has set at least one mac - logrus.Warn("No more static macs left using a random one") - } - *ipIndex++ + // FIXME This is very hard to support properly + // if len(options.StaticIPs) > *ipIndex { + // podOpt.Net.StaticIP = &options.StaticIPs[*ipIndex] + // } else if len(options.StaticIPs) > 0 { + // // only warn if the user has set at least one ip + // logrus.Warn("No more static ips left using a random one") + // } + // if len(options.StaticMACs) > *ipIndex { + // podOpt.Net.StaticMAC = &options.StaticMACs[*ipIndex] + // } else if len(options.StaticIPs) > 0 { + // // only warn if the user has set at least one mac + // logrus.Warn("No more static macs left using a random one") + // } + // *ipIndex++ p := specgen.NewPodSpecGenerator() if err != nil { diff --git a/pkg/specgen/container_validate.go b/pkg/specgen/container_validate.go index caea51ea84..cae231f0e4 100644 --- a/pkg/specgen/container_validate.go +++ b/pkg/specgen/container_validate.go @@ -29,25 +29,10 @@ func exclusiveOptions(opt1, opt2 string) error { // Validate verifies that the given SpecGenerator is valid and satisfies required // input for creating a container. func (s *SpecGenerator) Validate() error { - if rootless.IsRootless() && len(s.CNINetworks) == 0 { - if s.StaticIP != nil || s.StaticIPv6 != nil { - return ErrNoStaticIPRootless - } - if s.StaticMAC != nil { - return ErrNoStaticMACRootless - } - } - // Containers being added to a pod cannot have certain network attributes // associated with them because those should be on the infra container. if len(s.Pod) > 0 && s.NetNS.NSMode == FromPod { - if s.StaticIP != nil || s.StaticIPv6 != nil { - return errors.Wrap(define.ErrNetworkOnPodContainer, "static ip addresses must be defined when the pod is created") - } - if s.StaticMAC != nil { - return errors.Wrap(define.ErrNetworkOnPodContainer, "MAC addresses must be defined when the pod is created") - } - if len(s.CNINetworks) > 0 { + if len(s.Networks) > 0 { return errors.Wrap(define.ErrNetworkOnPodContainer, "networks must be defined when the pod is created") } if len(s.PortMappings) > 0 || s.PublishExposedPorts { diff --git a/pkg/specgen/generate/namespaces.go b/pkg/specgen/generate/namespaces.go index ebdd2abd03..7821566632 100644 --- a/pkg/specgen/generate/namespaces.go +++ b/pkg/specgen/generate/namespaces.go @@ -259,32 +259,28 @@ func namespaceOptions(ctx context.Context, s *specgen.SpecGenerator, rt *libpod. if err != nil { return nil, err } - if len(s.CNINetworks) == 0 { - rtConfig, err := rt.GetConfigNoCopy() - if err != nil { - return nil, err - } - s.CNINetworks = append(s.CNINetworks, rtConfig.Network.DefaultNetwork) - } - networks := make(map[string]types.PerNetworkOptions, len(s.CNINetworks)) - for i, netName := range s.CNINetworks { - opts := types.PerNetworkOptions{} - opts.Aliases = s.Aliases[netName] - if i == 0 { - if s.StaticIP != nil { - opts.StaticIPs = append(opts.StaticIPs, *s.StaticIP) + // if no network was specified use add the default + if len(s.Networks) == 0 { + // backwards config still allow the old cni networks list and convert to new format + if len(s.CNINetworks) > 0 { + logrus.Warn(`specgen "cni_networks" option is deprecated use the "networks" map instead`) + networks := make(map[string]types.PerNetworkOptions, len(s.CNINetworks)) + for _, net := range s.CNINetworks { + networks[net] = types.PerNetworkOptions{} } - if s.StaticIPv6 != nil { - opts.StaticIPs = append(opts.StaticIPs, *s.StaticIPv6) + s.Networks = networks + } else { + // no networks given but bridge is set so use default network + rtConfig, err := rt.GetConfigNoCopy() + if err != nil { + return nil, err } - if s.StaticMAC != nil { - opts.StaticMAC = *s.StaticMAC + s.Networks = map[string]types.PerNetworkOptions{ + rtConfig.Network.DefaultNetwork: {}, } } - networks[netName] = opts } - - toReturn = append(toReturn, libpod.WithNetNS(portMappings, expose, postConfigureNetNS, "bridge", networks)) + toReturn = append(toReturn, libpod.WithNetNS(portMappings, expose, postConfigureNetNS, "bridge", s.Networks)) } if s.UseImageHosts { diff --git a/pkg/specgen/generate/pod_create.go b/pkg/specgen/generate/pod_create.go index 72dd249e76..0a797c5712 100644 --- a/pkg/specgen/generate/pod_create.go +++ b/pkg/specgen/generate/pod_create.go @@ -218,9 +218,7 @@ func MapSpec(p *specgen.PodSpecGenerator) (*specgen.SpecGenerator, error) { case specgen.Host: logrus.Debugf("Pod will use host networking") if len(p.InfraContainerSpec.PortMappings) > 0 || - p.InfraContainerSpec.StaticIP != nil || - p.InfraContainerSpec.StaticMAC != nil || - len(p.InfraContainerSpec.CNINetworks) > 0 || + len(p.InfraContainerSpec.Networks) > 0 || p.InfraContainerSpec.NetNS.NSMode == specgen.NoNetwork { return nil, errors.Wrapf(define.ErrInvalidArg, "cannot set host network if network-related configuration is specified") } @@ -234,9 +232,7 @@ func MapSpec(p *specgen.PodSpecGenerator) (*specgen.SpecGenerator, error) { case specgen.NoNetwork: logrus.Debugf("Pod will not use networking") if len(p.InfraContainerSpec.PortMappings) > 0 || - p.InfraContainerSpec.StaticIP != nil || - p.InfraContainerSpec.StaticMAC != nil || - len(p.InfraContainerSpec.CNINetworks) > 0 || + len(p.InfraContainerSpec.Networks) > 0 || p.InfraContainerSpec.NetNS.NSMode == "host" { return nil, errors.Wrapf(define.ErrInvalidArg, "cannot disable pod network if network-related configuration is specified") } @@ -264,15 +260,13 @@ func MapSpec(p *specgen.PodSpecGenerator) (*specgen.SpecGenerator, error) { if len(p.DNSSearch) > 0 { p.InfraContainerSpec.DNSSearch = p.DNSSearch } - if p.StaticIP != nil { - p.InfraContainerSpec.StaticIP = p.StaticIP - } - if p.StaticMAC != nil { - p.InfraContainerSpec.StaticMAC = p.StaticMAC - } if p.NoManageResolvConf { p.InfraContainerSpec.UseImageResolvConf = true } + if len(p.Networks) > 0 { + p.InfraContainerSpec.Networks = p.Networks + } + // deprecated cni networks for api users if len(p.CNINetworks) > 0 { p.InfraContainerSpec.CNINetworks = p.CNINetworks } diff --git a/pkg/specgen/namespaces.go b/pkg/specgen/namespaces.go index bb5385ef1d..121e1ecf7d 100644 --- a/pkg/specgen/namespaces.go +++ b/pkg/specgen/namespaces.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/containers/common/pkg/cgroups" + "github.com/containers/podman/v3/libpod/network/types" "github.com/containers/podman/v3/pkg/rootless" "github.com/containers/podman/v3/pkg/util" "github.com/containers/storage" @@ -271,9 +272,9 @@ func ParseUserNamespace(ns string) (Namespace, error) { // ParseNetworkNamespace parses a network namespace specification in string // form. // Returns a namespace and (optionally) a list of CNI networks to join. -func ParseNetworkNamespace(ns string, rootlessDefaultCNI bool) (Namespace, []string, error) { +func ParseNetworkNamespace(ns string, rootlessDefaultCNI bool) (Namespace, map[string]types.PerNetworkOptions, error) { toReturn := Namespace{} - var cniNetworks []string + networks := make(map[string]types.PerNetworkOptions) // Net defaults to Slirp on rootless switch { case ns == string(Slirp), strings.HasPrefix(ns, string(Slirp)+":"): @@ -313,18 +314,22 @@ func ParseNetworkNamespace(ns string, rootlessDefaultCNI bool) (Namespace, []str default: // Assume we have been given a list of CNI networks. // Which only works in bridge mode, so set that. - cniNetworks = strings.Split(ns, ",") + networkList := strings.Split(ns, ",") + for _, net := range networkList { + networks[net] = types.PerNetworkOptions{} + } + toReturn.NSMode = Bridge } - return toReturn, cniNetworks, nil + return toReturn, networks, nil } -func ParseNetworkString(network string) (Namespace, []string, map[string][]string, error) { +func ParseNetworkString(network string) (Namespace, map[string]types.PerNetworkOptions, map[string][]string, error) { var networkOptions map[string][]string parts := strings.SplitN(network, ":", 2) - ns, cniNets, err := ParseNetworkNamespace(network, containerConfig.Containers.RootlessNetworking == "cni") + ns, nets, err := ParseNetworkNamespace(network, containerConfig.Containers.RootlessNetworking == "cni") if err != nil { return Namespace{}, nil, nil, err } @@ -332,9 +337,9 @@ func ParseNetworkString(network string) (Namespace, []string, map[string][]strin if len(parts) > 1 { networkOptions = make(map[string][]string) networkOptions[parts[0]] = strings.Split(parts[1], ",") - cniNets = nil + nets = nil } - return ns, cniNets, networkOptions, nil + return ns, nets, networkOptions, nil } func SetupUserNS(idmappings *storage.IDMappingOptions, userns Namespace, g *generate.Generator) (string, error) { diff --git a/pkg/specgen/pod_validate.go b/pkg/specgen/pod_validate.go index bca7b6dbe7..32c1159c60 100644 --- a/pkg/specgen/pod_validate.go +++ b/pkg/specgen/pod_validate.go @@ -1,7 +1,6 @@ package specgen import ( - "github.com/containers/podman/v3/pkg/rootless" "github.com/containers/podman/v3/pkg/util" "github.com/pkg/errors" ) @@ -19,15 +18,6 @@ func exclusivePodOptions(opt1, opt2 string) error { // Validate verifies the input is valid func (p *PodSpecGenerator) Validate() error { - if rootless.IsRootless() && len(p.CNINetworks) == 0 { - if p.StaticIP != nil { - return ErrNoStaticIPRootless - } - if p.StaticMAC != nil { - return ErrNoStaticMACRootless - } - } - // PodBasicConfig if p.NoInfra { if len(p.InfraCommand) > 0 { @@ -52,12 +42,6 @@ func (p *PodSpecGenerator) Validate() error { if p.NetNS.NSMode != Default && p.NetNS.NSMode != "" { return errors.New("NoInfra and network modes cannot be used together") } - if p.StaticIP != nil { - return exclusivePodOptions("NoInfra", "StaticIP") - } - if p.StaticMAC != nil { - return exclusivePodOptions("NoInfra", "StaticMAC") - } if len(p.DNSOption) > 0 { return exclusivePodOptions("NoInfra", "DNSOption") } @@ -78,8 +62,8 @@ func (p *PodSpecGenerator) Validate() error { if len(p.PortMappings) > 0 { return errors.New("PortMappings can only be used with Bridge or slirp4netns networking") } - if len(p.CNINetworks) > 0 { - return errors.New("CNINetworks can only be used with Bridge mode networking") + if len(p.Networks) > 0 { + return errors.New("Networks can only be used with Bridge mode networking") } } if p.NoManageResolvConf { diff --git a/pkg/specgen/podspecgen.go b/pkg/specgen/podspecgen.go index 948fb990cc..e59d11c0a7 100644 --- a/pkg/specgen/podspecgen.go +++ b/pkg/specgen/podspecgen.go @@ -86,33 +86,26 @@ type PodNetworkConfig struct { // Defaults to Bridge as root and Slirp as rootless. // Mandatory. NetNS Namespace `json:"netns,omitempty"` - // StaticIP sets a static IP for the infra container. As the infra - // container's network is used for the entire pod by default, this will - // thus be a static IP for the whole pod. - // Only available if NetNS is set to Bridge (the default for root). - // As such, conflicts with NoInfra=true by proxy. - // Optional. - StaticIP *net.IP `json:"static_ip,omitempty"` - // StaticMAC sets a static MAC for the infra container. As the infra - // container's network is used for the entire pod by default, this will - // thus be a static MAC for the entire pod. - // Only available if NetNS is set to Bridge (the default for root). - // As such, conflicts with NoInfra=true by proxy. - // Optional. - // swagger:strfmt string - StaticMAC *types.HardwareAddr `json:"static_mac,omitempty"` // PortMappings is a set of ports to map into the infra container. // As, by default, containers share their network with the infra // container, this will forward the ports to the entire pod. // Only available if NetNS is set to Bridge or Slirp. // Optional. PortMappings []types.PortMapping `json:"portmappings,omitempty"` - // CNINetworks is a list of CNI networks that the infra container will - // join. As, by default, containers share their network with the infra - // container, these networks will effectively be joined by the - // entire pod. - // Only available when NetNS is set to Bridge, the default for root. - // Optional. + // Map of networks names ot ids the container should join to. + // You can request additional settings for each network, you can + // set network aliases, static ips, static mac address and the + // network interface name for this container on the specifc network. + // If the map is empty and the bridge network mode is set the container + // will be joined to the default network. + Networks map[string]types.PerNetworkOptions + // CNINetworks is a list of CNI networks to join the container to. + // If this list is empty, the default CNI network will be joined + // instead. If at least one entry is present, we will not join the + // default network (unless it is part of this list). + // Only available if NetNS is set to bridge. + // Optional. + // Deprecated: as of podman 4.0 use "Networks" instead. CNINetworks []string `json:"cni_networks,omitempty"` // NoManageResolvConf indicates that /etc/resolv.conf should not be // managed by the pod. Instead, each container will create and manage a diff --git a/pkg/specgen/specgen.go b/pkg/specgen/specgen.go index 0e257ad4c4..e650c19660 100644 --- a/pkg/specgen/specgen.go +++ b/pkg/specgen/specgen.go @@ -394,26 +394,10 @@ type ContainerCgroupConfig struct { // ContainerNetworkConfig contains information on a container's network // configuration. type ContainerNetworkConfig struct { - // Aliases are a list of network-scoped aliases for container - // Optional - Aliases map[string][]string `json:"aliases"` // NetNS is the configuration to use for the container's network // namespace. // Mandatory. NetNS Namespace `json:"netns,omitempty"` - // StaticIP is the a IPv4 address of the container. - // Only available if NetNS is set to Bridge. - // Optional. - StaticIP *net.IP `json:"static_ip,omitempty"` - // StaticIPv6 is a static IPv6 address to set in the container. - // Only available if NetNS is set to Bridge. - // Optional. - StaticIPv6 *net.IP `json:"static_ipv6,omitempty"` - // StaticMAC is a static MAC address to set in the container. - // Only available if NetNS is set to bridge. - // Optional. - // swagger:strfmt string - StaticMAC *nettypes.HardwareAddr `json:"static_mac,omitempty"` // PortBindings is a set of ports to map into the container. // Only available if NetNS is set to bridge or slirp. // Optional. @@ -434,12 +418,20 @@ type ContainerNetworkConfig struct { // PublishExposedPorts is set. // Optional. Expose map[uint16]string `json:"expose,omitempty"` + // Map of networks names ot ids the container should join to. + // You can request additional settings for each network, you can + // set network aliases, static ips, static mac address and the + // network interface name for this container on the specifc network. + // If the map is empty and the bridge network mode is set the container + // will be joined to the default network. + Networks map[string]nettypes.PerNetworkOptions // CNINetworks is a list of CNI networks to join the container to. // If this list is empty, the default CNI network will be joined // instead. If at least one entry is present, we will not join the // default network (unless it is part of this list). // Only available if NetNS is set to bridge. // Optional. + // Deprecated: as of podman 4.0 use "Networks" instead. CNINetworks []string `json:"cni_networks,omitempty"` // UseImageResolvConf indicates that resolv.conf should not be managed // by Podman, but instead sourced from the image. diff --git a/pkg/specgenutil/specgen.go b/pkg/specgenutil/specgen.go index 5e4bd2f653..123c0073b1 100644 --- a/pkg/specgenutil/specgen.go +++ b/pkg/specgenutil/specgen.go @@ -11,7 +11,6 @@ import ( "github.com/containers/image/v5/manifest" "github.com/containers/podman/v3/cmd/podman/parse" "github.com/containers/podman/v3/libpod/define" - "github.com/containers/podman/v3/libpod/network/types" ann "github.com/containers/podman/v3/pkg/annotations" "github.com/containers/podman/v3/pkg/domain/entities" envLib "github.com/containers/podman/v3/pkg/env" @@ -434,19 +433,7 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions } if c.Net != nil { - s.CNINetworks = c.Net.CNINetworks - } - - // Network aliases - if c.Net != nil { - if len(c.Net.Aliases) > 0 { - // build a map of aliases where key=cniName - aliases := make(map[string][]string, len(s.CNINetworks)) - for _, cniNetwork := range s.CNINetworks { - aliases[cniNetwork] = c.Net.Aliases - } - s.Aliases = aliases - } + s.Networks = c.Net.Networks } if c.Net != nil { @@ -455,9 +442,6 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions s.DNSServers = c.Net.DNSServers s.DNSSearch = c.Net.DNSSearch s.DNSOptions = c.Net.DNSOptions - s.StaticIP = c.Net.StaticIP - // type cast to types.HardwareAddr - s.StaticMAC = (*types.HardwareAddr)(c.Net.StaticMAC) s.NetworkOptions = c.Net.NetworkOptions s.UseImageHosts = c.Net.NoHosts } From 535818414c2a6bdcf6434e36c33775ea1a43f1cf Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 10 Dec 2021 15:22:09 +0100 Subject: [PATCH 07/10] support advanced network configuration via cli Rework the --network parse logic to support multiple networks with specific network configuration settings. --network can now be set multiple times. For bridge network mode the following options have been added: - **alias=name**: Add network-scoped alias for the container. - **ip=IPv4**: Specify a static ipv4 address for this container. - **ip=IPv6**: Specify a static ipv6 address for this container. - **mac=MAC**: Specify a static mac address address for this container. - **interface_name**: Specify a name for the created network interface inside the container. So now you can set --network bridge:ip=10.88.0.10,mac=44:33:22:11:00:99 for the default bridge network as well as for network names. This is better than using --ip because we can set the ip per network without any confusion which network the ip address should be assigned to. The --ip, --mac-address and --network-alias options are still supported but --ip or --mac-address can only be set when only one network is set. This limitation already existed previously. The ability to specify a custom network interface name is new Fixes #11534 Signed-off-by: Paul Holzinger --- cmd/podman/common/create_opts.go | 9 +- cmd/podman/common/netflags.go | 131 +++++----- cmd/podman/containers/create.go | 2 +- cmd/podman/containers/run.go | 2 +- cmd/podman/pods/create.go | 4 +- docs/source/markdown/podman-create.1.md | 38 ++- docs/source/markdown/podman-pod-create.1.md | 48 +++- docs/source/markdown/podman-run.1.md | 37 ++- pkg/domain/infra/abi/play.go | 2 +- pkg/specgen/generate/namespaces.go | 16 +- pkg/specgen/namespaces.go | 164 +++++++++++- pkg/specgen/namespaces_test.go | 265 ++++++++++++++++++++ pkg/specgen/pod_validate.go | 5 + test/e2e/create_staticip_test.go | 6 - test/e2e/network_connect_disconnect_test.go | 11 +- test/e2e/run_staticip_test.go | 20 ++ test/system/030-run.bats | 4 +- 17 files changed, 644 insertions(+), 120 deletions(-) create mode 100644 pkg/specgen/namespaces_test.go diff --git a/cmd/podman/common/create_opts.go b/cmd/podman/common/create_opts.go index 8f0cd1be6d..990c1c0630 100644 --- a/cmd/podman/common/create_opts.go +++ b/cmd/podman/common/create_opts.go @@ -156,18 +156,11 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, rtc *c } // netMode - nsmode, networks, err := specgen.ParseNetworkNamespace(string(cc.HostConfig.NetworkMode), true) + nsmode, networks, netOpts, err := specgen.ParseNetworkFlag([]string{string(cc.HostConfig.NetworkMode)}) if err != nil { return nil, nil, err } - var netOpts map[string][]string - parts := strings.SplitN(string(cc.HostConfig.NetworkMode), ":", 2) - if len(parts) > 1 { - netOpts = make(map[string][]string) - netOpts[parts[0]] = strings.Split(parts[1], ",") - } - // network // Note: we cannot emulate compat exactly here. we only allow specifics of networks to be // defined when there is only one network. diff --git a/cmd/podman/common/netflags.go b/cmd/podman/common/netflags.go index f79a9fd88a..ba8ab7a8b1 100644 --- a/cmd/podman/common/netflags.go +++ b/cmd/podman/common/netflags.go @@ -61,8 +61,8 @@ func DefineNetFlags(cmd *cobra.Command) { _ = cmd.RegisterFlagCompletionFunc(macAddressFlagName, completion.AutocompleteNone) networkFlagName := "network" - netFlags.String( - networkFlagName, containerConfig.NetNS(), + netFlags.StringArray( + networkFlagName, nil, "Connect a container to a network", ) _ = cmd.RegisterFlagCompletionFunc(networkFlagName, AutocompleteNetworkFlag) @@ -88,9 +88,7 @@ func DefineNetFlags(cmd *cobra.Command) { } // NetFlagsToNetOptions parses the network flags for the given cmd. -// The netnsFromConfig bool is used to indicate if the --network flag -// should always be parsed regardless if it was set on the cli. -func NetFlagsToNetOptions(opts *entities.NetOptions, flags pflag.FlagSet, netnsFromConfig bool) (*entities.NetOptions, error) { +func NetFlagsToNetOptions(opts *entities.NetOptions, flags pflag.FlagSet) (*entities.NetOptions, error) { var ( err error ) @@ -168,79 +166,100 @@ func NetFlagsToNetOptions(opts *entities.NetOptions, flags pflag.FlagSet, netnsF return nil, err } - // parse the --network value only when the flag is set or we need to use - // the netns config value, e.g. when --pod is not used - if netnsFromConfig || flags.Changed("network") { - network, err := flags.GetString("network") + // parse the network only when network was changed + // otherwise we send default to server so that the server + // can pick the correct default instead of the client + if flags.Changed("network") { + network, err := flags.GetStringArray("network") if err != nil { return nil, err } - ns, networks, options, err := specgen.ParseNetworkString(network) + ns, networks, options, err := specgen.ParseNetworkFlag(network) if err != nil { return nil, err } - if len(options) > 0 { - opts.NetworkOptions = options - } + opts.NetworkOptions = options opts.Network = ns opts.Networks = networks } - ip, err := flags.GetString("ip") - if err != nil { - return nil, err - } - if ip != "" { - staticIP := net.ParseIP(ip) - if staticIP == nil { - return nil, errors.Errorf("%s is not an ip address", ip) - } - if !opts.Network.IsBridge() { - return nil, errors.Wrap(define.ErrInvalidArg, "--ip can only be set when the network mode is bridge") - } - if len(opts.Networks) != 1 { - return nil, errors.Wrap(define.ErrInvalidArg, "--ip can only be set for a single network") - } - for name, netOpts := range opts.Networks { - netOpts.StaticIPs = append(netOpts.StaticIPs, staticIP) - opts.Networks[name] = netOpts + if flags.Changed("ip") || flags.Changed("mac-address") || flags.Changed("network-alias") { + // if there is no network we add the default + if len(opts.Networks) == 0 { + opts.Networks = map[string]types.PerNetworkOptions{ + "default": {}, + } } - } - m, err := flags.GetString("mac-address") - if err != nil { - return nil, err - } - if len(m) > 0 { - mac, err := net.ParseMAC(m) + ip, err := flags.GetString("ip") if err != nil { return nil, err } - if !opts.Network.IsBridge() { - return nil, errors.Wrap(define.ErrInvalidArg, "--mac-address can only be set when the network mode is bridge") + if ip != "" { + // if pod create --infra=false + if infra, err := flags.GetBool("infra"); err == nil && !infra { + return nil, errors.Wrap(define.ErrInvalidArg, "cannot set --ip without infra container") + } + + staticIP := net.ParseIP(ip) + if staticIP == nil { + return nil, errors.Errorf("%s is not an ip address", ip) + } + if !opts.Network.IsBridge() && !opts.Network.IsDefault() { + return nil, errors.Wrap(define.ErrInvalidArg, "--ip can only be set when the network mode is bridge") + } + if len(opts.Networks) != 1 { + return nil, errors.Wrap(define.ErrInvalidArg, "--ip can only be set for a single network") + } + for name, netOpts := range opts.Networks { + netOpts.StaticIPs = append(netOpts.StaticIPs, staticIP) + opts.Networks[name] = netOpts + } } - if len(opts.Networks) != 1 { - return nil, errors.Wrap(define.ErrInvalidArg, "--mac-address can only be set for a single network") + + m, err := flags.GetString("mac-address") + if err != nil { + return nil, err } - for name, netOpts := range opts.Networks { - netOpts.StaticMAC = types.HardwareAddr(mac) - opts.Networks[name] = netOpts + if len(m) > 0 { + // if pod create --infra=false + if infra, err := flags.GetBool("infra"); err == nil && !infra { + return nil, errors.Wrap(define.ErrInvalidArg, "cannot set --mac without infra container") + } + mac, err := net.ParseMAC(m) + if err != nil { + return nil, err + } + if !opts.Network.IsBridge() && !opts.Network.IsDefault() { + return nil, errors.Wrap(define.ErrInvalidArg, "--mac-address can only be set when the network mode is bridge") + } + if len(opts.Networks) != 1 { + return nil, errors.Wrap(define.ErrInvalidArg, "--mac-address can only be set for a single network") + } + for name, netOpts := range opts.Networks { + netOpts.StaticMAC = types.HardwareAddr(mac) + opts.Networks[name] = netOpts + } } - } - aliases, err := flags.GetStringSlice("network-alias") - if err != nil { - return nil, err - } - if len(aliases) > 0 { - if !opts.Network.IsBridge() { - return nil, errors.Wrap(define.ErrInvalidArg, "--network-alias can only be set when the network mode is bridge") + aliases, err := flags.GetStringSlice("network-alias") + if err != nil { + return nil, err } - for name, netOpts := range opts.Networks { - netOpts.Aliases = aliases - opts.Networks[name] = netOpts + if len(aliases) > 0 { + // if pod create --infra=false + if infra, err := flags.GetBool("infra"); err == nil && !infra { + return nil, errors.Wrap(define.ErrInvalidArg, "cannot set --network-alias without infra container") + } + if !opts.Network.IsBridge() && !opts.Network.IsDefault() { + return nil, errors.Wrap(define.ErrInvalidArg, "--network-alias can only be set when the network mode is bridge") + } + for name, netOpts := range opts.Networks { + netOpts.Aliases = aliases + opts.Networks[name] = netOpts + } } } diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go index a17fcaa1c7..e004f4ab2d 100644 --- a/cmd/podman/containers/create.go +++ b/cmd/podman/containers/create.go @@ -105,7 +105,7 @@ func create(cmd *cobra.Command, args []string) error { err error ) flags := cmd.Flags() - cliVals.Net, err = common.NetFlagsToNetOptions(nil, *flags, cliVals.Pod == "" && cliVals.PodIDFile == "") + cliVals.Net, err = common.NetFlagsToNetOptions(nil, *flags) if err != nil { return err } diff --git a/cmd/podman/containers/run.go b/cmd/podman/containers/run.go index 9d1b040cc4..cfb89ce57f 100644 --- a/cmd/podman/containers/run.go +++ b/cmd/podman/containers/run.go @@ -120,7 +120,7 @@ func run(cmd *cobra.Command, args []string) error { } flags := cmd.Flags() - cliVals.Net, err = common.NetFlagsToNetOptions(nil, *flags, cliVals.Pod == "" && cliVals.PodIDFile == "") + cliVals.Net, err = common.NetFlagsToNetOptions(nil, *flags) if err != nil { return err } diff --git a/cmd/podman/pods/create.go b/cmd/podman/pods/create.go index 7399dd029c..f844812c21 100644 --- a/cmd/podman/pods/create.go +++ b/cmd/podman/pods/create.go @@ -116,7 +116,7 @@ func create(cmd *cobra.Command, args []string) error { return fmt.Errorf("cannot specify no-hosts without an infra container") } flags := cmd.Flags() - createOptions.Net, err = common.NetFlagsToNetOptions(nil, *flags, createOptions.Infra) + createOptions.Net, err = common.NetFlagsToNetOptions(nil, *flags) if err != nil { return err } @@ -133,7 +133,7 @@ func create(cmd *cobra.Command, args []string) error { } else { // reassign certain options for lbpod api, these need to be populated in spec flags := cmd.Flags() - infraOptions.Net, err = common.NetFlagsToNetOptions(nil, *flags, createOptions.Infra) + infraOptions.Net, err = common.NetFlagsToNetOptions(nil, *flags) if err != nil { return err } diff --git a/docs/source/markdown/podman-create.1.md b/docs/source/markdown/podman-create.1.md index b58fd1e183..c8f1ec3a52 100644 --- a/docs/source/markdown/podman-create.1.md +++ b/docs/source/markdown/podman-create.1.md @@ -476,9 +476,12 @@ Not implemented #### **--ip**=*ip* Specify a static IP address for the container, for example **10.88.64.128**. -This option can only be used if the container is joined to only a single network - i.e., `--network=_network-name_` is used at most once - -and if the container is not joining another container's network namespace via `--network=container:_id_`. -The address must be within the CNI network's IP address pool (default **10.88.0.0/16**). +This option can only be used if the container is joined to only a single network - i.e., **--network=network-name** is used at most once - +and if the container is not joining another container's network namespace via **--network=container:_id_**. +The address must be within the network's IP address pool (default **10.88.0.0/16**). + +To specify multiple static IP addresses per container, set multiple networks using the **--network** option with a static IP address specified for each using the `ip` mode for that option. + #### **--ipc**=*ipc* @@ -531,12 +534,16 @@ This option is currently supported only by the **journald** log driver. #### **--mac-address**=*address* -Container MAC address (e.g. 92:d0:c6:0a:29:33) +Container network interface MAC address (e.g. 92:d0:c6:0a:29:33) +This option can only be used if the container is joined to only a single network - i.e., **--network=_network-name_** is used at most once - +and if the container is not joining another container's network namespace via **--network=container:_id_**. Remember that the MAC address in an Ethernet network must be unique. The IPv6 link-local address will be based on the device's MAC address according to RFC4862. +To specify multiple static MAC addresses per container, set multiple networks using the **--network** option with a static MAC address specified for each using the `mac` mode for that option. + #### **--memory**, **-m**=*limit* Memory limit (format: `[]`, where unit = b (bytes), k (kilobytes), m (megabytes), or g (gigabytes)) @@ -668,15 +675,22 @@ This works for both background and foreground containers. #### **--network**=*mode*, **--net** -Set the network mode for the container. Invalid if using **--dns**, **--dns-opt**, or **--dns-search** with **--network** that is set to **none** or **container:**_id_. If used together with **--pod**, the container will not join the pod's network namespace. +Set the network mode for the container. Invalid if using **--dns**, **--dns-opt**, or **--dns-search** with **--network** set to **none** or **container:**_id_. If used together with **--pod**, the container will not join the pod's network namespace. Valid _mode_ values are: -- **bridge**: Create a network stack on the default bridge. This is the default for rootfull containers. +- **bridge[:OPTIONS,...]**: Create a network stack on the default bridge. This is the default for rootfull containers. It is possible to specify these additional options: + - **alias=name**: Add network-scoped alias for the container. + - **ip=IPv4**: Specify a static ipv4 address for this container. + - **ip=IPv6**: Specify a static ipv6 address for this container. + - **mac=MAC**: Specify a static mac address address for this container. + - **interface_name**: Specify a name for the created network interface inside the container. + + For example to set a static ipv4 address and a static mac address, use `--network bridge:ip=10.88.0.10,mac=44:33:22:11:00:99`. +- \[:OPTIONS,...]: Connect to a user-defined network; this is the network name or ID from a network created by **[podman network create](podman-network-create.1.md)**. Using the network name implies the bridge network mode. It is possible to specify the same options described under the bridge mode above. You can use the **--network** option multiple times to specify additional networks. - **none**: Create a network namespace for the container but do not configure network interfaces for it, thus the container has no network connectivity. - **container:**_id_: Reuse another container's network stack. - **host**: Do not create a network namespace, the container will use the host's network. Note: The host mode gives the container full access to local system services such as D-bus and is therefore considered insecure. -- **network**: 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. This will use the **bridge** mode for rootfull containers and **slirp4netns** for rootless ones. - **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: @@ -694,7 +708,9 @@ Valid _mode_ values are: #### **--network-alias**=*alias* -Add network-scoped alias for the container. NOTE: A container will only have access to aliases on the first network that it joins. This is a limitation that will be removed in a later release. +Add a network-scoped alias for the container, setting the alias for all networks that the container joins. To set a name only for a specific network, use the alias option as described under the **--network** option. +Network aliases work only with the bridge networking mode. This option can be specified multiple times. +NOTE: A container will only have access to aliases on the first network that it joins. This is a limitation that will be removed in a later release. #### **--no-healthcheck** @@ -1492,6 +1508,12 @@ $ podman create --name container1 --personaity=LINUX32 fedora bash $ podman create --name container1 --rootfs /path/to/rootfs:O bash ``` +### Create a container connected to two networks (called net1 and net2) with a static ip + +``` +$ podman create --network net1:ip=10.89.1.5 --network net2:ip=10.89.10.10 alpine ip addr +``` + ### Rootless Containers Podman runs as a non root user on most systems. This feature requires that a new enough version of shadow-utils diff --git a/docs/source/markdown/podman-pod-create.1.md b/docs/source/markdown/podman-pod-create.1.md index cca90c9429..b1b029429f 100644 --- a/docs/source/markdown/podman-pod-create.1.md +++ b/docs/source/markdown/podman-pod-create.1.md @@ -118,9 +118,14 @@ The custom image that will be used for the infra container. Unless specified, P The name that will be used for the pod's infra container. -#### **--ip**=*ipaddr* +#### **--ip**=*ip* -Set a static IP for the pod's shared network. +Specify a static IP address for the pod, for example **10.88.64.128**. +This option can only be used if the pod is joined to only a single network - i.e., **--network=network-name** is used at most once - +and if the pod is not joining another container's network namespace via **--network=container:_id_**. +The address must be within the network's IP address pool (default **10.88.0.0/16**). + +To specify multiple static IP addresses per pod, set multiple networks using the **--network** option with a static IP address specified for each using the `ip` mode for that option. #### **--label**=*label*, **-l** @@ -132,7 +137,16 @@ Read in a line delimited file of labels. #### **--mac-address**=*address* -Set a static MAC address for the pod's shared network. +Pod network interface MAC address (e.g. 92:d0:c6:0a:29:33) +This option can only be used if the pod is joined to only a single network - i.e., **--network=_network-name_** is used at most once - +and if the pod is not joining another container's network namespace via **--network=container:_id_**. + +Remember that the MAC address in an Ethernet network must be unique. +The IPv6 link-local address will be based on the device's MAC address +according to RFC4862. + +To specify multiple static MAC addresses per pod, set multiple networks using the **--network** option with a static MAC address specified for each using the `mac` mode for that option. + #### **--name**=*name*, **-n** @@ -140,11 +154,23 @@ Assign a name to the pod. #### **--network**=*mode*, **--net** -Set network mode for the pod. Supported values are: -- **bridge**: Create a network stack on the default bridge. This is the default for rootfull containers. +Set the network mode for the pod. Invalid if using **--dns**, **--dns-opt**, or **--dns-search** with **--network** that is set to **none** or **container:**_id_. + +Valid _mode_ values are: + +- **bridge[:OPTIONS,...]**: Create a network stack on the default bridge. This is the default for rootfull containers. It is possible to specify these additional options: + - **alias=name**: Add network-scoped alias for the container. + - **ip=IPv4**: Specify a static ipv4 address for this container. + - **ip=IPv6**: Specify a static ipv6 address for this container. + - **mac=MAC**: Specify a static mac address address for this container. + - **interface_name**: Specify a name for the created network interface inside the container. + + For example to set a static ipv4 address and a static mac address, use `--network bridge:ip=10.88.0.10,mac=44:33:22:11:00:99`. +- \[:OPTIONS,...]: Connect to a user-defined network; this is the network name or ID from a network created by **[podman network create](podman-network-create.1.md)**. Using the network name implies the bridge network mode. It is possible to specify the same options described under the bridge mode above. You can use the **--network** option multiple times to specify additional networks. - **none**: Create a network namespace for the container but do not configure network interfaces for it, thus the container has no network connectivity. -- **host**: Do not create a network namespace, all containers in the pod will use the host's network. Note: the host mode gives the container full access to local system services such as D-bus and is therefore considered insecure. -- **network**: Connect to a user-defined network, multiple networks should be comma-separated. +- **container:**_id_: Reuse another container's network stack. +- **host**: Do not create a network namespace, the container will use the host's network. Note: The host mode gives the container full access to local system services such as D-bus and is therefore considered insecure. +- **ns:**_path_: Path to a network namespace to join. - **private**: Create a new namespace for the container. This will use the **bridge** mode for rootfull containers and **slirp4netns** for rootless ones. - **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: - **allow_host_loopback=true|false**: Allow the slirp4netns to reach the host loopback IP (`10.0.2.2`, which is added to `/etc/hosts` as `host.containers.internal` for your convenience). Default is false. @@ -159,9 +185,11 @@ Set network mode for the pod. Supported values are: Note: Rootlesskit changes the source IP address of incoming packets to a IP address in the container network namespace, usually `10.0.2.100`. If your application requires the real source IP address, e.g. web server logs, use the slirp4netns port handler. The rootlesskit port handler is also used for rootless containers when connected to user-defined networks. - **port_handler=slirp4netns**: Use the slirp4netns port forwarding, it is slower than rootlesskit but preserves the correct source IP address. This port handler cannot be used for user-defined networks. -#### **--network-alias**=strings +#### **--network-alias**=*alias* -Add a DNS alias for the pod. When the pod is joined to a CNI network with support for the dnsname plugin, the containers inside the pod will be accessible through this name from other containers in the network. +Add a network-scoped alias for the pod, setting the alias for all networks that the pod joins. To set a name only for a specific network, use the alias option as described under the **--network** option. +Network aliases work only with the bridge networking mode. This option can be specified multiple times. +NOTE: A container will only have access to aliases on the first network that it joins. This is a limitation that will be removed in a later release. #### **--no-hosts** @@ -429,6 +457,8 @@ $ podman pod create --publish 8443:443 $ podman pod create --network slirp4netns:outbound_addr=127.0.0.1,allow_host_loopback=true $ podman pod create --network slirp4netns:cidr=192.168.0.0/24 + +$ podman pod create --network net1:ip=10.89.1.5 --network net2:ip=10.89.10.10 ``` ## SEE ALSO diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md index 0d9e6dbcda..a6687e6563 100644 --- a/docs/source/markdown/podman-run.1.md +++ b/docs/source/markdown/podman-run.1.md @@ -499,9 +499,11 @@ Not implemented. #### **--ip**=*ip* Specify a static IP address for the container, for example **10.88.64.128**. -This option can only be used if the container is joined to only a single network - i.e., `--network=_network-name_` is used at most once -and if the container is not joining another container's network namespace via `--network=container:_id_`. -The address must be within the CNI network's IP address pool (default **10.88.0.0/16**). +This option can only be used if the container is joined to only a single network - i.e., **--network=network-name** is used at most once - +and if the container is not joining another container's network namespace via **--network=container:_id_**. +The address must be within the network's IP address pool (default **10.88.0.0/16**). + +To specify multiple static IP addresses per container, set multiple networks using the **--network** option with a static IP address specified for each using the `ip` mode for that option. #### **--ipc**=*mode* @@ -557,12 +559,16 @@ This option is currently supported only by the **journald** log driver. #### **--mac-address**=*address* -Container MAC address (e.g. **92:d0:c6:0a:29:33**). +Container network interface MAC address (e.g. 92:d0:c6:0a:29:33) +This option can only be used if the container is joined to only a single network - i.e., **--network=_network-name_** is used at most once - +and if the container is not joining another container's network namespace via **--network=container:_id_**. Remember that the MAC address in an Ethernet network must be unique. The IPv6 link-local address will be based on the device's MAC address according to RFC4862. +To specify multiple static MAC addresses per container, set multiple networks using the **--network** option with a static MAC address specified for each using the `mac` mode for that option. + #### **--memory**, **-m**=_number_[_unit_] Memory limit. A _unit_ can be **b** (bytes), **k** (kilobytes), **m** (megabytes), or **g** (gigabytes). @@ -696,15 +702,22 @@ This works for both background and foreground containers. #### **--network**=*mode*, **--net** -Set the network mode for the container. Invalid if using **--dns**, **--dns-opt**, or **--dns-search** with **--network** that is set to **none** or **container:**_id_. If used together with **--pod**, the container will not join the pods network namespace. +Set the network mode for the container. Invalid if using **--dns**, **--dns-opt**, or **--dns-search** with **--network** set to **none** or **container:**_id_. If used together with **--pod**, the container will not join the pod's network namespace. Valid _mode_ values are: -- **bridge**: Create a network stack on the default bridge. This is the default for rootfull containers. +- **bridge[:OPTIONS,...]**: Create a network stack on the default bridge. This is the default for rootfull containers. It is possible to specify these additional options: + - **alias=name**: Add network-scoped alias for the container. + - **ip=IPv4**: Specify a static ipv4 address for this container. + - **ip=IPv6**: Specify a static ipv6 address for this container. + - **mac=MAC**: Specify a static mac address address for this container. + - **interface_name**: Specify a name for the created network interface inside the container. + + For example to set a static ipv4 address and a static mac address, use `--network bridge:ip=10.88.0.10,mac=44:33:22:11:00:99`. +- \[:OPTIONS,...]: Connect to a user-defined network; this is the network name or ID from a network created by **[podman network create](podman-network-create.1.md)**. Using the network name implies the bridge network mode. It is possible to specify the same options described under the bridge mode above. You can use the **--network** option multiple times to specify additional networks. - **none**: Create a network namespace for the container but do not configure network interfaces for it, thus the container has no network connectivity. - **container:**_id_: Reuse another container's network stack. - **host**: Do not create a network namespace, the container will use the host's network. Note: The host mode gives the container full access to local system services such as D-bus and is therefore considered insecure. -- **network**: 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. This will use the **bridge** mode for rootfull containers and **slirp4netns** for rootless ones. - **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: @@ -722,7 +735,9 @@ Valid _mode_ values are: #### **--network-alias**=*alias* -Add network-scoped alias for the container. NOTE: A container will only have access to aliases on the first network that it joins. This is a limitation that will be removed in a later release. +Add a network-scoped alias for the container, setting the alias for all networks that the container joins. To set a name only for a specific network, use the alias option as described under the **--network** option. +Network aliases work only with the bridge networking mode. This option can be specified multiple times. +NOTE: A container will only have access to aliases on the first network that it joins. This is a limitation that will be removed in a later release. #### **--no-healthcheck** @@ -1867,6 +1882,12 @@ Forcing UTC: Fri Nov 19 23:10:55 UTC 2021 ``` +### Run a container connected to two networks (called net1 and net2) with a static ip + +``` +$ podman run --network net1:ip=10.89.1.5 --network net2:ip=10.89.10.10 alpine ip addr +``` + ### Rootless Containers Podman runs as a non root user on most systems. This feature requires that a new enough version of **shadow-utils** diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index b0b68567a3..409ba938a4 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -196,7 +196,7 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY } if options.Network != "" { - ns, networks, netOpts, err := specgen.ParseNetworkString(options.Network) + ns, networks, netOpts, err := specgen.ParseNetworkFlag([]string{options.Network}) if err != nil { return nil, err } diff --git a/pkg/specgen/generate/namespaces.go b/pkg/specgen/generate/namespaces.go index 7821566632..a2bc37e349 100644 --- a/pkg/specgen/generate/namespaces.go +++ b/pkg/specgen/generate/namespaces.go @@ -67,7 +67,7 @@ func GetDefaultNamespaceMode(nsType string, cfg *config.Config, pod *libpod.Pod) case "cgroup": return specgen.ParseCgroupNamespace(cfg.Containers.CgroupNS) case "net": - ns, _, err := specgen.ParseNetworkNamespace(cfg.Containers.NetNS, cfg.Containers.RootlessNetworking == "cni") + ns, _, _, err := specgen.ParseNetworkFlag(nil) return ns, err } @@ -259,6 +259,11 @@ func namespaceOptions(ctx context.Context, s *specgen.SpecGenerator, rt *libpod. if err != nil { return nil, err } + + rtConfig, err := rt.GetConfigNoCopy() + if err != nil { + return nil, err + } // if no network was specified use add the default if len(s.Networks) == 0 { // backwards config still allow the old cni networks list and convert to new format @@ -271,15 +276,16 @@ func namespaceOptions(ctx context.Context, s *specgen.SpecGenerator, rt *libpod. s.Networks = networks } else { // no networks given but bridge is set so use default network - rtConfig, err := rt.GetConfigNoCopy() - if err != nil { - return nil, err - } s.Networks = map[string]types.PerNetworkOptions{ rtConfig.Network.DefaultNetwork: {}, } } } + // rename the "default" network to the correct default name + if opts, ok := s.Networks["default"]; ok { + s.Networks[rtConfig.Network.DefaultNetwork] = opts + delete(s.Networks, "default") + } toReturn = append(toReturn, libpod.WithNetNS(portMappings, expose, postConfigureNetNS, "bridge", s.Networks)) } diff --git a/pkg/specgen/namespaces.go b/pkg/specgen/namespaces.go index 121e1ecf7d..15a8ece171 100644 --- a/pkg/specgen/namespaces.go +++ b/pkg/specgen/namespaces.go @@ -2,10 +2,12 @@ package specgen import ( "fmt" + "net" "os" "strings" "github.com/containers/common/pkg/cgroups" + "github.com/containers/podman/v3/libpod/define" "github.com/containers/podman/v3/libpod/network/types" "github.com/containers/podman/v3/pkg/rootless" "github.com/containers/podman/v3/pkg/util" @@ -325,21 +327,163 @@ func ParseNetworkNamespace(ns string, rootlessDefaultCNI bool) (Namespace, map[s return toReturn, networks, nil } -func ParseNetworkString(network string) (Namespace, map[string]types.PerNetworkOptions, map[string][]string, error) { +// ParseNetworkFlag parses a network string slice into the network options +// If the input is nil or empty it will use the default setting from containers.conf +func ParseNetworkFlag(networks []string) (Namespace, map[string]types.PerNetworkOptions, map[string][]string, error) { var networkOptions map[string][]string - parts := strings.SplitN(network, ":", 2) + // by default we try to use the containers.conf setting + // if we get at least one value use this instead + ns := containerConfig.Containers.NetNS + if len(networks) > 0 { + ns = networks[0] + } - ns, nets, err := ParseNetworkNamespace(network, containerConfig.Containers.RootlessNetworking == "cni") - if err != nil { - return Namespace{}, nil, nil, err + toReturn := Namespace{} + podmanNetworks := make(map[string]types.PerNetworkOptions) + + switch { + case ns == string(Slirp), strings.HasPrefix(ns, string(Slirp)+":"): + parts := strings.SplitN(ns, ":", 2) + if len(parts) > 1 { + networkOptions = make(map[string][]string) + networkOptions[parts[0]] = strings.Split(parts[1], ",") + } + toReturn.NSMode = Slirp + case ns == string(FromPod): + toReturn.NSMode = FromPod + case ns == "" || ns == string(Default) || ns == string(Private): + // Net defaults to Slirp on rootless + if rootless.IsRootless() && containerConfig.Containers.RootlessNetworking != "cni" { + toReturn.NSMode = Slirp + break + } + // if not slirp we use bridge + fallthrough + case ns == string(Bridge), strings.HasPrefix(ns, string(Bridge)+":"): + toReturn.NSMode = Bridge + parts := strings.SplitN(ns, ":", 2) + netOpts := types.PerNetworkOptions{} + if len(parts) > 1 { + var err error + netOpts, err = parseBridgeNetworkOptions(parts[1]) + if err != nil { + return toReturn, nil, nil, err + } + } + // we have to set the special default network name here + podmanNetworks["default"] = netOpts + + case ns == string(NoNetwork): + toReturn.NSMode = NoNetwork + case ns == string(Host): + toReturn.NSMode = Host + case strings.HasPrefix(ns, "ns:"): + split := strings.SplitN(ns, ":", 2) + if len(split) != 2 { + return toReturn, nil, nil, errors.Errorf("must provide a path to a namespace when specifying ns:") + } + toReturn.NSMode = Path + toReturn.Value = split[1] + case strings.HasPrefix(ns, string(FromContainer)+":"): + split := strings.SplitN(ns, ":", 2) + if len(split) != 2 { + return toReturn, nil, nil, errors.Errorf("must provide name or ID or a container when specifying container:") + } + toReturn.NSMode = FromContainer + toReturn.Value = split[1] + default: + // we should have a normal network + parts := strings.SplitN(ns, ":", 2) + if len(parts) == 1 { + // Assume we have been given a comma separated list of networks for backwards compat. + networkList := strings.Split(ns, ",") + for _, net := range networkList { + podmanNetworks[net] = types.PerNetworkOptions{} + } + } else { + if parts[0] == "" { + return toReturn, nil, nil, errors.New("network name cannot be empty") + } + netOpts, err := parseBridgeNetworkOptions(parts[1]) + if err != nil { + return toReturn, nil, nil, errors.Wrapf(err, "invalid option for network %s", parts[0]) + } + podmanNetworks[parts[0]] = netOpts + } + + // networks need bridge mode + toReturn.NSMode = Bridge } - if len(parts) > 1 { - networkOptions = make(map[string][]string) - networkOptions[parts[0]] = strings.Split(parts[1], ",") - nets = nil + if len(networks) > 1 { + if !toReturn.IsBridge() { + return toReturn, nil, nil, errors.Wrapf(define.ErrInvalidArg, "cannot set multiple networks without bridge network mode, selected mode %s", toReturn.NSMode) + } + + for _, network := range networks[1:] { + parts := strings.SplitN(network, ":", 2) + if parts[0] == "" { + return toReturn, nil, nil, errors.Wrapf(define.ErrInvalidArg, "network name cannot be empty") + } + if util.StringInSlice(parts[0], []string{string(Bridge), string(Slirp), string(FromPod), string(NoNetwork), + string(Default), string(Private), string(Path), string(FromContainer), string(Host)}) { + return toReturn, nil, nil, errors.Wrapf(define.ErrInvalidArg, "can only set extra network names, selected mode %s conflicts with bridge", parts[0]) + } + netOpts := types.PerNetworkOptions{} + if len(parts) > 1 { + var err error + netOpts, err = parseBridgeNetworkOptions(parts[1]) + if err != nil { + return toReturn, nil, nil, errors.Wrapf(err, "invalid option for network %s", parts[0]) + } + } + podmanNetworks[parts[0]] = netOpts + } + } + + return toReturn, podmanNetworks, networkOptions, nil +} + +func parseBridgeNetworkOptions(opts string) (types.PerNetworkOptions, error) { + netOpts := types.PerNetworkOptions{} + if len(opts) == 0 { + return netOpts, nil + } + allopts := strings.Split(opts, ",") + for _, opt := range allopts { + split := strings.SplitN(opt, "=", 2) + switch split[0] { + case "ip", "ip6": + ip := net.ParseIP(split[1]) + if ip == nil { + return netOpts, errors.Errorf("invalid ip address %q", split[1]) + } + netOpts.StaticIPs = append(netOpts.StaticIPs, ip) + + case "mac": + mac, err := net.ParseMAC(split[1]) + if err != nil { + return netOpts, err + } + netOpts.StaticMAC = types.HardwareAddr(mac) + + case "alias": + if split[1] == "" { + return netOpts, errors.New("alias cannot be empty") + } + netOpts.Aliases = append(netOpts.Aliases, split[1]) + + case "interface_name": + if split[1] == "" { + return netOpts, errors.New("interface_name cannot be empty") + } + netOpts.InterfaceName = split[1] + + default: + return netOpts, errors.Errorf("unknown bridge network option: %s", split[0]) + } } - return ns, nets, networkOptions, nil + return netOpts, nil } func SetupUserNS(idmappings *storage.IDMappingOptions, userns Namespace, g *generate.Generator) (string, error) { diff --git a/pkg/specgen/namespaces_test.go b/pkg/specgen/namespaces_test.go new file mode 100644 index 0000000000..4f69e6b980 --- /dev/null +++ b/pkg/specgen/namespaces_test.go @@ -0,0 +1,265 @@ +package specgen + +import ( + "net" + "testing" + + "github.com/containers/podman/v3/libpod/network/types" + "github.com/containers/podman/v3/pkg/rootless" + "github.com/stretchr/testify/assert" +) + +func parsMacNoErr(mac string) types.HardwareAddr { + m, _ := net.ParseMAC(mac) + return types.HardwareAddr(m) +} + +func TestParseNetworkFlag(t *testing.T) { + // root and rootless have different defaults + defaultNetName := "default" + defaultNetworks := map[string]types.PerNetworkOptions{ + defaultNetName: {}, + } + defaultNsMode := Namespace{NSMode: Bridge} + if rootless.IsRootless() { + defaultNsMode = Namespace{NSMode: Slirp} + defaultNetworks = map[string]types.PerNetworkOptions{} + } + + tests := []struct { + name string + args []string + nsmode Namespace + networks map[string]types.PerNetworkOptions + options map[string][]string + err string + }{ + { + name: "empty input", + args: nil, + nsmode: defaultNsMode, + networks: defaultNetworks, + }, + { + name: "empty string as input", + args: []string{}, + nsmode: defaultNsMode, + networks: defaultNetworks, + }, + { + name: "default mode", + args: []string{"default"}, + nsmode: defaultNsMode, + networks: defaultNetworks, + }, + { + name: "private mode", + args: []string{"private"}, + nsmode: defaultNsMode, + networks: defaultNetworks, + }, + { + name: "bridge mode", + args: []string{"bridge"}, + nsmode: Namespace{NSMode: Bridge}, + networks: map[string]types.PerNetworkOptions{ + defaultNetName: {}, + }, + }, + { + name: "slirp4netns mode", + args: []string{"slirp4netns"}, + nsmode: Namespace{NSMode: Slirp}, + networks: map[string]types.PerNetworkOptions{}, + }, + { + name: "from pod mode", + args: []string{"pod"}, + nsmode: Namespace{NSMode: FromPod}, + networks: map[string]types.PerNetworkOptions{}, + }, + { + name: "no network mode", + args: []string{"none"}, + nsmode: Namespace{NSMode: NoNetwork}, + networks: map[string]types.PerNetworkOptions{}, + }, + { + name: "container mode", + args: []string{"container:abc"}, + nsmode: Namespace{NSMode: FromContainer, Value: "abc"}, + networks: map[string]types.PerNetworkOptions{}, + }, + { + name: "ns path mode", + args: []string{"ns:/path"}, + nsmode: Namespace{NSMode: Path, Value: "/path"}, + networks: map[string]types.PerNetworkOptions{}, + }, + { + name: "slirp4netns mode with options", + args: []string{"slirp4netns:cidr=10.0.0.0/24"}, + nsmode: Namespace{NSMode: Slirp}, + networks: map[string]types.PerNetworkOptions{}, + options: map[string][]string{ + "slirp4netns": {"cidr=10.0.0.0/24"}, + }, + }, + { + name: "bridge mode with options 1", + args: []string{"bridge:ip=10.0.0.1,mac=11:22:33:44:55:66"}, + nsmode: Namespace{NSMode: Bridge}, + networks: map[string]types.PerNetworkOptions{ + defaultNetName: { + StaticIPs: []net.IP{net.ParseIP("10.0.0.1")}, + StaticMAC: parsMacNoErr("11:22:33:44:55:66"), + }, + }, + }, + { + name: "bridge mode with options 2", + args: []string{"bridge:ip=10.0.0.1,ip=10.0.0.5"}, + nsmode: Namespace{NSMode: Bridge}, + networks: map[string]types.PerNetworkOptions{ + defaultNetName: { + StaticIPs: []net.IP{net.ParseIP("10.0.0.1"), net.ParseIP("10.0.0.5")}, + }, + }, + }, + { + name: "bridge mode with ip6 option", + args: []string{"bridge:ip6=fd10::"}, + nsmode: Namespace{NSMode: Bridge}, + networks: map[string]types.PerNetworkOptions{ + defaultNetName: { + StaticIPs: []net.IP{net.ParseIP("fd10::")}, + }, + }, + }, + { + name: "bridge mode with alias option", + args: []string{"bridge:alias=myname,alias=myname2"}, + nsmode: Namespace{NSMode: Bridge}, + networks: map[string]types.PerNetworkOptions{ + defaultNetName: { + Aliases: []string{"myname", "myname2"}, + }, + }, + }, + { + name: "bridge mode with alias option", + args: []string{"bridge:alias=myname,alias=myname2"}, + nsmode: Namespace{NSMode: Bridge}, + networks: map[string]types.PerNetworkOptions{ + defaultNetName: { + Aliases: []string{"myname", "myname2"}, + }, + }, + }, + { + name: "bridge mode with interface option", + args: []string{"bridge:interface_name=eth123"}, + nsmode: Namespace{NSMode: Bridge}, + networks: map[string]types.PerNetworkOptions{ + defaultNetName: { + InterfaceName: "eth123", + }, + }, + }, + { + name: "bridge mode with invalid option", + args: []string{"bridge:abc=123"}, + nsmode: Namespace{NSMode: Bridge}, + err: "unknown bridge network option: abc", + }, + { + name: "bridge mode with invalid ip", + args: []string{"bridge:ip=10..1"}, + nsmode: Namespace{NSMode: Bridge}, + err: "invalid ip address \"10..1\"", + }, + { + name: "bridge mode with invalid mac", + args: []string{"bridge:mac=123"}, + nsmode: Namespace{NSMode: Bridge}, + err: "address 123: invalid MAC address", + }, + { + name: "network name", + args: []string{"someName"}, + nsmode: Namespace{NSMode: Bridge}, + networks: map[string]types.PerNetworkOptions{ + "someName": {}, + }, + }, + { + name: "network name with options", + args: []string{"someName:ip=10.0.0.1"}, + nsmode: Namespace{NSMode: Bridge}, + networks: map[string]types.PerNetworkOptions{ + "someName": {StaticIPs: []net.IP{net.ParseIP("10.0.0.1")}}, + }, + }, + { + name: "multiple networks", + args: []string{"someName", "net2"}, + nsmode: Namespace{NSMode: Bridge}, + networks: map[string]types.PerNetworkOptions{ + "someName": {}, + "net2": {}, + }, + }, + { + name: "multiple networks with options", + args: []string{"someName:ip=10.0.0.1", "net2:ip=10.10.0.1"}, + nsmode: Namespace{NSMode: Bridge}, + networks: map[string]types.PerNetworkOptions{ + "someName": {StaticIPs: []net.IP{net.ParseIP("10.0.0.1")}}, + "net2": {StaticIPs: []net.IP{net.ParseIP("10.10.0.1")}}, + }, + }, + { + name: "multiple networks with bridge mode first should map to default net", + args: []string{"bridge", "net2"}, + nsmode: Namespace{NSMode: Bridge}, + networks: map[string]types.PerNetworkOptions{ + defaultNetName: {}, + "net2": {}, + }, + }, + { + name: "conflicting network modes should error", + args: []string{"bridge", "host"}, + nsmode: Namespace{NSMode: Bridge}, + err: "can only set extra network names, selected mode host conflicts with bridge: invalid argument", + }, + { + name: "multiple networks empty name should error", + args: []string{"someName", ""}, + nsmode: Namespace{NSMode: Bridge}, + err: "network name cannot be empty: invalid argument", + }, + { + name: "multiple networks on invalid mode should error", + args: []string{"host", "net2"}, + nsmode: Namespace{NSMode: Host}, + err: "cannot set multiple networks without bridge network mode, selected mode host: invalid argument", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + got, got1, got2, err := ParseNetworkFlag(tt.args) + if tt.err != "" { + assert.EqualError(t, err, tt.err, tt.name) + } else { + assert.NoError(t, err, tt.name) + } + + assert.Equal(t, tt.nsmode, got, tt.name) + assert.Equal(t, tt.networks, got1, tt.name) + assert.Equal(t, tt.options, got2, tt.name) + }) + } +} diff --git a/pkg/specgen/pod_validate.go b/pkg/specgen/pod_validate.go index 32c1159c60..224a5b12d0 100644 --- a/pkg/specgen/pod_validate.go +++ b/pkg/specgen/pod_validate.go @@ -42,6 +42,11 @@ func (p *PodSpecGenerator) Validate() error { if p.NetNS.NSMode != Default && p.NetNS.NSMode != "" { return errors.New("NoInfra and network modes cannot be used together") } + // Note that networks might be set when --ip or --mac was set + // so we need to check that no networks are set without the infra + if len(p.Networks) > 0 { + return errors.New("cannot set networks options without infra container") + } if len(p.DNSOption) > 0 { return exclusivePodOptions("NoInfra", "DNSOption") } diff --git a/test/e2e/create_staticip_test.go b/test/e2e/create_staticip_test.go index 205855fd67..ded4c03e0a 100644 --- a/test/e2e/create_staticip_test.go +++ b/test/e2e/create_staticip_test.go @@ -43,12 +43,6 @@ var _ = Describe("Podman create with --ip flag", func() { Expect(result).To(ExitWithError()) }) - It("Podman create --ip with v6 address", func() { - result := podmanTest.Podman([]string{"create", "--name", "test", "--ip", "2001:db8:bad:beef::1", ALPINE, "ls"}) - result.WaitWithDefaultTimeout() - Expect(result).To(ExitWithError()) - }) - It("Podman create --ip with non-allocatable IP", func() { SkipIfRootless("--ip not supported without network in rootless mode") result := podmanTest.Podman([]string{"create", "--name", "test", "--ip", "203.0.113.124", ALPINE, "ls"}) diff --git a/test/e2e/network_connect_disconnect_test.go b/test/e2e/network_connect_disconnect_test.go index be94ecb2d3..23281fe054 100644 --- a/test/e2e/network_connect_disconnect_test.go +++ b/test/e2e/network_connect_disconnect_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gexec" + "github.com/onsi/gomega/types" ) var _ = Describe("Podman network connect and disconnect", func() { @@ -330,11 +331,17 @@ var _ = Describe("Podman network connect and disconnect", func() { exec := podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth0"}) exec.WaitWithDefaultTimeout() - Expect(exec).Should(Exit(0)) + + // because the network interface order is not guaranteed to be the same we have to check both eth0 and eth1 + // if eth0 did not exists eth1 has to exists + var exitMatcher types.GomegaMatcher = ExitWithError() + if exec.ExitCode() > 0 { + exitMatcher = Exit(0) + } exec = podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth1"}) exec.WaitWithDefaultTimeout() - Expect(exec).Should(ExitWithError()) + Expect(exec).Should(exitMatcher) }) It("podman network disconnect and run with network ID", func() { diff --git a/test/e2e/run_staticip_test.go b/test/e2e/run_staticip_test.go index 6dd7a14d0e..eb7dc9d11e 100644 --- a/test/e2e/run_staticip_test.go +++ b/test/e2e/run_staticip_test.go @@ -65,6 +65,26 @@ var _ = Describe("Podman run with --ip flag", func() { Expect(result.OutputToString()).To(ContainSubstring(ip + "/16")) }) + It("Podman run with --network bridge:ip=", func() { + ip := GetRandomIPAddress() + result := podmanTest.Podman([]string{"run", "-ti", "--network", "bridge:ip=" + ip, ALPINE, "ip", "addr"}) + result.WaitWithDefaultTimeout() + Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(ip + "/16")) + }) + + It("Podman run with --network net:ip=,mac=,interface_name=", func() { + ip := GetRandomIPAddress() + mac := "44:33:22:11:00:99" + intName := "myeth" + result := podmanTest.Podman([]string{"run", "-ti", "--network", "bridge:ip=" + ip + ",mac=" + mac + ",interface_name=" + intName, ALPINE, "ip", "addr"}) + result.WaitWithDefaultTimeout() + Expect(result).Should(Exit(0)) + Expect(result.OutputToString()).To(ContainSubstring(ip + "/16")) + Expect(result.OutputToString()).To(ContainSubstring(mac)) + Expect(result.OutputToString()).To(ContainSubstring(intName)) + }) + It("Podman run two containers with the same IP", func() { ip := GetRandomIPAddress() result := podmanTest.Podman([]string{"run", "-dt", "--ip", ip, nginx}) diff --git a/test/system/030-run.bats b/test/system/030-run.bats index 5937d38f8c..6f1fa600a6 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -586,9 +586,7 @@ json-file | f @test "podman run with --net=host and --port prints warning" { rand=$(random_string 10) - # Please keep the duplicate "--net" options; this tests against #8507, - # a regression in which subsequent --net options did not override earlier. - run_podman run --rm -p 8080 --net=none --net=host $IMAGE echo $rand + run_podman run --rm -p 8080 --net=host $IMAGE echo $rand is "${lines[0]}" \ "Port mappings have been discarded as one of the Host, Container, Pod, and None network modes are in use" \ "Warning is emitted before container output" From 3e9af2029f1f92fbc069f81ba9ca90c090617e9c Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 13 Dec 2021 15:56:20 +0100 Subject: [PATCH 08/10] play kube add support for multiple networks Allow the same --network options for play kube as for podman run/create. Signed-off-by: Paul Holzinger --- cmd/podman/play/kube.go | 2 +- docs/source/markdown/podman-play-kube.1.md | 18 ++++-- pkg/api/handlers/libpod/play.go | 4 +- pkg/api/server/register_play.go | 6 +- pkg/bindings/play/types.go | 4 +- pkg/bindings/play/types_kube_options.go | 6 +- pkg/domain/entities/play.go | 4 +- pkg/domain/infra/abi/play.go | 69 +++++++++++++--------- pkg/domain/infra/tunnel/play.go | 2 +- test/e2e/play_kube_test.go | 35 +++++++++++ test/system/700-play.bats | 2 - 11 files changed, 105 insertions(+), 47 deletions(-) diff --git a/cmd/podman/play/kube.go b/cmd/podman/play/kube.go index 11b5d7d348..ae4066a6f2 100644 --- a/cmd/podman/play/kube.go +++ b/cmd/podman/play/kube.go @@ -69,7 +69,7 @@ func init() { _ = kubeCmd.RegisterFlagCompletionFunc(staticMACFlagName, completion.AutocompleteNone) networkFlagName := "network" - flags.StringVar(&kubeOptions.Network, networkFlagName, "", "Connect pod to CNI network(s)") + flags.StringArrayVar(&kubeOptions.Networks, networkFlagName, nil, "Connect pod to network(s) or network mode") _ = kubeCmd.RegisterFlagCompletionFunc(networkFlagName, common.AutocompleteNetworkFlag) staticIPFlagName := "ip" diff --git a/docs/source/markdown/podman-play-kube.1.md b/docs/source/markdown/podman-play-kube.1.md index 075fbbe81f..81ab651765 100644 --- a/docs/source/markdown/podman-play-kube.1.md +++ b/docs/source/markdown/podman-play-kube.1.md @@ -142,6 +142,7 @@ removed. Any volumes created are left intact. #### **--ip**=*IP address* Assign a static ip address to the pod. This option can be specified several times when play kube creates more than one pod. +Note: When joining multiple networks you should use the **--network name:ip=\** syntax. #### **--log-driver**=driver @@ -167,15 +168,24 @@ This option is currently supported only by the **journald** log driver. #### **--mac-address**=*MAC address* Assign a static mac address to the pod. This option can be specified several times when play kube creates more than one pod. +Note: When joining multiple networks you should use the **--network name:mac=\** syntax. #### **--network**=*mode*, **--net** -Change the network mode of the pod. The host and bridge network mode should be configured in the yaml file. +Change the network mode of the pod. The host network mode should be configured in the YAML file. Valid _mode_ values are: +- **bridge[:OPTIONS,...]**: Create a network stack on the default bridge. This is the default for rootfull containers. It is possible to specify these additional options: + - **alias=name**: Add network-scoped alias for the container. + - **ip=IPv4**: Specify a static ipv4 address for this container. + - **ip=IPv6**: Specify a static ipv6 address for this container. + - **mac=MAC**: Specify a static mac address address for this container. + - **interface_name**: Specify a name for the created network interface inside the container. + + For example to set a static ipv4 address and a static mac address, use `--network bridge:ip=10.88.0.10,mac=44:33:22:11:00:99`. +- \[:OPTIONS,...]: Connect to a user-defined network; this is the network name or ID from a network created by **[podman network create](podman-network-create.1.md)**. Using the network name implies the bridge network mode. It is possible to specify the same options described under the bridge mode above. You can use the **--network** option multiple times to specify additional networks. - **none**: Create a network namespace for the container but do not configure network interfaces for it, thus the container has no network connectivity. - **container:**_id_: Reuse another container's network stack. -- **network**: 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. This will use the **bridge** mode for rootfull containers and **slirp4netns** for rootless ones. - **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: @@ -253,9 +263,9 @@ $ podman play kube demo.yml --configmap configmap-foo.yml --configmap configmap- 52182811df2b1e73f36476003a66ec872101ea59034ac0d4d3a7b40903b955a6 ``` -CNI network(s) can be specified as comma-separated list using ``--network`` +Create a pod connected to two networks (called net1 and net2) with a static ip ``` -$ podman play kube demo.yml --network cni1,cni2 +$ podman play kube demo.yml --network net1:ip=10.89.1.5 --network net2:ip=10.89.10.10 52182811df2b1e73f36476003a66ec872101ea59034ac0d4d3a7b40903b955a6 ``` diff --git a/pkg/api/handlers/libpod/play.go b/pkg/api/handlers/libpod/play.go index f943fc2402..312aa32de5 100644 --- a/pkg/api/handlers/libpod/play.go +++ b/pkg/api/handlers/libpod/play.go @@ -23,7 +23,7 @@ func PlayKube(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime) decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder) query := struct { - Network string `schema:"network"` + Network []string `schema:"network"` TLSVerify bool `schema:"tlsVerify"` LogDriver string `schema:"logDriver"` LogOptions []string `schema:"logOptions"` @@ -103,7 +103,7 @@ func PlayKube(w http.ResponseWriter, r *http.Request) { Authfile: authfile, Username: username, Password: password, - Network: query.Network, + Networks: query.Network, NoHosts: query.NoHosts, Quiet: true, LogDriver: query.LogDriver, diff --git a/pkg/api/server/register_play.go b/pkg/api/server/register_play.go index 915d0d02ed..5ace01929f 100644 --- a/pkg/api/server/register_play.go +++ b/pkg/api/server/register_play.go @@ -18,8 +18,10 @@ func (s *APIServer) registerPlayHandlers(r *mux.Router) error { // parameters: // - in: query // name: network - // type: string - // description: Connect the pod to this network. + // type: array + // description: USe the network mode or specify an array of networks. + // items: + // type: string // - in: query // name: tlsVerify // type: boolean diff --git a/pkg/bindings/play/types.go b/pkg/bindings/play/types.go index 011f7f9caa..ca639e46b9 100644 --- a/pkg/bindings/play/types.go +++ b/pkg/bindings/play/types.go @@ -15,8 +15,8 @@ type KubeOptions struct { Username *string // Password for authenticating against the registry. Password *string - // Network - name of the CNI network to connect to. - Network *string + // Network - name of the networks to connect to. + Network *[]string // NoHosts - do not generate /etc/hosts file in pod's containers NoHosts *bool // Quiet - suppress output when pulling images. diff --git a/pkg/bindings/play/types_kube_options.go b/pkg/bindings/play/types_kube_options.go index 344771e0c9..593f026a33 100644 --- a/pkg/bindings/play/types_kube_options.go +++ b/pkg/bindings/play/types_kube_options.go @@ -79,15 +79,15 @@ func (o *KubeOptions) GetPassword() string { } // WithNetwork set field Network to given value -func (o *KubeOptions) WithNetwork(value string) *KubeOptions { +func (o *KubeOptions) WithNetwork(value []string) *KubeOptions { o.Network = &value return o } // GetNetwork returns value of field Network -func (o *KubeOptions) GetNetwork() string { +func (o *KubeOptions) GetNetwork() []string { if o.Network == nil { - var z string + var z []string return z } return *o.Network diff --git a/pkg/domain/entities/play.go b/pkg/domain/entities/play.go index ad35dfe254..39234caf8a 100644 --- a/pkg/domain/entities/play.go +++ b/pkg/domain/entities/play.go @@ -26,8 +26,8 @@ type PlayKubeOptions struct { Username string // Password for authenticating against the registry. Password string - // Network - name of the CNI network to connect to. - Network string + // Networks - name of the network to connect to. + Networks []string // Quiet - suppress output when pulling images. Quiet bool // SignaturePolicy - path to a signature-policy file. diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index 409ba938a4..6b3b04a0bd 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -17,6 +17,7 @@ import ( "github.com/containers/image/v5/types" "github.com/containers/podman/v3/libpod" "github.com/containers/podman/v3/libpod/define" + nettypes "github.com/containers/podman/v3/libpod/network/types" "github.com/containers/podman/v3/pkg/autoupdate" "github.com/containers/podman/v3/pkg/domain/entities" "github.com/containers/podman/v3/pkg/specgen" @@ -195,39 +196,51 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY return nil, err } - if options.Network != "" { - ns, networks, netOpts, err := specgen.ParseNetworkFlag([]string{options.Network}) - if err != nil { - return nil, err - } + ns, networks, netOpts, err := specgen.ParseNetworkFlag(options.Networks) + if err != nil { + return nil, err + } - if (ns.IsBridge() && len(networks) == 0) || ns.IsHost() { - return nil, errors.Errorf("invalid value passed to --network: bridge or host networking must be configured in YAML") - } + if (ns.IsBridge() && len(networks) == 0) || ns.IsHost() { + return nil, errors.Errorf("invalid value passed to --network: bridge or host networking must be configured in YAML") + } - podOpt.Net.Network = ns - if len(networks) > 0 { - podOpt.Net.Networks = networks + podOpt.Net.Network = ns + podOpt.Net.Networks = networks + podOpt.Net.NetworkOptions = netOpts + + // FIXME This is very hard to support properly with a good ux + if len(options.StaticIPs) > *ipIndex { + if !podOpt.Net.Network.IsBridge() { + errors.Wrap(define.ErrInvalidArg, "static ip addresses can only be set when the network mode is bridge") + } + if len(podOpt.Net.Networks) != 1 { + return nil, errors.Wrap(define.ErrInvalidArg, "cannot set static ip addresses for more than network, use netname:ip= syntax to specify ips for more than network") } - if len(netOpts) > 0 { - podOpt.Net.NetworkOptions = netOpts + for name, netOpts := range podOpt.Net.Networks { + netOpts.StaticIPs = append(netOpts.StaticIPs, options.StaticIPs[*ipIndex]) + podOpt.Net.Networks[name] = netOpts } + } else if len(options.StaticIPs) > 0 { + // only warn if the user has set at least one ip + logrus.Warn("No more static ips left using a random one") } - - // FIXME This is very hard to support properly - // if len(options.StaticIPs) > *ipIndex { - // podOpt.Net.StaticIP = &options.StaticIPs[*ipIndex] - // } else if len(options.StaticIPs) > 0 { - // // only warn if the user has set at least one ip - // logrus.Warn("No more static ips left using a random one") - // } - // if len(options.StaticMACs) > *ipIndex { - // podOpt.Net.StaticMAC = &options.StaticMACs[*ipIndex] - // } else if len(options.StaticIPs) > 0 { - // // only warn if the user has set at least one mac - // logrus.Warn("No more static macs left using a random one") - // } - // *ipIndex++ + if len(options.StaticMACs) > *ipIndex { + if !podOpt.Net.Network.IsBridge() { + errors.Wrap(define.ErrInvalidArg, "static mac address can only be set when the network mode is bridge") + } + if len(podOpt.Net.Networks) != 1 { + return nil, errors.Wrap(define.ErrInvalidArg, "cannot set static mac address for more than network, use netname:mac= syntax to specify mac for more than network") + } + for name, netOpts := range podOpt.Net.Networks { + netOpts.StaticMAC = nettypes.HardwareAddr(options.StaticMACs[*ipIndex]) + podOpt.Net.Networks[name] = netOpts + } + } else if len(options.StaticIPs) > 0 { + // only warn if the user has set at least one mac + logrus.Warn("No more static macs left using a random one") + } + *ipIndex++ p := specgen.NewPodSpecGenerator() if err != nil { diff --git a/pkg/domain/infra/tunnel/play.go b/pkg/domain/infra/tunnel/play.go index 75952ce2cc..103be0cf12 100644 --- a/pkg/domain/infra/tunnel/play.go +++ b/pkg/domain/infra/tunnel/play.go @@ -11,7 +11,7 @@ import ( func (ic *ContainerEngine) PlayKube(ctx context.Context, path string, opts entities.PlayKubeOptions) (*entities.PlayKubeReport, error) { options := new(play.KubeOptions).WithAuthfile(opts.Authfile).WithUsername(opts.Username).WithPassword(opts.Password) options.WithCertDir(opts.CertDir).WithQuiet(opts.Quiet).WithSignaturePolicy(opts.SignaturePolicy).WithConfigMaps(opts.ConfigMaps) - options.WithLogDriver(opts.LogDriver).WithNetwork(opts.Network).WithSeccompProfileRoot(opts.SeccompProfileRoot) + options.WithLogDriver(opts.LogDriver).WithNetwork(opts.Networks).WithSeccompProfileRoot(opts.SeccompProfileRoot) options.WithStaticIPs(opts.StaticIPs).WithStaticMACs(opts.StaticMACs) if len(opts.LogOptions) > 0 { options.WithLogOptions(opts.LogOptions) diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 36010704fe..f79710ee69 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -2136,6 +2136,41 @@ spec: } }) + It("podman play kube with multiple networks", func() { + ctr := getCtr(withImage(ALPINE)) + pod := getPod(withCtr(ctr)) + err := generateKubeYaml("pod", pod, kubeYaml) + Expect(err).To(BeNil()) + + net1 := "net1" + stringid.GenerateNonCryptoID() + net2 := "net2" + stringid.GenerateNonCryptoID() + + net := podmanTest.Podman([]string{"network", "create", "--subnet", "10.0.11.0/24", net1}) + net.WaitWithDefaultTimeout() + defer podmanTest.removeCNINetwork(net1) + Expect(net).Should(Exit(0)) + + net = podmanTest.Podman([]string{"network", "create", "--subnet", "10.0.12.0/24", net2}) + net.WaitWithDefaultTimeout() + defer podmanTest.removeCNINetwork(net2) + Expect(net).Should(Exit(0)) + + ip1 := "10.0.11.5" + ip2 := "10.0.12.10" + + kube := podmanTest.Podman([]string{"play", "kube", "--network", net1 + ":ip=" + ip1, "--network", net2 + ":ip=" + ip2, kubeYaml}) + kube.WaitWithDefaultTimeout() + Expect(kube).Should(Exit(0)) + + inspect := podmanTest.Podman([]string{"exec", getCtrNameInPod(pod), "ip", "addr"}) + inspect.WaitWithDefaultTimeout() + Expect(inspect).Should(Exit(0)) + Expect(inspect.OutputToString()).To(ContainSubstring(ip1)) + Expect(inspect.OutputToString()).To(ContainSubstring(ip2)) + Expect(inspect.OutputToString()).To(ContainSubstring("eth0")) + Expect(inspect.OutputToString()).To(ContainSubstring("eth1")) + }) + It("podman play kube test with network portbindings", func() { ip := "127.0.0.100" port := "5000" diff --git a/test/system/700-play.bats b/test/system/700-play.bats index b77d419205..88c7cad87a 100644 --- a/test/system/700-play.bats +++ b/test/system/700-play.bats @@ -104,8 +104,6 @@ RELABEL="system_u:object_r:container_file_t:s0" TESTDIR=$PODMAN_TMPDIR/testdir mkdir -p $TESTDIR echo "$testYaml" | sed "s|TESTDIR|${TESTDIR}|g" > $PODMAN_TMPDIR/test.yaml - run_podman 125 play kube --network bridge $PODMAN_TMPDIR/test.yaml - is "$output" ".*invalid value passed to --network: bridge or host networking must be configured in YAML" "podman plan-network should fail with --network host" run_podman 125 play kube --network host $PODMAN_TMPDIR/test.yaml is "$output" ".*invalid value passed to --network: bridge or host networking must be configured in YAML" "podman plan-network should fail with --network host" run_podman play kube --network slirp4netns:port_handler=slirp4netns $PODMAN_TMPDIR/test.yaml From 094e1d70dee1f5cc04987169615898bd6a4af499 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 13 Dec 2021 18:20:31 +0100 Subject: [PATCH 09/10] container restore/import: store networks from db It is important that we store the current networks from the db in the config. Also make sure to properly handle aliases and ignore static ip/mac addresses. Signed-off-by: Paul Holzinger --- libpod/container_internal.go | 18 ++++++++++++++++++ pkg/checkpoint/checkpoint_restore.go | 12 ++++++++++++ 2 files changed, 30 insertions(+) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index d8187c609a..a68de31735 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -2206,6 +2206,24 @@ func (c *Container) canWithPrevious() error { // prepareCheckpointExport writes the config and spec to // JSON files for later export func (c *Container) prepareCheckpointExport() error { + networks, err := c.networks() + if err != nil { + return err + } + // make sure to exclude the short ID alias since the container gets a new ID on restore + for net, opts := range networks { + newAliases := make([]string, 0, len(opts.Aliases)) + for _, alias := range opts.Aliases { + if alias != c.config.ID[:12] { + newAliases = append(newAliases, alias) + } + } + opts.Aliases = newAliases + networks[net] = opts + } + + // add the networks from the db to the config so that the exported checkpoint still stores all current networks + c.config.Networks = networks // save live config if _, err := metadata.WriteJSONFile(c.config, c.bundlePath(), metadata.ConfigDumpFile); err != nil { return err diff --git a/pkg/checkpoint/checkpoint_restore.go b/pkg/checkpoint/checkpoint_restore.go index c371adf5b1..34bd8a124f 100644 --- a/pkg/checkpoint/checkpoint_restore.go +++ b/pkg/checkpoint/checkpoint_restore.go @@ -78,6 +78,18 @@ func CRImportCheckpoint(ctx context.Context, runtime *libpod.Runtime, restoreOpt } } + if restoreOptions.IgnoreStaticIP || restoreOptions.IgnoreStaticMAC { + for net, opts := range ctrConfig.Networks { + if restoreOptions.IgnoreStaticIP { + opts.StaticIPs = nil + } + if restoreOptions.IgnoreStaticMAC { + opts.StaticMAC = nil + } + ctrConfig.Networks[net] = opts + } + } + ctrID := ctrConfig.ID newName := false From ef325bc8c4824537e4bfb21aa7e6114a6e5a8c09 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 13 Dec 2021 20:36:25 +0100 Subject: [PATCH 10/10] specgen: check that networks are only set with bridge Because we cannot reqad the networking mode in the frontent because we should always use the server default we have to parse the mac and ip address to the server via a default network. Now when the server reads the default nsmode it has to reject the provided networks when the mode is not set to bridge. Signed-off-by: Paul Holzinger --- pkg/specgen/container_validate.go | 5 +++++ pkg/specgen/pod_validate.go | 4 +--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/specgen/container_validate.go b/pkg/specgen/container_validate.go index cae231f0e4..d06a047c1b 100644 --- a/pkg/specgen/container_validate.go +++ b/pkg/specgen/container_validate.go @@ -189,5 +189,10 @@ func (s *SpecGenerator) Validate() error { if err := validateNetNS(&s.NetNS); err != nil { return err } + if s.NetNS.NSMode != Bridge && len(s.Networks) > 0 { + // Note that we also get the ip and mac in the networks map + return errors.New("Networks and static ip/mac address can only be used with Bridge mode networking") + } + return nil } diff --git a/pkg/specgen/pod_validate.go b/pkg/specgen/pod_validate.go index 224a5b12d0..c5a66189c9 100644 --- a/pkg/specgen/pod_validate.go +++ b/pkg/specgen/pod_validate.go @@ -67,10 +67,8 @@ func (p *PodSpecGenerator) Validate() error { if len(p.PortMappings) > 0 { return errors.New("PortMappings can only be used with Bridge or slirp4netns networking") } - if len(p.Networks) > 0 { - return errors.New("Networks can only be used with Bridge mode networking") - } } + if p.NoManageResolvConf { if len(p.DNSServer) > 0 { return exclusivePodOptions("NoManageResolvConf", "DNSServer")