From 5490be67b3620c7a13319f4a18519ab691c20bef Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 7 Dec 2021 19:04:13 +0100 Subject: 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(-) (limited to 'libpod') diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index 1242a8d6b..d322ccc53 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 2b74a1943..006661d5b 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 ef9f13f44..f4b629a83 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 5fb97dd73..3f56be855 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 4b711bae9..fe7b86fdb 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 -- cgit v1.2.3-54-g00ecf From 4e8ad039cee5debcc1afe93a455cd8a25e88d16b Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 7 Dec 2021 20:43:04 +0100 Subject: 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(-) (limited to 'libpod') diff --git a/libpod/container.go b/libpod/container.go index 006661d5b..1270f2112 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 269cd2d27..bd5167fa5 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 9a2f0a3ba..59a6d0d78 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 ee7403ed5..49c7dc60d 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 028de9e81..c8c7b0d9e 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 90ad23f49..391bd54d6 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 } -- cgit v1.2.3-54-g00ecf From 9ce6b64133bc37433efac391bcaf9234c3890fc5 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 7 Dec 2021 23:04:47 +0100 Subject: 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(-) (limited to 'libpod') diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index d322ccc53..9669cf921 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 e71d82736..5f0f6ba7d 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 57f5b92ac..adc585fa1 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 91ebe93fb..ca5ce8b2a 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 105641e70..37fa11461 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 8f2d5cb15..dbcc50741 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 80ecb690a..95a82721e 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 6c46eb747..59a1fd153 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 fe7b86fdb..21525107f 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 5c3b0d7f7..b0793127d 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 df5d2e8ff..331c9393a 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 7d63fc10f..ebdd2abd0 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)) } -- cgit v1.2.3-54-g00ecf From 4791595b5cb3b2350c66b0d1ba91a6a171652409 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 8 Dec 2021 15:31:49 +0100 Subject: 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 ++++++++++++++- docs/source/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 (limited to 'libpod') diff --git a/cmd/podman/networks/connect.go b/cmd/podman/networks/connect.go index 0d62a45df..b0ffbfe6d 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 b998d4b7e..c3eef4038 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 3f56be855..a931774f8 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 8aab29658..db3af7d0b 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 1f7f2e26c..a28c3c57c 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 172598be1..66e01a016 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 8088de061..b82c0e438 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 b7a465999..000000000 --- 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 d7389a699..34b89ae7d 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 49c7dc60d..c7b12663c 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 069982d30..b5050345a 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 2205a1263..be94ecb2d 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"}) -- cgit v1.2.3-54-g00ecf From d072167fe2f75db9648bf1be4181b42e9b7db9a4 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 9 Dec 2021 15:59:54 +0100 Subject: 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(-) (limited to 'libpod') diff --git a/cmd/podman/common/create_opts.go b/cmd/podman/common/create_opts.go index 7d6471fd4..8f0cd1be6 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 17b47dd16..601078b61 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 d11f3c9d2..f79a9fd88 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 dbcc50741..e6fa987a8 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 b255785c2..14127e468 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 e062b9442..0348c0af5 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 4c024a3d8..b0b68567a 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 caea51ea8..cae231f0e 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 ebdd2abd0..782156663 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 72dd249e7..0a797c571 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 bb5385ef1..121e1ecf7 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 bca7b6dbe..32c1159c6 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 948fb990c..e59d11c0a 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 0e257ad4c..e650c1966 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 5e4bd2f65..123c0073b 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 } -- cgit v1.2.3-54-g00ecf From 094e1d70dee1f5cc04987169615898bd6a4af499 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 13 Dec 2021 18:20:31 +0100 Subject: 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(+) (limited to 'libpod') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index d8187c609..a68de3173 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 c371adf5b..34bd8a124 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 -- cgit v1.2.3-54-g00ecf