Merge pull request #8228 from yifan-gu/rkt_image_cat

pkg/kubelet/rkt/image.go

@@ -38,7 +38,7 @@ type image struct {
 // sha512-91e98d7f1679a097c878203c9659f2a26ae394656b3147963324c61fa3832f15	coreos.com/etcd:v2.0.9
 //
 func (im *image) parseString(input string) error {
-	idName := strings.Split(input, "\t")
+	idName := strings.Split(strings.TrimSpace(input), "\t")
 	if len(idName) != 2 {
 		return fmt.Errorf("invalid image information from 'rkt images': %q", input)
 	}

pkg/kubelet/rkt/rkt.go

@@ -148,6 +148,7 @@ func New(config *Config,
 		generator:           generator,
 		recorder:            recorder,
 		readinessManager:    readinessManager,
+		volumeGetter:        volumeGetter,
 	}
 	rkt.prober = prober.New(rkt, readinessManager, containerRefManager, recorder)
 
@@ -174,7 +175,7 @@ func (r *runtime) buildCommand(args ...string) *exec.Cmd {
 }
 
 // runCommand invokes rkt binary with arguments and returns the result
-// from stdout in a list of strings.
+// from stdout in a list of strings. Each string in the list is a line.
 func (r *runtime) runCommand(args ...string) ([]string, error) {
 	glog.V(4).Info("rkt: Run command:", args)
 
@@ -285,7 +286,7 @@ func setIsolators(app *appctypes.App, c *api.Container) error {
 
 // setApp overrides the app's fields if any of them are specified in the
 // container's spec.
-func setApp(app *appctypes.App, c *api.Container) error {
+func setApp(app *appctypes.App, c *api.Container, opts *kubecontainer.RunContainerOptions) error {
 	// Override the exec.
 	// TOOD(yifan): Revisit this for the overriding rule.
 	if len(c.Command) > 0 || len(c.Args) > 0 {
@@ -302,8 +303,7 @@ func setApp(app *appctypes.App, c *api.Container) error {
 	}
 
 	// Override the environment.
-	// TODO(yifan): Use RunContainerOptions.
-	if len(c.Env) > 0 {
+	if len(opts.Envs) > 0 {
 		app.Environment = []appctypes.EnvironmentVariable{}
 	}
 	for _, env := range c.Env {
@@ -314,30 +314,31 @@ func setApp(app *appctypes.App, c *api.Container) error {
 	}
 
 	// Override the mount points.
-	if len(c.VolumeMounts) > 0 {
+	if len(opts.Mounts) > 0 {
 		app.MountPoints = []appctypes.MountPoint{}
 	}
-	for _, m := range c.VolumeMounts {
+	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.MountPath,
+			Path:     m.ContainerPath,
 			ReadOnly: m.ReadOnly,
 		})
 	}
 
 	// Override the ports.
-	if len(c.Ports) > 0 {
+	if len(opts.PortMappings) > 0 {
 		app.Ports = []appctypes.Port{}
 	}
-	for _, p := range c.Ports {
-		portName, err := appctypes.NewACName(p.Name)
+	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),
@@ -349,12 +350,38 @@ func setApp(app *appctypes.App, c *api.Container) error {
 	return setIsolators(app, c)
 }
 
+// getImageManifest invokes 'rkt image cat-manifest' to retrive the image manifest
+// for the image.
+func (r *runtime) getImageManifest(image string) (*appcschema.ImageManifest, error) {
+	var manifest appcschema.ImageManifest
+
+	// TODO(yifan): Assume docker images for now.
+	output, err := r.runCommand("image", "cat-manifest", "--quiet", dockerPrefix+image)
+	if err != nil {
+		return nil, err
+	}
+	if len(output) != 1 {
+		return nil, fmt.Errorf("invalid output: %v", output)
+	}
+	return &manifest, json.Unmarshal([]byte(output[0]), &manifest)
+}
+
 // makePodManifest transforms a kubelet pod spec to the rkt pod manifest.
 // TODO(yifan): Use the RunContainerOptions generated by GenerateRunContainerOptions().
 func (r *runtime) makePodManifest(pod *api.Pod) (*appcschema.PodManifest, error) {
+	var globalPortMappings []kubecontainer.PortMapping
 	manifest := appcschema.BlankPodManifest()
 
 	for _, c := range pod.Spec.Containers {
+		imgManifest, err := r.getImageManifest(c.Image)
+		if err != nil {
+			return nil, err
+		}
+
+		if imgManifest.App == nil {
+			return nil, fmt.Errorf("no app section in image manifest for image: %q", c.Image)
+		}
+
 		img, err := r.getImageByName(c.Image)
 		if err != nil {
 			return nil, err
@@ -364,22 +391,23 @@ func (r *runtime) makePodManifest(pod *api.Pod) (*appcschema.PodManifest, error)
 			return nil, err
 		}
 
-		// TODO(yifan): Override the image manifest's app and store it in the pod manifest.
-		// We need to get the image manifest. https://github.com/coreos/rkt/issues/850
-		app := &appctypes.App{}
-		if err := setApp(app, &c); err != nil {
+		opts, err := r.generator.GenerateRunContainerOptions(pod, &c)
+		if err != nil {
 			return nil, err
 		}
-		imageName, err := appctypes.NewACName(img.name)
-		if err != nil {
+
+		globalPortMappings = append(globalPortMappings, opts.PortMappings...)
+
+		if err := setApp(imgManifest.App, &c, opts); err != nil {
 			return nil, err
 		}
+
 		manifest.Apps = append(manifest.Apps, appcschema.RuntimeApp{
 			// TODO(yifan): We should allow app name to be different with
 			// image name. See https://github.com/coreos/rkt/pull/640.
-			Name:  *imageName,
+			Name:  imgManifest.Name,
 			Image: appcschema.RuntimeImage{ID: *hash},
-			App:   app,
+			App:   imgManifest.App,
 		})
 	}
 
@@ -402,17 +430,16 @@ func (r *runtime) makePodManifest(pod *api.Pod) (*appcschema.PodManifest, error)
 	}
 
 	// Set global ports.
-	for _, c := range pod.Spec.Containers {
-		for _, port := range c.Ports {
-			portName, err := appctypes.NewACName(port.Name)
-			if err != nil {
-				return nil, fmt.Errorf("cannot use the port's name %q as ACName: %v", port.Name, err)
-			}
-			manifest.Ports = append(manifest.Ports, appctypes.ExposedPort{
-				Name:     *portName,
-				HostPort: uint(port.HostPort),
-			})
+	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,
+			HostPort: uint(port.HostPort),
+		})
 	}
 	// TODO(yifan): Set pod-level isolators once it's supported in kubernetes.
 	return manifest, nil
@@ -793,7 +820,6 @@ func (r *runtime) SyncPod(pod *api.Pod, runningPod kubecontainer.Pod, podStatus
 	podFullName := kubecontainer.GetPodFullName(pod)
 	if len(runningPod.Containers) == 0 {
 		glog.V(4).Infof("Pod %q is not running, will start it", podFullName)
-		// TODO(yifan): Use RunContainerOptionsGeneratior to get volumeMaps, etc.
 		return r.RunPod(pod)
 	}
 
@@ -1118,18 +1144,33 @@ func (r *runtime) listImages() ([]image, error) {
 }
 
 // getImageByName tries to find the image info with the given image name.
-func (r *runtime) getImageByName(name string) (image, error) {
-	// TODO(yifan): Use rkt image cat-file when that is ready:
-	// https://github.com/coreos/rkt/pull/649
+func (r *runtime) getImageByName(imageName string) (image, error) {
+	// TODO(yifan): Print hash in rkt image?
 	images, err := r.listImages()
 	if err != nil {
 		return image{}, err
 	}
 
+	var name, version string
+	nameVersion := strings.Split(imageName, ":")
+
+	// TODO(yifan): Currently the name cannot include "_", it is replaced
+	// by "-". See the issue in appc/spec: https://github.com/appc/spec/issues/406.
+	name, err = appctypes.SanitizeACName(nameVersion[0])
+	if err != nil {
+		return image{}, err
+	}
+
+	if len(nameVersion) == 2 {
+		version = nameVersion[1]
+	}
+
 	for _, img := range images {
 		if img.name == name {
-			return img, nil
+			if version == "" || img.version == version {
+				return img, nil
+			}
 		}
 	}
-	return image{}, fmt.Errorf("cannot find the image %q", name)
+	return image{}, fmt.Errorf("cannot find the image %q", imageName)
 }