From 0bc3d35791a86e53bedadd72afff2c0d53a290f2 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 6 Dec 2022 16:15:26 +0100 Subject: [PATCH] libpod: move NetNS into state db instead of extra bucket This should simplify the db logic. We no longer need a extra db bucket for the netns, it is still supported in read only mode for backwards compat. The old version required us to always open the netns before we could attach it to the container state struct which caused problem in some cases were the netns was no longer valid. Now we use the netns as string throughout the code, this allow us to only open it when needed reducing possible errors. [NO NEW TESTS NEEDED] Existing tests should cover it and it is only a flake so hard to reproduce the error. Fixes #16140 Signed-off-by: Paul Holzinger --- libpod/boltdb_state.go | 24 ++-------- libpod/boltdb_state_freebsd.go | 25 ---------- libpod/boltdb_state_internal.go | 6 --- libpod/boltdb_state_linux.go | 57 ----------------------- libpod/container.go | 5 +- libpod/container_freebsd.go | 19 +------- libpod/container_internal.go | 3 ++ libpod/container_internal_freebsd.go | 18 ++++---- libpod/container_internal_linux.go | 13 +++--- libpod/container_linux.go | 8 ---- libpod/networking_common.go | 20 ++++---- libpod/networking_freebsd.go | 42 ++++++++--------- libpod/networking_linux.go | 69 ++++++---------------------- libpod/networking_pasta_linux.go | 5 +- libpod/networking_slirp4netns.go | 4 +- 15 files changed, 73 insertions(+), 245 deletions(-) delete mode 100644 libpod/boltdb_state_freebsd.go delete mode 100644 libpod/boltdb_state_linux.go diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index 4962fd5668..de9e88c737 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "net" - "os" "strconv" "strings" "sync" @@ -817,7 +816,6 @@ func (s *BoltState) UpdateContainer(ctr *Container) error { } newState := new(ContainerState) - netNSPath := "" ctrID := []byte(ctr.ID()) @@ -848,9 +846,10 @@ func (s *BoltState) UpdateContainer(ctr *Container) error { return fmt.Errorf("unmarshalling container %s state: %w", ctr.ID(), err) } + // backwards compat, previously we used a extra bucket for the netns so try to get it from there netNSBytes := ctrToUpdate.Get(netNSKey) - if netNSBytes != nil { - netNSPath = string(netNSBytes) + if netNSBytes != nil && newState.NetNS == "" { + newState.NetNS = string(netNSBytes) } return nil @@ -859,15 +858,6 @@ func (s *BoltState) UpdateContainer(ctr *Container) error { return err } - // Handle network namespace. - if os.Geteuid() == 0 { - // Do it only when root, either on the host or as root in the - // user namespace. - if err := replaceNetNS(netNSPath, ctr, newState); err != nil { - return err - } - } - // New state compiled successfully, swap it into the current state ctr.state = newState @@ -892,7 +882,7 @@ func (s *BoltState) SaveContainer(ctr *Container) error { if err != nil { return fmt.Errorf("marshalling container %s state to JSON: %w", ctr.ID(), err) } - netNSPath := getNetNSPath(ctr) + netNSPath := ctr.state.NetNS ctrID := []byte(ctr.ID()) @@ -919,11 +909,7 @@ func (s *BoltState) SaveContainer(ctr *Container) error { return fmt.Errorf("updating container %s state in DB: %w", ctr.ID(), err) } - if netNSPath != "" { - if err := ctrToSave.Put(netNSKey, []byte(netNSPath)); err != nil { - return fmt.Errorf("updating network namespace path for container %s in DB: %w", ctr.ID(), err) - } - } else { + if netNSPath == "" { // Delete the existing network namespace if err := ctrToSave.Delete(netNSKey); err != nil { return fmt.Errorf("removing network namespace path for container %s in DB: %w", ctr.ID(), err) diff --git a/libpod/boltdb_state_freebsd.go b/libpod/boltdb_state_freebsd.go deleted file mode 100644 index d0a2d4f28e..0000000000 --- a/libpod/boltdb_state_freebsd.go +++ /dev/null @@ -1,25 +0,0 @@ -//go:build freebsd -// +build freebsd - -package libpod - -// replaceNetNS handle network namespace transitions after updating a -// container's state. -func replaceNetNS(netNSPath string, ctr *Container, newState *ContainerState) error { - if netNSPath != "" { - // On FreeBSD, we just record the network jail's name in our state. - newState.NetNS = &jailNetNS{Name: netNSPath} - } else { - newState.NetNS = nil - } - return nil -} - -// getNetNSPath retrieves the netns path to be stored in the database -func getNetNSPath(ctr *Container) string { - if ctr.state.NetNS != nil { - return ctr.state.NetNS.Name - } else { - return "" - } -} diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go index 7f2d49b316..48aa0acc49 100644 --- a/libpod/boltdb_state_internal.go +++ b/libpod/boltdb_state_internal.go @@ -590,7 +590,6 @@ func (s *BoltState) addContainer(ctr *Container, pod *Pod) error { if err != nil { return fmt.Errorf("marshalling container %s state to JSON: %w", ctr.ID(), err) } - netNSPath := getNetNSPath(ctr) dependsCtrs := ctr.Dependencies() ctrID := []byte(ctr.ID()) @@ -741,11 +740,6 @@ func (s *BoltState) addContainer(ctr *Container, pod *Pod) error { return fmt.Errorf("adding container %s pod to DB: %w", ctr.ID(), err) } } - if netNSPath != "" { - if err := newCtrBkt.Put(netNSKey, []byte(netNSPath)); err != nil { - return fmt.Errorf("adding container %s netns path to DB: %w", ctr.ID(), err) - } - } if len(networks) > 0 { ctrNetworksBkt, err := newCtrBkt.CreateBucket(networksBkt) if err != nil { diff --git a/libpod/boltdb_state_linux.go b/libpod/boltdb_state_linux.go deleted file mode 100644 index 3d9f5b6a2f..0000000000 --- a/libpod/boltdb_state_linux.go +++ /dev/null @@ -1,57 +0,0 @@ -//go:build linux -// +build linux - -package libpod - -import ( - "fmt" - - "github.com/containers/podman/v4/libpod/define" - "github.com/sirupsen/logrus" -) - -// replaceNetNS handle network namespace transitions after updating a -// container's state. -func replaceNetNS(netNSPath string, ctr *Container, newState *ContainerState) error { - if netNSPath != "" { - // Check if the container's old state has a good netns - if ctr.state.NetNS != nil && netNSPath == ctr.state.NetNS.Path() { - newState.NetNS = ctr.state.NetNS - } else { - // Close the existing namespace. - // Whoever removed it from the database already tore it down. - if err := ctr.runtime.closeNetNS(ctr); err != nil { - return err - } - - // Open the new network namespace - ns, err := joinNetNS(netNSPath) - if err == nil { - newState.NetNS = ns - } else { - if ctr.ensureState(define.ContainerStateRunning, define.ContainerStatePaused) { - return fmt.Errorf("joining network namespace of container %s: %w", ctr.ID(), err) - } - - logrus.Errorf("Joining network namespace for container %s: %v", ctr.ID(), err) - ctr.state.NetNS = nil - } - } - } else { - // The container no longer has a network namespace - // Close the old one, whoever removed it from the DB should have - // cleaned it up already. - if err := ctr.runtime.closeNetNS(ctr); err != nil { - return err - } - } - return nil -} - -// getNetNSPath retrieves the netns path to be stored in the database -func getNetNSPath(ctr *Container) string { - if ctr.state.NetNS != nil { - return ctr.state.NetNS.Path() - } - return "" -} diff --git a/libpod/container.go b/libpod/container.go index 255e353ea4..091fa3340d 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -169,6 +169,8 @@ type ContainerState struct { // Podman. // These are DEPRECATED and will be removed in a future release. LegacyExecSessions map[string]*legacyExecSession `json:"execSessions,omitempty"` + // NetNS is the path or name of the NetNS + NetNS string `json:"netns,omitempty"` // NetworkStatusOld contains the configuration results for all networks // the pod is attached to. Only populated if we created a network // namespace for the container, and the network namespace is currently @@ -228,9 +230,6 @@ type ContainerState struct { // `podman-play-kube`. Service Service - // containerPlatformState holds platform-specific container state. - containerPlatformState - // Following checkpoint/restore related information is displayed // if the container has been checkpointed or restored. CheckpointedTime time.Time `json:"checkpointedTime,omitempty"` diff --git a/libpod/container_freebsd.go b/libpod/container_freebsd.go index 87fb494dd6..cb0a465b23 100644 --- a/libpod/container_freebsd.go +++ b/libpod/container_freebsd.go @@ -3,29 +3,12 @@ package libpod -type containerPlatformState struct { - // NetNS is the name of the container's network VNET - // jail. Will only be set if config.CreateNetNS is true, or - // the container was told to join another container's network - // namespace. - NetNS *jailNetNS `json:"-"` -} - -type jailNetNS struct { - Name string `json:"-"` -} - -func (ns *jailNetNS) Path() string { - // The jail name approximately corresponds to the Linux netns path - return ns.Name -} - func networkDisabled(c *Container) (bool, error) { if c.config.CreateNetNS { return false, nil } if !c.config.PostConfigureNetNS { - return c.state.NetNS != nil, nil + return c.state.NetNS != "", nil } return false, nil } diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 165ef7ec35..4abd701d49 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -625,6 +625,9 @@ func resetState(state *ContainerState) { state.StartupHCPassed = false state.StartupHCSuccessCount = 0 state.StartupHCFailureCount = 0 + state.NetNS = "" + state.NetworkStatus = nil + state.NetworkStatusOld = nil } // Refresh refreshes the container's state after a restart. diff --git a/libpod/container_internal_freebsd.go b/libpod/container_internal_freebsd.go index 5edb045908..995116a8c7 100644 --- a/libpod/container_internal_freebsd.go +++ b/libpod/container_internal_freebsd.go @@ -36,7 +36,7 @@ func (c *Container) unmountSHM(path string) error { func (c *Container) prepare() error { var ( wg sync.WaitGroup - ctrNS *jailNetNS + ctrNS string networkStatus map[string]types.StatusBlock createNetNSErr, mountStorageErr error mountPoint string @@ -48,7 +48,7 @@ func (c *Container) prepare() error { go func() { defer wg.Done() // Set up network namespace if not already set up - noNetNS := c.state.NetNS == nil + noNetNS := c.state.NetNS == "" if c.config.CreateNetNS && noNetNS && !c.config.PostConfigureNetNS { ctrNS, networkStatus, createNetNSErr = c.runtime.createNetNS(c) if createNetNSErr != nil { @@ -168,8 +168,8 @@ func (c *Container) addNetworkContainer(g *generate.Generator, ctr string) error return fmt.Errorf("retrieving dependency %s of container %s from state: %w", ctr, c.ID(), err) } c.runtime.state.UpdateContainer(nsCtr) - if nsCtr.state.NetNS != nil { - g.AddAnnotation("org.freebsd.parentJail", nsCtr.state.NetNS.Name) + if nsCtr.state.NetNS != "" { + g.AddAnnotation("org.freebsd.parentJail", nsCtr.state.NetNS) } return nil } @@ -193,7 +193,7 @@ func openDirectory(path string) (fd int, err error) { func (c *Container) addNetworkNamespace(g *generate.Generator) error { if c.config.CreateNetNS { - if c.state.NetNS == nil { + if c.state.NetNS == "" { // This should not happen since network setup // errors should be propagated correctly from // (*Runtime).createNetNS. Check for it anyway @@ -201,7 +201,7 @@ func (c *Container) addNetworkNamespace(g *generate.Generator) error { // the past (see #16333). return fmt.Errorf("Inconsistent state: c.config.CreateNetNS is set but c.state.NetNS is nil") } - g.AddAnnotation("org.freebsd.parentJail", c.state.NetNS.Name) + g.AddAnnotation("org.freebsd.parentJail", c.state.NetNS) } return nil } @@ -286,7 +286,7 @@ func (c *Container) isSlirp4netnsIPv6() (bool, error) { // check for net=none func (c *Container) hasNetNone() bool { - return c.state.NetNS == nil + return c.state.NetNS == "" } func setVolumeAtime(mountPoint string, st os.FileInfo) error { @@ -310,8 +310,8 @@ func (c *Container) getConmonPidFd() int { } func (c *Container) jailName() string { - if c.state.NetNS != nil { - return c.state.NetNS.Name + "." + c.ID() + if c.state.NetNS != "" { + return c.state.NetNS + "." + c.ID() } else { return c.ID() } diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 7161383a08..b309b32cd0 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -14,7 +14,6 @@ import ( "syscall" "time" - "github.com/containernetworking/plugins/pkg/ns" "github.com/containers/common/libnetwork/types" "github.com/containers/common/pkg/cgroups" "github.com/containers/common/pkg/config" @@ -56,7 +55,7 @@ func (c *Container) unmountSHM(mount string) error { func (c *Container) prepare() error { var ( wg sync.WaitGroup - netNS ns.NetNS + netNS string networkStatus map[string]types.StatusBlock createNetNSErr, mountStorageErr error mountPoint string @@ -68,7 +67,7 @@ func (c *Container) prepare() error { go func() { defer wg.Done() // Set up network namespace if not already set up - noNetNS := c.state.NetNS == nil + noNetNS := c.state.NetNS == "" if c.config.CreateNetNS && noNetNS && !c.config.PostConfigureNetNS { netNS, networkStatus, createNetNSErr = c.runtime.createNetNS(c) if createNetNSErr != nil { @@ -159,7 +158,7 @@ func (c *Container) cleanupNetwork() error { if netDisabled { return nil } - if c.state.NetNS == nil { + if c.state.NetNS == "" { logrus.Debugf("Network is already cleaned up, skipping...") return nil } @@ -169,7 +168,7 @@ func (c *Container) cleanupNetwork() error { logrus.Errorf("Unable to clean up network for container %s: %q", c.ID(), err) } - c.state.NetNS = nil + c.state.NetNS = "" c.state.NetworkStatus = nil c.state.NetworkStatusOld = nil @@ -411,7 +410,7 @@ func (c *Container) setupRootlessNetwork() error { // set up rootlesskit port forwarder again since it dies when conmon exits // we use rootlesskit port forwarder only as rootless and when bridge network is used if rootless.IsRootless() && c.config.NetMode.IsBridge() && len(c.config.PortMappings) > 0 { - err := c.runtime.setupRootlessPortMappingViaRLK(c, c.state.NetNS.Path(), c.state.NetworkStatus) + err := c.runtime.setupRootlessPortMappingViaRLK(c, c.state.NetNS, c.state.NetworkStatus) if err != nil { return err } @@ -430,7 +429,7 @@ func (c *Container) addNetworkNamespace(g *generate.Generator) error { return err } } else { - if err := g.AddOrReplaceLinuxNamespace(string(spec.NetworkNamespace), c.state.NetNS.Path()); err != nil { + if err := g.AddOrReplaceLinuxNamespace(string(spec.NetworkNamespace), c.state.NetNS); err != nil { return err } } diff --git a/libpod/container_linux.go b/libpod/container_linux.go index 9c17a1966e..a1a64f5dda 100644 --- a/libpod/container_linux.go +++ b/libpod/container_linux.go @@ -4,17 +4,9 @@ package libpod import ( - "github.com/containernetworking/plugins/pkg/ns" spec "github.com/opencontainers/runtime-spec/specs-go" ) -type containerPlatformState struct { - // NetNSPath is the path of the container's network namespace - // Will only be set if config.CreateNetNS is true, or the container was - // told to join another container's network namespace - NetNS ns.NetNS `json:"-"` -} - func networkDisabled(c *Container) (bool, error) { if c.config.CreateNetNS { return false, nil diff --git a/libpod/networking_common.go b/libpod/networking_common.go index 08b6a1b2fb..ac914493b2 100644 --- a/libpod/networking_common.go +++ b/libpod/networking_common.go @@ -121,12 +121,12 @@ func (r *Runtime) teardownNetworkBackend(ns string, opts types.NetworkOptions) e // Tear down a container's network backend configuration, but do not tear down the // namespace itself. func (r *Runtime) teardownNetwork(ctr *Container) error { - if ctr.state.NetNS == nil { + if ctr.state.NetNS == "" { // The container has no network namespace, we're set return nil } - 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, ctr.ID()) networks, err := ctr.networks() if err != nil { @@ -136,7 +136,7 @@ func (r *Runtime) teardownNetwork(ctr *Container) error { if !ctr.config.NetMode.IsSlirp4netns() && !ctr.config.NetMode.IsPasta() && len(networks) > 0 { netOpts := ctr.getNetworkOptions(networks) - return r.teardownNetworkBackend(ctr.state.NetNS.Path(), netOpts) + return r.teardownNetworkBackend(ctr.state.NetNS, netOpts) } return nil } @@ -158,7 +158,7 @@ func isBridgeNetMode(n namespaces.NetworkMode) error { // Only works on containers with bridge networking at present, though in the future we could // extend this to stop + restart slirp4netns func (r *Runtime) reloadContainerNetwork(ctr *Container) (map[string]types.StatusBlock, error) { - if ctr.state.NetNS == nil { + if ctr.state.NetNS == "" { return nil, fmt.Errorf("container %s network is not configured, refusing to reload: %w", ctr.ID(), define.ErrCtrStateInvalid) } if err := isBridgeNetMode(ctr.config.NetMode); err != nil { @@ -234,7 +234,7 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e return nil, err } - if c.state.NetNS == nil { + if c.state.NetNS == "" { if networkNSPath := c.joinedNetworkNSPath(); networkNSPath != "" { if result, err := c.inspectJoinedNetworkNS(networkNSPath); err == nil { // fallback to dummy configuration @@ -262,7 +262,7 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e } // Set network namespace path - settings.SandboxKey = c.state.NetNS.Path() + settings.SandboxKey = c.state.NetNS netStatus := c.getNetworkStatus() // If this is empty, we're probably slirp4netns @@ -394,7 +394,7 @@ func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) erro return nil } - if c.state.NetNS == nil { + if c.state.NetNS == "" { return fmt.Errorf("unable to disconnect %s from %s: %w", nameOrID, netName, define.ErrNoNetwork) } @@ -407,7 +407,7 @@ func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) erro netName: networks[netName], } - if err := c.runtime.teardownNetworkBackend(c.state.NetNS.Path(), opts); err != nil { + if err := c.runtime.teardownNetworkBackend(c.state.NetNS, opts); err != nil { return err } @@ -517,7 +517,7 @@ func (c *Container) NetworkConnect(nameOrID, netName string, netOpts types.PerNe if !c.ensureState(define.ContainerStateRunning, define.ContainerStateCreated) { return nil } - if c.state.NetNS == nil { + if c.state.NetNS == "" { return fmt.Errorf("unable to connect %s to %s: %w", nameOrID, netName, define.ErrNoNetwork) } @@ -530,7 +530,7 @@ func (c *Container) NetworkConnect(nameOrID, netName string, netOpts types.PerNe netName: netOpts, } - results, err := c.runtime.setUpNetwork(c.state.NetNS.Path(), opts) + results, err := c.runtime.setUpNetwork(c.state.NetNS, opts) if err != nil { return err } diff --git a/libpod/networking_freebsd.go b/libpod/networking_freebsd.go index 637e0ea08f..26899faf2e 100644 --- a/libpod/networking_freebsd.go +++ b/libpod/networking_freebsd.go @@ -116,7 +116,7 @@ func (r *Runtime) setupNetNS(ctr *Container) error { } // Create and configure a new network namespace for a container -func (r *Runtime) configureNetNS(ctr *Container, ctrNS *jailNetNS) (status map[string]types.StatusBlock, rerr error) { +func (r *Runtime) configureNetNS(ctr *Container, ctrNS string) (status map[string]types.StatusBlock, rerr error) { if err := r.exposeMachinePorts(ctr.config.PortMappings); err != nil { return nil, err } @@ -139,7 +139,7 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS *jailNetNS) (status map[s } netOpts := ctr.getNetworkOptions(networks) - netStatus, err := r.setUpNetwork(ctrNS.Name, netOpts) + netStatus, err := r.setUpNetwork(ctrNS, netOpts) if err != nil { return nil, err } @@ -148,16 +148,16 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS *jailNetNS) (status map[s } // Create and configure a new network namespace for a container -func (r *Runtime) createNetNS(ctr *Container) (n *jailNetNS, q map[string]types.StatusBlock, retErr error) { +func (r *Runtime) createNetNS(ctr *Container) (n string, q map[string]types.StatusBlock, retErr error) { b := make([]byte, 16) _, err := rand.Reader.Read(b) if err != nil { - return nil, nil, fmt.Errorf("failed to generate random vnet name: %v", err) + return "", nil, fmt.Errorf("failed to generate random vnet name: %v", err) } - ctrNS := &jailNetNS{Name: fmt.Sprintf("vnet-%x-%x-%x-%x-%x", b[0:4], b[4:6], b[6:8], b[8:10], b[10:])} + netns := fmt.Sprintf("vnet-%x-%x-%x-%x-%x", b[0:4], b[4:6], b[6:8], b[8:10], b[10:]) jconf := jail.NewConfig() - jconf.Set("name", ctrNS.Name) + jconf.Set("name", netns) jconf.Set("vnet", jail.NEW) jconf.Set("children.max", 1) jconf.Set("persist", true) @@ -168,22 +168,22 @@ func (r *Runtime) createNetNS(ctr *Container) (n *jailNetNS, q map[string]types. jconf.Set("securelevel", -1) j, err := jail.Create(jconf) if err != nil { - return nil, nil, fmt.Errorf("Failed to create vnet jail %s for container %s: %w", ctrNS.Name, ctr.ID(), err) + return "", nil, fmt.Errorf("Failed to create vnet jail %s for container %s: %w", netns, ctr.ID(), err) } - logrus.Debugf("Created vnet jail %s for container %s", ctrNS.Name, ctr.ID()) + logrus.Debugf("Created vnet jail %s for container %s", netns, ctr.ID()) var networkStatus map[string]types.StatusBlock - networkStatus, err = r.configureNetNS(ctr, ctrNS) + networkStatus, err = r.configureNetNS(ctr, netns) if err != nil { jconf := jail.NewConfig() jconf.Set("persist", false) if err := j.Set(jconf); err != nil { // Log this error and return the error from configureNetNS - logrus.Errorf("failed to destroy vnet jail %s: %w", ctrNS.Name, err) + logrus.Errorf("failed to destroy vnet jail %s: %w", netns, err) } } - return ctrNS, networkStatus, err + return netns, networkStatus, err } // Tear down a network namespace, undoing all state associated with it. @@ -196,28 +196,28 @@ func (r *Runtime) teardownNetNS(ctr *Container) error { return err } - if ctr.state.NetNS != nil { + if ctr.state.NetNS != "" { // Rather than destroying the jail immediately, reset the // persist flag so that it will live until the container is // done. - netjail, err := jail.FindByName(ctr.state.NetNS.Name) + netjail, err := jail.FindByName(ctr.state.NetNS) if err != nil { - return fmt.Errorf("finding network jail %s: %w", ctr.state.NetNS.Name, err) + return fmt.Errorf("finding network jail %s: %w", ctr.state.NetNS, err) } jconf := jail.NewConfig() jconf.Set("persist", false) if err := netjail.Set(jconf); err != nil { - return fmt.Errorf("releasing network jail %s: %w", ctr.state.NetNS.Name, err) + return fmt.Errorf("releasing network jail %s: %w", ctr.state.NetNS, err) } - ctr.state.NetNS = nil + ctr.state.NetNS = "" } return nil } func getContainerNetIO(ctr *Container) (*LinkStatistics64, error) { - if ctr.state.NetNS == nil { + if ctr.state.NetNS == "" { // If NetNS is nil, it was set as none, and no netNS // was set up this is a valid state and thus return no // error, nor any statistics @@ -225,7 +225,7 @@ func getContainerNetIO(ctr *Container) (*LinkStatistics64, error) { } // FIXME get the interface from the container netstatus - cmd := exec.Command("jexec", ctr.state.NetNS.Name, "netstat", "-bI", "eth0", "--libxo", "json") + cmd := exec.Command("jexec", ctr.state.NetNS, "netstat", "-bI", "eth0", "--libxo", "json") out, err := cmd.Output() if err != nil { return nil, err @@ -255,11 +255,7 @@ func getContainerNetIO(ctr *Container) (*LinkStatistics64, error) { } func (c *Container) joinedNetworkNSPath() string { - if c.state.NetNS != nil { - return c.state.NetNS.Name - } else { - return "" - } + return c.state.NetNS } func (c *Container) inspectJoinedNetworkNS(networkns string) (q types.StatusBlock, retErr error) { diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 0a6b7de109..c1381aa046 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -281,7 +281,7 @@ func (r *RootlessNetNS) Cleanup(runtime *Runtime) error { // only check for an active netns, we cannot use the container state // because not running does not mean that the netns does not need cleanup // only if the netns is empty we know that we do not need cleanup - return c.state.NetNS != nil + return c.state.NetNS != "" } ctrs, err := runtime.GetContainers(activeNetns) if err != nil { @@ -548,7 +548,7 @@ func (r *Runtime) GetRootlessNetNs(new bool) (*RootlessNetNS, error) { } // Create and configure a new network namespace for a container -func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) (status map[string]types.StatusBlock, rerr error) { +func (r *Runtime) configureNetNS(ctr *Container, ctrNS string) (status map[string]types.StatusBlock, rerr error) { if err := r.exposeMachinePorts(ctr.config.PortMappings); err != nil { return nil, err } @@ -577,7 +577,7 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) (status map[str } netOpts := ctr.getNetworkOptions(networks) - netStatus, err := r.setUpNetwork(ctrNS.Path(), netOpts) + netStatus, err := r.setUpNetwork(ctrNS, netOpts) if err != nil { return nil, err } @@ -587,21 +587,20 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) (status map[str // not set up port because they are still active if rootless.IsRootless() && len(ctr.config.PortMappings) > 0 && ctr.getNetworkStatus() == nil { // set up port forwarder for rootless netns - netnsPath := ctrNS.Path() // TODO: support slirp4netns port forwarder as well // make sure to fix this in container.handleRestartPolicy() as well // Important we have to call this after r.setUpNetwork() so that // we can use the proper netStatus - err = r.setupRootlessPortMappingViaRLK(ctr, netnsPath, netStatus) + err = r.setupRootlessPortMappingViaRLK(ctr, ctrNS, netStatus) } return netStatus, err } // Create and configure a new network namespace for a container -func (r *Runtime) createNetNS(ctr *Container) (n ns.NetNS, q map[string]types.StatusBlock, retErr error) { +func (r *Runtime) createNetNS(ctr *Container) (n string, q map[string]types.StatusBlock, retErr error) { ctrNS, err := netns.NewNS() if err != nil { - return nil, nil, fmt.Errorf("creating network namespace for container %s: %w", ctr.ID(), err) + return "", nil, fmt.Errorf("creating network namespace for container %s: %w", ctr.ID(), err) } defer func() { if retErr != nil { @@ -617,8 +616,8 @@ func (r *Runtime) createNetNS(ctr *Container) (n ns.NetNS, q map[string]types.St logrus.Debugf("Made network namespace at %s for container %s", ctrNS.Path(), ctr.ID()) var networkStatus map[string]types.StatusBlock - networkStatus, err = r.configureNetNS(ctr, ctrNS) - return ctrNS, networkStatus, err + networkStatus, err = r.configureNetNS(ctr, ctrNS.Path()) + return ctrNS.Path(), networkStatus, err } // Configure the network namespace using the container process @@ -652,46 +651,14 @@ func (r *Runtime) setupNetNS(ctr *Container) error { return fmt.Errorf("cannot mount %s: %w", nsPath, err) } - netNS, err := ns.GetNS(nsPath) - if err != nil { - return err - } - networkStatus, err := r.configureNetNS(ctr, netNS) + networkStatus, err := r.configureNetNS(ctr, nsPath) // Assign NetNS attributes to container - ctr.state.NetNS = netNS + ctr.state.NetNS = nsPath ctr.state.NetworkStatus = networkStatus return err } -// Join an existing network namespace -func joinNetNS(path string) (ns.NetNS, error) { - netNS, err := ns.GetNS(path) - if err != nil { - return nil, fmt.Errorf("retrieving network namespace at %s: %w", path, err) - } - - return netNS, nil -} - -// Close a network namespace. -// Differs from teardownNetNS() in that it will not attempt to undo the setup of -// the namespace, but will instead only close the open file descriptor -func (r *Runtime) closeNetNS(ctr *Container) error { - if ctr.state.NetNS == nil { - // The container has no network namespace, we're set - return nil - } - - if err := ctr.state.NetNS.Close(); err != nil { - return fmt.Errorf("closing network namespace for container %s: %w", ctr.ID(), err) - } - - ctr.state.NetNS = nil - - return nil -} - // Tear down a network namespace, undoing all state associated with it. func (r *Runtime) teardownNetNS(ctr *Container) error { if err := r.unexposeMachinePorts(ctr.config.PortMappings); err != nil { @@ -705,29 +672,21 @@ func (r *Runtime) teardownNetNS(ctr *Container) error { prevErr := r.teardownNetwork(ctr) // First unmount the namespace - if err := netns.UnmountNS(ctr.state.NetNS.Path()); err != nil { + if err := netns.UnmountNS(ctr.state.NetNS); err != nil { if prevErr != nil { logrus.Error(prevErr) } return fmt.Errorf("unmounting network namespace for container %s: %w", ctr.ID(), err) } - // Now close the open file descriptor - if err := ctr.state.NetNS.Close(); err != nil { - if prevErr != nil { - logrus.Error(prevErr) - } - return fmt.Errorf("closing network namespace for container %s: %w", ctr.ID(), err) - } - - ctr.state.NetNS = nil + ctr.state.NetNS = "" return prevErr } func getContainerNetNS(ctr *Container) (string, *Container, error) { - if ctr.state.NetNS != nil { - return ctr.state.NetNS.Path(), nil, nil + if ctr.state.NetNS != "" { + return ctr.state.NetNS, nil, nil } if ctr.config.NetNsCtr != "" { c, err := ctr.runtime.GetContainer(ctr.config.NetNsCtr) diff --git a/libpod/networking_pasta_linux.go b/libpod/networking_pasta_linux.go index 8296a5ebe8..bf28a117e9 100644 --- a/libpod/networking_pasta_linux.go +++ b/libpod/networking_pasta_linux.go @@ -12,11 +12,10 @@ import ( "os/exec" "strings" - "github.com/containernetworking/plugins/pkg/ns" "github.com/sirupsen/logrus" ) -func (r *Runtime) setupPasta(ctr *Container, netns ns.NetNS) error { +func (r *Runtime) setupPasta(ctr *Container, netns string) error { var NoTCPInitPorts = true var NoUDPInitPorts = true var NoTCPNamespacePorts = true @@ -93,7 +92,7 @@ func (r *Runtime) setupPasta(ctr *Container, netns ns.NetNS) error { cmdArgs = append(cmdArgs, "--no-map-gw") } - cmdArgs = append(cmdArgs, "--netns", netns.Path()) + cmdArgs = append(cmdArgs, "--netns", netns) logrus.Debugf("pasta arguments: %s", strings.Join(cmdArgs, " ")) diff --git a/libpod/networking_slirp4netns.go b/libpod/networking_slirp4netns.go index 4026b6b488..48e1aae0f2 100644 --- a/libpod/networking_slirp4netns.go +++ b/libpod/networking_slirp4netns.go @@ -212,7 +212,7 @@ func createBasicSlirp4netnsCmdArgs(options *slirp4netnsNetworkOptions, features } // setupSlirp4netns can be called in rootful as well as in rootless -func (r *Runtime) setupSlirp4netns(ctr *Container, netns ns.NetNS) error { +func (r *Runtime) setupSlirp4netns(ctr *Container, netns string) error { path := r.config.Engine.NetworkCmdPath if path == "" { var err error @@ -267,7 +267,7 @@ func (r *Runtime) setupSlirp4netns(ctr *Container, netns ns.NetNS) error { if err != nil { return fmt.Errorf("failed to create rootless network sync pipe: %w", err) } - netnsPath = netns.Path() + netnsPath = netns cmdArgs = append(cmdArgs, "--netns-type=path", netnsPath, "tap0") } else { defer errorhandling.CloseQuiet(ctr.rootlessSlirpSyncR)