feature: Update CRI from v1alpha1 to v1alpha2 & support down-level compatibility #1359

starnop · 2018-05-21T02:07:11Z

Basically, Pouch has not supported Kubernetes 1.10+ because of update CRI from v1alpha1 to v1alpha2.
Signed-off-by: Starnop [email protected]

Ⅰ. Describe what this PR did

Update CRI from v1alpha1 to v1alpha2 & support down-level compatibility

Ⅱ. Does this pull request fix one issue?

NONE

Ⅲ. Describe how you did it

update package from cri/v1alpha1/runtime to cri/runtime/v1alpha2
fix: convert GetHostNetwork() to GetNetwork(),etc
add ReopenContainerLog function definition
nothing should be configured when PidMode equals NamespaceMode_CONTAINER
maintain two pieces of code at the same time.

Ⅳ. Describe how to verify it

It has passed CRI validation tests.
Deploying with released Kubernetes 1.5.+ will be successful.
Use command

pouchd --enable-cri --cri-version v1alpha1/v1alpha2

to support CRI v1alpha1/v1alpha2

Ⅴ. Special notes for reviews

The doc will be updated in the back PR.
The function of ReopenContainerLog has not been implemented, I'll fix it in the back PR.

codecov-io · 2018-05-21T02:21:37Z

Codecov Report

Merging #1359 into master will decrease coverage by 1%.
The diff coverage is 8.97%.

@@            Coverage Diff             @@
##           master    #1359      +/-   ##
==========================================
- Coverage   17.29%   16.29%   -1.01%     
==========================================
  Files         190      197       +7     
  Lines       11911    13387    +1476     
==========================================
+ Hits         2060     2181     +121     
- Misses       9703    11056    +1353     
- Partials      148      150       +2

Impacted Files	Coverage Δ
cri/v1alpha1/cri_types.go	`0% <ø> (ø)`
cri/v1alpha1/cri_network.go	`0% <ø> (ø)`
cri/v1alpha1/cri_wrapper.go	`0% <ø> (ø)`
cri/v1alpha1/cri_utils.go	`29.31% <ø> (ø)`
cri/v1alpha1/cri.go	`0% <ø> (ø)`
cri/v1alpha2/cri_network.go	`0% <0%> (ø)`
cri/v1alpha2/cri_types.go	`0% <0%> (ø)`
cri/v1alpha2/cri_stream.go	`0% <0%> (ø)`
cri/v1alpha1/server.go	`0% <0%> (ø)`
cri/v1alpha1/cri_stream.go	`0% <0%> (ø)`
... and 21 more

YaoZengzeng · 2018-05-21T03:38:13Z

cri/criservice.go

+	case "v1alpha1":
+		runv1alpha1(daemonconfig, containerMgr, imageMgr)
+	case "v1alpha2":
+		runv1alpha2(daemonconfig, containerMgr, imageMgr)


If user doesn't specify the CRI version, use "v1alpha2" by default.

OK, I'll change it.

If user doesn't specify the CRI version, use "v1alpha2" by default.

Yes, I think this is reasonable to set a default one. And throw out a line of related log.

YaoZengzeng · 2018-05-21T03:53:40Z

internal/generator.go

@@ -5,7 +5,7 @@ import (
 	"path"

 	"github.com/alibaba/pouch/apis/plugins"
-	cri "github.com/alibaba/pouch/cri/src"
+	cri "github.com/alibaba/pouch/cri/v1alpha1"


GenCriMgr in this file is no longer needed.

YaoZengzeng · 2018-05-21T04:02:49Z

cri/criservice.go

+	return
+}
+
+func runv1alpha1(daemonconfig *config.Config, containerMgr mgr.ContainerMgr, imageMgr mgr.ImageMgr) {


Print a log here, like Start CRI services, version: v1alpha1(just an example, could be better?), which may be helpful in debugging.

The same for function runv1alpha2.

YaoZengzeng · 2018-05-21T04:59:17Z

cri/service/cri.go

@@ -5,7 +5,7 @@ import (
 	"os"
 	"syscall"

-	cri "github.com/alibaba/pouch/cri/src"
+	cri "github.com/alibaba/pouch/cri/v1alpha1"


This directory could be deprecated as well?

@YaoZengzeng All mentioned above has been done.

YaoZengzeng · 2018-05-22T02:20:20Z

cri/criservice.go

+}
+
+func runv1alpha1(daemonconfig *config.Config, containerMgr mgr.ContainerMgr, imageMgr mgr.ImageMgr) {
+	logrus.Infof("Start CRI services with CRI version: v1alpha1")


CRI services -> CRI service, please be consistent with other places :)

YaoZengzeng · 2018-05-22T02:20:37Z

cri/criservice.go

+}
+
+func runv1alpha2(daemonconfig *config.Config, containerMgr mgr.ContainerMgr, imageMgr mgr.ImageMgr) {
+	logrus.Infof("Start CRI services with CRI version: v1alpha2")


Same as above.

fuweid · 2018-05-22T13:09:13Z

cri/criservice.go

+		close(streamServerCloseCh)
+	}()
+
+	<-streamServerCloseCh


Could you please create issue to track the thing that it is to make server close gracefully? Thanks

Maybe with a TODO annotation ?

fuweid · 2018-05-22T13:12:27Z

cri/v1alpha2/cri_network.go

+	}
+
+	defer func() {
+		if err != nil {


I don't think the defer function work here, because it has finished if the err is not null.

fuweid · 2018-05-22T13:14:59Z

cri/v1alpha2/cri_stream.go

+	if err != nil {
+		return 0, fmt.Errorf("failed to start exec for container %q: %v", containerID, err)
+	}
+


Please add the todo/fixme as reminder. I think we can find the better way to handle this case gracefully instead of the dead loop. WDYT?

@fuweid All mentioned above has been done.

Signed-off-by: Starnop <[email protected]>

fuweid · 2018-05-23T03:17:58Z

LGTM

cherry-pick: cherry-pick #1359 from master to 0.5.x

idealhack · 2018-05-31T08:17:42Z

Hi @starnop, may I ask a question?

Since vendor.json only have k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime now, not k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2, how do we want to manage these dependencies?

I think we may want to track the newer version, not the old one.

starnop · 2018-05-31T12:55:21Z

@idealhack I'm sorry for my negligence. I'll fix that, thank you!

pouchrobot added kind/feature size/XXL labels May 21, 2018

fuweid self-assigned this May 21, 2018

starnop force-pushed the cri-upgrade branch from 24d6578 to 6e1e47e Compare May 21, 2018 03:23

YaoZengzeng reviewed May 21, 2018

View reviewed changes

starnop force-pushed the cri-upgrade branch from 6e1e47e to 7715590 Compare May 21, 2018 03:47

YaoZengzeng reviewed May 21, 2018

View reviewed changes

starnop force-pushed the cri-upgrade branch 7 times, most recently from 7b8a2be to cfbce09 Compare May 21, 2018 09:36

HusterWan added this to the v0.5.0-milestone milestone May 21, 2018

starnop force-pushed the cri-upgrade branch 2 times, most recently from 3fcb71f to 4b148ec Compare May 22, 2018 01:46

YaoZengzeng reviewed May 22, 2018

View reviewed changes

starnop force-pushed the cri-upgrade branch 3 times, most recently from ac3fcd0 to 2e09bb7 Compare May 22, 2018 07:34

fuweid reviewed May 22, 2018

View reviewed changes

starnop force-pushed the cri-upgrade branch from 2e09bb7 to 0990726 Compare May 23, 2018 02:33

feature:cri-upgrade and support down-level compatibility

206b027

Signed-off-by: Starnop <[email protected]>

starnop force-pushed the cri-upgrade branch from 0990726 to 206b027 Compare May 23, 2018 03:10

fuweid merged commit 0df5157 into AliyunContainerService:master May 23, 2018

HusterWan added a commit that referenced this pull request May 23, 2018

Merge pull request #1383 from HusterWan/0.5.x

f908208

cherry-pick: cherry-pick #1359 from master to 0.5.x

pouchrobot mentioned this pull request May 24, 2018

Weekly Report of Pouch from 2018-05-17 to 2018-05-24 #1408

Closed

idealhack mentioned this pull request May 31, 2018

[question] do we have a plan to revendor libnetwork to the latest version or stable one #1344

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: Update CRI from v1alpha1 to v1alpha2 & support down-level compatibility #1359

feature: Update CRI from v1alpha1 to v1alpha2 & support down-level compatibility #1359

starnop commented May 21, 2018 •

edited

Loading

codecov-io commented May 21, 2018 •

edited

Loading

YaoZengzeng May 21, 2018

starnop May 21, 2018

allencloud May 21, 2018

YaoZengzeng May 21, 2018

YaoZengzeng May 21, 2018

YaoZengzeng May 21, 2018

starnop May 21, 2018

YaoZengzeng May 22, 2018

YaoZengzeng May 22, 2018

fuweid May 22, 2018

starnop May 23, 2018

fuweid May 22, 2018

fuweid May 22, 2018

starnop May 23, 2018

fuweid commented May 23, 2018

idealhack commented May 31, 2018

starnop commented May 31, 2018

feature: Update CRI from v1alpha1 to v1alpha2 & support down-level compatibility #1359

feature: Update CRI from v1alpha1 to v1alpha2 & support down-level compatibility #1359

Conversation

starnop commented May 21, 2018 • edited Loading

Ⅰ. 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 May 21, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fuweid commented May 23, 2018

idealhack commented May 31, 2018

starnop commented May 31, 2018

starnop commented May 21, 2018 •

edited

Loading

codecov-io commented May 21, 2018 •

edited

Loading