feature: make runtime choosing supported in CRI managers for Kubernetes #1593

starnop · 2018-06-25T07:49:20Z

Signed-off-by: Starnop [email protected]

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

fixes part of #1580

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Prerequisites Installation, the runtime binaries you will use is necessary
You should start pouchd with the configuration like

pouchd --enable-cri --cri-version v1alpha2 --add-runtime runv=runv >pouchd.log 2>&1  &

After setting up your kubernetes cluster, you can create a deployment like this :

apiVersion: apps/v1
kind: Deployment
metadata:
  name: pouch
  labels:
    pouch: pouch
spec:
  selector:
    matchLabels:
      pouch: pouch
  template:
    metadata:
      labels:
        pouch: pouch
      annotations:
        io.kubernetes.runtime: runv
    spec:
      containers:
      - name: pouch
        image: docker.io/library/nginx:latest
        ports:
        - containerPort: 80

Use command pouch ps to observe the runtime of the container.

Ⅴ. Special notes for reviews

codecov-io · 2018-06-25T09:37:57Z

Codecov Report

Merging #1593 into master will decrease coverage by 1.15%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #1593      +/-   ##
==========================================
- Coverage   41.97%   40.82%   -1.16%     
==========================================
  Files         270      270              
  Lines       17609    18106     +497     
==========================================
  Hits         7392     7392              
- Misses       9307     9806     +499     
+ Partials      910      908       -2

Impacted Files	Coverage Δ
cri/v1alpha2/cri_types.go	`0% <ø> (ø)`	⬆️
cri/v1alpha1/cri_types.go	`0% <ø> (ø)`	⬆️
cri/v1alpha2/cri_utils.go	`27.86% <0%> (-0.49%)`	⬇️
cri/v1alpha1/cri.go	`0% <0%> (ø)`	⬆️
cri/v1alpha2/cri.go	`0% <0%> (ø)`	⬆️
cri/v1alpha1/cri_utils.go	`28.73% <0%> (-0.53%)`	⬇️
daemon/mgr/image.go	`54.09% <0%> (-14.38%)`	⬇️
daemon/logger/syslog/validate.go	`40.7% <0%> (-11.93%)`	⬇️
daemon/mgr/container.go	`38.52% <0%> (-11.68%)`	⬇️
daemon/mgr/container_exec.go	`67.77% <0%> (-2.34%)`	⬇️
... and 1 more

YaoZengzeng · 2018-06-25T09:58:33Z

daemon/mgr/container.go

-	if err != nil {
-		return nil, err
+	id := ""
+	if id = config.ContainerID; id == "" {


Why not code like this:

id := config.ContainerID if id == "" { ... }

It seems more concise. 😄

shaloulcy · 2018-06-28T02:08:21Z

cri/v1alpha2/cri_utils.go


 // makeSandboxPouchConfig returns apitypes.ContainerCreateConfig based on runtime.PodSandboxConfig.
-func makeSandboxPouchConfig(config *runtime.PodSandboxConfig, image string) (*apitypes.ContainerCreateConfig, error) {
+func makeSandboxPouchConfig(config *runtime.PodSandboxConfig, sandboxID string, image string) (*apitypes.ContainerCreateConfig, error) {


sandboxID, image string

shaloulcy · 2018-06-28T02:12:49Z

daemon/annotations/annotations.go

@@ -0,0 +1,19 @@
+package annotations


maybe we should put the annotations into cri package

shaloulcy · 2018-06-28T02:22:46Z

cri/v1alpha1/cri.go

 		return nil, err
 	}

+	id, err := c.ContainerMgr.GenerateID()


what's the purpose of generating ContainerID in CRI?

It will be used before sandbox has been created.

fuweid · 2018-07-05T07:10:11Z

apis/types/container_config.go

 	Cmd []string `json:"Cmd"`

+	// The ID of the container
+	ContainerID string `json:"ContainerID,omitempty"`


ContainerID is used to be as SandboxID? I think it's a little confusing in the API level. Could we use annotation to handle this part?
cc @YaoZengzeng @starnop

fuweid · 2018-07-05T07:41:00Z

cri/v1alpha2/cri_utils.go

 func (c *CriManager) updateCreateConfig(createConfig *apitypes.ContainerCreateConfig, config *runtime.ContainerConfig, sandboxConfig *runtime.PodSandboxConfig, podSandboxID string) error {
+	// Apply runtime options.
+	if annotations := config.GetAnnotations(); annotations != nil {
+		createConfig.HostConfig.Runtime = annotations[anno.KubernetesRuntime]


if the annotations[anno.KubernetesRuntime] is emtpy?

createConfig.HostConfig.Runtime will be "", and then PouchContainer will use the DefaultRuntime,which is runc.

// set container runtime if config.HostConfig.Runtime == "" { config.HostConfig.Runtime = mgr.Config.DefaultRuntime }

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

fuweid · 2018-07-05T08:36:40Z

LGTM @YaoZengzeng please take a look

starnop force-pushed the cri-annotation branch 4 times, most recently from c84d56a to a0f3373 Compare June 25, 2018 08:56

YaoZengzeng reviewed Jun 25, 2018

View reviewed changes

starnop force-pushed the cri-annotation branch from a0f3373 to 9fa1ee5 Compare June 25, 2018 11:37

shaloulcy reviewed Jun 28, 2018

View reviewed changes

daemon/annotations/annotations.go

@@ -0,0 +1,19 @@

package annotations

Copy link

Contributor

shaloulcy Jun 28, 2018

Choose a reason for hiding this comment

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

maybe we should put the annotations into cri package

shaloulcy reviewed Jun 28, 2018

View reviewed changes

starnop force-pushed the cri-annotation branch 2 times, most recently from cb2eebf to 6f67f23 Compare July 3, 2018 12:10

pouchrobot added the size/L label Jul 3, 2018

starnop force-pushed the cri-annotation branch from 6f67f23 to 998b367 Compare July 3, 2018 12:58

pouchrobot added size/XL and removed size/L labels Jul 3, 2018

starnop force-pushed the cri-annotation branch 3 times, most recently from dca0326 to 364bf2b Compare July 4, 2018 02:22

pouchrobot added size/L and removed size/XL labels Jul 4, 2018

starnop force-pushed the cri-annotation branch from 364bf2b to 59e7ba3 Compare July 5, 2018 04:55

pouchrobot added size/XL and removed size/L labels Jul 5, 2018

starnop force-pushed the cri-annotation branch from 59e7ba3 to 301f43a Compare July 5, 2018 05:00

fuweid reviewed Jul 5, 2018

View reviewed changes

starnop force-pushed the cri-annotation branch from 301f43a to 7e6a9ff Compare July 5, 2018 07:27

pouchrobot added size/L and removed size/XL labels Jul 5, 2018

starnop force-pushed the cri-annotation branch from 7e6a9ff to 0df1f26 Compare July 5, 2018 07:32

fuweid reviewed Jul 5, 2018

View reviewed changes

use cri annotations to support runtime parameters

9400882

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

starnop force-pushed the cri-annotation branch from 0df1f26 to 9400882 Compare July 5, 2018 08:24

pouchrobot added areas/orchestration kind/feature labels Jul 5, 2018

fuweid merged commit c456006 into AliyunContainerService:master Jul 5, 2018

pouchrobot mentioned this pull request Jul 12, 2018

Weekly Report of PouchContainer from 2018-07-05 to 2018-07-12 #1722

Closed

starnop mentioned this pull request Oct 16, 2018

[feature request] pouch support kubernetes runtime class #2314

Closed

feature: make runtime choosing supported in CRI managers for Kubernetes #1593

feature: make runtime choosing supported in CRI managers for Kubernetes #1593

Uh oh!

Conversation

starnop commented Jun 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ⅰ. 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

Uh oh!

codecov-io commented Jun 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fuweid commented Jul 5, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

starnop commented Jun 25, 2018 •

edited

Loading

codecov-io commented Jun 25, 2018 •

edited

Loading