From b4fa3c622ddc58469bd6c52c69731bae8ad46601 Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Mon, 16 Dec 2019 12:25:44 -0500 Subject: [PATCH] namespaces: Remove symlink methods the symlink functionality was used as a crutch for a bug in runc (cri-o#237). that bug has since been fixed (runc#1149), so remove this patch as it clutters the code unnecessarily Signed-off-by: Peter Hunt --- internal/lib/sandbox/namespaces.go | 65 +++++------------------ internal/lib/sandbox/namespaces_linux.go | 66 ++---------------------- internal/pkg/criocli/criocli.go | 4 +- 3 files changed, 17 insertions(+), 118 deletions(-) diff --git a/internal/lib/sandbox/namespaces.go b/internal/lib/sandbox/namespaces.go index fb4a491794f..fc02212596e 100644 --- a/internal/lib/sandbox/namespaces.go +++ b/internal/lib/sandbox/namespaces.go @@ -37,8 +37,8 @@ type NamespaceIface interface { // Remove ensures this network namespace handle is closed and removed Remove() error - // SymlinkCreate creates all necessary symlinks - SymlinkCreate(string) error + // TODO FIXME add comment what is path? + Path() string } func (s *Sandbox) CreateSandboxNamespaces(managedNamespaces []string) (map[string]string, error) { @@ -63,22 +63,16 @@ func (s *Sandbox) CreateSandboxNamespaces(managedNamespaces []string) (map[strin } }() - // TODO FIXME I'm not sure we need this anymore - err = namespaceIface.SymlinkCreate(s.name) - if err != nil { - return nil, err - } - switch namespace.nsType { case NETNS: s.netns = namespaceIface - nsTypeToPath[NETNS] = namespace.symlink.Name() + nsTypeToPath[NETNS] = namespace.Path() case IPCNS: s.ipcns = namespaceIface - nsTypeToPath[IPCNS] = namespace.symlink.Name() + nsTypeToPath[IPCNS] = namespace.Path() case UTSNS: s.utsns = namespaceIface - nsTypeToPath[UTSNS] = namespace.symlink.Name() + nsTypeToPath[UTSNS] = namespace.Path() default: // This should never happen err = errors.New("Invalid namespace type") @@ -197,19 +191,19 @@ func (s *Sandbox) RemoveSandboxManagedNamespaces() error { errs := make([]error, 0) var directory string if s.utsns != nil { - directory = filepath.Dir(s.utsns.Get().Path()) + directory = filepath.Dir(s.utsns.Path()) if err := s.utsns.Remove(); err != nil { errs = append(errs, err) } } if s.ipcns != nil { - directory = filepath.Dir(s.ipcns.Get().Path()) + directory = filepath.Dir(s.ipcns.Path()) if err := s.ipcns.Remove(); err != nil { errs = append(errs, err) } } if s.netns != nil { - directory = filepath.Dir(s.netns.Get().Path()) + directory = filepath.Dir(s.netns.Path()) if err := s.netns.Remove(); err != nil { errs = append(errs, err) } @@ -230,15 +224,14 @@ func (s *Sandbox) RemoveSandboxManagedNamespaces() error { // nsPath returns the path to a namespace of the sandbox. // If the sandbox uses the host namespace, nil is returned func (s *Sandbox) nsPath(ns NamespaceIface, nsType string) string { - if ns == nil || ns.Get() == nil || - ns.Get().symlink == nil { + if ns == nil || ns.Get() == nil { if s.infraContainer != nil { return fmt.Sprintf("/proc/%v/ns/%s", s.infraContainer.State().Pid, nsType) } return "" } - return ns.Get().symlink.Name() + return ns.Path() } // NsGet returns the Namespace associated with the given nspath and name @@ -247,50 +240,16 @@ func (s *Sandbox) NsGet(nspath, name string) (*Namespace, error) { return nil, ErrClosedNS } - symlink, symlinkErr := isSymbolicLink(nspath) - if symlinkErr != nil { - return nil, symlinkErr - } - - var resolvedNsPath string - if symlink { - path, err := os.Readlink(nspath) - if err != nil { - return nil, err - } - resolvedNsPath = path - } else { - resolvedNsPath = nspath - } - - ns, err := getNamespace(resolvedNsPath) + ns, err := getNamespace(nspath) if err != nil { return nil, err } - if symlink { - fd, err := os.Open(nspath) - if err != nil { - return nil, err - } - - ns.symlink = fd - } else if err := ns.SymlinkCreate(name); err != nil { - return nil, err - } + ns.nsPath = nspath return ns, nil } -func isSymbolicLink(path string) (bool, error) { - fi, err := os.Lstat(path) - if err != nil { - return false, err - } - - return fi.Mode()&os.ModeSymlink == os.ModeSymlink, nil -} - // UserNsPath returns the path to the user namespace of the sandbox. // If the sandbox uses the host namespace, nil is returned func (s *Sandbox) UserNsPath() string { diff --git a/internal/lib/sandbox/namespaces_linux.go b/internal/lib/sandbox/namespaces_linux.go index 4ac7adae2c7..2995097d269 100644 --- a/internal/lib/sandbox/namespaces_linux.go +++ b/internal/lib/sandbox/namespaces_linux.go @@ -36,11 +36,11 @@ func findPinnsPath() string { type Namespace struct { sync.Mutex ns NS - symlink *os.File closed bool restored bool initialized bool nsType string + nsPath string } // NS is a wrapper for the containernetworking plugin's NetNS interface @@ -140,6 +140,7 @@ func createNewNamespaces(nsTypes []string) ([]*Namespace, error) { returnedNamespaces = append(returnedNamespaces, &Namespace{ ns: ret.(NS), nsType: info.nsType, + nsPath: info.path, }) } return returnedNamespaces, nil @@ -154,57 +155,12 @@ func getNamespace(nsPath string) (*Namespace, error) { return &Namespace{ns: ns, closed: false, restored: true}, nil } -// SymlinkCreate creates the necessary symlinks for the Namespace -func (n *Namespace) SymlinkCreate(name string) error { - if n.ns == nil { - return errors.New("no ns set up") - } - b := make([]byte, 4) - _, randErr := rand.Reader.Read(b) - if randErr != nil { - return randErr - } - - nsRunDir := getRunDirGivenType(n.nsType) - - nsName := fmt.Sprintf("%s-%x", name, b) - symlinkPath := filepath.Join(nsRunDir, nsName) - - if err := os.MkdirAll(nsRunDir, 0755); err != nil { - return err - } - - if err := os.Symlink(n.Path(), symlinkPath); err != nil { - return err - } - - fd, err := os.Open(symlinkPath) - if err != nil { - if removeErr := os.RemoveAll(symlinkPath); removeErr != nil { - return removeErr - } - - return err - } - - n.symlink = fd - - return nil -} - -func getRunDirGivenType(nsType string) string { - // runDir is the default directory in which running namespaces - // are stored - const runDir = "/var/run" - return fmt.Sprintf("%s/%sns", runDir, nsType) -} - // Path returns the path of the namespace handle func (n *Namespace) Path() string { if n == nil || n.ns == nil { return "" } - return n.ns.Path() + return n.nsPath } // Close closes this namespace @@ -226,10 +182,6 @@ func (n *Namespace) Remove() error { return nil } - if err := n.symlinkRemove(); err != nil { - return err - } - if err := n.Close(); err != nil { return err } @@ -262,15 +214,3 @@ func (n *Namespace) Remove() error { return nil } - -func (n *Namespace) symlinkRemove() error { - if err := n.symlink.Close(); err != nil { - return fmt.Errorf("failed to close ns symlink: %v", err) - } - - if err := os.RemoveAll(n.symlink.Name()); err != nil { - return fmt.Errorf("failed to remove ns symlink: %v", err) - } - - return nil -} diff --git a/internal/pkg/criocli/criocli.go b/internal/pkg/criocli/criocli.go index fcc9ac448cd..c8b278332af 100644 --- a/internal/pkg/criocli/criocli.go +++ b/internal/pkg/criocli/criocli.go @@ -631,12 +631,12 @@ func getCrioFlags(defConf *libconfig.Config, systemContext *types.SystemContext) &cli.BoolFlag{ Name: "manage-network-ns-lifecycle", Usage: "Deprecated: this option is being replaced by `manage_ns_lifecycle`, which is described below", - EnvVar: "CONTAINER_MANAGE_NETWORK_NS_LIFECYCLE", + EnvVars: []string{"CONTAINER_MANAGE_NETWORK_NS_LIFECYCLE"}, }, &cli.BoolFlag{ Name: "manage-ns-lifecycle", Usage: fmt.Sprintf("Determines whether we pin and remove namespaces and manage their lifecycle (default: %v)", defConf.ManageNetworkNSLifecycle), - EnvVar: "CONTAINER_MANAGE_NS_LIFECYCLE", + EnvVars: []string{"CONTAINER_MANAGE_NS_LIFECYCLE"}, }, &cli.BoolFlag{ Name: "no-pivot",