Skip to content

Commit

Permalink
libcontainer: remove LinuxFactory
Browse files Browse the repository at this point in the history
Since LinuxFactory has become the means to specify containers state
top directory (aka --root), and is only used by two methods (Create
and Load), it is easier to pass root to them directly.

Modify all the users and the docs accordingly.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
  • Loading branch information
kolyshkin committed Feb 12, 2022
1 parent 87d2685 commit 0f79e80
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 100 deletions.
18 changes: 4 additions & 14 deletions libcontainer/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,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
Expand Down Expand Up @@ -242,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
Expand Down
8 changes: 4 additions & 4 deletions libcontainer/cgroups/manager/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
50 changes: 21 additions & 29 deletions libcontainer/factory_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,35 +27,23 @@ const (

var idRegex = regexp.MustCompile(`^[\w+-\.]+$`)

// New returns a linux based container factory based in the root directory.
func New(root string) (*LinuxFactory, error) {
if err := os.MkdirAll(root, 0o700); err != nil {
return nil, err
}
return &LinuxFactory{
Root: root,
}, nil
}

// LinuxFactory implements the default factory interface for linux based systems.
type LinuxFactory struct {
// Root directory for the factory to store state.
Root string
}

// Create creates a new container with the given id and starts the initial
// process inside it.
// Create creates a new container with the given id inside a given state
// directory (root), and starts the initial process inside it.
//
// 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 consists of only the following characters:
// ASCII letters, digits, underscore, plus, minus, period. The id must be
// unique and non-existent for the factory with same root path.
// unique and non-existent for the given root path.
//
// Returns the new container with a running process.
//
// On error, any partially created container parts are cleaned up (the
// operation is atomic).
func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, error) {
if l.Root == "" {
func Create(root, id string, config *configs.Config) (Container, error) {
if root == "" {
return nil, errors.New("root not set")
}
if err := validateID(id); err != nil {
Expand All @@ -64,7 +52,10 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, 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
}
Expand Down Expand Up @@ -108,7 +99,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{
Expand All @@ -122,18 +114,18 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err
return c, nil
}

// 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.
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 the container information from the stateDir. 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 := 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
}
Expand Down
27 changes: 3 additions & 24 deletions libcontainer/factory_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,9 @@ import (
"github.com/opencontainers/runtime-spec/specs-go"
)

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")
}
if factory.Root != root {
t.Fatalf("expected factory root to be %q but received %q", root, factory.Root)
}
}

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")
}
Expand Down Expand Up @@ -77,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)
}
Expand Down
7 changes: 3 additions & 4 deletions libcontainer/integration/checkpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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()
Expand Down
6 changes: 1 addition & 5 deletions libcontainer/integration/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions libcontainer/restored_process.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 1 addition & 6 deletions list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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
Expand Down
16 changes: 4 additions & 12 deletions utils_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 0f79e80

Please sign in to comment.