Make c.networks() list include the default network

This makes things a lot more clear - if we are actually joining a
CNI network, we are guaranteed to get a non-zero length list of
networks.

We do, however, need to know if the network we are joining is the
default network for inspecting containers as it determines how we
populate the response struct. To handle this, add a bool to
indicate that the network listed was the default network, and
only the default network.

Signed-off-by: Matthew Heon <mheon@redhat.com>
This commit is contained in:
Matthew Heon
2020-11-20 13:49:40 -05:00
parent 864fe21ed0
commit ce775248ad
4 changed files with 31 additions and 23 deletions

View File

@ -13,6 +13,7 @@ import (
"github.com/containers/image/v5/manifest" "github.com/containers/image/v5/manifest"
"github.com/containers/podman/v2/libpod/define" "github.com/containers/podman/v2/libpod/define"
"github.com/containers/podman/v2/libpod/lock" "github.com/containers/podman/v2/libpod/lock"
"github.com/containers/podman/v2/pkg/rootless"
"github.com/containers/storage" "github.com/containers/storage"
"github.com/cri-o/ocicni/pkg/ocicni" "github.com/cri-o/ocicni/pkg/ocicni"
spec "github.com/opencontainers/runtime-spec/specs-go" spec "github.com/opencontainers/runtime-spec/specs-go"
@ -1095,13 +1096,17 @@ func (c *Container) Umask() string {
// values at runtime via network connect and disconnect. // values at runtime via network connect and disconnect.
// If the container is configured to use CNI and this function returns an empty // If the container is configured to use CNI and this function returns an empty
// array, the container will still be connected to the default network. // array, the container will still be connected to the default network.
func (c *Container) Networks() ([]string, error) { // The second return parameter, a bool, indicates that the container container
// 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) {
if !c.batched { if !c.batched {
c.lock.Lock() c.lock.Lock()
defer c.lock.Unlock() defer c.lock.Unlock()
if err := c.syncContainer(); err != nil { if err := c.syncContainer(); err != nil {
return nil, err return nil, false, err
} }
} }
@ -1109,19 +1114,22 @@ func (c *Container) Networks() ([]string, error) {
} }
// Unlocked accessor for networks // Unlocked accessor for networks
func (c *Container) networks() ([]string, error) { func (c *Container) networks() ([]string, bool, error) {
networks, err := c.runtime.state.GetNetworks(c) networks, err := c.runtime.state.GetNetworks(c)
if err != nil && errors.Cause(err) == define.ErrNoSuchNetwork { if err != nil && errors.Cause(err) == define.ErrNoSuchNetwork {
return c.config.Networks, nil if len(c.config.Networks) == 0 && !rootless.IsRootless() {
return []string{c.runtime.netPlugin.GetDefaultNetworkName()}, true, nil
}
return c.config.Networks, false, nil
} }
return networks, err return networks, false, err
} }
// networksByNameIndex provides us with a map of container networks where key // networksByNameIndex provides us with a map of container networks where key
// is network name and value is the index position // is network name and value is the index position
func (c *Container) networksByNameIndex() (map[string]int, error) { func (c *Container) networksByNameIndex() (map[string]int, error) {
networks, err := c.networks() networks, _, err := c.networks()
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@ -641,18 +641,13 @@ func (c *Container) removeIPv4Allocations() error {
cniDefaultNetwork = c.runtime.netPlugin.GetDefaultNetworkName() cniDefaultNetwork = c.runtime.netPlugin.GetDefaultNetworkName()
} }
networks, err := c.networks() networks, _, err := c.networks()
if err != nil { if err != nil {
return err return err
} }
switch { if len(networks) != len(c.state.NetworkStatus) {
case len(networks) > 0 && len(networks) != len(c.state.NetworkStatus):
return errors.Wrapf(define.ErrInternal, "network mismatch: asked to join %d CNI networks but got %d CNI results", len(networks), len(c.state.NetworkStatus)) return errors.Wrapf(define.ErrInternal, "network mismatch: asked to join %d CNI networks but got %d CNI results", len(networks), len(c.state.NetworkStatus))
case len(networks) == 0 && len(c.state.NetworkStatus) != 1:
return errors.Wrapf(define.ErrInternal, "network mismatch: did not specify CNI networks but joined more than one (%d)", len(c.state.NetworkStatus))
case len(networks) == 0 && cniDefaultNetwork == "":
return errors.Wrapf(define.ErrInternal, "could not retrieve name of CNI default network")
} }
for index, result := range c.state.NetworkStatus { for index, result := range c.state.NetworkStatus {

View File

@ -110,10 +110,15 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) ([]*cnitypes.Re
podName := getCNIPodName(ctr) podName := getCNIPodName(ctr)
networks, err := ctr.networks() networks, _, err := ctr.networks()
if err != nil { if err != nil {
return nil, err return nil, err
} }
// All networks have been removed from the container.
// This is effectively forcing net=none.
if len(networks) == 0 {
return nil, nil
}
// Update container map of interface descriptions // Update container map of interface descriptions
if err := ctr.setupNetworkDescriptions(networks); err != nil { if err := ctr.setupNetworkDescriptions(networks); err != nil {
@ -224,7 +229,7 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) error {
if ctr.config.NetMode.IsSlirp4netns() { if ctr.config.NetMode.IsSlirp4netns() {
return r.setupSlirp4netns(ctr) return r.setupSlirp4netns(ctr)
} }
networks, err := ctr.networks() networks, _, err := ctr.networks()
if err != nil { if err != nil {
return err return err
} }
@ -744,13 +749,13 @@ func (r *Runtime) teardownNetNS(ctr *Container) error {
logrus.Debugf("Tearing down network namespace at %s for container %s", ctr.state.NetNS.Path(), ctr.ID()) 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 { if err != nil {
return err return err
} }
// rootless containers do not use the CNI plugin directly // rootless containers do not use the CNI plugin directly
if !rootless.IsRootless() && !ctr.config.NetMode.IsSlirp4netns() { if !rootless.IsRootless() && !ctr.config.NetMode.IsSlirp4netns() && len(networks) > 0 {
var requestedIP net.IP var requestedIP net.IP
if ctr.requestedIP != nil { if ctr.requestedIP != nil {
requestedIP = ctr.requestedIP requestedIP = ctr.requestedIP
@ -863,7 +868,7 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e
settings := new(define.InspectNetworkSettings) settings := new(define.InspectNetworkSettings)
settings.Ports = makeInspectPortBindings(c.config.PortMappings) settings.Ports = makeInspectPortBindings(c.config.PortMappings)
networks, err := c.networks() networks, isDefault, err := c.networks()
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -872,7 +877,7 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e
if c.state.NetNS == nil { if c.state.NetNS == nil {
// We still want to make dummy configurations for each CNI net // We still want to make dummy configurations for each CNI net
// the container joined. // the container joined.
if len(networks) > 0 { if len(networks) > 0 && !isDefault {
settings.Networks = make(map[string]*define.InspectAdditionalNetwork, len(networks)) settings.Networks = make(map[string]*define.InspectAdditionalNetwork, len(networks))
for _, net := range networks { for _, net := range networks {
cniNet := new(define.InspectAdditionalNetwork) cniNet := new(define.InspectAdditionalNetwork)
@ -893,7 +898,7 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e
} }
// If we have CNI networks - handle that here // If we have CNI networks - handle that here
if len(networks) > 0 { if len(networks) > 0 && !isDefault {
if len(networks) != len(c.state.NetworkStatus) { if len(networks) != len(c.state.NetworkStatus) {
return nil, errors.Wrapf(define.ErrInternal, "network inspection mismatch: asked to join %d CNI networks but have information on %d networks", len(networks), len(c.state.NetworkStatus)) return nil, errors.Wrapf(define.ErrInternal, "network inspection mismatch: asked to join %d CNI networks but have information on %d networks", len(networks), len(c.state.NetworkStatus))
} }
@ -1101,7 +1106,7 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e
return err return err
} }
ctrNetworks, err := c.networks() ctrNetworks, _, err := c.networks()
if err != nil { if err != nil {
return err return err
} }

View File

@ -40,7 +40,7 @@ const (
// //
// AllocRootlessCNI does not lock c. c should be already locked. // AllocRootlessCNI does not lock c. c should be already locked.
func AllocRootlessCNI(ctx context.Context, c *Container) (ns.NetNS, []*cnitypes.Result, error) { func AllocRootlessCNI(ctx context.Context, c *Container) (ns.NetNS, []*cnitypes.Result, error) {
networks, err := c.networks() networks, _, err := c.networks()
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -81,7 +81,7 @@ func AllocRootlessCNI(ctx context.Context, c *Container) (ns.NetNS, []*cnitypes.
// //
// DeallocRootlessCNI does not lock c. c should be already locked. // DeallocRootlessCNI does not lock c. c should be already locked.
func DeallocRootlessCNI(ctx context.Context, c *Container) error { func DeallocRootlessCNI(ctx context.Context, c *Container) error {
networks, err := c.networks() networks, _, err := c.networks()
if err != nil { if err != nil {
return err return err
} }