diff --git a/init.go b/init.go index bddc237f6e5..2f44ce8a555 100644 --- a/init.go +++ b/init.go @@ -32,8 +32,7 @@ func init() { logrus.SetFormatter(new(logrus.JSONFormatter)) logrus.Debug("child process in init()") - factory, _ := libcontainer.New("") - if err := factory.StartInitialization(); err != nil { + if err := libcontainer.StartInitialization(); err != nil { // as the error is sent back to the parent there is no need to log // or write it to stderr because the parent process will handle this os.Exit(1) diff --git a/libcontainer/README.md b/libcontainer/README.md index 2850db2aade..addd6a95681 100644 --- a/libcontainer/README.md +++ b/libcontainer/README.md @@ -32,8 +32,7 @@ func init() { if len(os.Args) > 1 && os.Args[1] == "init" { runtime.GOMAXPROCS(1) runtime.LockOSThread() - factory, _ := libcontainer.New("") - if err := factory.StartInitialization(); err != nil { + if err := libcontainer.StartInitialization(); err != nil { logrus.Fatal(err) } panic("--this line should have never been executed, congratulations--") @@ -41,18 +40,7 @@ func init() { } ``` -Then to create a container you first have to initialize an instance of a factory -that will handle the creation and initialization for a container. - -```go -factory, err := libcontainer.New("/var/lib/container") -if err != nil { - logrus.Fatal(err) - return -} -``` - -Once you have an instance of the factory created we can create a configuration +Then to create a container you first have to create a configuration struct describing how the container is to be created. A sample would look similar to this: ```go @@ -243,10 +231,11 @@ config := &configs.Config{ } ``` -Once you have the configuration populated you can create a container: +Once you have the configuration populated you can create a container +with a specified ID under a specified state directory: ```go -container, err := factory.Create("container-id", config) +container, err := libcontainer.Create("/run/containers", "container-id", config) if err != nil { logrus.Fatal(err) return diff --git a/libcontainer/cgroups/manager/new.go b/libcontainer/cgroups/manager/new.go index 5df120d0f09..a7bf155cf81 100644 --- a/libcontainer/cgroups/manager/new.go +++ b/libcontainer/cgroups/manager/new.go @@ -55,10 +55,10 @@ func NewWithPaths(config *configs.Cgroup, paths map[string]string) (cgroups.Mana return fs.NewManager(config, paths) } -// getUnifiedPath is an implementation detail of libcontainer factory. -// Historically, it saves cgroup paths as per-subsystem path map (as returned -// by cm.GetPaths(""), but with v2 we only have one single unified path -// (with "" as a key). +// getUnifiedPath is an implementation detail of libcontainer. +// Historically, libcontainer.Create saves cgroup paths as per-subsystem path +// map (as returned by cm.GetPaths(""), but with v2 we only have one single +// unified path (with "" as a key). // // This function converts from that map to string (using "" as a key), // and also checks that the map itself is sane. diff --git a/libcontainer/factory.go b/libcontainer/factory.go deleted file mode 100644 index 9f9e8fc583c..00000000000 --- a/libcontainer/factory.go +++ /dev/null @@ -1,30 +0,0 @@ -package libcontainer - -import ( - "github.com/opencontainers/runc/libcontainer/configs" -) - -type Factory interface { - // Creates a new container with the given id and starts the initial process inside it. - // id must be a string containing only letters, digits and underscores and must contain - // between 1 and 1024 characters, inclusive. - // - // The id must not already be in use by an existing container. Containers created using - // a factory with the same path (and filesystem) must have distinct ids. - // - // Returns the new container with a running process. - // - // On error, any partially created container parts are cleaned up (the operation is atomic). - Create(id string, config *configs.Config) (Container, error) - - // Load takes an ID for an existing container and returns the container information - // from the state. This presents a read only view of the container. - Load(id string) (Container, error) - - // StartInitialization is an internal API to libcontainer used during the reexec of the - // container. - StartInitialization() error - - // Type returns info string about factory type (e.g. lxc, libcontainer...) - Type() string -} diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index a0ea4fe3f2b..9d4bc4cfe60 100644 --- a/libcontainer/factory_linux.go +++ b/libcontainer/factory_linux.go @@ -10,7 +10,6 @@ import ( "strconv" securejoin "github.com/cyphar/filepath-securejoin" - "github.com/moby/sys/mountinfo" "golang.org/x/sys/unix" "github.com/opencontainers/runc/libcontainer/cgroups/manager" @@ -28,60 +27,30 @@ const ( var idRegex = regexp.MustCompile(`^[\w+-\.]+$`) -// TmpfsRoot is an option func to mount LinuxFactory.Root to tmpfs. -func TmpfsRoot(l *LinuxFactory) error { - mounted, err := mountinfo.Mounted(l.Root) - if err != nil { - return err - } - if !mounted { - if err := mount("tmpfs", l.Root, "", "tmpfs", 0, ""); err != nil { - return err - } - } - return nil -} - -// New returns a linux based container factory based in the root directory and -// configures the factory with the provided option funcs. -func New(root string, options ...func(*LinuxFactory) error) (Factory, error) { - if root != "" { - if err := os.MkdirAll(root, 0o700); err != nil { - return nil, err - } - } - l := &LinuxFactory{ - Root: root, - } - - for _, opt := range options { - if opt == nil { - continue - } - if err := opt(l); err != nil { - return nil, err - } - } - return l, nil -} - -// LinuxFactory implements the default factory interface for linux based systems. -type LinuxFactory struct { - // Root directory for the factory to store state. - Root string -} - -func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, error) { - if l.Root == "" { +// Create creates a new container with the given id inside a given state +// directory (root), and returns a Container object. +// +// The root is a state directory which many containers can share. It can be +// used later to get the list of containers, or to get information about a +// particular container (see Load). +// +// The id must not be empty and consist of only the following characters: +// ASCII letters, digits, underscore, plus, minus, period. The id must be +// unique and non-existent for the given root path. +func Create(root, id string, config *configs.Config) (Container, error) { + if root == "" { return nil, errors.New("root not set") } - if err := l.validateID(id); err != nil { + if err := validateID(id); err != nil { return nil, err } if err := validate.Validate(config); err != nil { return nil, err } - containerRoot, err := securejoin.SecureJoin(l.Root, id) + if err := os.MkdirAll(root, 0o700); err != nil { + return nil, err + } + containerRoot, err := securejoin.SecureJoin(root, id) if err != nil { return nil, err } @@ -125,7 +94,8 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err return nil, errors.New("container's cgroup unexpectedly frozen") } - if err := os.MkdirAll(containerRoot, 0o711); err != nil { + // Parent directory is already created above, so Mkdir is enough. + if err := os.Mkdir(containerRoot, 0o711); err != nil { return nil, err } c := &linuxContainer{ @@ -139,19 +109,22 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err return c, nil } -func (l *LinuxFactory) Load(id string) (Container, error) { - if l.Root == "" { +// Load takes a path to the state directory (root) and an id of an existing +// container, and returns a Container object reconstructed from the saved +// state. This presents a read only view of the container. +func Load(root, id string) (Container, error) { + if root == "" { return nil, errors.New("root not set") } // when load, we need to check id is valid or not. - if err := l.validateID(id); err != nil { + if err := validateID(id); err != nil { return nil, err } - containerRoot, err := securejoin.SecureJoin(l.Root, id) + containerRoot, err := securejoin.SecureJoin(root, id) if err != nil { return nil, err } - state, err := l.loadState(containerRoot) + state, err := loadState(containerRoot) if err != nil { return nil, err } @@ -181,13 +154,10 @@ func (l *LinuxFactory) Load(id string) (Container, error) { return c, nil } -func (l *LinuxFactory) Type() string { - return "libcontainer" -} - -// StartInitialization loads a container by opening the pipe fd from the parent to read the configuration and state -// This is a low level implementation detail of the reexec and should not be consumed externally -func (l *LinuxFactory) StartInitialization() (err error) { +// StartInitialization loads a container by opening the pipe fd from the parent +// to read the configuration and state. This is a low level implementation +// detail of the reexec and should not be consumed externally. +func StartInitialization() (err error) { // Get the INITPIPE. envInitPipe := os.Getenv("_LIBCONTAINER_INITPIPE") pipefd, err := strconv.Atoi(envInitPipe) @@ -269,7 +239,7 @@ func (l *LinuxFactory) StartInitialization() (err error) { return i.Init() } -func (l *LinuxFactory) loadState(root string) (*State, error) { +func loadState(root string) (*State, error) { stateFilePath, err := securejoin.SecureJoin(root, stateFilename) if err != nil { return nil, err @@ -289,7 +259,7 @@ func (l *LinuxFactory) loadState(root string) (*State, error) { return state, nil } -func (l *LinuxFactory) validateID(id string) error { +func validateID(id string) error { if !idRegex.MatchString(id) || string(os.PathSeparator)+id != utils.CleanPath(string(os.PathSeparator)+id) { return ErrInvalidID } diff --git a/libcontainer/factory_linux_test.go b/libcontainer/factory_linux_test.go index 47f3069953b..c9e2b0e0908 100644 --- a/libcontainer/factory_linux_test.go +++ b/libcontainer/factory_linux_test.go @@ -7,89 +7,14 @@ import ( "reflect" "testing" - "github.com/moby/sys/mountinfo" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/utils" "github.com/opencontainers/runtime-spec/specs-go" - - "golang.org/x/sys/unix" ) -func TestFactoryNew(t *testing.T) { - root := t.TempDir() - factory, err := New(root) - if err != nil { - t.Fatal(err) - } - if factory == nil { - t.Fatal("factory should not be nil") - } - lfactory, ok := factory.(*LinuxFactory) - if !ok { - t.Fatal("expected linux factory returned on linux based systems") - } - if lfactory.Root != root { - t.Fatalf("expected factory root to be %q but received %q", root, lfactory.Root) - } - - if factory.Type() != "libcontainer" { - t.Fatalf("unexpected factory type: %q, expected %q", factory.Type(), "libcontainer") - } -} - -func TestFactoryNewTmpfs(t *testing.T) { - root := t.TempDir() - factory, err := New(root, TmpfsRoot) - if err != nil { - t.Fatal(err) - } - if factory == nil { - t.Fatal("factory should not be nil") - } - lfactory, ok := factory.(*LinuxFactory) - if !ok { - t.Fatal("expected linux factory returned on linux based systems") - } - if lfactory.Root != root { - t.Fatalf("expected factory root to be %q but received %q", root, lfactory.Root) - } - - if factory.Type() != "libcontainer" { - t.Fatalf("unexpected factory type: %q, expected %q", factory.Type(), "libcontainer") - } - mounted, err := mountinfo.Mounted(lfactory.Root) - if err != nil { - t.Fatal(err) - } - if !mounted { - t.Fatalf("Factory Root is not mounted") - } - mounts, err := mountinfo.GetMounts(mountinfo.SingleEntryFilter(lfactory.Root)) - if err != nil { - t.Fatal(err) - } - if len(mounts) != 1 { - t.Fatalf("Factory Root is not listed in mounts list") - } - m := mounts[0] - if m.FSType != "tmpfs" { - t.Fatalf("FSType of root: %s, expected %s", m.FSType, "tmpfs") - } - if m.Source != "tmpfs" { - t.Fatalf("Source of root: %s, expected %s", m.Source, "tmpfs") - } - err = unix.Unmount(root, unix.MNT_DETACH) - if err != nil { - t.Error("failed to unmount root:", err) - } -} - func TestFactoryLoadNotExists(t *testing.T) { - factory, err := New(t.TempDir()) - if err != nil { - t.Fatal(err) - } - _, err = factory.Load("nocontainer") + stateDir := t.TempDir() + _, err := Load(stateDir, "nocontainer") if err == nil { t.Fatal("expected nil error loading non-existing container") } @@ -135,11 +60,7 @@ func TestFactoryLoadContainer(t *testing.T) { if err := marshal(filepath.Join(root, id, stateFilename), expectedState); err != nil { t.Fatal(err) } - factory, err := New(root) - if err != nil { - t.Fatal(err) - } - container, err := factory.Load(id) + container, err := Load(root, id) if err != nil { t.Fatal(err) } diff --git a/libcontainer/integration/checkpoint_test.go b/libcontainer/integration/checkpoint_test.go index c86ec8a0c47..cc45134b621 100644 --- a/libcontainer/integration/checkpoint_test.go +++ b/libcontainer/integration/checkpoint_test.go @@ -62,10 +62,9 @@ func testCheckpoint(t *testing.T, userns bool) { } config := newTemplateConfig(t, &tParam{userns: userns}) - factory, err := libcontainer.New(t.TempDir()) - ok(t, err) + stateDir := t.TempDir() - container, err := factory.Create("test", config) + container, err := libcontainer.Create(stateDir, "test", config) ok(t, err) defer destroyContainer(container) @@ -143,7 +142,7 @@ func testCheckpoint(t *testing.T, userns bool) { ok(t, err) // reload the container - container, err = factory.Load("test") + container, err = libcontainer.Load(stateDir, "test") ok(t, err) restoreStdinR, restoreStdinW, err := os.Pipe() diff --git a/libcontainer/integration/init_test.go b/libcontainer/integration/init_test.go index effcde06d60..b6a3714d493 100644 --- a/libcontainer/integration/init_test.go +++ b/libcontainer/integration/init_test.go @@ -19,11 +19,7 @@ func init() { } runtime.GOMAXPROCS(1) runtime.LockOSThread() - factory, err := libcontainer.New("") - if err != nil { - logrus.Fatalf("unable to initialize for container: %s", err) - } - if err := factory.StartInitialization(); err != nil { + if err := libcontainer.StartInitialization(); err != nil { logrus.Fatal(err) } } diff --git a/libcontainer/integration/utils_test.go b/libcontainer/integration/utils_test.go index def29fc0cd6..c54a40bbe11 100644 --- a/libcontainer/integration/utils_test.go +++ b/libcontainer/integration/utils_test.go @@ -169,11 +169,7 @@ func newContainer(t *testing.T, config *configs.Config) (libcontainer.Container, name := strings.ReplaceAll(t.Name(), "/", "_") + strconv.FormatInt(-int64(time.Now().Nanosecond()), 35) root := t.TempDir() - f, err := libcontainer.New(root) - if err != nil { - return nil, err - } - return f.Create(name, config) + return libcontainer.Create(root, name, config) } // runContainer runs the container with the specific config and arguments diff --git a/libcontainer/restored_process.go b/libcontainer/restored_process.go index cdffbd3aff5..2e81cdf68f1 100644 --- a/libcontainer/restored_process.go +++ b/libcontainer/restored_process.go @@ -79,8 +79,8 @@ func (p *restoredProcess) forwardChildLogs() chan error { } // nonChildProcess represents a process where the calling process is not -// the parent process. This process is created when a factory loads a container from -// a persisted state. +// the parent process. This process is created when Load loads a container +// from a persisted state. type nonChildProcess struct { processPid int processStartTime uint64 diff --git a/list.go b/list.go index 80721e70c78..01e282714fd 100644 --- a/list.go +++ b/list.go @@ -121,11 +121,6 @@ func getContainers(context *cli.Context) ([]containerState, error) { // Report other errors, including non-existent custom --root. return nil, err } - factory, err := libcontainer.New(root) - if err != nil { - return nil, err - } - var s []containerState for _, item := range list { if !item.IsDir() { @@ -146,7 +141,7 @@ func getContainers(context *cli.Context) ([]containerState, error) { owner.Name = fmt.Sprintf("#%d", uid) } - container, err := factory.Load(item.Name()) + container, err := libcontainer.Load(root, item.Name()) if err != nil { fmt.Fprintf(os.Stderr, "load container %s: %v\n", item.Name(), err) continue diff --git a/utils_linux.go b/utils_linux.go index 7ec7b8a79ac..4124d9a7e1d 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -23,19 +23,15 @@ import ( var errEmptyID = errors.New("container id cannot be empty") -// getContainer returns the specified container instance by loading it from state -// with the default factory. +// getContainer returns the specified container instance by loading it from +// a state directory (root). func getContainer(context *cli.Context) (libcontainer.Container, error) { id := context.Args().First() if id == "" { return nil, errEmptyID } root := context.GlobalString("root") - factory, err := libcontainer.New(root) - if err != nil { - return nil, err - } - return factory.Load(id) + return libcontainer.Load(root, id) } func getDefaultImagePath() string { @@ -185,11 +181,7 @@ func createContainer(context *cli.Context, id string, spec *specs.Spec) (libcont } root := context.GlobalString("root") - factory, err := libcontainer.New(root) - if err != nil { - return nil, err - } - return factory.Create(id, config) + return libcontainer.Create(root, id, config) } type runner struct {