From b6954758bb17f846f81fada71385a94b5072e91c Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 9 Mar 2020 16:06:43 -0400 Subject: [PATCH] Attempt manual removal of CNI IP allocations on refresh We previously attempted to work within CNI to do this, without success. So let's do it manually, instead. We know where the files should live, so we can remove them ourselves instead. This solves issues around sudden reboots where containers do not have time to fully tear themselves down, and leave IP address allocations which, for various reasons, are not stored in tmpfs and persist through reboot. Fixes #5433 Signed-off-by: Matthew Heon --- libpod/container_internal.go | 70 ++++++++++++++++++++++-- libpod/container_internal_linux.go | 7 --- libpod/container_internal_unsupported.go | 4 -- libpod/networking_linux.go | 7 +++ libpod/networking_unsupported.go | 4 ++ 5 files changed, 75 insertions(+), 17 deletions(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 12a13a0ce7..7a85c1f046 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -470,9 +470,9 @@ func (c *Container) teardownStorage() error { return nil } -// Reset resets state fields to default values -// It is performed before a refresh and clears the state after a reboot -// It does not save the results - assumes the database will do that for us +// Reset resets state fields to default values. +// It is performed before a refresh and clears the state after a reboot. +// It does not save the results - assumes the database will do that for us. func resetState(state *ContainerState) error { state.PID = 0 state.ConmonPID = 0 @@ -483,7 +483,6 @@ func resetState(state *ContainerState) error { } state.ExecSessions = make(map[string]*ExecSession) state.LegacyExecSessions = nil - state.NetworkStatus = nil state.BindMounts = make(map[string]string) state.StoppedByUser = false state.RestartPolicyMatch = false @@ -539,6 +538,18 @@ func (c *Container) refresh() error { } c.lock = lock + // Try to delete any lingering IP allocations. + // If this fails, just log and ignore. + // I'm a little concerned that this is so far down in refresh() and we + // could fail before getting to it - but the worst that would happen is + // that Inspect() would return info on IPs we no longer own. + if len(c.state.NetworkStatus) > 0 { + if err := c.removeIPv4Allocations(); err != nil { + logrus.Errorf("Error removing IP allocations for container %s: %v", c.ID(), err) + } + } + c.state.NetworkStatus = nil + if err := c.save(); err != nil { return errors.Wrapf(err, "error refreshing state for container %s", c.ID()) } @@ -548,11 +559,58 @@ func (c *Container) refresh() error { return err } - if rootless.IsRootless() { + return nil +} + +// Try and remove IP address allocations. Presently IPv4 only. +// Should be safe as rootless because NetworkStatus should only be populated if +// CNI is running. +func (c *Container) removeIPv4Allocations() error { + cniNetworksDir, err := getCNINetworksDir() + if err != nil { + return err + } + + if len(c.state.NetworkStatus) == 0 { return nil } - return c.refreshCNI() + cniDefaultNetwork := "" + if c.runtime.netPlugin != nil { + cniDefaultNetwork = c.runtime.netPlugin.GetDefaultNetworkName() + } + + switch { + case len(c.config.Networks) > 0 && len(c.config.Networks) != len(c.state.NetworkStatus): + return errors.Wrapf(define.ErrInternal, "network mismatch: asked to join %d CNI networks but got %d CNI results", len(c.config.Networks), len(c.state.NetworkStatus)) + case len(c.config.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(c.config.Networks) == 0 && cniDefaultNetwork == "": + return errors.Wrapf(define.ErrInternal, "could not retrieve name of CNI default network") + } + + for index, result := range c.state.NetworkStatus { + for _, ctrIP := range result.IPs { + if ctrIP.Version != "4" { + continue + } + candidate := "" + if len(c.config.Networks) > 0 { + // CNI returns networks in order we passed them. + // So our index into results should be our index + // into networks. + candidate = filepath.Join(cniNetworksDir, c.config.Networks[index], ctrIP.Address.IP.String()) + } else { + candidate = filepath.Join(cniNetworksDir, cniDefaultNetwork, ctrIP.Address.IP.String()) + } + logrus.Debugf("Going to try removing IP address reservation file %q for container %s", candidate, c.ID()) + if err := os.Remove(candidate); err != nil && !os.IsNotExist(err) { + return errors.Wrapf(err, "error removing CNI IP reservation file %q for container %s", candidate, c.ID()) + } + } + } + + return nil } // Remove conmon attach socket and terminal resize FIFO diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 63968918cb..2f0f59c244 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -1436,13 +1436,6 @@ func (c *Container) copyOwnerAndPerms(source, dest string) error { return nil } -// Teardown CNI config on refresh -func (c *Container) refreshCNI() error { - // Let's try and delete any lingering network config... - podNetwork := c.runtime.getPodNetwork(c.ID(), c.config.Name, "", c.config.Networks, c.config.PortMappings, c.config.StaticIP, c.config.StaticMAC) - return c.runtime.netPlugin.TearDownPod(podNetwork) -} - // Get cgroup path in a format suitable for the OCI spec func (c *Container) getOCICgroupPath() (string, error) { unified, err := cgroups.IsCgroup2UnifiedMode() diff --git a/libpod/container_internal_unsupported.go b/libpod/container_internal_unsupported.go index 4abaa63621..395271b2a2 100644 --- a/libpod/container_internal_unsupported.go +++ b/libpod/container_internal_unsupported.go @@ -41,10 +41,6 @@ func (c *Container) copyOwnerAndPerms(source, dest string) error { return nil } -func (c *Container) refreshCNI() error { - return define.ErrNotImplemented -} - func (c *Container) getOCICgroupPath() (string, error) { return "", define.ErrNotImplemented } diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 5a27a2abb2..f1bf79ce7d 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -657,6 +657,13 @@ func resultToBasicNetworkConfig(result *cnitypes.Result) (InspectBasicNetworkCon return config, nil } +// This is a horrible hack, necessary because CNI does not properly clean up +// after itself on an unclean reboot. Return what we're pretty sure is the path +// to CNI's internal files (it's not really exposed to us). +func getCNINetworksDir() (string, error) { + return "/var/lib/cni/networks", nil +} + type logrusDebugWriter struct { prefix string } diff --git a/libpod/networking_unsupported.go b/libpod/networking_unsupported.go index 7f343cf356..32b354a442 100644 --- a/libpod/networking_unsupported.go +++ b/libpod/networking_unsupported.go @@ -23,3 +23,7 @@ func (r *Runtime) createNetNS(ctr *Container) (err error) { func (c *Container) getContainerNetworkInfo() (*InspectNetworkSettings, error) { return nil, define.ErrNotImplemented } + +func getCNINetworksDir() (string, error) { + return "", define.ErrNotImplemented +}