From 51fbf3da9ee34a8143df5baeda6032c1747446d2 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 25 Apr 2022 15:15:52 +0200 Subject: enable gocritic linter The linter ensures a common code style. - use switch/case instead of else if - use if instead of switch/case for single case statement - add space between comment and text - detect the use of defer with os.Exit() - use short form var += "..." instead of var = var + "..." - detect problems with append() ``` newSlice := append(orgSlice, val) ``` This could lead to nasty bugs because the orgSlice will be changed in place if it has enough capacity too hold the new elements. Thus we newSlice might not be a copy. Of course most of the changes are just cosmetic and do not cause any logic errors but I think it is a good idea to enforce a common style. This should help maintainability. Signed-off-by: Paul Holzinger --- test/e2e/common_test.go | 4 ++-- test/e2e/container_clone_test.go | 2 +- test/e2e/generate_kube_test.go | 10 +++++----- test/e2e/images_test.go | 4 ++-- test/e2e/load_test.go | 2 +- test/e2e/namespace_test.go | 2 +- test/e2e/pod_initcontainers_test.go | 2 +- test/e2e/run_cgroup_parent_test.go | 6 +++--- test/e2e/run_signal_test.go | 4 ++-- test/e2e/run_volume_test.go | 4 ++-- test/utils/utils.go | 8 ++++---- 11 files changed, 24 insertions(+), 24 deletions(-) (limited to 'test') diff --git a/test/e2e/common_test.go b/test/e2e/common_test.go index 6846a5677..59252fcb0 100644 --- a/test/e2e/common_test.go +++ b/test/e2e/common_test.go @@ -212,7 +212,7 @@ func PodmanTestCreateUtil(tempDir string, remote bool) *PodmanTestIntegration { podmanRemoteBinary = os.Getenv("PODMAN_REMOTE_BINARY") } - conmonBinary := filepath.Join("/usr/libexec/podman/conmon") + conmonBinary := "/usr/libexec/podman/conmon" altConmonBinary := "/usr/bin/conmon" if _, err := os.Stat(conmonBinary); os.IsNotExist(err) { conmonBinary = altConmonBinary @@ -344,7 +344,7 @@ func imageTarPath(image string) string { } // e.g., registry.com/fubar:latest -> registry.com-fubar-latest.tar - imageCacheName := strings.Replace(strings.Replace(image, ":", "-", -1), "/", "-", -1) + ".tar" + imageCacheName := strings.ReplaceAll(strings.ReplaceAll(image, ":", "-"), "/", "-") + ".tar" return filepath.Join(cacheDir, imageCacheName) } diff --git a/test/e2e/container_clone_test.go b/test/e2e/container_clone_test.go index 1ff4b3b5f..42b477da0 100644 --- a/test/e2e/container_clone_test.go +++ b/test/e2e/container_clone_test.go @@ -184,7 +184,7 @@ var _ = Describe("Podman container clone", func() { Expect(checkCreate).Should(Exit(0)) createArray := checkCreate.OutputToStringArray() - Expect(cloneArray).To(ContainElements(createArray[:])) + Expect(cloneArray).To(ContainElements(createArray)) ctrInspect := podmanTest.Podman([]string{"inspect", clone.OutputToString()}) ctrInspect.WaitWithDefaultTimeout() diff --git a/test/e2e/generate_kube_test.go b/test/e2e/generate_kube_test.go index 9c99c3d93..1e9d725b7 100644 --- a/test/e2e/generate_kube_test.go +++ b/test/e2e/generate_kube_test.go @@ -75,7 +75,7 @@ var _ = Describe("Podman generate kube", func() { numContainers := 0 for range pod.Spec.Containers { - numContainers = numContainers + 1 + numContainers++ } Expect(numContainers).To(Equal(1)) }) @@ -169,7 +169,7 @@ var _ = Describe("Podman generate kube", func() { numContainers := 0 for range pod.Spec.Containers { - numContainers = numContainers + 1 + numContainers++ } Expect(numContainers).To(Equal(1)) }) @@ -478,11 +478,11 @@ var _ = Describe("Podman generate kube", func() { // for k8s Expect(port.Protocol).To(BeEmpty()) if port.HostPort == 4000 { - foundPort4000 = foundPort4000 + 1 + foundPort4000++ } else if port.HostPort == 5000 { - foundPort5000 = foundPort5000 + 1 + foundPort5000++ } else { - foundOtherPort = foundOtherPort + 1 + foundOtherPort++ } } } diff --git a/test/e2e/images_test.go b/test/e2e/images_test.go index d34c911ad..fc1c48c15 100644 --- a/test/e2e/images_test.go +++ b/test/e2e/images_test.go @@ -438,7 +438,7 @@ RUN > file2 Expect(result).Should(Exit(0)) Expect(result.OutputToStringArray()).To(HaveLen(1)) - //check if really abc is removed + // check if really abc is removed result = podmanTest.Podman([]string{"image", "list", "--filter", "label=abc"}) Expect(result.OutputToStringArray()).To(BeEmpty()) @@ -459,7 +459,7 @@ RUN > file2 Expect(result).Should(Exit(0)) Expect(result.OutputToStringArray()).To(HaveLen(1)) - //check if really abc is removed + // check if really abc is removed result = podmanTest.Podman([]string{"image", "list", "--filter", "label=abc"}) Expect(result.OutputToStringArray()).To(BeEmpty()) diff --git a/test/e2e/load_test.go b/test/e2e/load_test.go index 8e9f0ad13..1e3f9089a 100644 --- a/test/e2e/load_test.go +++ b/test/e2e/load_test.go @@ -64,7 +64,7 @@ var _ = Describe("Podman load", func() { compress := SystemExec("gzip", []string{outfile}) Expect(compress).Should(Exit(0)) - outfile = outfile + ".gz" + outfile += ".gz" rmi := podmanTest.Podman([]string{"rmi", ALPINE}) rmi.WaitWithDefaultTimeout() diff --git a/test/e2e/namespace_test.go b/test/e2e/namespace_test.go index c3e0e2d23..bc9db4cd9 100644 --- a/test/e2e/namespace_test.go +++ b/test/e2e/namespace_test.go @@ -51,7 +51,7 @@ var _ = Describe("Podman namespaces", func() { numCtrs := 0 for _, outputLine := range output { if outputLine != "" { - numCtrs = numCtrs + 1 + numCtrs++ } } Expect(numCtrs).To(Equal(0)) diff --git a/test/e2e/pod_initcontainers_test.go b/test/e2e/pod_initcontainers_test.go index a99e24908..cefaa3dc1 100644 --- a/test/e2e/pod_initcontainers_test.go +++ b/test/e2e/pod_initcontainers_test.go @@ -114,7 +114,7 @@ var _ = Describe("Podman init containers", func() { check := podmanTest.Podman([]string{"container", "exists", initContainerID}) check.WaitWithDefaultTimeout() // Container was rm'd - //Expect(check).Should(Exit(1)) + // Expect(check).Should(Exit(1)) Expect(check.ExitCode()).To(Equal(1), "I dont understand why the other way does not work") // Lets double check with a stop and start stopPod := podmanTest.Podman([]string{"pod", "stop", "foobar"}) diff --git a/test/e2e/run_cgroup_parent_test.go b/test/e2e/run_cgroup_parent_test.go index 0aba367f3..34715be22 100644 --- a/test/e2e/run_cgroup_parent_test.go +++ b/test/e2e/run_cgroup_parent_test.go @@ -84,12 +84,12 @@ var _ = Describe("Podman run with --cgroup-parent", func() { exec.WaitWithDefaultTimeout() Expect(exec).Should(Exit(0)) - containerCgroup := strings.TrimRight(strings.Replace(exec.OutputToString(), "0::", "", -1), "\n") + containerCgroup := strings.TrimRight(strings.ReplaceAll(exec.OutputToString(), "0::", ""), "\n") // Move the container process to a sub cgroup content, err := ioutil.ReadFile(filepath.Join(cgroupRoot, containerCgroup, "cgroup.procs")) Expect(err).To(BeNil()) - oldSubCgroupPath := filepath.Join(filepath.Join(cgroupRoot, containerCgroup, "old-container")) + oldSubCgroupPath := filepath.Join(cgroupRoot, containerCgroup, "old-container") err = os.MkdirAll(oldSubCgroupPath, 0755) Expect(err).To(BeNil()) err = ioutil.WriteFile(filepath.Join(oldSubCgroupPath, "cgroup.procs"), content, 0644) @@ -102,7 +102,7 @@ var _ = Describe("Podman run with --cgroup-parent", func() { run = podmanTest.Podman([]string{"--cgroup-manager=cgroupfs", "run", "--rm", "--cgroupns=host", fmt.Sprintf("--cgroup-parent=%s", newCgroup), fedoraMinimal, "cat", "/proc/self/cgroup"}) run.WaitWithDefaultTimeout() Expect(run).Should(Exit(0)) - cgroupEffective := strings.TrimRight(strings.Replace(run.OutputToString(), "0::", "", -1), "\n") + cgroupEffective := strings.TrimRight(strings.ReplaceAll(run.OutputToString(), "0::", ""), "\n") Expect(newCgroup).To(Equal(filepath.Dir(cgroupEffective))) }) diff --git a/test/e2e/run_signal_test.go b/test/e2e/run_signal_test.go index 71efa770a..d40a5a1b4 100644 --- a/test/e2e/run_signal_test.go +++ b/test/e2e/run_signal_test.go @@ -66,7 +66,7 @@ var _ = Describe("Podman run with --sig-proxy", func() { counter := 0 for { buf := make([]byte, 1024) - n, err := uds.Read(buf[:]) + n, err := uds.Read(buf) if err != nil && err != io.EOF { fmt.Println(err) return @@ -92,7 +92,7 @@ var _ = Describe("Podman run with --sig-proxy", func() { counter = 0 for { buf := make([]byte, 1024) - n, err := uds.Read(buf[:]) + n, err := uds.Read(buf) if err != nil { fmt.Println(err) return diff --git a/test/e2e/run_volume_test.go b/test/e2e/run_volume_test.go index 0be84e11b..a31c1959e 100644 --- a/test/e2e/run_volume_test.go +++ b/test/e2e/run_volume_test.go @@ -226,7 +226,7 @@ var _ = Describe("Podman run with volumes", func() { mountPath := filepath.Join(podmanTest.TempDir, "secrets") os.Mkdir(mountPath, 0755) - //Container should be able to start with custom overlay volume + // Container should be able to start with custom overlay volume session := podmanTest.Podman([]string{"run", "--rm", "-v", mountPath + ":/data:O", "--workdir=/data", ALPINE, "echo", "hello"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) @@ -729,7 +729,7 @@ VOLUME /test/`, ALPINE) Expect(session).Should(Exit(0)) Expect(session.OutputToString()).To(ContainSubstring("888:888")) - vol = vol + ",O" + vol += ",O" session = podmanTest.Podman([]string{"run", "--rm", "--user", "888:888", "--userns", "keep-id", "-v", vol, ALPINE, "stat", "-c", "%u:%g", dest}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) diff --git a/test/utils/utils.go b/test/utils/utils.go index 0867570c1..f3e14c784 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -108,8 +108,8 @@ func (p *PodmanTest) PodmanAsUserBase(args []string, uid, gid uint32, cwd string timeCmd := append([]string{"/usr/bin/time"}, timeArgs...) wrapper = append(timeCmd, wrapper...) } - - runCmd := append(wrapper, podmanBinary) + runCmd := wrapper + runCmd = append(runCmd, podmanBinary) if !p.RemoteTest && p.NetworkBackend == Netavark { runCmd = append(runCmd, []string{"--network-backend", "netavark"}...) } @@ -449,10 +449,10 @@ func GetHostDistributionInfo() HostOS { host.Arch = runtime.GOARCH for l.Scan() { if strings.HasPrefix(l.Text(), "ID=") { - host.Distribution = strings.Replace(strings.TrimSpace(strings.Join(strings.Split(l.Text(), "=")[1:], "")), "\"", "", -1) + host.Distribution = strings.ReplaceAll(strings.TrimSpace(strings.Join(strings.Split(l.Text(), "=")[1:], "")), "\"", "") } if strings.HasPrefix(l.Text(), "VERSION_ID=") { - host.Version = strings.Replace(strings.TrimSpace(strings.Join(strings.Split(l.Text(), "=")[1:], "")), "\"", "", -1) + host.Version = strings.ReplaceAll(strings.TrimSpace(strings.Join(strings.Split(l.Text(), "=")[1:], "")), "\"", "") } } return host -- cgit v1.2.3-54-g00ecf From 4f8ece76fff31d31570af56e0ec4a4092e015b33 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 26 Apr 2022 16:53:08 +0200 Subject: play kube: do not skip containers by name We should not exclude contianers by name. If a users has a container with the name "inf" it is currently skipped. This is wrong. The k8s yaml does not contain infra containers so we do not have to skip them. Signed-off-by: Paul Holzinger --- pkg/domain/infra/abi/play.go | 86 ++++++++++++++++++++++---------------------- test/e2e/play_kube_test.go | 2 +- 2 files changed, 43 insertions(+), 45 deletions(-) (limited to 'test') diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index f914a53d0..45500b23c 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -435,53 +435,51 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY initContainers = append(initContainers, ctr) } for _, container := range podYAML.Spec.Containers { - if !strings.Contains(container.Name, "infra") { - // Error out if the same name is used for more than one container - if _, ok := ctrNames[container.Name]; ok { - return nil, errors.Errorf("the pod %q is invalid; duplicate container name %q detected", podName, container.Name) - } - ctrNames[container.Name] = "" - pulledImage, labels, err := ic.getImageAndLabelInfo(ctx, cwd, annotations, writer, container, options) - if err != nil { - return nil, err - } + // Error out if the same name is used for more than one container + if _, ok := ctrNames[container.Name]; ok { + return nil, errors.Errorf("the pod %q is invalid; duplicate container name %q detected", podName, container.Name) + } + ctrNames[container.Name] = "" + pulledImage, labels, err := ic.getImageAndLabelInfo(ctx, cwd, annotations, writer, container, options) + if err != nil { + return nil, err + } - for k, v := range podSpec.PodSpecGen.Labels { // add podYAML labels - labels[k] = v - } + for k, v := range podSpec.PodSpecGen.Labels { // add podYAML labels + labels[k] = v + } - specgenOpts := kube.CtrSpecGenOptions{ - Annotations: annotations, - Container: container, - Image: pulledImage, - Volumes: volumes, - PodID: pod.ID(), - PodName: podName, - PodInfraID: podInfraID, - ConfigMaps: configMaps, - SeccompPaths: seccompPaths, - RestartPolicy: ctrRestartPolicy, - NetNSIsHost: p.NetNS.IsHost(), - SecretsManager: secretsManager, - LogDriver: options.LogDriver, - LogOptions: options.LogOptions, - Labels: labels, - } - specGen, err := kube.ToSpecGen(ctx, &specgenOpts) - if err != nil { - return nil, err - } - specGen.RawImageName = container.Image - rtSpec, spec, opts, err := generate.MakeContainer(ctx, ic.Libpod, specGen, false, nil) - if err != nil { - return nil, err - } - ctr, err := generate.ExecuteCreate(ctx, ic.Libpod, rtSpec, spec, false, opts...) - if err != nil { - return nil, err - } - containers = append(containers, ctr) + specgenOpts := kube.CtrSpecGenOptions{ + Annotations: annotations, + Container: container, + Image: pulledImage, + Volumes: volumes, + PodID: pod.ID(), + PodName: podName, + PodInfraID: podInfraID, + ConfigMaps: configMaps, + SeccompPaths: seccompPaths, + RestartPolicy: ctrRestartPolicy, + NetNSIsHost: p.NetNS.IsHost(), + SecretsManager: secretsManager, + LogDriver: options.LogDriver, + LogOptions: options.LogOptions, + Labels: labels, + } + specGen, err := kube.ToSpecGen(ctx, &specgenOpts) + if err != nil { + return nil, err + } + specGen.RawImageName = container.Image + rtSpec, spec, opts, err := generate.MakeContainer(ctx, ic.Libpod, specGen, false, nil) + if err != nil { + return nil, err + } + ctr, err := generate.ExecuteCreate(ctx, ic.Libpod, rtSpec, spec, false, opts...) + if err != nil { + return nil, err } + containers = append(containers, ctr) } if options.Start != types.OptionalBoolFalse { diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 4dd05c755..6d89a844e 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -2995,7 +2995,7 @@ invalid kube kind It("podman play kube with auto update annotations for all containers", func() { ctr01Name := "ctr01" - ctr02Name := "ctr02" + ctr02Name := "infra" podName := "foo" autoUpdateRegistry := "io.containers.autoupdate" autoUpdateRegistryValue := "registry" -- cgit v1.2.3-54-g00ecf