Merge pull request #22713 from yifan-gu/rkt_fix_entrypoint

pkg/kubelet/rkt/rkt.go

@@ -94,6 +94,11 @@ const (
 	// hence, setting ndots to be 5.
 	// TODO(yifan): Move this and dockertools.ndotsDNSOption to a common package.
 	defaultDNSOption = "ndots:5"
+
+	// Annotations for the ENTRYPOINT and CMD for an ACI that's converted from Docker image.
+	// TODO(yifan): Import them from docker2aci. See https://github.com/appc/docker2aci/issues/133.
+	appcDockerEntrypoint = "appc.io/docker/entrypoint"
+	appcDockerCmd        = "appc.io/docker/cmd"
 )
 
 // Runtime implements the Containerruntime for rkt. The implementation
@@ -414,10 +419,29 @@ func setSupplementaryGIDs(app *appctypes.App, podCtx *api.PodSecurityContext) {
 }
 
 // setApp merges the container spec with the image's manifest.
-func setApp(app *appctypes.App, c *api.Container, opts *kubecontainer.RunContainerOptions, ctx *api.SecurityContext, podCtx *api.PodSecurityContext) error {
-	// TODO(yifan): If ENTRYPOINT and CMD are both specified in the image,
-	// we cannot override just one of these at this point as they are already mixed.
-	command, args := kubecontainer.ExpandContainerCommandAndArgs(c, opts.Envs)
+func setApp(imgManifest *appcschema.ImageManifest, c *api.Container, opts *kubecontainer.RunContainerOptions, ctx *api.SecurityContext, podCtx *api.PodSecurityContext) error {
+	app := imgManifest.App
+
+	// Set up Exec.
+	var command, args []string
+	cmd, ok := imgManifest.Annotations.Get(appcDockerEntrypoint)
+	if ok {
+		command = strings.Fields(cmd)
+	}
+	ag, ok := imgManifest.Annotations.Get(appcDockerCmd)
+	if ok {
+		args = strings.Fields(ag)
+	}
+	userCommand, userArgs := kubecontainer.ExpandContainerCommandAndArgs(c, opts.Envs)
+
+	if len(userCommand) > 0 {
+		command = userCommand
+		args = nil // If 'command' is specified, then drop the default args.
+	}
+	if len(userArgs) > 0 {
+		args = userArgs
+	}
+
 	exec := append(command, args...)
 	if len(exec) > 0 {
 		app.Exec = exec
@@ -592,7 +616,7 @@ func (r *Runtime) newAppcRuntimeApp(pod *api.Pod, c api.Container, pullSecrets [
 	}
 
 	ctx := securitycontext.DetermineEffectiveSecurityContext(pod, &c)
-	if err := setApp(imgManifest.App, &c, opts, ctx, pod.Spec.SecurityContext); err != nil {
+	if err := setApp(imgManifest, &c, opts, ctx, pod.Spec.SecurityContext); err != nil {
 		return err
 	}
 

pkg/kubelet/rkt/rkt_test.go

@@ -743,7 +743,7 @@ func generateMemoryIsolator(t *testing.T, request, limit string) appctypes.Isola
 
 func baseApp(t *testing.T) *appctypes.App {
 	return &appctypes.App{
-		Exec:              appctypes.Exec{"/bin/foo"},
+		Exec:              appctypes.Exec{"/bin/foo", "bar"},
 		SupplementaryGIDs: []int{4, 5, 6},
 		WorkingDirectory:  "/foo",
 		Environment: []appctypes.EnvironmentVariable{
@@ -764,6 +764,13 @@ func baseApp(t *testing.T) *appctypes.App {
 	}
 }
 
+func baseImageManifest(t *testing.T) *appcschema.ImageManifest {
+	img := &appcschema.ImageManifest{App: baseApp(t)}
+	img.Annotations.Set(*appctypes.MustACIdentifier(appcDockerEntrypoint), "/bin/foo")
+	img.Annotations.Set(*appctypes.MustACIdentifier(appcDockerCmd), "bar")
+	return img
+}
+
 func baseAppWithRootUserGroup(t *testing.T) *appctypes.App {
 	app := baseApp(t)
 	app.User, app.Group = "0", "0"
@@ -844,11 +851,43 @@ func TestSetApp(t *testing.T) {
 			err:    fmt.Errorf("container has no runAsUser and image will run as root"),
 		},
 
+		// app's args should be changed.
+		{
+			container: &api.Container{
+				Args: []string{"foo"},
+			},
+			opts:   &kubecontainer.RunContainerOptions{},
+			ctx:    nil,
+			podCtx: nil,
+			expect: &appctypes.App{
+				Exec:              appctypes.Exec{"/bin/foo", "foo"},
+				User:              "0",
+				Group:             "0",
+				SupplementaryGIDs: []int{4, 5, 6},
+				WorkingDirectory:  "/foo",
+				Environment: []appctypes.EnvironmentVariable{
+					{"env-foo", "bar"},
+				},
+				MountPoints: []appctypes.MountPoint{
+					{Name: *appctypes.MustACName("mnt-foo"), Path: "/mnt-foo", ReadOnly: false},
+				},
+				Ports: []appctypes.Port{
+					{Name: *appctypes.MustACName("port-foo"), Protocol: "TCP", Port: 4242},
+				},
+				Isolators: []appctypes.Isolator{
+					generateCapRetainIsolator(t, "CAP_SYS_ADMIN"),
+					generateCapRevokeIsolator(t, "CAP_NET_ADMIN"),
+					generateCPUIsolator(t, "100m", "200m"),
+					generateMemoryIsolator(t, "10M", "20M"),
+				},
+			},
+			err: nil,
+		},
+
 		// app should be changed.
 		{
 			container: &api.Container{
-				Command:    []string{"/bin/bar"},
-				Args:       []string{"hello", "world"},
+				Command:    []string{"/bin/bar", "$(env-bar)"},
 				WorkingDir: tmpDir,
 				Resources: api.ResourceRequirements{
 					Limits:   api.ResourceList{"cpu": resource.MustParse("50m"), "memory": resource.MustParse("50M")},
@@ -879,7 +918,7 @@ func TestSetApp(t *testing.T) {
 				FSGroup:            &fsgid,
 			},
 			expect: &appctypes.App{
-				Exec:              appctypes.Exec{"/bin/bar", "hello", "world"},
+				Exec:              appctypes.Exec{"/bin/bar", "foo"},
 				User:              "42",
 				Group:             "0",
 				SupplementaryGIDs: []int{1, 2, 3},
@@ -909,7 +948,7 @@ func TestSetApp(t *testing.T) {
 		{
 			container: &api.Container{
 				Name:       "hello-world",
-				Command:    []string{"/bin/bar", "$(env-foo)"},
+				Command:    []string{"/bin/hello", "$(env-foo)"},
 				Args:       []string{"hello", "world", "$(env-bar)"},
 				WorkingDir: tmpDir,
 				Resources: api.ResourceRequirements{
@@ -942,7 +981,7 @@ func TestSetApp(t *testing.T) {
 				FSGroup:            &fsgid,
 			},
 			expect: &appctypes.App{
-				Exec:              appctypes.Exec{"/bin/bar", "foo", "hello", "world", "bar"},
+				Exec:              appctypes.Exec{"/bin/hello", "foo", "hello", "world", "bar"},
 				User:              "42",
 				Group:             "0",
 				SupplementaryGIDs: []int{1, 2, 3},
@@ -969,15 +1008,15 @@ func TestSetApp(t *testing.T) {
 
 	for i, tt := range tests {
 		testCaseHint := fmt.Sprintf("test case #%d", i)
-		app := baseApp(t)
-		err := setApp(app, tt.container, tt.opts, tt.ctx, tt.podCtx)
+		img := baseImageManifest(t)
+		err := setApp(img, tt.container, tt.opts, tt.ctx, tt.podCtx)
 		if err == nil && tt.err != nil || err != nil && tt.err == nil {
 			t.Errorf("%s: expect %v, saw %v", testCaseHint, tt.err, err)
 		}
 		if err == nil {
 			sortAppFields(tt.expect)
-			sortAppFields(app)
-			assert.Equal(t, tt.expect, app, testCaseHint)
+			sortAppFields(img.App)
+			assert.Equal(t, tt.expect, img.App, testCaseHint)
 		}
 	}
 }