bugfix: container cannot start after first start failed #1190
Conversation
HusterWan
force-pushed
the
zr/fix-start-failed
branch
from
586e70c to
de9e32d
Compare
HusterWan
force-pushed
the
zr/fix-start-failed
branch
from
de9e32d to
90cb4c4
Compare
daemon/mgr/container.go
Outdated
| c.meta.State.StartedAt = time.Now().UTC().Format(utils.TimeLayout) | ||
| pid, err := mgr.Client.ContainerPID(ctx, c.ID()) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "failed to get PID of container: %s", c.ID()) |
There was a problem hiding this comment.
please remove : in message, since it will be failed to get PID of container: xxxxxx: khljk
not so readable.
daemon/mgr/container.go
Outdated
| c.meta.State.ExitCode = 0 | ||
|
|
||
| // set Snapshot MergedDir | ||
| c.meta.Snapshotter.Data["MergedDir"] = c.meta.BaseFS | ||
|
|
||
| c.Write(mgr.Store) | ||
| return err |
There was a problem hiding this comment.
return nil is better, right?
HusterWan
force-pushed
the
zr/fix-start-failed
branch
from
90cb4c4 to
0adb39f
Compare
daemon/mgr/container.go
Outdated
| c.meta.State.ExitCode = 0 | ||
|
|
||
| // set Snapshot MergedDir | ||
| c.meta.Snapshotter.Data["MergedDir"] = c.meta.BaseFS | ||
|
|
||
| c.Write(mgr.Store) |
There was a problem hiding this comment.
since the c.Write may return the error, how about return c.Write(mgr.Store)?
There was a problem hiding this comment.
Totally agree with you. @fuweid @HusterWan
Signed-off-by: Michael Wan <zirenwan@gmail.com>
HusterWan
force-pushed
the
zr/fix-start-failed
branch
from
0adb39f to
3364441
Compare
|
ping @HusterWan CI fails according integration system. If this is flaky test, welcome to track this with profiling an issue. build url: https://travis-ci.org/alibaba/pouch/builds/370377809 |
|
LGTM i will merge this pr to fix circle ci failed, if has any problem, feel free to contact me, thanks a lot |
pouchrobot
added
the
LGTM
Signed-off-by: Michael Wan zirenwan@gmail.com
Ⅰ. Describe what this PR did
Fix container start failed problem: if start failed once, all start action will fail because of network endpoint not deleted.
Ⅱ. Does this pull request fix one issue?
fixes #1068
Ⅲ. Describe how you did it
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews