diff --git a/pkg/kubelet/container/runtime.go b/pkg/kubelet/container/runtime.go index bfb14ce4bfc..1c86fa14b46 100644 --- 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 diff --git a/pkg/kubelet/rkt/rkt.go b/pkg/kubelet/rkt/rkt.go index 31ef2462100..ae527ab036b 100644 --- a/pkg/kubelet/rkt/rkt.go +++ b/pkg/kubelet/rkt/rkt.go @@ -451,21 +451,16 @@ func mergeEnv(app *appctypes.App, optEnv []kubecontainer.EnvVar) { } } -// mergeMounts merges the optMounts with the image's mount points. +// mergeMounts merges the mountPoints 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) +// with the same container path. +func mergeMounts(app *appctypes.App, mountPoints []appctypes.MountPoint) { + mountMap := make(map[string]appctypes.MountPoint) for _, m := range app.MountPoints { - mountMap[m.Name] = m + mountMap[m.Path] = m } - for _, m := range optMounts { - mpName := convertToACName(m.Name) - mountMap[mpName] = appctypes.MountPoint{ - Name: mpName, - Path: m.ContainerPath, - ReadOnly: m.ReadOnly, - } + for _, m := range mountPoints { + mountMap[m.Path] = m } app.MountPoints = nil for _, mount := range mountMap { @@ -473,21 +468,16 @@ func mergeMounts(app *appctypes.App, optMounts []kubecontainer.Mount) { } } -// mergePortMappings merges the optPortMappings with the image's port mappings. +// mergePortMappings merges the containerPorts with the image's container ports. // 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) { +func mergePortMappings(app *appctypes.App, containerPorts []appctypes.Port) { 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), - } + for _, p := range containerPorts { + portMap[p.Name] = p } app.Ports = nil for _, port := range portMap { @@ -525,7 +515,10 @@ func setSupplementalGIDs(app *appctypes.App, podCtx *api.PodSecurityContext, sup } // setApp merges the container spec with the image's manifest. -func setApp(imgManifest *appcschema.ImageManifest, c *api.Container, opts *kubecontainer.RunContainerOptions, ctx *api.SecurityContext, podCtx *api.PodSecurityContext, supplementalGids []int64) error { +func setApp(imgManifest *appcschema.ImageManifest, c *api.Container, + mountPoints []appctypes.MountPoint, containerPorts []appctypes.Port, envs []kubecontainer.EnvVar, + ctx *api.SecurityContext, podCtx *api.PodSecurityContext, supplementalGids []int64) error { + app := imgManifest.App // Set up Exec. @@ -544,7 +537,7 @@ func setApp(imgManifest *appcschema.ImageManifest, c *api.Container, opts *kubec return fmt.Errorf("cannot unmarshal CMD %q: %v", ag, err) } } - userCommand, userArgs := kubecontainer.ExpandContainerCommandAndArgs(c, opts.Envs) + userCommand, userArgs := kubecontainer.ExpandContainerCommandAndArgs(c, envs) if len(userCommand) > 0 { command = userCommand @@ -589,9 +582,9 @@ func setApp(imgManifest *appcschema.ImageManifest, c *api.Container, opts *kubec // 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) + mergeMounts(app, mountPoints) + mergeEnv(app, envs) + mergePortMappings(app, containerPorts) return setIsolators(app, c, ctx) } @@ -752,6 +745,17 @@ func (r *Runtime) makeContainerLogMount(opts *kubecontainer.RunContainerOptions, } func (r *Runtime) newAppcRuntimeApp(pod *api.Pod, podIP string, c api.Container, requiresPrivileged bool, pullSecrets []api.Secret, manifest *appcschema.PodManifest) error { + var annotations appctypes.Annotations = []appctypes.Annotation{ + { + Name: *appctypes.MustACIdentifier(k8sRktContainerHashAnno), + Value: strconv.FormatUint(kubecontainer.HashContainer(&c), 10), + }, + { + Name: *appctypes.MustACIdentifier(types.KubernetesContainerNameLabel), + Value: c.Name, + }, + } + if requiresPrivileged && !securitycontext.HasPrivilegedRequest(&c) { return fmt.Errorf("cannot make %q: running a custom stage1 requires a privileged security context", format.Pod(pod)) } @@ -782,93 +786,51 @@ func (r *Runtime) newAppcRuntimeApp(pod *api.Pod, podIP string, c api.Container, return err } - // create the container log file and make a mount pair. - mnt, err := r.makeContainerLogMount(opts, &c) + // Create additional mount for termintation message path. + mount, err := r.makeContainerLogMount(opts, &c) if err != nil { return err } + mounts := append(opts.Mounts, *mount) + annotations = append(annotations, appctypes.Annotation{ + Name: *appctypes.MustACIdentifier(k8sRktTerminationMessagePathAnno), + Value: mount.HostPath, + }) - // If run in 'hostnetwork' mode, then copy and mount the host's /etc/resolv.conf and /etc/hosts, - // and add volumes. - var hostsMnt, resolvMnt *kubecontainer.Mount + // If run in 'hostnetwork' mode, then copy the host's /etc/resolv.conf and /etc/hosts, + // and add mounts. if kubecontainer.IsHostNetworkPod(pod) { - hostsMnt, resolvMnt, err = makeHostNetworkMount(opts) + hostsMount, resolvMount, err := makeHostNetworkMount(opts) if err != nil { return err } - manifest.Volumes = append(manifest.Volumes, appctypes.Volume{ - Name: convertToACName(hostsMnt.Name), - Kind: "host", - Source: hostsMnt.HostPath, - }) - manifest.Volumes = append(manifest.Volumes, appctypes.Volume{ - Name: convertToACName(resolvMnt.Name), - Kind: "host", - Source: resolvMnt.HostPath, - }) + mounts = append(mounts, *hostsMount, *resolvMount) } supplementalGids := r.runtimeHelper.GetExtraSupplementalGroupsForPod(pod) ctx := securitycontext.DetermineEffectiveSecurityContext(pod, &c) - if err := setApp(imgManifest, &c, opts, ctx, pod.Spec.SecurityContext, supplementalGids); err != nil { + + volumes, mountPoints := convertKubeMounts(mounts) + containerPorts, hostPorts := convertKubePortMappings(opts.PortMappings) + + if err := setApp(imgManifest, &c, mountPoints, containerPorts, opts.Envs, ctx, pod.Spec.SecurityContext, supplementalGids); err != nil { return err } - for _, mnt := range opts.Mounts { - readOnly := mnt.ReadOnly - manifest.Volumes = append(manifest.Volumes, appctypes.Volume{ - Name: convertToACName(mnt.Name), - Source: mnt.HostPath, - Kind: "host", - ReadOnly: &readOnly, - }) - } - ra := appcschema.RuntimeApp{ - Name: convertToACName(c.Name), - Image: appcschema.RuntimeImage{ID: *hash}, - App: imgManifest.App, - Annotations: []appctypes.Annotation{ - { - Name: *appctypes.MustACIdentifier(k8sRktContainerHashAnno), - Value: strconv.FormatUint(kubecontainer.HashContainer(&c), 10), - }, - { - Name: *appctypes.MustACIdentifier(types.KubernetesContainerNameLabel), - Value: c.Name, - }, - }, + Name: convertToACName(c.Name), + Image: appcschema.RuntimeImage{ID: *hash}, + App: imgManifest.App, + Annotations: annotations, } if c.SecurityContext != nil && c.SecurityContext.ReadOnlyRootFilesystem != nil { ra.ReadOnlyRootFS = *c.SecurityContext.ReadOnlyRootFilesystem } - if mnt != nil { - ra.Annotations = append(ra.Annotations, appctypes.Annotation{ - Name: *appctypes.MustACIdentifier(k8sRktTerminationMessagePathAnno), - Value: mnt.HostPath, - }) - - manifest.Volumes = append(manifest.Volumes, appctypes.Volume{ - Name: convertToACName(mnt.Name), - Kind: "host", - Source: mnt.HostPath, - }) - } - manifest.Apps = append(manifest.Apps, ra) - - // Set global ports. - for _, port := range opts.PortMappings { - if port.HostPort == 0 { - continue - } - manifest.Ports = append(manifest.Ports, appctypes.ExposedPort{ - Name: convertToACName(port.Name), - HostPort: uint(port.HostPort), - }) - } + manifest.Volumes = append(manifest.Volumes, volumes...) + manifest.Ports = append(manifest.Ports, hostPorts...) return nil } @@ -2347,3 +2309,75 @@ func getOSReleaseInfo() (map[string]string, error) { } return result, nil } + +// convertKubeMounts creates appc volumes and mount points according to the given mounts. +// Only one volume will be created for every unique host path. +// Only one mount point will be created for every unique container path. +func convertKubeMounts(mounts []kubecontainer.Mount) ([]appctypes.Volume, []appctypes.MountPoint) { + volumeMap := make(map[string]*appctypes.Volume) + mountPointMap := make(map[string]*appctypes.MountPoint) + + for _, mnt := range mounts { + readOnly := mnt.ReadOnly + + if _, existed := volumeMap[mnt.HostPath]; !existed { + volumeMap[mnt.HostPath] = &appctypes.Volume{ + Name: *appctypes.MustACName(string(uuid.NewUUID())), + Kind: "host", + Source: mnt.HostPath, + ReadOnly: &readOnly, + } + } + + if _, existed := mountPointMap[mnt.ContainerPath]; existed { + glog.Warningf("Multiple mount points with the same container path %v, ignore it", mnt) + continue + } + + mountPointMap[mnt.ContainerPath] = &appctypes.MountPoint{ + Name: volumeMap[mnt.HostPath].Name, + Path: mnt.ContainerPath, + ReadOnly: readOnly, + } + } + + volumes := make([]appctypes.Volume, 0, len(volumeMap)) + mountPoints := make([]appctypes.MountPoint, 0, len(mountPointMap)) + + for _, vol := range volumeMap { + volumes = append(volumes, *vol) + } + for _, mnt := range mountPointMap { + mountPoints = append(mountPoints, *mnt) + } + + return volumes, mountPoints +} + +// convertKubePortMappings creates appc container ports and host ports according to the given port mappings. +// The container ports and host ports are mapped by PortMapping.Name. +func convertKubePortMappings(portMappings []kubecontainer.PortMapping) ([]appctypes.Port, []appctypes.ExposedPort) { + containerPorts := make([]appctypes.Port, 0, len(portMappings)) + hostPorts := make([]appctypes.ExposedPort, 0, len(portMappings)) + + for _, p := range portMappings { + // This matches the docker code's behaviour. + if p.HostPort == 0 { + continue + } + + portName := convertToACName(p.Name) + containerPorts = append(containerPorts, appctypes.Port{ + Name: portName, + Protocol: string(p.Protocol), + Port: uint(p.ContainerPort), + }) + + hostPorts = append(hostPorts, appctypes.ExposedPort{ + Name: portName, + HostPort: uint(p.HostPort), + }) + } + + return containerPorts, hostPorts +} diff --git a/pkg/kubelet/rkt/rkt_test.go b/pkg/kubelet/rkt/rkt_test.go index 89c28d01d31..d34f4e22a0e 100644 --- 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) }