diff options
author | baude <bbaude@redhat.com> | 2020-10-14 13:53:12 -0500 |
---|---|---|
committer | baude <bbaude@redhat.com> | 2020-10-20 12:06:59 -0500 |
commit | eb91d66c4aa0d2d75a5787ab7013cef88d8c9f4f (patch) | |
tree | 1dc9988ebc7470e05bd80622982ab9a5926d0d5b | |
parent | 35b4cb196545eee3b072083e716ad4588e0bb486 (diff) | |
download | podman-eb91d66c4aa0d2d75a5787ab7013cef88d8c9f4f.tar.gz podman-eb91d66c4aa0d2d75a5787ab7013cef88d8c9f4f.tar.bz2 podman-eb91d66c4aa0d2d75a5787ab7013cef88d8c9f4f.zip |
refactor api compatibility container creation to specgen
when using the compatibility layer to create containers, it used code paths to the pkg/spec which is the old implementation of containers. it is error prone and no longer being maintained. rather that fixing things in spec, migrating to specgen usage seems to make the most sense. furthermore, any fixes to the compat create will not need to be ported later.
Signed-off-by: baude <bbaude@redhat.com>
-rw-r--r-- | cmd/podman/common/create_opts.go | 291 | ||||
-rw-r--r-- | pkg/api/handlers/compat/containers_create.go | 237 | ||||
-rw-r--r-- | pkg/specgen/generate/oci.go | 2 | ||||
-rw-r--r-- | test/apiv2/20-containers.at | 10 |
4 files changed, 309 insertions, 231 deletions
diff --git a/cmd/podman/common/create_opts.go b/cmd/podman/common/create_opts.go index 83a25f4ab..f4fecf4b7 100644 --- a/cmd/podman/common/create_opts.go +++ b/cmd/podman/common/create_opts.go @@ -1,6 +1,15 @@ package common -import "github.com/containers/podman/v2/pkg/domain/entities" +import ( + "fmt" + "net" + "strconv" + "strings" + + "github.com/containers/podman/v2/pkg/api/handlers" + "github.com/containers/podman/v2/pkg/domain/entities" + "github.com/containers/podman/v2/pkg/specgen" +) type ContainerCLIOpts struct { Annotation []string @@ -111,3 +120,283 @@ type ContainerCLIOpts struct { CgroupConf []string } + +func stringMaptoArray(m map[string]string) []string { + a := make([]string, 0, len(m)) + for k, v := range m { + a = append(a, fmt.Sprintf("%s=%s", k, v)) + } + return a +} + +// ContainerCreateToContainerCLIOpts converts a compat input struct to cliopts so it can be converted to +// a specgen spec. +func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig) (*ContainerCLIOpts, []string, error) { + var ( + capAdd []string + cappDrop []string + entrypoint string + init bool + specPorts []specgen.PortMapping + ) + + if cc.HostConfig.Init != nil { + init = *cc.HostConfig.Init + } + + // Iterate devices and convert back to string + devices := make([]string, 0, len(cc.HostConfig.Devices)) + for _, dev := range cc.HostConfig.Devices { + devices = append(devices, fmt.Sprintf("%s:%s:%s", dev.PathOnHost, dev.PathInContainer, dev.CgroupPermissions)) + } + + // iterate blkreaddevicebps + readBps := make([]string, 0, len(cc.HostConfig.BlkioDeviceReadBps)) + for _, dev := range cc.HostConfig.BlkioDeviceReadBps { + readBps = append(readBps, dev.String()) + } + + // iterate blkreaddeviceiops + readIops := make([]string, 0, len(cc.HostConfig.BlkioDeviceReadIOps)) + for _, dev := range cc.HostConfig.BlkioDeviceReadIOps { + readIops = append(readIops, dev.String()) + } + + // iterate blkwritedevicebps + writeBps := make([]string, 0, len(cc.HostConfig.BlkioDeviceWriteBps)) + for _, dev := range cc.HostConfig.BlkioDeviceWriteBps { + writeBps = append(writeBps, dev.String()) + } + + // iterate blkwritedeviceiops + writeIops := make([]string, 0, len(cc.HostConfig.BlkioDeviceWriteIOps)) + for _, dev := range cc.HostConfig.BlkioDeviceWriteIOps { + writeIops = append(writeIops, dev.String()) + } + + // entrypoint + // can be a string or slice. if it is a slice, we need to + // marshall it to json; otherwise it should just be the string + // value + if len(cc.Config.Entrypoint) > 0 { + entrypoint = cc.Config.Entrypoint[0] + if len(cc.Config.Entrypoint) > 1 { + b, err := json.Marshal(cc.Config.Entrypoint) + if err != nil { + return nil, nil, err + } + entrypoint = string(b) + } + } + + // expose ports + expose := make([]string, 0, len(cc.Config.ExposedPorts)) + for p := range cc.Config.ExposedPorts { + expose = append(expose, fmt.Sprintf("%s/%s", p.Port(), p.Proto())) + } + + // mounts type=tmpfs/bind,source=,dest=,opt=val + // TODO options + mounts := make([]string, 0, len(cc.HostConfig.Mounts)) + for _, m := range cc.HostConfig.Mounts { + mount := fmt.Sprintf("type=%s", m.Type) + if len(m.Source) > 0 { + mount += fmt.Sprintf("source=%s", m.Source) + } + if len(m.Target) > 0 { + mount += fmt.Sprintf("dest=%s", m.Target) + } + mounts = append(mounts, mount) + } + + //volumes + volumes := make([]string, 0, len(cc.Config.Volumes)) + for v := range cc.Config.Volumes { + volumes = append(volumes, v) + } + + // dns + dns := make([]net.IP, 0, len(cc.HostConfig.DNS)) + for _, d := range cc.HostConfig.DNS { + dns = append(dns, net.ParseIP(d)) + } + + // publish + for port, pbs := range cc.HostConfig.PortBindings { + for _, pb := range pbs { + hostport, err := strconv.Atoi(pb.HostPort) + if err != nil { + return nil, nil, err + } + tmpPort := specgen.PortMapping{ + HostIP: pb.HostIP, + ContainerPort: uint16(port.Int()), + HostPort: uint16(hostport), + Range: 0, + Protocol: port.Proto(), + } + specPorts = append(specPorts, tmpPort) + } + } + + // network names + endpointsConfig := cc.NetworkingConfig.EndpointsConfig + cniNetworks := make([]string, 0, len(endpointsConfig)) + for netName := range endpointsConfig { + cniNetworks = append(cniNetworks, netName) + } + + // netMode + nsmode, _, err := specgen.ParseNetworkNamespace(cc.HostConfig.NetworkMode.NetworkName()) + if err != nil { + return nil, nil, err + } + + netNS := specgen.Namespace{ + NSMode: nsmode.NSMode, + Value: nsmode.Value, + } + + // network + // Note: we cannot emulate compat exactly here. we only allow specifics of networks to be + // defined when there is only one network. + netInfo := entities.NetOptions{ + AddHosts: cc.HostConfig.ExtraHosts, + CNINetworks: cniNetworks, + DNSOptions: cc.HostConfig.DNSOptions, + DNSSearch: cc.HostConfig.DNSSearch, + DNSServers: dns, + Network: netNS, + PublishPorts: specPorts, + } + + // static IP and MAC + if len(endpointsConfig) == 1 { + for _, ep := range endpointsConfig { + // if IP address is provided + if len(ep.IPAddress) > 0 { + staticIP := net.ParseIP(ep.IPAddress) + netInfo.StaticIP = &staticIP + } + // If MAC address is provided + if len(ep.MacAddress) > 0 { + staticMac, err := net.ParseMAC(ep.MacAddress) + if err != nil { + return nil, nil, err + } + netInfo.StaticMAC = &staticMac + } + break + } + } + + // Note: several options here are marked as "don't need". this is based + // on speculation by Matt and I. We think that these come into play later + // like with start. We believe this is just a difference in podman/compat + cliOpts := ContainerCLIOpts{ + //Attach: nil, // dont need? + Authfile: "", + BlkIOWeight: strconv.Itoa(int(cc.HostConfig.BlkioWeight)), + BlkIOWeightDevice: nil, // TODO + CapAdd: append(capAdd, cc.HostConfig.CapAdd...), + CapDrop: append(cappDrop, cc.HostConfig.CapDrop...), + CGroupParent: cc.HostConfig.CgroupParent, + CIDFile: cc.HostConfig.ContainerIDFile, + CPUPeriod: uint64(cc.HostConfig.CPUPeriod), + CPUQuota: cc.HostConfig.CPUQuota, + CPURTPeriod: uint64(cc.HostConfig.CPURealtimePeriod), + CPURTRuntime: cc.HostConfig.CPURealtimeRuntime, + CPUShares: uint64(cc.HostConfig.CPUShares), + //CPUS: 0, // dont need? + CPUSetCPUs: cc.HostConfig.CpusetCpus, + CPUSetMems: cc.HostConfig.CpusetMems, + //Detach: false, // dont need + //DetachKeys: "", // dont need + Devices: devices, + DeviceCGroupRule: nil, + DeviceReadBPs: readBps, + DeviceReadIOPs: readIops, + DeviceWriteBPs: writeBps, + DeviceWriteIOPs: writeIops, + Entrypoint: &entrypoint, + Env: cc.Config.Env, + Expose: expose, + GroupAdd: cc.HostConfig.GroupAdd, + Hostname: cc.Config.Hostname, + ImageVolume: "bind", + Init: init, + Interactive: cc.Config.OpenStdin, + IPC: string(cc.HostConfig.IpcMode), + Label: stringMaptoArray(cc.Config.Labels), + LogDriver: cc.HostConfig.LogConfig.Type, + LogOptions: stringMaptoArray(cc.HostConfig.LogConfig.Config), + Memory: strconv.Itoa(int(cc.HostConfig.Memory)), + MemoryReservation: strconv.Itoa(int(cc.HostConfig.MemoryReservation)), + MemorySwap: strconv.Itoa(int(cc.HostConfig.MemorySwap)), + Name: cc.Name, + OOMScoreAdj: cc.HostConfig.OomScoreAdj, + OverrideArch: "", + OverrideOS: "", + OverrideVariant: "", + PID: string(cc.HostConfig.PidMode), + PIDsLimit: cc.HostConfig.PidsLimit, + Privileged: cc.HostConfig.Privileged, + PublishAll: cc.HostConfig.PublishAllPorts, + Quiet: false, + ReadOnly: cc.HostConfig.ReadonlyRootfs, + ReadOnlyTmpFS: true, // podman default + Rm: cc.HostConfig.AutoRemove, + SecurityOpt: cc.HostConfig.SecurityOpt, + ShmSize: strconv.Itoa(int(cc.HostConfig.ShmSize)), + StopSignal: cc.Config.StopSignal, + StoreageOpt: stringMaptoArray(cc.HostConfig.StorageOpt), + Sysctl: stringMaptoArray(cc.HostConfig.Sysctls), + Systemd: "true", // podman default + TmpFS: stringMaptoArray(cc.HostConfig.Tmpfs), + TTY: cc.Config.Tty, + //Ulimit: cc.HostConfig.Ulimits, // ask dan, no documented format + User: cc.Config.User, + UserNS: string(cc.HostConfig.UsernsMode), + UTS: string(cc.HostConfig.UTSMode), + Mount: mounts, + Volume: volumes, + VolumesFrom: cc.HostConfig.VolumesFrom, + Workdir: cc.Config.WorkingDir, + Net: &netInfo, + } + + if cc.Config.StopTimeout != nil { + cliOpts.StopTimeout = uint(*cc.Config.StopTimeout) + } + + if cc.HostConfig.KernelMemory > 0 { + cliOpts.KernelMemory = strconv.Itoa(int(cc.HostConfig.KernelMemory)) + } + if len(cc.HostConfig.RestartPolicy.Name) > 0 { + policy := cc.HostConfig.RestartPolicy.Name + // only add restart count on failure + if cc.HostConfig.RestartPolicy.IsOnFailure() { + policy += fmt.Sprintf(":%d", cc.HostConfig.RestartPolicy.MaximumRetryCount) + } + cliOpts.Restart = policy + } + + if cc.HostConfig.MemorySwappiness != nil { + cliOpts.MemorySwappiness = *cc.HostConfig.MemorySwappiness + } + if cc.HostConfig.OomKillDisable != nil { + cliOpts.OOMKillDisable = *cc.HostConfig.OomKillDisable + } + if cc.Config.Healthcheck != nil { + cliOpts.HealthCmd = strings.Join(cc.Config.Healthcheck.Test, " ") + cliOpts.HealthInterval = cc.Config.Healthcheck.Interval.String() + cliOpts.HealthRetries = uint(cc.Config.Healthcheck.Retries) + cliOpts.HealthStartPeriod = cc.Config.Healthcheck.StartPeriod.String() + cliOpts.HealthTimeout = cc.Config.Healthcheck.Timeout.String() + } + + // specgen assumes the image name is arg[0] + cmd := []string{cc.Image} + cmd = append(cmd, cc.Config.Cmd...) + return &cliOpts, cmd, nil +} diff --git a/pkg/api/handlers/compat/containers_create.go b/pkg/api/handlers/compat/containers_create.go index 8a0b3c922..87c95a24c 100644 --- a/pkg/api/handlers/compat/containers_create.go +++ b/pkg/api/handlers/compat/containers_create.go @@ -1,27 +1,19 @@ package compat import ( - "context" "encoding/json" - "fmt" "net/http" - "strings" - "github.com/containers/common/pkg/config" + "github.com/containers/podman/v2/cmd/podman/common" "github.com/containers/podman/v2/libpod" "github.com/containers/podman/v2/libpod/define" - image2 "github.com/containers/podman/v2/libpod/image" "github.com/containers/podman/v2/pkg/api/handlers" "github.com/containers/podman/v2/pkg/api/handlers/utils" - "github.com/containers/podman/v2/pkg/namespaces" - "github.com/containers/podman/v2/pkg/rootless" - "github.com/containers/podman/v2/pkg/signal" - createconfig "github.com/containers/podman/v2/pkg/spec" + "github.com/containers/podman/v2/pkg/domain/entities" + "github.com/containers/podman/v2/pkg/domain/infra/abi" "github.com/containers/podman/v2/pkg/specgen" - "github.com/containers/storage" "github.com/gorilla/schema" "github.com/pkg/errors" - "golang.org/x/sys/unix" ) func CreateContainer(w http.ResponseWriter, r *http.Request) { @@ -56,220 +48,27 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) { utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "NewFromLocal()")) return } - containerConfig, err := runtime.GetConfig() + + // Take input structure and convert to cliopts + cliOpts, args, err := common.ContainerCreateToContainerCLIOpts(input) if err != nil { - utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "GetConfig()")) + utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "make cli opts()")) return } - cc, err := makeCreateConfig(r.Context(), containerConfig, input, newImage) - if err != nil { - utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "makeCreatConfig()")) + sg := specgen.NewSpecGenerator(newImage.ID(), cliOpts.RootFS) + if err := common.FillOutSpecGen(sg, cliOpts, args); err != nil { + utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "fill out specgen")) return } - cc.Name = query.Name - utils.CreateContainer(r.Context(), w, runtime, &cc) -} - -func makeCreateConfig(ctx context.Context, containerConfig *config.Config, input handlers.CreateContainerConfig, newImage *image2.Image) (createconfig.CreateConfig, error) { - var ( - err error - init bool - ) - env := make(map[string]string) - stopSignal := unix.SIGTERM - if len(input.StopSignal) > 0 { - stopSignal, err = signal.ParseSignal(input.StopSignal) - if err != nil { - return createconfig.CreateConfig{}, err - } - } - - workDir, err := newImage.WorkingDir(ctx) + ic := abi.ContainerEngine{Libpod: runtime} + report, err := ic.ContainerCreate(r.Context(), sg) if err != nil { - return createconfig.CreateConfig{}, err - } - if workDir == "" { - workDir = "/" - } - if len(input.WorkingDir) > 0 { - workDir = input.WorkingDir - } - - // Only use image's Cmd when the user does not set the entrypoint - if input.Entrypoint == nil && len(input.Cmd) == 0 { - cmdSlice, err := newImage.Cmd(ctx) - if err != nil { - return createconfig.CreateConfig{}, err - } - input.Cmd = cmdSlice - } - - if input.Entrypoint == nil { - entrypointSlice, err := newImage.Entrypoint(ctx) - if err != nil { - return createconfig.CreateConfig{}, err - } - input.Entrypoint = entrypointSlice - } - - stopTimeout := containerConfig.Engine.StopTimeout - if input.StopTimeout != nil { - stopTimeout = uint(*input.StopTimeout) - } - c := createconfig.CgroupConfig{ - Cgroups: "", // podman - Cgroupns: "", // podman - CgroupParent: "", // podman - CgroupMode: "", // podman - } - security := createconfig.SecurityConfig{ - CapAdd: input.HostConfig.CapAdd, - CapDrop: input.HostConfig.CapDrop, - LabelOpts: nil, // podman - NoNewPrivs: false, // podman - ApparmorProfile: "", // podman - SeccompProfilePath: "", - SecurityOpts: input.HostConfig.SecurityOpt, - Privileged: input.HostConfig.Privileged, - ReadOnlyRootfs: input.HostConfig.ReadonlyRootfs, - ReadOnlyTmpfs: false, // podman-only - Sysctl: input.HostConfig.Sysctls, - } - - var netmode namespaces.NetworkMode - if rootless.IsRootless() { - netmode = namespaces.NetworkMode(specgen.Slirp) - } - - network := createconfig.NetworkConfig{ - DNSOpt: input.HostConfig.DNSOptions, - DNSSearch: input.HostConfig.DNSSearch, - DNSServers: input.HostConfig.DNS, - ExposedPorts: input.ExposedPorts, - HTTPProxy: false, // podman - IP6Address: "", - IPAddress: "", - LinkLocalIP: nil, // docker-only - MacAddress: input.MacAddress, - NetMode: netmode, - Network: input.HostConfig.NetworkMode.NetworkName(), - NetworkAlias: nil, // docker-only now - PortBindings: input.HostConfig.PortBindings, - Publish: nil, // podmanseccompPath - PublishAll: input.HostConfig.PublishAllPorts, - } - - uts := createconfig.UtsConfig{ - UtsMode: namespaces.UTSMode(input.HostConfig.UTSMode), - NoHosts: false, //podman - HostAdd: input.HostConfig.ExtraHosts, - Hostname: input.Hostname, - } - - z := createconfig.UserConfig{ - GroupAdd: input.HostConfig.GroupAdd, - IDMappings: &storage.IDMappingOptions{}, // podman //TODO <--- fix this, - UsernsMode: namespaces.UsernsMode(input.HostConfig.UsernsMode), - User: input.User, - } - pidConfig := createconfig.PidConfig{PidMode: namespaces.PidMode(input.HostConfig.PidMode)} - // TODO: We should check that these binds are all listed in the `Volumes` - // key since it doesn't make sense to define a `Binds` element for a - // container path which isn't defined as a volume - volumes := input.HostConfig.Binds - - // Docker is more flexible about its input where podman throws - // away incorrectly formatted variables so we cannot reuse the - // parsing of the env input - // [Foo Other=one Blank=] - imgEnv, err := newImage.Env(ctx) - if err != nil { - return createconfig.CreateConfig{}, err - } - input.Env = append(imgEnv, input.Env...) - for _, e := range input.Env { - splitEnv := strings.Split(e, "=") - switch len(splitEnv) { - case 0: - continue - case 1: - env[splitEnv[0]] = "" - default: - env[splitEnv[0]] = strings.Join(splitEnv[1:], "=") - } - } - - // format the tmpfs mounts into a []string from map - tmpfs := make([]string, 0, len(input.HostConfig.Tmpfs)) - for k, v := range input.HostConfig.Tmpfs { - tmpfs = append(tmpfs, fmt.Sprintf("%s:%s", k, v)) - } - - if input.HostConfig.Init != nil && *input.HostConfig.Init { - init = true - } - - m := createconfig.CreateConfig{ - Annotations: nil, // podman - Args: nil, - Cgroup: c, - CidFile: "", - ConmonPidFile: "", // podman - Command: input.Cmd, - UserCommand: input.Cmd, // podman - Detach: false, // - // Devices: input.HostConfig.Devices, - Entrypoint: input.Entrypoint, - Env: env, - HealthCheck: nil, // - Init: init, - InitPath: "", // tbd - Image: input.Image, - ImageID: newImage.ID(), - BuiltinImgVolumes: nil, // podman - ImageVolumeType: "", // podman - Interactive: input.OpenStdin, - // IpcMode: input.HostConfig.IpcMode, - Labels: input.Labels, - LogDriver: input.HostConfig.LogConfig.Type, // is this correct - // LogDriverOpt: input.HostConfig.LogConfig.Config, - Name: input.Name, - Network: network, - Pod: "", // podman - PodmanPath: "", // podman - Quiet: false, // front-end only - Resources: createconfig.CreateResourceConfig{MemorySwappiness: -1}, - RestartPolicy: input.HostConfig.RestartPolicy.Name, - Rm: input.HostConfig.AutoRemove, - StopSignal: stopSignal, - StopTimeout: stopTimeout, - Systemd: false, // podman - Tmpfs: tmpfs, - User: z, - Uts: uts, - Tty: input.Tty, - Mounts: nil, // we populate - // MountsFlag: input.HostConfig.Mounts, - NamedVolumes: nil, // we populate - Volumes: volumes, - VolumesFrom: input.HostConfig.VolumesFrom, - WorkDir: workDir, - Rootfs: "", // podman - Security: security, - Syslog: false, // podman - - Pid: pidConfig, + utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "container create")) + return } - - fullCmd := append(input.Entrypoint, input.Cmd...) - if len(fullCmd) > 0 { - m.PodmanPath = fullCmd[0] - if len(fullCmd) == 1 { - m.Args = fullCmd - } else { - m.Args = fullCmd[1:] - } + createResponse := entities.ContainerCreateResponse{ + ID: report.Id, + Warnings: []string{}, } - - return m, nil + utils.WriteResponse(w, http.StatusCreated, createResponse) } diff --git a/pkg/specgen/generate/oci.go b/pkg/specgen/generate/oci.go index f02432f5b..8454458a8 100644 --- a/pkg/specgen/generate/oci.go +++ b/pkg/specgen/generate/oci.go @@ -110,7 +110,7 @@ func makeCommand(ctx context.Context, s *specgen.SpecGenerator, img *image.Image // Only use image command if the user did not manually set an // entrypoint. command := s.Command - if command == nil && img != nil && s.Entrypoint == nil { + if (command == nil || len(command) == 0) && img != nil && (s.Entrypoint == nil || len(s.Entrypoint) == 0) { newCmd, err := img.Cmd(ctx) if err != nil { return nil, err diff --git a/test/apiv2/20-containers.at b/test/apiv2/20-containers.at index 7fbcd2e9c..c7055dfc4 100644 --- a/test/apiv2/20-containers.at +++ b/test/apiv2/20-containers.at @@ -206,16 +206,6 @@ t POST containers/${cid_top}/stop "" 204 t DELETE containers/$cid 204 t DELETE containers/$cid_top 204 -# test the apiv2 create, shouldn't ignore the ENV and WORKDIR from the image -t POST containers/create '"Image":"'$ENV_WORKDIR_IMG'","Env":["testKey1"]' 201 \ - .Id~[0-9a-f]\\{64\\} -cid=$(jq -r '.Id' <<<"$output") -t GET containers/$cid/json 200 \ - .Config.Env~.*REDIS_VERSION= \ - .Config.Env~.*testKey1= \ - .Config.WorkingDir="/data" # default is /data -t DELETE containers/$cid 204 - # test the WORKDIR and StopSignal t POST containers/create '"Image":"'$ENV_WORKDIR_IMG'","WorkingDir":"/dataDir","StopSignal":"9"' 201 \ .Id~[0-9a-f]\\{64\\} |