From 9ee50fe2c7d31e5a6209b63f7735a965dc204131 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 15 Apr 2019 15:26:29 -0400 Subject: Migrate to unified volume handling code Unify handling for the --volume, --mount, --volumes-from, --tmpfs and --init flags into a single file and set of functions. This will greatly improve readability and maintainability. Further, properly handle superceding and conflicting mounts. Our current patchwork has serious issues when mounts conflict, or when a mount from --volumes-from or an image volume should be overwritten by a user volume or named volume. Signed-off-by: Matthew Heon --- pkg/spec/spec_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'pkg/spec/spec_test.go') diff --git a/pkg/spec/spec_test.go b/pkg/spec/spec_test.go index c037bf69e..b7c9c9c0d 100644 --- a/pkg/spec/spec_test.go +++ b/pkg/spec/spec_test.go @@ -8,22 +8,22 @@ import ( "github.com/stretchr/testify/assert" ) -func TestCreateConfig_GetVolumeMounts(t *testing.T) { +func TestGetVolumeMountsOneVolume(t *testing.T) { data := spec.Mount{ Destination: "/foobar", Type: "bind", - Source: "foobar", + Source: "/foobar", Options: []string{"ro", "rbind", "rprivate"}, } config := CreateConfig{ - Volumes: []string{"foobar:/foobar:ro"}, + Volumes: []string{"/foobar:/foobar:ro"}, } - specMount, err := config.GetVolumeMounts([]spec.Mount{}) + specMount, _, err := config.getVolumeMounts() assert.NoError(t, err) - assert.True(t, reflect.DeepEqual(data, specMount[0])) + assert.True(t, reflect.DeepEqual(data, specMount[data.Destination])) } -func TestCreateConfig_GetTmpfsMounts(t *testing.T) { +func TestGetTmpfsMounts(t *testing.T) { data := spec.Mount{ Destination: "/homer", Type: "tmpfs", @@ -33,7 +33,7 @@ func TestCreateConfig_GetTmpfsMounts(t *testing.T) { config := CreateConfig{ Tmpfs: []string{"/homer:rw,size=787448k,mode=1777"}, } - tmpfsMount := config.GetTmpfsMounts() - assert.True(t, reflect.DeepEqual(data, tmpfsMount[0])) - + tmpfsMount, err := config.getTmpfsMounts() + assert.NoError(t, err) + assert.True(t, reflect.DeepEqual(data, tmpfsMount[data.Destination])) } -- cgit v1.2.3-54-g00ecf From 2e00d417dd87dd6648f6c3b604fd25db0c05d9f1 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 17 Apr 2019 16:08:39 -0400 Subject: Migrate unit tests from cmd/podman into pkg/spec Several changes made in the interface of pkg/spec make interacting with it without a runtime difficult to impossible, so move the existing limited testing from cmd/podman (which mostly tested pkg/spec) into pkg/spec itself where we can call individual functions that don't break things. Signed-off-by: Matthew Heon --- cmd/podman/run_test.go | 162 -------------------------------------------- pkg/spec/containerconfig.go | 2 - pkg/spec/spec_test.go | 103 ++++++++++++++++++++++------ pkg/spec/storage_test.go | 39 +++++++++++ 4 files changed, 120 insertions(+), 186 deletions(-) delete mode 100644 cmd/podman/run_test.go create mode 100644 pkg/spec/storage_test.go (limited to 'pkg/spec/spec_test.go') diff --git a/cmd/podman/run_test.go b/cmd/podman/run_test.go deleted file mode 100644 index af9e6923c..000000000 --- a/cmd/podman/run_test.go +++ /dev/null @@ -1,162 +0,0 @@ -package main - -import ( - "runtime" - "testing" - - "github.com/containers/libpod/cmd/podman/cliconfig" - "github.com/containers/libpod/cmd/podman/shared" - "github.com/containers/libpod/pkg/inspect" - cc "github.com/containers/libpod/pkg/spec" - "github.com/containers/libpod/pkg/sysinfo" - "github.com/docker/go-units" - ociv1 "github.com/opencontainers/image-spec/specs-go/v1" - spec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/spf13/cobra" - "github.com/stretchr/testify/assert" -) - -var ( - sysInfo = sysinfo.New(true) - cmd = []string{"podman", "test", "alpine"} - CLI *cliconfig.PodmanCommand -) - -// generates a mocked ImageData structure based on alpine -func generateAlpineImageData() *inspect.ImageData { - config := &ociv1.ImageConfig{ - User: "", - ExposedPorts: nil, - Env: []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"}, - Entrypoint: []string{}, - Cmd: []string{"/bin/sh"}, - Volumes: nil, - WorkingDir: "", - Labels: nil, - StopSignal: "", - } - - data := &inspect.ImageData{ - ID: "e21c333399e0aeedfd70e8827c9fba3f8e9b170ef8a48a29945eb7702bf6aa5f", - RepoTags: []string{"docker.io/library/alpine:latest"}, - RepoDigests: []string{"docker.io/library/alpine@sha256:5cb04fce748f576d7b72a37850641de8bd725365519673c643ef2d14819b42c6"}, - Comment: "Created:2017-12-01 18:48:48.949613376 +0000", - Author: "", - Architecture: "amd64", - Os: "linux", - Version: "17.06.2-ce", - Config: config, - } - return data -} - -// sets a global CLI -func testCmd(c *cobra.Command) error { - CLI = &cliconfig.PodmanCommand{Command: c} - return nil -} - -// creates the mocked cli pointing to our create flags -// global flags like log-level are not implemented -func createCLI(args []string) *cliconfig.PodmanCommand { - var testCommand = &cliconfig.PodmanCommand{ - Command: &cobra.Command{ - Use: "test", - RunE: func(cmd *cobra.Command, args []string) error { - return testCmd(cmd) - }, - }, - } - rootCmd := testCommand - getCreateFlags(rootCmd) - rootCmd.ParseFlags(args) - return rootCmd -} - -func getRuntimeSpec(c *cliconfig.PodmanCommand) (*spec.Spec, error) { - /* - TODO: This test has never worked. Need to install content - runtime, err := getRuntime(c) - if err != nil { - return nil, err - } - createConfig, err := parseCreateOpts(c, runtime, "alpine", generateAlpineImageData()) - */ - ctx := getContext() - genericResults := shared.NewIntermediateLayer(c, false) - createConfig, err := shared.ParseCreateOpts(ctx, &genericResults, nil, "alpine", generateAlpineImageData()) - if err != nil { - return nil, err - } - runtimeSpec, err := cc.CreateConfigToOCISpec(createConfig) - if err != nil { - return nil, err - } - return runtimeSpec, nil -} - -// TestPIDsLimit verifies the inputted pid-limit is correctly defined in the spec -func TestPIDsLimit(t *testing.T) { - // The default configuration of podman enables seccomp, which is not available on non-Linux systems. - // Thus, any tests that use the default seccomp setting would fail. - // Skip the tests on non-Linux platforms rather than explicitly disable seccomp in the test and possibly affect the test result. - if runtime.GOOS != "linux" { - t.Skip("seccomp, which is enabled by default, is only supported on Linux") - } - if !sysInfo.PidsLimit { - t.Skip("running test not supported by the host system") - } - args := []string{"--pids-limit", "22"} - a := createCLI(args) - a.InputArgs = args - //a.Run(append(cmd, args...)) - runtimeSpec, err := getRuntimeSpec(a) - if err != nil { - t.Fatalf(err.Error()) - } - assert.Equal(t, runtimeSpec.Linux.Resources.Pids.Limit, int64(22)) -} - -// TestBLKIOWeightDevice verifies the inputted blkio weigh device is correctly defined in the spec -func TestBLKIOWeightDevice(t *testing.T) { - // The default configuration of podman enables seccomp, which is not available on non-Linux systems. - // Thus, any tests that use the default seccomp setting would fail. - // Skip the tests on non-Linux platforms rather than explicitly disable seccomp in the test and possibly affect the test result. - if runtime.GOOS != "linux" { - t.Skip("seccomp, which is enabled by default, is only supported on Linux") - } - if !sysInfo.BlkioWeightDevice { - t.Skip("running test not supported by the host system") - } - args := []string{"--blkio-weight-device", "/dev/zero:100"} - a := createCLI(args) - a.InputArgs = args - runtimeSpec, err := getRuntimeSpec(a) - if err != nil { - t.Fatalf(err.Error()) - } - assert.Equal(t, *runtimeSpec.Linux.Resources.BlockIO.WeightDevice[0].Weight, uint16(100)) -} - -// TestMemorySwap verifies that the inputted memory swap is correctly defined in the spec -func TestMemorySwap(t *testing.T) { - // The default configuration of podman enables seccomp, which is not available on non-Linux systems. - // Thus, any tests that use the default seccomp setting would fail. - // Skip the tests on non-Linux platforms rather than explicitly disable seccomp in the test and possibly affect the test result. - if runtime.GOOS != "linux" { - t.Skip("seccomp, which is enabled by default, is only supported on Linux") - } - if !sysInfo.SwapLimit { - t.Skip("running test not supported by the host system") - } - args := []string{"--memory-swap", "45m", "--memory", "40m"} - a := createCLI(args) - a.InputArgs = args - //a.Run(append(cmd, args...)) - runtimeSpec, err := getRuntimeSpec(a) - if err != nil { - t.Fatalf(err.Error()) - } - mem, _ := units.RAMInBytes("45m") - assert.Equal(t, *runtimeSpec.Linux.Resources.Memory.Swap, mem) -} diff --git a/pkg/spec/containerconfig.go b/pkg/spec/containerconfig.go index f9bb075b8..b2f8a268f 100644 --- a/pkg/spec/containerconfig.go +++ b/pkg/spec/containerconfig.go @@ -24,8 +24,6 @@ func (config *CreateConfig) MakeContainerConfig(runtime *libpod.Runtime, pod *li return nil, nil, err } - logrus.Debugf("got mounts as %v", mounts) - runtimeSpec, err := config.createConfigToOCISpec(runtime, mounts) if err != nil { return nil, nil, err diff --git a/pkg/spec/spec_test.go b/pkg/spec/spec_test.go index b7c9c9c0d..0abff491b 100644 --- a/pkg/spec/spec_test.go +++ b/pkg/spec/spec_test.go @@ -1,39 +1,98 @@ package createconfig import ( - "reflect" + "runtime" "testing" - spec "github.com/opencontainers/runtime-spec/specs-go" + "github.com/containers/libpod/pkg/sysinfo" + "github.com/containers/storage" + "github.com/containers/storage/pkg/idtools" + "github.com/docker/go-units" "github.com/stretchr/testify/assert" ) -func TestGetVolumeMountsOneVolume(t *testing.T) { - data := spec.Mount{ - Destination: "/foobar", - Type: "bind", - Source: "/foobar", - Options: []string{"ro", "rbind", "rprivate"}, +var ( + sysInfo = sysinfo.New(true) +) + +// Make createconfig to test with +func makeTestCreateConfig() *CreateConfig { + cc := new(CreateConfig) + cc.Resources = CreateResourceConfig{} + cc.IDMappings = new(storage.IDMappingOptions) + cc.IDMappings.UIDMap = []idtools.IDMap{} + cc.IDMappings.GIDMap = []idtools.IDMap{} + + return cc +} + +// TestPIDsLimit verifies the given pid-limit is correctly defined in the spec +func TestPIDsLimit(t *testing.T) { + // The default configuration of podman enables seccomp, which is not available on non-Linux systems. + // Thus, any tests that use the default seccomp setting would fail. + // Skip the tests on non-Linux platforms rather than explicitly disable seccomp in the test and possibly affect the test result. + if runtime.GOOS != "linux" { + t.Skip("seccomp, which is enabled by default, is only supported on Linux") } - config := CreateConfig{ - Volumes: []string{"/foobar:/foobar:ro"}, + if !sysInfo.PidsLimit { + t.Skip("running test not supported by the host system") } - specMount, _, err := config.getVolumeMounts() + + cc := makeTestCreateConfig() + cc.Resources.PidsLimit = 22 + + spec, err := cc.createConfigToOCISpec(nil, nil) assert.NoError(t, err) - assert.True(t, reflect.DeepEqual(data, specMount[data.Destination])) + + assert.Equal(t, spec.Linux.Resources.Pids.Limit, int64(22)) } -func TestGetTmpfsMounts(t *testing.T) { - data := spec.Mount{ - Destination: "/homer", - Type: "tmpfs", - Source: "tmpfs", - Options: []string{"rw", "size=787448k", "mode=1777"}, +// TestBLKIOWeightDevice verifies the given blkio weight is correctly set in the +// spec. +func TestBLKIOWeightDevice(t *testing.T) { + // The default configuration of podman enables seccomp, which is not available on non-Linux systems. + // Thus, any tests that use the default seccomp setting would fail. + // Skip the tests on non-Linux platforms rather than explicitly disable seccomp in the test and possibly affect the test result. + if runtime.GOOS != "linux" { + t.Skip("seccomp, which is enabled by default, is only supported on Linux") } - config := CreateConfig{ - Tmpfs: []string{"/homer:rw,size=787448k,mode=1777"}, + if !sysInfo.BlkioWeightDevice { + t.Skip("running test not supported by the host system") } - tmpfsMount, err := config.getTmpfsMounts() + + cc := makeTestCreateConfig() + cc.Resources.BlkioWeightDevice = []string{"/dev/zero:100"} + + spec, err := cc.createConfigToOCISpec(nil, nil) assert.NoError(t, err) - assert.True(t, reflect.DeepEqual(data, tmpfsMount[data.Destination])) + + // /dev/zero is guaranteed 1,5 by the Linux kernel + assert.Equal(t, spec.Linux.Resources.BlockIO.WeightDevice[0].Major, int64(1)) + assert.Equal(t, spec.Linux.Resources.BlockIO.WeightDevice[0].Minor, int64(5)) + assert.Equal(t, *(spec.Linux.Resources.BlockIO.WeightDevice[0].Weight), uint16(100)) +} + +// TestMemorySwap verifies that the given swap memory limit is correctly set in +// the spec. +func TestMemorySwap(t *testing.T) { + // The default configuration of podman enables seccomp, which is not available on non-Linux systems. + // Thus, any tests that use the default seccomp setting would fail. + // Skip the tests on non-Linux platforms rather than explicitly disable seccomp in the test and possibly affect the test result. + if runtime.GOOS != "linux" { + t.Skip("seccomp, which is enabled by default, is only supported on Linux") + } + if !sysInfo.SwapLimit { + t.Skip("running test not supported by the host system") + } + + swapLimit, err := units.RAMInBytes("45m") + assert.NoError(t, err) + + cc := makeTestCreateConfig() + cc.Resources.MemorySwap = swapLimit + + spec, err := cc.createConfigToOCISpec(nil, nil) + assert.NoError(t, err) + + assert.Equal(t, *(spec.Linux.Resources.Memory.Swap), swapLimit) } diff --git a/pkg/spec/storage_test.go b/pkg/spec/storage_test.go new file mode 100644 index 000000000..4d96a2de6 --- /dev/null +++ b/pkg/spec/storage_test.go @@ -0,0 +1,39 @@ +package createconfig + +import ( + "reflect" + "testing" + + spec "github.com/opencontainers/runtime-spec/specs-go" + "github.com/stretchr/testify/assert" +) + +func TestGetVolumeMountsOneVolume(t *testing.T) { + data := spec.Mount{ + Destination: "/foobar", + Type: "bind", + Source: "/tmp", + Options: []string{"ro", "rbind", "rprivate"}, + } + config := CreateConfig{ + Volumes: []string{"/tmp:/foobar:ro"}, + } + specMount, _, err := config.getVolumeMounts() + assert.NoError(t, err) + assert.True(t, reflect.DeepEqual(data, specMount[data.Destination])) +} + +func TestGetTmpfsMounts(t *testing.T) { + data := spec.Mount{ + Destination: "/homer", + Type: "tmpfs", + Source: "tmpfs", + Options: []string{"rw", "size=787448k", "mode=1777"}, + } + config := CreateConfig{ + Tmpfs: []string{"/homer:rw,size=787448k,mode=1777"}, + } + tmpfsMount, err := config.getTmpfsMounts() + assert.NoError(t, err) + assert.True(t, reflect.DeepEqual(data, tmpfsMount[data.Destination])) +} -- cgit v1.2.3-54-g00ecf