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

[WIP] Upgrade to containerd v1.2 #2530

Closed
wants to merge 2 commits into from
Closed

[WIP] Upgrade to containerd v1.2 #2530

wants to merge 2 commits into from

Conversation

fuweid
Copy link
Contributor

@fuweid fuweid commented Dec 4, 2018

Ⅰ. Describe what this PR did

The containerd has been improved a lot in v1.2, which makes the service
stable than before. Based on this, we need to do the upgrade.

using following commands to update the vendor

govendor fetch github.com/containerd/containerd/...@v1.2.0
govendor fetch github.com/gogo/protobuf/...@v1.0.0
govendor fetch github.com/gogo/googleapis/...@08a7655d27152912db7aaf4f983275eaf8d128ef
govendor fetch google.golang.org/grpc/...@v1.12.0
govendor fetch github.com/golang/protobuf/...@v1.1.0

Basically, both the contaienrd and shim API interfaces are stable so
that we don't need to worry about the remote API calls. However, the
package level interface has been changed a lot. The commit is used to
align with containerd@v1.2.0.

1. events part:
   using exchange event helper instead of raw grpc conn

2. image part:
  a. import:
     new Import interface makes the import easier without Importer.

  b. commit:
     content helper has been upgraded with less arguments for caller.
     and `diff` package separates the Applier and Comparer interfaces.
     The changes can make the code clear in PouchContainer

3. lease:
  containerd client doesn't provide ListLeases interface directly.
  PouchContainer needs to get LeasesService and make the call by itself.

4. container IO:
  remove codes which comes from containerd. The code was used to align
  with containerd v1.2, which can make upgrade work easier. For now, we
  can remove this.

I also use subshell to run the integration::ping_pouchd command so that the error log can show up in CI.

Ⅱ. Does this pull request fix one issue?

NONE

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

no need

Ⅳ. Describe how to verify it

use existing CI testing case

Ⅴ. Special notes for reviews

I separate the change into two commits because the vendor change is too big.
There are somethings need to do after this PR, for example, it is time to remove the ctrd/wrapper_client.go because the grpc has been updated and it can support the defaultMaxStreamsClient

related to containerd/containerd#2862

using following commands:

govendor fetch github.com/containerd/containerd/...@v1.2.0
govendor fetch github.com/gogo/protobuf/...@v1.0.0
govendor fetch github.com/gogo/googleapis/...@08a7655d27152912db7aaf4f983275eaf8d128ef
govendor fetch google.golang.org/grpc/...@v1.12.0
govendor fetch github.com/golang/protobuf/...@v1.1.0

Signed-off-by: Wei Fu <fuweid89@gmail.com>
@codecov
Copy link

codecov bot commented Dec 4, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@9ed34bb). Click here to learn what that means.
The diff coverage is 42.1%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2530   +/-   ##
=========================================
  Coverage          ?   54.87%           
=========================================
  Files             ?      277           
  Lines             ?    18426           
  Branches          ?        0           
=========================================
  Hits              ?    10112           
  Misses            ?     7054           
  Partials          ?     1260
Flag Coverage Δ
#criv1alpha1test 31.16% <42.1%> (?)
#criv1alpha2test 35.18% <36.84%> (?)
#unittest 26.86% <0%> (?)
Impacted Files Coverage Δ
pkg/jsonstream/image_progress.go 0% <ø> (ø)
daemon/mgr/image_load.go 0% <0%> (ø)
ctrd/image.go 58.87% <0%> (ø)
ctrd/utils.go 69.01% <0%> (ø)
daemon/mgr/image.go 38.22% <0%> (ø)
ctrd/image_commit.go 0% <0%> (ø)
daemon/mgr/image_utils.go 85.45% <100%> (ø)
daemon/containerio/io.go 70.87% <100%> (ø)
daemon/containerio/cio.go 70% <100%> (ø)
ctrd/snapshot.go 64% <100%> (ø)
... and 3 more

@fuweid
Copy link
Contributor Author

fuweid commented Dec 4, 2018

the first commit is for vendor update and the second one is for code change. hope it can help.

The containerd has been improved a lot in v1.2, which makes the service
stable before. Based on this, we need to do the upgrade.

Basically, both the contaienrd and shim API interfaces are stable so
that we don't need to worry about the remote API calls. However, the
package level interface has been changed a lot. The commit is used to
align with containerd@v1.2.0.

1. events part:
   using exchange event helper instead of raw grpc conn

2. image part:
  a. import:
     new Import interface makes the import easier without Importer.

  b. commit:
     content helper has been upgraded with less arguments for caller.
     and `diff` package separates the Applier and Comparer interfaces.
     The changes can make the code clear in PouchContainer

3. lease:
  containerd client doesn't provide ListLeases interface directly.
  PouchContainer needs to get LeasesService and make the call by itself.

4. container IO:
  remove codes which comes from containerd. The code was used to align
  with containerd v1.2, which can make upgrade work easier. For now, we
  can remove this.

Signed-off-by: Wei Fu <fuweid89@gmail.com>
@fuweid fuweid changed the title [RFC] Upgrade to containerd v1.2 [WIP|RFC] Upgrade to containerd v1.2 Dec 4, 2018
@fuweid
Copy link
Contributor Author

fuweid commented Dec 4, 2018

still investigate the failed cases about load/save functionality. It seems that the containerd export functionality doesn't support the image which has several manifests.

@fuweid fuweid changed the title [WIP|RFC] Upgrade to containerd v1.2 [WIP] Upgrade to containerd v1.2 Dec 5, 2018
@Ace-Tang
Copy link
Contributor

Ace-Tang commented Dec 5, 2018

Hold this pr

@pouchrobot
Copy link
Collaborator

ping @fuweid
Conflict happens after merging a previous commit.
Please rebase the branch against master and push it back again. Thanks a lot.

@fuweid
Copy link
Contributor Author

fuweid commented Feb 25, 2019

close it and re-create other one

@fuweid fuweid closed this Feb 25, 2019
@fuweid fuweid deleted the upgrade_v1.2 branch March 25, 2019 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants