Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update state after update #1558

Merged
merged 1 commit into from
Aug 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about external users of libcontainer, but runc's own update.go modifies container config in place, meaning &config and c.config is exactly the same thing, meaning this "rollback" does not make sense.

@hqhq do you remember why you introduced it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kolyshkin Libcontainer was supposed to be used without runc, so the config parameter can be different from c.config, if I remember right, before I added runc update, docker was using this Set function of libcontainer directly for docker update, this rollback was valid at that time.

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 {
Expand Down Expand Up @@ -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
Expand Down
63 changes: 63 additions & 0 deletions libcontainer/container_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
}