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

feature: upgrade containerd version to v1.0.3 #1148

Merged
merged 1 commit into from
Apr 18, 2018
Merged

feature: upgrade containerd version to v1.0.3 #1148

merged 1 commit into from
Apr 18, 2018

Conversation

fuweid
Copy link
Contributor

@fuweid fuweid commented Apr 17, 2018

Ⅰ. Describe what this PR did

Upgrade containerd to make the pouchd stable.

The version of containerd will be changed from v1.0.0 to v1.0.3

Ⅱ. Does this pull request fix one issue?

fixes #1141

Ⅲ. Describe how you did it

# for the vendor
$ govendor fetch github.com/containerd/containerd@v1.0.3

Ⅳ. Describe how to verify it

It's the dependency upgrade. I have tested pouch run -it busybox /bin/sh in local and it passed.
It should be covered by CI.

Ⅴ. Special notes for reviews

NONE

@codecov-io
Copy link

codecov-io commented Apr 17, 2018

Codecov Report

Merging #1148 into master will decrease coverage by 0.7%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1148      +/-   ##
==========================================
- Coverage   16.03%   15.32%   -0.71%     
==========================================
  Files         173      173              
  Lines        9904     9779     -125     
==========================================
- Hits         1588     1499      -89     
+ Misses       8200     8172      -28     
+ Partials      116      108       -8
Impacted Files Coverage Δ
client/client.go 28.57% <0%> (-30.38%) ⬇️
pkg/kernel/kernel.go 72.72% <0%> (-6.23%) ⬇️
cli/command.go 0% <0%> (ø) ⬆️
daemon/mgr/system.go 0% <0%> (ø) ⬆️
cli/info.go 0% <0%> (ø) ⬆️
cli/cli.go 0% <0%> (ø) ⬆️
apis/opts/labels.go
apis/opts/memory_swappiness.go
apis/opts/memory.go
apis/opts/restart_policy.go
... and 37 more

@allencloud
Copy link
Collaborator

Please help to review if the containerd 1.0.3 would influence the current pouchd. @HusterWan

In addition, please help review the installation part to check if more improvement in the package building is needed. @Letty5411

@@ -48,10 +48,12 @@
"revisionTime": "2017-09-25T15:48:32Z"
},
{
"checksumSHA1": "3tWZA853zC/N1yoGDEkkye76Vv0=",
"checksumSHA1": "1ny6UV3Oq4kaqpQtM8bs9W9e3vw=",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am afraid that beside this package github.com/containerd/containerd we still have some sub packages in vendor.json, such as github.com/containerd/containerd/api/services/containers/v1, github.com/containerd/containerd/api/services/content/v1.

I have two parts to confirm:

  • Do we still need all the sub packages?
  • If the above answer is yes, do we need to make all sub packages and containerd package with the same version v1.0.3?

Seems to have discussion offline with @rudyfly , please add some comment if you have any thoughts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I will check the sub-packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@allencloud and @rudyfly

I use govendor fetch github.com/containerd/containerd/...@v1.0.3 to fetch the v1.0.3. But this command will import the github.com/Microsoft/go-winio package in the vendor. Since we have the package in the libnetwork/Godeps, in order to keep the consistent, I decided to remove the package after update command govendor fetch github.com/containerd/containerd/...@v1.0.3.

If you have any questions, please add the comment. Thanks

@Letty5411
Copy link
Contributor

@fuweid the following script also need to be updated, they are used to build package:

./hack/package/deb/Makefile:	wget --quiet https://github.com/containerd/containerd/releases/download/v1.0.0/containerd-1.0.0.linux-amd64.tar.gz -P $(TMP)
./hack/package/rpm/build.sh:    wget --quiet https://github.com/containerd/containerd/releases/download/v1.0.0/containerd-1.0.0.linux-amd64.tar.gz -P $TMP

@pouchrobot pouchrobot added size/XL and removed size/M labels Apr 18, 2018
Not just vendor, but also update the docs.

fix #1141

Signed-off-by: Wei Fu <fhfuwei@163.com>
@fuweid
Copy link
Contributor Author

fuweid commented Apr 18, 2018

@Letty5411 I have updated based on your comment. Please help me to check. Thanks

@Letty5411
Copy link
Contributor

Please help me to check. Thanks

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Apr 18, 2018
@HusterWan
Copy link
Contributor

containerd/containerd#2186, i think upgraded containerd should contain this pr,
grpc-go has some bug in old version which used by containerd, so we should also upgrade grpc-go version.

@fuweid @allencloud WDYT?

@fuweid
Copy link
Contributor Author

fuweid commented Apr 18, 2018

containerd/containerd#2186, i think upgraded containerd should contain this pr,
grpc-go has some bug in old version which used by containerd, so we should also upgrade grpc-go version.

Hi @HusterWan ,

could you help us to verify the "hang" case in pouch?
It seems that we can just only update the client version because the grpc-go-1.10.1 updated the client part by grpc/grpc-go#1943

@allencloud
Copy link
Collaborator

I think upgraded containerd should contain this pr. grpc-go has some bug in old version which used by containerd, so we should also upgrade grpc-go version.

@HusterWan Actually I am not the side of using a non-released version of containerd, although the non-release version covered the grpc-go bug you mentioned.

@allencloud
Copy link
Collaborator

LGTM

@allencloud allencloud merged commit b14e144 into AliyunContainerService:master Apr 18, 2018
@fuweid fuweid deleted the feature_update_containerd_to_v103 branch August 3, 2018 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature LGTM one maintainer or community participant agrees to merge the pull reuqest. size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature request] vendor containerd 1.0.3 for project
6 participants