Merge pull request #30934 from yifan-gu/fix_subpath

Automatic merge from submit-queue rkt: Support subPath volume mounts feature So that at most one volume object will be created for every unique host path. Also the volume's name is random generated UUID to avoid collision since the mount point's name passed by kubelet is not guaranteed to be unique when 'subpath' is specified. Should partially fix https://github.com/kubernetes/kubernetes/issues/26986 The non-existing host path creation issue is not touched here. cc @kubernetes/sig-rktnetes also cc @kubernetes/sig-node for the Mount name comments I added.

Merge pull request #30934 from yifan-gu/fix_subpath
f75910b7 · Kubernetes Submit Queue · GitHub · 4e571eaf · ce15f0e8 · f75910b7
Commit f75910b7 authored Aug 21, 2016 by Kubernetes Submit Queue Committed by GitHub Aug 21, 2016
Expand all Hide whitespace changes
Inline Side-by-side

Showing with 162 additions and 118 deletions

runtime.go pkg/kubelet/container/runtime.go +2 -0

rkt.go pkg/kubelet/rkt/rkt.go +124 -90

rkt_test.go pkg/kubelet/rkt/rkt_test.go +36 -28

No files found.
--- a/pkg/kubelet/container/runtime.go
+++ b/pkg/kubelet/container/runtime.go
@@ -342,6 +342,8 @@ type EnvVar struct {

 type Mount struct {
 	// Name of the volume mount.
+	// TODO(yifan): Remove this field, as this is not representing the unique name of the mount,
+	// but the volume name only.
 	Name string
 	// Path of the mount within the container.
 	ContainerPath string

--- a/pkg/kubelet/rkt/rkt.go
+++ b/pkg/kubelet/rkt/rkt.go
--- a/pkg/kubelet/rkt/rkt_test.go
+++ b/pkg/kubelet/rkt/rkt_test.go
@@ -949,7 +949,9 @@ func TestSetApp(t *testing.T) {

 	tests := []struct {
 		container        *api.Container
-		opts             *kubecontainer.RunContainerOptions
+		mountPoints      []appctypes.MountPoint
+		containerPorts   []appctypes.Port
+		envs             []kubecontainer.EnvVar
 		ctx              *api.SecurityContext
 		podCtx           *api.PodSecurityContext
 		supplementalGids []int64
@@ -959,7 +961,9 @@ func TestSetApp(t *testing.T) {
 		// Nothing should change, but the "User" and "Group" should be filled.
 		{
 			container:        &api.Container{},
-			opts:             &kubecontainer.RunContainerOptions{},
+			mountPoints:      []appctypes.MountPoint{},
+			containerPorts:   []appctypes.Port{},
+			envs:             []kubecontainer.EnvVar{},
 			ctx:              nil,
 			podCtx:           nil,
 			supplementalGids: nil,
@@ -969,8 +973,10 @@ func TestSetApp(t *testing.T) {

 		// error verifying non-root.
 		{
-			container: &api.Container{},
-			opts:      &kubecontainer.RunContainerOptions{},
+			container:      &api.Container{},
+			mountPoints:    []appctypes.MountPoint{},
+			containerPorts: []appctypes.Port{},
+			envs:           []kubecontainer.EnvVar{},
 			ctx: &api.SecurityContext{
 				RunAsNonRoot: &runAsNonRootTrue,
 				RunAsUser:    &rootUser,
@@ -986,7 +992,9 @@ func TestSetApp(t *testing.T) {
 			container: &api.Container{
 				Args: []string{"foo"},
 			},
-			opts:             &kubecontainer.RunContainerOptions{},
+			mountPoints:      []appctypes.MountPoint{},
+			containerPorts:   []appctypes.Port{},
+			envs:             []kubecontainer.EnvVar{},
 			ctx:              nil,
 			podCtx:           nil,
 			supplementalGids: nil,
@@ -1025,16 +1033,14 @@ func TestSetApp(t *testing.T) {
 					Requests: api.ResourceList{"cpu": resource.MustParse("5m"), "memory": resource.MustParse("5M")},
 				},
 			},
-			opts: &kubecontainer.RunContainerOptions{
-				Envs: []kubecontainer.EnvVar{
-					{Name: "env-bar", Value: "foo"},
-				},
-				Mounts: []kubecontainer.Mount{
-					{Name: "mnt-bar", ContainerPath: "/mnt-bar", ReadOnly: true},
-				},
-				PortMappings: []kubecontainer.PortMapping{
-					{Name: "port-bar", Protocol: api.ProtocolTCP, ContainerPort: 1234},
-				},
+			mountPoints: []appctypes.MountPoint{
+				{Name: *appctypes.MustACName("mnt-bar"), Path: "/mnt-bar", ReadOnly: true},
+			},
+			containerPorts: []appctypes.Port{
+				{Name: *appctypes.MustACName("port-bar"), Protocol: "TCP", Port: 1234},
+			},
+			envs: []kubecontainer.EnvVar{
+				{Name: "env-bar", Value: "foo"},
 			},
 			ctx: &api.SecurityContext{
 				Capabilities: &api.Capabilities{
@@ -1088,17 +1094,15 @@ func TestSetApp(t *testing.T) {
 					Requests: api.ResourceList{"memory": resource.MustParse("5M")},
 				},
 			},
-			opts: &kubecontainer.RunContainerOptions{
-				Envs: []kubecontainer.EnvVar{
-					{Name: "env-foo", Value: "foo"},
-					{Name: "env-bar", Value: "bar"},
-				},
-				Mounts: []kubecontainer.Mount{
-					{Name: "mnt-foo", ContainerPath: "/mnt-bar", ReadOnly: true},
-				},
-				PortMappings: []kubecontainer.PortMapping{
-					{Name: "port-foo", Protocol: api.ProtocolTCP, ContainerPort: 1234},
-				},
+			mountPoints: []appctypes.MountPoint{
+				{Name: *appctypes.MustACName("mnt-foo"), Path: "/mnt-foo", ReadOnly: true},
+			},
+			containerPorts: []appctypes.Port{
+				{Name: *appctypes.MustACName("port-foo"), Protocol: "TCP", Port: 1234},
+			},
+			envs: []kubecontainer.EnvVar{
+				{Name: "env-foo", Value: "foo"},
+				{Name: "env-bar", Value: "bar"},
 			},
 			ctx: &api.SecurityContext{
 				Capabilities: &api.Capabilities{
@@ -1124,7 +1128,7 @@ func TestSetApp(t *testing.T) {
 					{Name: "env-bar", Value: "bar"},
 				},
 				MountPoints: []appctypes.MountPoint{
-					{Name: *appctypes.MustACName("mnt-foo"), Path: "/mnt-bar", ReadOnly: true},
+					{Name: *appctypes.MustACName("mnt-foo"), Path: "/mnt-foo", ReadOnly: true},
 				},
 				Ports: []appctypes.Port{
 					{Name: *appctypes.MustACName("port-foo"), Protocol: "TCP", Port: 1234},
@@ -1142,7 +1146,11 @@ func TestSetApp(t *testing.T) {
 	for i, tt := range tests {
 		testCaseHint := fmt.Sprintf("test case #%d", i)
 		img := baseImageManifest(t)
-		err := setApp(img, tt.container, tt.opts, tt.ctx, tt.podCtx, tt.supplementalGids)
+
+		err := setApp(img, tt.container,
+			tt.mountPoints, tt.containerPorts, tt.envs,
+			tt.ctx, tt.podCtx, tt.supplementalGids)
+
 		if err == nil && tt.err != nil || err != nil && tt.err == nil {
 			t.Errorf("%s: expect %v, saw %v", testCaseHint, tt.err, err)
 		}