feature: extend cri apis for special needs #1617

starnop · 2018-07-03T04:18:55Z

Signed-off-by: Starnop starnop@163.com

Ⅰ. Describe what this PR did

At present, the CRI interface of Kubernetes cannot meet the customized development of Kubelet. Therefore, it is necessary to provide the required functions of Kubelet by extending CRI.

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

codecov-io · 2018-07-03T07:13:35Z

Codecov Report

Merging #1617 into master will decrease coverage by 27.79%.
The diff coverage is 88.46%.

@@            Coverage Diff             @@
##           master    #1617      +/-   ##
==========================================
- Coverage   41.92%   14.13%   -27.8%     
==========================================
  Files         279      281       +2     
  Lines       18185    56755   +38570     
==========================================
+ Hits         7624     8021     +397     
- Misses       9638    47809   +38171     
- Partials      923      925       +2

Impacted Files	Coverage Δ
cri/v1alpha2/cri_types.go	`0% <ø> (ø)`	⬆️
cri/v1alpha1/cri_types.go	`0% <ø> (ø)`	⬆️
cri/apis/v1alpha2/api.pb.go	`0.57% <ø> (ø)`
cri/apis/v1alpha1/api.pb.go	`0.56% <ø> (ø)`
daemon/containerio/cri_log_file.go	`11.76% <ø> (ø)`	⬆️
cri/v1alpha1/server.go	`0% <ø> (ø)`	⬆️
cri/v1alpha2/server.go	`0% <ø> (ø)`	⬆️
cri/v1alpha2/cri_network.go	`0% <ø> (ø)`	⬆️
cri/v1alpha2/service/cri.go	`0% <ø> (ø)`	⬆️
cri/v1alpha1/service/cri.go	`0% <ø> (ø)`	⬆️
... and 11 more

YaoZengzeng · 2018-07-04T07:00:17Z

cri/apis/cri/runtime/v1alpha2/api.proto

@@ -0,0 +1,1281 @@
+// To regenerate api.pb.go run hack/update-generated-runtime.sh


cri/apis/cri/v1alpha1 ---> cri/apis/v1alpha1
cri/apis/cri/runtime/v1alpha2 ---> cri/apis/v1alpha2

make the directory structure more clear

YaoZengzeng · 2018-07-04T07:13:37Z

cri/v1alpha2/cri.go

 		},
 		HostConfig: &apitypes.HostConfig{
-			Binds: generateMountBindings(config.GetMounts()),
+			Binds:     generateMountBindings(config.GetMounts()),
+			Resources: parseResourceFromAPI(resources),


rename parseResourceFromAPI to parseResourcesFromAPI may more consistent :)

YaoZengzeng · 2018-07-04T07:16:17Z

cri/v1alpha2/cri_utils.go

+	if runtimeUlimit == nil {
+		return nil
+	}
+	var ulimit []*apitypes.Ulimit


Please make sure how to use slice in golang.

pouchrobot · 2018-07-05T06:23:39Z

ping @starnop
We found that this PR is 14 commits, which is more than 10 commits, behind master.
Please rebase the branch against master and push it back again. Thanks a lot.

fuweid · 2018-07-12T07:49:42Z

cri/apis/v1alpha1/api.pb.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2017 The Kubernetes Authors.
+Copyright YEAR AUTHORS.


YEAR AUTHORS?

fuweid · 2018-07-12T07:54:34Z

cri/v1alpha1/cri_utils.go

+// CRI extension related tool functions.
+
+// parseResourceFromAPI parse Resources from runtime.LinuxContainerResources to apitypes.Resources
+func parseResourcesFromAPI(runtimeResources *runtime.LinuxContainerResources) apitypes.Resources {


s/parseResoucesFromAPI/pbLinuxContainerResourcesToAPIResouces/g WDYT?

fuweid · 2018-07-12T07:59:33Z

cri/v1alpha1/cri_utils.go

+func parseVolumesFromPouch(containerVolumes map[string]interface{}) map[string]*runtime.Volume {
+	volumes := make(map[string]*runtime.Volume)
+	for k, v := range containerVolumes {
+		volumes[k] = v.(*runtime.Volume)


always (*runtime.Volume) ? nil or other type?

Could we use two-values assertion to get the value? if fails, log it or return error

Done. thanks for your comment.

YaoZengzeng · 2018-07-13T02:21:52Z

cri/v1alpha1/cri.go

@@ -728,7 +734,9 @@ func (c *CriManager) ContainerStatus(ctx context.Context, r *runtime.ContainerSt
 		Message:     message,
 		Labels:      labels,
 		Annotations: annotations,
-		// TODO: LogPath.
+		LogPath:     container.LogPath,


The container.LogPath is not the log path of cri container

I'll fix it in my next PR, please leave it empty now :)

fuweid · 2018-07-16T07:12:32Z

hack/protoc.sh

+}
+
+main(){
+    generateproto


API_ROOT="${DIR}/cri/apis/v1alpha1" generateproto
API_ROOT="${DIR}/cri/apis/v1alpha2" generateproto

fuweid

LGTM @YaoZengzeng

Signed-off-by: Starnop <starnop@163.com>

YaoZengzeng · 2018-07-16T09:42:50Z

I've used this PR to do node-e2e test, it works fine.

LGTM

pouchrobot added areas/test kind/feature size/XXL labels Jul 3, 2018

starnop force-pushed the cri-apiextend branch 3 times, most recently from df75c95 to f4730a5 Compare July 3, 2018 06:40

YaoZengzeng reviewed Jul 4, 2018

View reviewed changes

starnop force-pushed the cri-apiextend branch 4 times, most recently from 46a34b4 to bacfb54 Compare July 4, 2018 08:26

pouchrobot added the gap/needs-rebase label Jul 5, 2018

pouchrobot added conflict/needs-rebase and removed conflict/needs-rebase labels Jul 5, 2018

starnop force-pushed the cri-apiextend branch from bacfb54 to d70d7c4 Compare July 6, 2018 08:30

pouchrobot removed the conflict/needs-rebase label Jul 6, 2018

starnop force-pushed the cri-apiextend branch 3 times, most recently from 25a2907 to 0b9f2f1 Compare July 9, 2018 02:33

starnop force-pushed the cri-apiextend branch 3 times, most recently from f6ec45b to fd9da28 Compare July 12, 2018 07:06

fuweid reviewed Jul 12, 2018

View reviewed changes

starnop force-pushed the cri-apiextend branch 4 times, most recently from baa10e9 to 58412d3 Compare July 12, 2018 10:13

YaoZengzeng reviewed Jul 13, 2018

View reviewed changes

starnop force-pushed the cri-apiextend branch 8 times, most recently from a624b1d to ccd77d4 Compare July 16, 2018 06:18

fuweid reviewed Jul 16, 2018

View reviewed changes

starnop force-pushed the cri-apiextend branch 3 times, most recently from deefe75 to 9dbcbec Compare July 16, 2018 07:50

fuweid reviewed Jul 16, 2018

View reviewed changes

starnop force-pushed the cri-apiextend branch from 9dbcbec to d90fe1f Compare July 16, 2018 08:04

extend cri apis for special needs

5ea03ae

Signed-off-by: Starnop <starnop@163.com>

starnop force-pushed the cri-apiextend branch from d90fe1f to 5ea03ae Compare July 16, 2018 08:35

allencloud merged commit 165899c into AliyunContainerService:master Jul 16, 2018

pouchrobot mentioned this pull request Jul 19, 2018

Weekly Report of PouchContainer from 2018-07-12 to 2018-07-19 #1752

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: extend cri apis for special needs #1617

feature: extend cri apis for special needs #1617

starnop commented Jul 3, 2018

codecov-io commented Jul 3, 2018 •

edited

Loading

YaoZengzeng Jul 4, 2018

YaoZengzeng Jul 4, 2018

YaoZengzeng Jul 4, 2018

pouchrobot commented Jul 5, 2018

fuweid Jul 12, 2018

fuweid Jul 12, 2018

fuweid Jul 12, 2018 •

edited

Loading

starnop Jul 12, 2018

YaoZengzeng Jul 13, 2018

fuweid Jul 16, 2018

fuweid left a comment

YaoZengzeng commented Jul 16, 2018

		@@ -0,0 +1,1281 @@
		// To regenerate api.pb.go run hack/update-generated-runtime.sh

feature: extend cri apis for special needs #1617

feature: extend cri apis for special needs #1617

Conversation

starnop commented Jul 3, 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 Jul 3, 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

pouchrobot commented Jul 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fuweid Jul 12, 2018 • edited Loading

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 left a comment

Choose a reason for hiding this comment

YaoZengzeng commented Jul 16, 2018

codecov-io commented Jul 3, 2018 •

edited

Loading

fuweid Jul 12, 2018 •

edited

Loading