From c54b7a76a2abac30a22369179bf12fa0335feffa Mon Sep 17 00:00:00 2001 From: Antonios Motakis Date: Fri, 30 Jun 2017 17:59:00 +0200 Subject: [PATCH 1/3] Add container state as defined by OCI Track the OCI container states: - creating - created - running - stopped Signed-off-by: Antonios Motakis --- containerd/api/grpc/server/server.go | 2 +- containerd/containerd.go | 4 ++-- supervisor/container.go | 27 +++++++++++++++++++++++++++ supervisor/events.go | 8 +++++--- 4 files changed, 35 insertions(+), 6 deletions(-) diff --git a/containerd/api/grpc/server/server.go b/containerd/api/grpc/server/server.go index fbf5175e..497dd707 100644 --- a/containerd/api/grpc/server/server.go +++ b/containerd/api/grpc/server/server.go @@ -230,7 +230,7 @@ func supervisorContainer2ApiContainer(c *supervisor.Container) *types.Container return &types.Container{ Id: c.Id, BundlePath: c.BundlePath, - Status: "running", + Status: c.Status, Runtime: "runv", } } diff --git a/containerd/containerd.go b/containerd/containerd.go index 0088a9d6..c12c1672 100644 --- a/containerd/containerd.go +++ b/containerd/containerd.go @@ -189,9 +189,9 @@ func namespaceShare(sv *supervisor.Supervisor, namespace, state string) { events := sv.Events.Events(time.Time{}) containerCount := 0 for e := range events { - if e.Type == supervisor.EventContainerStart { + if e.Type == supervisor.EventContainerCreate { containerCount++ - } else if e.Type == supervisor.EventExit && e.PID == "init" { + } else if e.Type == supervisor.EventContainerDelete { containerCount-- if containerCount == 0 { syscall.Kill(0, syscall.SIGQUIT) diff --git a/supervisor/container.go b/supervisor/container.go index e31ccb81..f49eba48 100644 --- a/supervisor/container.go +++ b/supervisor/container.go @@ -20,8 +20,17 @@ import ( "github.com/opencontainers/runtime-spec/specs-go" ) +// container states +const ( + ContainerStateCreating = "creating" + ContainerStateCreated = "created" + ContainerStateRunning = "running" + ContainerStateStopped = "stopped" +) + type Container struct { Id string + Status string BundlePath string Spec *specs.Spec Processes map[string]*Process @@ -50,6 +59,7 @@ func (c *Container) start(p *Process) error { } go func() { + c.Status = ContainerStateRunning e := Event{ ID: c.Id, Type: EventContainerStart, @@ -58,6 +68,7 @@ func (c *Container) start(p *Process) error { c.ownerPod.sv.Events.notifySubscribers(e) exit, err := c.wait(p, res) + c.Status = ContainerStateStopped e = Event{ ID: c.Id, Type: EventExit, @@ -75,6 +86,7 @@ func (c *Container) start(p *Process) error { } func (c *Container) create() error { + c.Status = ContainerStateCreating glog.V(3).Infof("prepare hypervisor info") config := api.ContainerDescriptionFromOCF(c.Id, c.Spec) @@ -182,6 +194,14 @@ func (c *Container) create() error { return err } + c.Status = ContainerStateCreated + e := Event{ + ID: c.Id, + Type: EventContainerCreate, + Timestamp: time.Now(), + } + c.ownerPod.sv.Events.notifySubscribers(e) + return nil } @@ -354,6 +374,13 @@ func (c *Container) reap() { utils.Umount(containerRoot) os.RemoveAll(containerRoot) os.RemoveAll(filepath.Join(c.ownerPod.sv.StateDir, c.Id)) + + e := Event{ + ID: c.Id, + Type: EventContainerDelete, + Timestamp: time.Now(), + } + c.ownerPod.sv.Events.notifySubscribers(e) } func mountToRootfs(m *specs.Mount, rootfs, mountLabel string) error { diff --git a/supervisor/events.go b/supervisor/events.go index 3ac2ea49..bc433089 100644 --- a/supervisor/events.go +++ b/supervisor/events.go @@ -12,9 +12,11 @@ import ( ) const ( - EventExit = "exit" - EventContainerStart = "start-container" - EventProcessStart = "start-process" + EventExit = "exit" + EventContainerCreate = "create-container" + EventContainerStart = "start-container" + EventContainerDelete = "delete-container" + EventProcessStart = "start-process" ) var ( From 9eec33d93fad3c5782f64fdb96b7d5718bd5dba2 Mon Sep 17 00:00:00 2001 From: Antonios Motakis Date: Mon, 3 Jul 2017 15:25:34 +0200 Subject: [PATCH 2/3] Return the right container status in OCI interface The list and state commands of the runv OCI interface always return "running" as the status of the container. Return the right status of each container via the containerd API. Signed-off-by: Antonios Motakis --- list.go | 10 +++++++++- state.go | 9 ++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/list.go b/list.go index 5aabc4a4..6980448c 100644 --- a/list.go +++ b/list.go @@ -9,6 +9,7 @@ import ( "text/tabwriter" "time" + "github.com/hyperhq/runv/supervisor" "github.com/opencontainers/runtime-spec/specs-go" "github.com/urfave/cli" ) @@ -110,15 +111,22 @@ func getContainers(context *cli.Context) ([]containerState, error) { if err != nil && !os.IsNotExist(err) { return nil, fmt.Errorf("Stat file %s error: %s", stateFile, err.Error()) } + state, err := loadStateFile(stateFile) if err != nil { return nil, fmt.Errorf("Load state file %s failed: %s", stateFile, err.Error()) } + status := supervisor.ContainerStateStopped // if we can't connect to runv-containerd then the container is stopped + if c, err := getContainerApi(context, item.Name()); err == nil { + status = c.Status + } + + // FIXME: refactor to get container state only via API s = append(s, containerState{ ID: state.ID, InitProcessPid: state.Pid, - Status: "running", + Status: status, Bundle: state.Bundle, Created: fi.ModTime(), }) diff --git a/state.go b/state.go index a3a7bfb4..3c79d8c1 100644 --- a/state.go +++ b/state.go @@ -9,6 +9,7 @@ import ( "path/filepath" "time" + "github.com/hyperhq/runv/supervisor" "github.com/urfave/cli" ) @@ -79,11 +80,17 @@ func getContainer(context *cli.Context, name string) (*cState, error) { return nil, fmt.Errorf("Load state file %s failed: %s", stateFile, err.Error()) } + status := supervisor.ContainerStateStopped // if we can't connect to runv-containerd then the container is stopped + if c, err := getContainerApi(context, name); err == nil { + status = c.Status + } + + // FIXME: refactor to get container state only via API s := &cState{ Version: state.Version, ID: state.ID, InitProcessPid: state.Pid, - Status: "running", + Status: status, Bundle: state.Bundle, Rootfs: filepath.Join(state.Bundle, "rootfs"), Created: fi.ModTime(), From b6889b7254414ed983758915a4534717dfe318d0 Mon Sep 17 00:00:00 2001 From: Antonios Motakis Date: Fri, 30 Jun 2017 18:05:49 +0200 Subject: [PATCH 3/3] Add explicit delete command to the OCI interface Currently runv automatically destroys any container that finish executing. The OCI spec defines that the container shall enter a stopped status instead; an explicit delete command is defined for cleaning up any container resources. Don't delete the container automatically on exit, and add an explicit delete command to the OCI interface of runv. To get around the limited containerd interface of runv, use a special status for containers that are about to be deleted. Signed-off-by: Antonios Motakis --- containerd/api/grpc/server/server.go | 12 ++++++- delete.go | 52 ++++++++++++++++++++++++++++ integration-test/kill_test.go | 4 +-- main.go | 1 + supervisor/container.go | 1 + supervisor/supervisor.go | 28 ++++++++++----- 6 files changed, 87 insertions(+), 11 deletions(-) create mode 100644 delete.go diff --git a/containerd/api/grpc/server/server.go b/containerd/api/grpc/server/server.go index 497dd707..be29033c 100644 --- a/containerd/api/grpc/server/server.go +++ b/containerd/api/grpc/server/server.go @@ -147,7 +147,17 @@ func (s *apiServer) State(ctx context.Context, r *types.StateRequest) (*types.St } func (s *apiServer) UpdateContainer(ctx context.Context, r *types.UpdateContainerRequest) (*types.UpdateContainerResponse, error) { - return nil, errors.New("UpdateContainer() not implemented yet") + glog.V(3).Infof("gRPC handle UpdateContainer") + + if r.Status != supervisor.ContainerStateDeleted { + return nil, fmt.Errorf("UpdateContainer() status %s not supported", r.Status) + } + + err := s.sv.DeleteContainer(r.Id) + if err != nil { + return nil, err + } + return &types.UpdateContainerResponse{}, nil } func (s *apiServer) UpdateProcess(ctx context.Context, r *types.UpdateProcessRequest) (*types.UpdateProcessResponse, error) { diff --git a/delete.go b/delete.go new file mode 100644 index 00000000..ec812175 --- /dev/null +++ b/delete.go @@ -0,0 +1,52 @@ +package main + +import ( + "fmt" + "os" + "path/filepath" + + "github.com/hyperhq/runv/containerd/api/grpc/types" + "github.com/hyperhq/runv/supervisor" + "github.com/urfave/cli" + netcontext "golang.org/x/net/context" +) + +var deleteCommand = cli.Command{ + Name: "delete", + Usage: "delete a stopped container", + ArgsUsage: ``, + Action: func(context *cli.Context) error { + container := context.Args().First() + if container == "" { + return cli.NewExitError("container id cannot be empty", -1) + } + + containerPath := filepath.Join(context.GlobalString("root"), container) + + if dir, err := os.Stat(containerPath); err != nil || !dir.IsDir() { + return fmt.Errorf("container %s does not exist", container) + } + + api, err := getClient(filepath.Join(containerPath, "namespace/namespaced.sock")) + if err != nil { + return fmt.Errorf("failed to get client: %v", err) + } + + _, err = api.GetServerVersion(netcontext.Background(), nil) + if err != nil { + // if we can't connect to the api, runv was killed before it could clean up the stopped containers + err := os.RemoveAll(containerPath) + if err != nil { + return fmt.Errorf("delete stale container %s failed, %v", container, err) + } + return nil + } + + _, err = api.UpdateContainer(netcontext.Background(), &types.UpdateContainerRequest{Id: container, Status: supervisor.ContainerStateDeleted}) + if err != nil { + return fmt.Errorf("delete container %s failed, %v", container, err) + } + + return nil + }, +} diff --git a/integration-test/kill_test.go b/integration-test/kill_test.go index 199ad0d7..d1382533 100644 --- a/integration-test/kill_test.go +++ b/integration-test/kill_test.go @@ -35,9 +35,9 @@ func (s *RunVSuite) TestKillKILL(c *check.C) { timeout := true for count := 0; count < 10; count++ { - out, exitCode := s.runvCommand(c, "list") + out, exitCode := s.runvCommand(c, "state", ctrName) c.Assert(exitCode, checker.Equals, 0) - if !strings.Contains(out, ctrName) { + if !strings.Contains(out, "running") { timeout = false break } diff --git a/main.go b/main.go index 64d2aea8..e833ba98 100644 --- a/main.go +++ b/main.go @@ -162,6 +162,7 @@ func main() { pauseCommand, resumeCommand, containerd.ContainerdCommand, + deleteCommand, } if err := app.Run(os.Args); err != nil { fmt.Fprintf(os.Stderr, "%v", err) diff --git a/supervisor/container.go b/supervisor/container.go index f49eba48..1b34b34e 100644 --- a/supervisor/container.go +++ b/supervisor/container.go @@ -26,6 +26,7 @@ const ( ContainerStateCreated = "created" ContainerStateRunning = "running" ContainerStateStopped = "stopped" + ContainerStateDeleted = "deleted" // special state that will lead to deleting the container resources ) type Container struct { diff --git a/supervisor/supervisor.go b/supervisor/supervisor.go index ca92672e..374b9695 100644 --- a/supervisor/supervisor.go +++ b/supervisor/supervisor.go @@ -94,6 +94,26 @@ func (sv *Supervisor) StartContainer(container string, spec *specs.Spec) (c *Con return nil, nil, fmt.Errorf("container %s is not found for StartContainer()", container) } +func (sv *Supervisor) DeleteContainer(container string) error { + glog.Infof("delete container %s", container) + sv.Lock() + defer sv.Unlock() + + if c, ok := sv.Containers[container]; ok { + c.reap() + delete(c.ownerPod.Containers, container) + delete(sv.Containers, container) + + if len(c.ownerPod.Containers) == 0 { + c.ownerPod.reap() + } + + return nil + } + + return fmt.Errorf("Container %s not found", container) +} + func (sv *Supervisor) AddProcess(container, processId, stdin, stdout, stderr string, spec *specs.Process) (*Process, error) { sv.Lock() defer sv.Unlock() @@ -164,14 +184,6 @@ func (sv *Supervisor) reap(container, processId string) { // TODO: kill all the other existing processes in the same container } } - if len(c.Processes) == 0 { - c.reap() - delete(c.ownerPod.Containers, container) - delete(sv.Containers, container) - } - if len(c.ownerPod.Containers) == 0 { - c.ownerPod.reap() - } } }