feature: finish upgrade interface #923

HusterWan · 2018-03-20T02:37:28Z

Signed-off-by: Michael Wan zirenwan@gmail.com

Ⅰ. Describe what this PR did

finish upgrade interface daemon part

root@osboxes:wanziren -> pouch upgrade --name test1 registry.hub.docker.com/library/busybox:latest
test1

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

1. Create a new container

root@osboxes:pouch (zr/finish-upgrade) -> pouch inspect test2
{
    "Args": null,
    "Config": {
        "Cmd": [
            "sh"
        ],
        "Entrypoint": null,
        "Env": [
            "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
        ],
        "Image": "registry.hub.docker.com/library/busybox:latest",
        "OnBuild": null,
        "Shell": null
    },
    "Created": "2018-03-20T02:19:05.418084614Z",
    "HostConfig": {
        "NetworkMode": "bridge",
        "RestartPolicy": {
            "Name": "no"
        },
        "Runtime": "runc",
        "BlkioDeviceReadBps": null,
        "BlkioDeviceReadIOps": null,
        "BlkioDeviceWriteBps": null,
        "BlkioDeviceWriteIOps": null,
        "BlkioWeightDevice": null,
        "CgroupParent": "default",
        "DeviceCgroupRules": null,
        "Devices": [],
        "Memory": 20971520,
        "MemoryExtra": 0,
        "MemorySwappiness": -1,
        "MemoryWmarkRatio": 0,
        "Ulimits": null
    },
    "Id": "4c58d27f58d38776dda31c01c897bbf554c802a9b80ae4dc20be1337f8a969f2",
    "Image": "registry.hub.docker.com/library/busybox:latest",
    "Mounts": null,
    "Name": "test2",
    "NetworkSettings": {
        "Networks": {
            "bridge": {
                "Aliases": null,
                "EndpointID": "e3845861204350e6ec1ac2fbfaece7d3fe384ed0f5c01b41b53478d89b70a131",
                "Gateway": "172.17.0.1",
                "IPAddress": "172.17.0.2/24",
                "IPPrefixLen": 24,
                "Links": null,
                "MacAddress": "02:42:ac:11:00:02",
                "NetworkID": "59a8a8a365171ea1862f11534be913a81a6b4fc39f516b3121fb71c52b3dc2f8"
            }
        },
        "SecondaryIPAddresses": null,
        "SecondaryIPv6Addresses": null
    },
    "State": {
        "Pid": 4895,
        "StartedAt": "2018-03-20T02:19:05.780853724Z",
        "Status": "running"
    }
}

1. Update container's image

root@osboxes:pouch (zr/finish-upgrade) -> pouch upgrade --name test2 registry.hub.docker.com/library/ubuntu:14.04
test2

1. Check the container's status

root@osboxes:pouch (zr/finish-upgrade) -> pouch ps
Name    ID       Status          Created          Image                                                Runtime
test2   4c58d2   Up 2 minutes    15 minutes ago   registry.hub.docker.com/library/ubuntu:14.04         runc

1. Inspect the container's meta info

root@osboxes:pouch (zr/finish-upgrade) -> pouch inspect test2
{
    "Args": null,
    "Config": {
        "Cmd": [
            "sh"
        ],
        "Entrypoint": null,
        "Env": [
            "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
        ],
        "Image": "registry.hub.docker.com/library/ubuntu:14.04",
        "OnBuild": null,
        "Shell": null
    },
    "Created": "2018-03-20T02:19:05.418084614Z",
    "HostConfig": {
        "NetworkMode": "bridge",
        "RestartPolicy": {
            "Name": "no"
        },
        "Runtime": "runc",
        "BlkioDeviceReadBps": null,
        "BlkioDeviceReadIOps": null,
        "BlkioDeviceWriteBps": null,
        "BlkioDeviceWriteIOps": null,
        "BlkioWeightDevice": null,
        "CgroupParent": "default",
        "DeviceCgroupRules": null,
        "Devices": [],
        "Memory": 20971520,
        "MemoryExtra": 0,
        "MemorySwappiness": -1,
        "MemoryWmarkRatio": 0,
        "Ulimits": null
    },
    "Id": "4c58d27f58d38776dda31c01c897bbf554c802a9b80ae4dc20be1337f8a969f2",
    "Image": "registry.hub.docker.com/library/ubuntu:14.04",
    "Mounts": null,
    "Name": "test2",
    "NetworkSettings": {
        "Networks": {
            "bridge": {
                "Aliases": null,
                "EndpointID": "e3845861204350e6ec1ac2fbfaece7d3fe384ed0f5c01b41b53478d89b70a131",
                "Gateway": "172.17.0.1",
                "IPAddress": "172.17.0.2/24",
                "IPPrefixLen": 24,
                "Links": null,
                "MacAddress": "02:42:ac:11:00:02",
                "NetworkID": "59a8a8a365171ea1862f11534be913a81a6b4fc39f516b3121fb71c52b3dc2f8"
            }
        },
        "SecondaryIPAddresses": null,
        "SecondaryIPv6Addresses": null
    },
    "State": {
        "Pid": 8251,
        "StartedAt": "2018-03-20T02:32:43.874327747Z",
        "Status": "running"
    }
}

Ⅴ. Special notes for reviews

codecov-io · 2018-03-20T07:28:51Z

Codecov Report

Merging #923 into master will decrease coverage by 0.14%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #923      +/-   ##
==========================================
- Coverage   13.06%   12.92%   -0.15%     
==========================================
  Files         123      123              
  Lines        8235     8325      +90     
==========================================
  Hits         1076     1076              
- Misses       7063     7153      +90     
  Partials       96       96

Impacted Files	Coverage Δ
pkg/utils/utils.go	`82.08% <ø> (ø)`	⬆️
client/container.go	`0% <0%> (ø)`	⬆️
cli/upgrade.go	`0% <0%> (ø)`	⬆️
daemon/mgr/container.go	`2.06% <0%> (-0.19%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9f3816...c26160c. Read the comment docs.

allencloud · 2018-03-20T09:23:04Z

Thanks a lot for the work you made. I am wondering adding some integration test will guarantee this much better. @HusterWan 💟

allencloud · 2018-03-24T00:12:18Z

daemon/mgr/container.go

+	// check the image existed or not, and convert image id to image ref
+	image, err := mgr.ImageMgr.GetImage(ctx, config.Image)
+	if err != nil {
+		return errors.Wrap(err, "get image failed")


failed to get image

allencloud · 2018-03-24T00:13:28Z

daemon/mgr/container.go

+	if len(image.RepoTags) > 0 {
+		ref = image.RepoTags[0]
+	} else {
+		ref = image.RepoDigests[0]


Is there possibility that this image.RepoDigests has a length of 0?
also ping @YaoZengzeng @zeppp

allencloud · 2018-03-24T00:16:28Z

daemon/mgr/container.go

+		if needRollback {
+			c.meta = &backupContainerMeta
+			if err := mgr.Client.CreateSnapshot(ctx, c.ID(), c.meta.Image); err != nil {
+				logrus.Errorf("Upgrade rollback failed to create snapshot: %v", err)


failed to create snapshot when rollback upgrade: %v

allencloud · 2018-03-24T00:18:24Z

daemon/mgr/container.go

+				time.Sleep(100 * time.Millisecond)
+
+				msg, err := mgr.Client.GetSnapshot(ctx, c.ID())
+				logrus.Info("snapshots %+v", msg)


If err != nil, just double check whether msg has data in itself.

if error occured , msg will be a empty Snapshot.Info{}

allencloud · 2018-03-24T00:35:36Z

test/cli_upgrade_test.go

+	command.PouchRun("run", "-d", "--name", name, busyboxImage).Assert(c, icmd.Success)
+
+	res := command.PouchRun("upgrade", "--name", name, busyboxImage)
+	c.Assert(res.Error, check.IsNil)


I am wondering if we do not upgrade anything, should we return an error indicating nothing to upgrade and no status update happens in daemon side. @HusterWan WDYT

allencloud · 2018-03-24T05:21:02Z

CI fails:

FAIL: /go/src/github.com/alibaba/pouch/test/cli_upgrade_test.go:58: PouchUpgradeSuite.TestPouchUpgradeNoChange
/go/src/github.com/alibaba/pouch/test/cli_upgrade_test.go:64:
    c.Assert(res.Error, check.NotNil)
... value = nil

@HusterWan

Signed-off-by: Michael Wan <zirenwan@gmail.com>

allencloud · 2018-03-25T16:10:36Z

LGTM
And I think we need to add a document to tell users how to take advantages of this API. WDYT? @HusterWan

HusterWan · 2018-03-26T05:09:13Z

And I think we need to add a document to tell users how to take advantages of this API. WDYT? @HusterWan

Yes, i will add the doc after i have a tiime, but i will create a issue to reminder me.

HusterWan force-pushed the zr/finish-upgrade branch from c3f1390 to 2f087b2 Compare March 20, 2018 06:59

pouchrobot added the size/XXL label Mar 20, 2018

pouchrobot added the conflict/needs-rebase label Mar 23, 2018

HusterWan force-pushed the zr/finish-upgrade branch from 2f087b2 to 938d668 Compare March 23, 2018 07:00

pouchrobot removed the conflict/needs-rebase label Mar 23, 2018

HusterWan force-pushed the zr/finish-upgrade branch from 938d668 to a501cd6 Compare March 23, 2018 15:29

HusterWan changed the title ~~[WIP] feature: finish upgrade interface~~ feature: finish upgrade interface Mar 23, 2018

pouchrobot added the kind/feature label Mar 23, 2018

allencloud reviewed Mar 24, 2018

View reviewed changes

HusterWan force-pushed the zr/finish-upgrade branch from a501cd6 to ca2c657 Compare March 24, 2018 01:18

feature: finish upgrade interface

c26160c

Signed-off-by: Michael Wan <zirenwan@gmail.com>

HusterWan force-pushed the zr/finish-upgrade branch from ca2c657 to c26160c Compare March 24, 2018 14:59

pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Mar 25, 2018

allencloud merged commit 5b6ed3e into AliyunContainerService:master Mar 25, 2018

HusterWan mentioned this pull request Mar 26, 2018

[feature request] Add a new api "POST /containers/upgrade" to upgrade a container #823

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: finish upgrade interface #923

feature: finish upgrade interface #923

HusterWan commented Mar 20, 2018

codecov-io commented Mar 20, 2018 •

edited

Loading

allencloud commented Mar 20, 2018

allencloud Mar 24, 2018

allencloud Mar 24, 2018

allencloud Mar 24, 2018

allencloud Mar 24, 2018

HusterWan Mar 24, 2018

allencloud Mar 24, 2018

allencloud commented Mar 24, 2018

allencloud commented Mar 25, 2018

HusterWan commented Mar 26, 2018

feature: finish upgrade interface #923

feature: finish upgrade interface #923

Conversation

HusterWan commented Mar 20, 2018

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

codecov-io commented Mar 20, 2018 • edited Loading

Codecov Report

allencloud commented Mar 20, 2018

allencloud Mar 24, 2018

Choose a reason for hiding this comment

allencloud Mar 24, 2018

Choose a reason for hiding this comment

allencloud Mar 24, 2018

Choose a reason for hiding this comment

allencloud Mar 24, 2018

Choose a reason for hiding this comment

HusterWan Mar 24, 2018

Choose a reason for hiding this comment

allencloud Mar 24, 2018

Choose a reason for hiding this comment

allencloud commented Mar 24, 2018

allencloud commented Mar 25, 2018

HusterWan commented Mar 26, 2018

codecov-io commented Mar 20, 2018 •

edited

Loading