From e6e1c34a7dea384a771798616f1330fcc9122e04 Mon Sep 17 00:00:00 2001 From: Qiang Huang Date: Tue, 15 Aug 2017 14:30:58 +0800 Subject: [PATCH] Update state after update state.json should be a reflection of the container's realtime state, including resource configurations, so we should update state.json after updating container resources. Signed-off-by: Qiang Huang --- libcontainer/container_linux.go | 15 ++++++- libcontainer/container_linux_test.go | 63 ++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index d7e7516e5..cbdfe0d5c 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -186,8 +186,17 @@ func (c *linuxContainer) Set(config configs.Config) error { if status == Stopped { return newGenericError(fmt.Errorf("container not running"), ContainerNotRunning) } + if err := c.cgroupManager.Set(&config); err != nil { + // Set configs back + if err2 := c.cgroupManager.Set(c.config); err2 != nil { + logrus.Warnf("Setting back cgroup configs failed due to error: %v, your state.json and actual configs might be inconsistent.", err2) + } + return err + } + // After config setting succeed, update config and states c.config = &config - return c.cgroupManager.Set(c.config) + _, err = c.updateState(nil) + return err } func (c *linuxContainer) Start(process *Process) error { @@ -1388,7 +1397,9 @@ func (c *linuxContainer) criuNotifications(resp *criurpc.CriuResp, process *Proc } func (c *linuxContainer) updateState(process parentProcess) (*State, error) { - c.initProcess = process + if process != nil { + c.initProcess = process + } state, err := c.currentState() if err != nil { return nil, err diff --git a/libcontainer/container_linux_test.go b/libcontainer/container_linux_test.go index 3fdd4a809..75c289852 100644 --- a/libcontainer/container_linux_test.go +++ b/libcontainer/container_linux_test.go @@ -9,6 +9,7 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/configs" + "github.com/opencontainers/runc/libcontainer/system" ) type mockCgroupManager struct { @@ -216,3 +217,65 @@ func TestGetContainerState(t *testing.T) { } } } + +func TestGetContainerStateAfterUpdate(t *testing.T) { + var ( + pid = os.Getpid() + ) + stat, err := system.Stat(pid) + if err != nil { + t.Fatal(err) + } + container := &linuxContainer{ + id: "myid", + config: &configs.Config{ + Namespaces: []configs.Namespace{ + {Type: configs.NEWPID}, + {Type: configs.NEWNS}, + {Type: configs.NEWNET}, + {Type: configs.NEWUTS}, + {Type: configs.NEWIPC}, + }, + Cgroups: &configs.Cgroup{ + Resources: &configs.Resources{ + Memory: 1024, + }, + }, + }, + initProcess: &mockProcess{ + _pid: pid, + started: stat.StartTime, + }, + cgroupManager: &mockCgroupManager{}, + } + container.state = &createdState{c: container} + state, err := container.State() + if err != nil { + t.Fatal(err) + } + if state.InitProcessPid != pid { + t.Fatalf("expected pid %d but received %d", pid, state.InitProcessPid) + } + if state.InitProcessStartTime != stat.StartTime { + t.Fatalf("expected process start time %d but received %d", stat.StartTime, state.InitProcessStartTime) + } + if state.Config.Cgroups.Resources.Memory != 1024 { + t.Fatalf("expected Memory to be 1024 but received %q", state.Config.Cgroups.Memory) + } + + // Set initProcessStartTime so we fake to be running + container.initProcessStartTime = state.InitProcessStartTime + container.state = &runningState{c: container} + newConfig := container.Config() + newConfig.Cgroups.Resources.Memory = 2048 + if err := container.Set(newConfig); err != nil { + t.Fatal(err) + } + state, err = container.State() + if err != nil { + t.Fatal(err) + } + if state.Config.Cgroups.Resources.Memory != 2048 { + t.Fatalf("expected Memory to be 2048 but received %q", state.Config.Cgroups.Memory) + } +}