Merge pull request #19269 from yifan-gu/rkt_refactor

Auto commit by PR queue bot

pkg/kubelet/rkt/rkt.go

@@ -196,6 +196,14 @@ func (r *Runtime) buildCommand(args ...string) *exec.Cmd {
 	return cmd
 }
 
+// convertToACName converts a string into ACName.
+func convertToACName(name string) appctypes.ACName {
+	// Note that as the 'name' already matches 'DNS_LABEL'
+	// defined in pkg/api/types.go, there shouldn't be error or panic.
+	acname, _ := appctypes.SanitizeACName(name)
+	return *appctypes.MustACName(acname)
+}
+
 // runCommand invokes rkt binary with arguments and returns the result
 // from stdout in a list of strings. Each string in the list is a line.
 func (r *Runtime) runCommand(args ...string) ([]string, error) {
@@ -320,14 +328,68 @@ func setIsolators(app *appctypes.App, c *api.Container) error {
 	return nil
 }
 
-// findEnvInList returns the index of environment variable in the environment whose Name equals env.Name.
-func findEnvInList(envs appctypes.Environment, env kubecontainer.EnvVar) int {
-	for i, e := range envs {
-		if e.Name == env.Name {
-			return i
+// mergeEnv merges the optEnv with the image's environments.
+// The environments defined in the image will be overridden by
+// the ones with the same name in optEnv.
+func mergeEnv(app *appctypes.App, optEnv []kubecontainer.EnvVar) {
+	envMap := make(map[string]string)
+	for _, e := range app.Environment {
+		envMap[e.Name] = e.Value
+	}
+	for _, e := range optEnv {
+		envMap[e.Name] = e.Value
+	}
+	app.Environment = nil
+	for name, value := range envMap {
+		app.Environment = append(app.Environment, appctypes.EnvironmentVariable{
+			Name:  name,
+			Value: value,
+		})
+	}
+}
+
+// mergeMounts merges the optMounts with the image's mount points.
+// The mount points defined in the image will be overridden by the ones
+// with the same name in optMounts.
+func mergeMounts(app *appctypes.App, optMounts []kubecontainer.Mount) {
+	mountMap := make(map[appctypes.ACName]appctypes.MountPoint)
+	for _, m := range app.MountPoints {
+		mountMap[m.Name] = m
+	}
+	for _, m := range optMounts {
+		mpName := convertToACName(m.Name)
+		mountMap[mpName] = appctypes.MountPoint{
+			Name:     mpName,
+			Path:     m.ContainerPath,
+			ReadOnly: m.ReadOnly,
 		}
 	}
-	return -1
+	app.MountPoints = nil
+	for _, mount := range mountMap {
+		app.MountPoints = append(app.MountPoints, mount)
+	}
+}
+
+// mergePortMappings merges the optPortMappings with the image's port mappings.
+// The port mappings defined in the image will be overridden by the ones
+// with the same name in optPortMappings.
+func mergePortMappings(app *appctypes.App, optPortMappings []kubecontainer.PortMapping) {
+	portMap := make(map[appctypes.ACName]appctypes.Port)
+	for _, p := range app.Ports {
+		portMap[p.Name] = p
+	}
+	for _, p := range optPortMappings {
+		pName := convertToACName(p.Name)
+		portMap[pName] = appctypes.Port{
+			Name:     pName,
+			Protocol: string(p.Protocol),
+			Port:     uint(p.ContainerPort),
+		}
+	}
+	app.Ports = nil
+	for _, port := range portMap {
+		app.Ports = append(app.Ports, port)
+	}
 }
 
 // setApp overrides the app's fields if any of them are specified in the
@@ -351,50 +413,11 @@ func setApp(app *appctypes.App, c *api.Container, opts *kubecontainer.RunContain
 		app.WorkingDirectory = c.WorkingDir
 	}
 
-	// Merge the environment. Override the image with the ones defined in the spec if necessary.
-	for _, env := range opts.Envs {
-		if ix := findEnvInList(app.Environment, env); ix >= 0 {
-			app.Environment[ix].Value = env.Value
-			continue
-		}
-		app.Environment = append(app.Environment, appctypes.EnvironmentVariable{
-			Name:  env.Name,
-			Value: env.Value,
-		})
-	}
-
-	// Override the mount points.
-	if len(opts.Mounts) > 0 {
-		app.MountPoints = []appctypes.MountPoint{}
-	}
-	for _, m := range opts.Mounts {
-		mountPointName, err := appctypes.NewACName(m.Name)
-		if err != nil {
-			return err
-		}
-		app.MountPoints = append(app.MountPoints, appctypes.MountPoint{
-			Name:     *mountPointName,
-			Path:     m.ContainerPath,
-			ReadOnly: m.ReadOnly,
-		})
-	}
-
-	// Override the ports.
-	if len(opts.PortMappings) > 0 {
-		app.Ports = []appctypes.Port{}
-	}
-	for _, p := range opts.PortMappings {
-		name, err := appctypes.SanitizeACName(p.Name)
-		if err != nil {
-			return err
-		}
-		portName := appctypes.MustACName(name)
-		app.Ports = append(app.Ports, appctypes.Port{
-			Name:     *portName,
-			Protocol: string(p.Protocol),
-			Port:     uint(p.ContainerPort),
-		})
-	}
+	// Notes that we don't create Mounts section in the pod manifest here,
+	// as Mounts will be automatically generated by rkt.
+	mergeMounts(app, opts.Mounts)
+	mergeEnv(app, opts.Envs)
+	mergePortMappings(app, opts.PortMappings)
 
 	// Override isolators.
 	return setIsolators(app, c)
@@ -456,13 +479,9 @@ func (r *Runtime) makePodManifest(pod *api.Pod, pullSecrets []api.Secret) (*appc
 	}
 
 	// Set global volumes.
-	for name, volume := range volumeMap {
-		volName, err := appctypes.NewACName(name)
-		if err != nil {
-			return nil, fmt.Errorf("cannot use the volume's name %q as ACName: %v", name, err)
-		}
+	for vname, volume := range volumeMap {
 		manifest.Volumes = append(manifest.Volumes, appctypes.Volume{
-			Name:   *volName,
+			Name:   convertToACName(vname),
 			Kind:   "host",
 			Source: volume.Builder.GetPath(),
 		})
@@ -470,13 +489,8 @@ func (r *Runtime) makePodManifest(pod *api.Pod, pullSecrets []api.Secret) (*appc
 
 	// Set global ports.
 	for _, port := range globalPortMappings {
-		name, err := appctypes.SanitizeACName(port.Name)
-		if err != nil {
-			return nil, fmt.Errorf("cannot use the port's name %q as ACName: %v", port.Name, err)
-		}
-		portName := appctypes.MustACName(name)
 		manifest.Ports = append(manifest.Ports, appctypes.ExposedPort{
-			Name:     *portName,
+			Name:     convertToACName(port.Name),
 			HostPort: uint(port.HostPort),
 		})
 	}
@@ -515,22 +529,14 @@ func (r *Runtime) newAppcRuntimeApp(pod *api.Pod, c api.Container, pullSecrets [
 		return nil, nil, err
 	}
 
-	name, err := appctypes.SanitizeACName(c.Name)
-	if err != nil {
-		return nil, nil, err
-	}
-	appName := appctypes.MustACName(name)
-
-	kubehash := kubecontainer.HashContainer(&c)
-
 	return &appcschema.RuntimeApp{
-		Name:  *appName,
+		Name:  convertToACName(c.Name),
 		Image: appcschema.RuntimeImage{ID: *hash},
 		App:   imgManifest.App,
 		Annotations: []appctypes.Annotation{
 			{
 				Name:  *appctypes.MustACIdentifier(k8sRktContainerHashAnno),
-				Value: strconv.FormatUint(kubehash, 10),
+				Value: strconv.FormatUint(kubecontainer.HashContainer(&c), 10),
 			},
 		},
 	}, opts.PortMappings, nil