From a35da3ad87b5a962e20961efbc57d85427df682c Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 19 Jun 2023 14:04:01 +0200 Subject: [PATCH 1/8] bump golangci-lint to v1.53.3 Disable new linters and drop comments on them. Signed-off-by: Valentin Rothberg --- .golangci.yml | 11 +++++++++++ Makefile | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 56b551ca31..237b523e66 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -13,6 +13,17 @@ linters: enable-all: true disable: # All these break for one reason or another + - rowserrcheck # FIXME (urgent): sqlite error checking + # too many reports but requires attention + - depguard + - revive + - tagalign + # useful hints that should be addressed + - ginkgolinter + - nakedret + - mirror + - wastedassign + - gosmopolitan # usage of time.Local in pkg/k8s.io - tagliatelle # too many JSON keys cannot be changed due to compat - nosnakecase # too many false positives due to the `unix` package - dupword # too many false positives (e.g., in tests) diff --git a/Makefile b/Makefile index c454c0ca66..53f0365dc3 100644 --- a/Makefile +++ b/Makefile @@ -941,7 +941,7 @@ install.tools: .install.golangci-lint ## Install needed tools .PHONY: .install.golangci-lint .install.golangci-lint: - VERSION=1.51.1 ./hack/install_golangci.sh + VERSION=1.53.3 ./hack/install_golangci.sh .PHONY: .install.swagger .install.swagger: From 60a5a59475027da374669238f215a2459828d268 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 19 Jun 2023 14:08:04 +0200 Subject: [PATCH 2/8] make lint: enable mirror Helpful reports to avoid unnecessary allocations. Signed-off-by: Valentin Rothberg --- .golangci.yml | 1 - cmd/podman/system/connection/add.go | 4 ++-- cmd/quadlet/main.go | 2 +- libpod/oci_conmon_common.go | 2 +- pkg/rootless/rootless_linux.go | 2 +- test/e2e/play_kube_test.go | 4 ++-- utils/utils_supported.go | 2 +- 7 files changed, 8 insertions(+), 9 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 237b523e66..f72b12739b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -21,7 +21,6 @@ linters: # useful hints that should be addressed - ginkgolinter - nakedret - - mirror - wastedassign - gosmopolitan # usage of time.Local in pkg/k8s.io - tagliatelle # too many JSON keys cannot be changed due to compat diff --git a/cmd/podman/system/connection/add.go b/cmd/podman/system/connection/add.go index 7f89c4ac63..2e89cff56d 100644 --- a/cmd/podman/system/connection/add.go +++ b/cmd/podman/system/connection/add.go @@ -106,7 +106,7 @@ func add(cmd *cobra.Command, args []string) error { Default: cOpts.Default, } dest := args[1] - if match, err := regexp.Match("^[A-Za-z][A-Za-z0-9+.-]*://", []byte(dest)); err != nil { + if match, err := regexp.MatchString("^[A-Za-z][A-Za-z0-9+.-]*://", dest); err != nil { return fmt.Errorf("invalid destination: %w", err) } else if !match { dest = "ssh://" + dest @@ -203,7 +203,7 @@ func create(cmd *cobra.Command, args []string) error { if err != nil { return err } - if match, err := regexp.Match("^[A-Za-z][A-Za-z0-9+.-]*://", []byte(dest)); err != nil { + if match, err := regexp.MatchString("^[A-Za-z][A-Za-z0-9+.-]*://", dest); err != nil { return fmt.Errorf("invalid destination: %w", err) } else if !match { dest = "ssh://" + dest diff --git a/cmd/quadlet/main.go b/cmd/quadlet/main.go index 21544b15a4..7549886eae 100644 --- a/cmd/quadlet/main.go +++ b/cmd/quadlet/main.go @@ -64,7 +64,7 @@ func logToKmsg(s string) bool { kmsgFile = f } - if _, err := kmsgFile.Write([]byte(s)); err != nil { + if _, err := kmsgFile.WriteString(s); err != nil { kmsgFile.Close() kmsgFile = nil return false diff --git a/libpod/oci_conmon_common.go b/libpod/oci_conmon_common.go index 2393ff9204..38fd07bdea 100644 --- a/libpod/oci_conmon_common.go +++ b/libpod/oci_conmon_common.go @@ -343,7 +343,7 @@ func generateResourceFile(res *spec.LinuxResources) (string, []string, error) { if err != nil { return "", nil, err } - _, err = f.WriteString(string(j)) + _, err = f.Write(j) if err != nil { return "", nil, err } diff --git a/pkg/rootless/rootless_linux.go b/pkg/rootless/rootless_linux.go index 323daec723..76d7b982a5 100644 --- a/pkg/rootless/rootless_linux.go +++ b/pkg/rootless/rootless_linux.go @@ -374,7 +374,7 @@ func becomeRootInUserNS(pausePid, fileToRead string, fileOutput *os.File) (_ boo } } - _, err = w.Write([]byte("0")) + _, err = w.WriteString("0") if err != nil { return false, -1, fmt.Errorf("write to sync pipe: %w", err) } diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 7d4077a2fc..4b9c2f7faa 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -1739,7 +1739,7 @@ func createSourceTarFile(fileName, fileContent, tarFilePath string) error { return err } - _, err = file.Write([]byte(fileContent)) + _, err = file.WriteString(fileContent) if err != nil { return err } @@ -4996,7 +4996,7 @@ spec: file, err := os.Create(filepath.Join(hostPathLocation, "testing", "onlythis", "123.txt")) Expect(err).ToNot(HaveOccurred()) - _, err = file.Write([]byte("hi")) + _, err = file.WriteString("hi") Expect(err).ToNot(HaveOccurred()) err = file.Close() diff --git a/utils/utils_supported.go b/utils/utils_supported.go index 418c1f871a..ecd0c4c024 100644 --- a/utils/utils_supported.go +++ b/utils/utils_supported.go @@ -175,7 +175,7 @@ func moveUnderCgroup(cgroup, subtree string, processes []uint32) error { if len(processes) > 0 { for _, pid := range processes { - if _, err := f.Write([]byte(fmt.Sprintf("%d\n", pid))); err != nil { + if _, err := f.WriteString(fmt.Sprintf("%d\n", pid)); err != nil { logrus.Debugf("Cannot move process %d to cgroup %q: %v", pid, newCgroup, err) } } From f07aa1bfdc558b8fc2739858ba89da245a21f9ac Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 19 Jun 2023 14:14:48 +0200 Subject: [PATCH 3/8] make lint: enable wastedassign Because we shouldn't waste assigns. Signed-off-by: Valentin Rothberg --- .golangci.yml | 1 - libpod/container_internal_common.go | 1 - libpod/container_internal_linux.go | 2 +- libpod/oci_conmon_common.go | 2 +- 4 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index f72b12739b..f414aeb026 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -21,7 +21,6 @@ linters: # useful hints that should be addressed - ginkgolinter - nakedret - - wastedassign - gosmopolitan # usage of time.Local in pkg/k8s.io - tagliatelle # too many JSON keys cannot be changed due to compat - nosnakecase # too many false positives due to the `unix` package diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index d7cb65e43b..401a3986fc 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -2463,7 +2463,6 @@ func (c *Container) setHomeEnvIfNeeded() error { } // Ensure HOME is not already set in Env - home := "" for _, s := range c.config.Spec.Process.Env { if strings.HasPrefix(s, "HOME=") { return nil diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 0a4f99a24e..22efb94f76 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -750,7 +750,7 @@ func (c *Container) safeMountSubPath(mountPoint, subpath string) (s *safeMountIn if err != nil { return nil, err } - npath := "" + var npath string switch { case fi.Mode()&fs.ModeSymlink != 0: return nil, fmt.Errorf("file %q is a symlink", joinedPath) diff --git a/libpod/oci_conmon_common.go b/libpod/oci_conmon_common.go index 38fd07bdea..89a025b8f9 100644 --- a/libpod/oci_conmon_common.go +++ b/libpod/oci_conmon_common.go @@ -1488,7 +1488,7 @@ func readConmonPipeData(runtimeName string, pipe *os.File, ociLog string) (int, ch <- syncStruct{si: si} }() - data := -1 + var data int select { case ss := <-ch: if ss.err != nil { From 2efa7c3fa142606d8c15850da3245be51ec52e21 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 19 Jun 2023 14:31:40 +0200 Subject: [PATCH 4/8] make lint: enable rowserrcheck It turns out, after iterating over rows, we need to check for errors. It also turns out that we did not do that at all. Signed-off-by: Valentin Rothberg --- .golangci.yml | 2 -- libpod/sqlite_state.go | 54 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index f414aeb026..13f87bdeb3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -12,8 +12,6 @@ run: linters: enable-all: true disable: - # All these break for one reason or another - - rowserrcheck # FIXME (urgent): sqlite error checking # too many reports but requires attention - depguard - revive diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index fd207acb36..de30988adf 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -139,6 +139,9 @@ func (s *SQLiteState) Refresh() (defErr error) { ctrStates[id] = string(newJSON) } + if err := ctrRows.Err(); err != nil { + return err + } podRows, err := s.conn.Query("SELECT ID, JSON FROM PodState;") if err != nil { @@ -170,6 +173,9 @@ func (s *SQLiteState) Refresh() (defErr error) { podStates[id] = string(newJSON) } + if err := podRows.Err(); err != nil { + return err + } volRows, err := s.conn.Query("SELECT Name, JSON FROM VolumeState;") if err != nil { @@ -202,6 +208,9 @@ func (s *SQLiteState) Refresh() (defErr error) { volumeStates[name] = string(newJSON) } + if err := volRows.Err(); err != nil { + return err + } // Write updated states back to DB, and perform additional maintenance // (Remove exit codes and exec sessions) @@ -503,6 +512,9 @@ func (s *SQLiteState) LookupContainerID(idOrName string) (string, error) { } resCount++ } + if err := rows.Err(); err != nil { + return "", err + } if resCount == 0 { return "", define.ErrNoSuchCtr } else if resCount > 1 { @@ -544,6 +556,9 @@ func (s *SQLiteState) LookupContainer(idOrName string) (*Container, error) { } resCount++ } + if err := rows.Err(); err != nil { + return nil, err + } if !exactName { if resCount == 0 { return nil, fmt.Errorf("no container with name or ID %q found: %w", idOrName, define.ErrNoSuchCtr) @@ -730,6 +745,9 @@ func (s *SQLiteState) ContainerInUse(ctr *Container) ([]string, error) { } deps = append(deps, dep) } + if err := rows.Err(); err != nil { + return nil, err + } return deps, nil } @@ -770,6 +788,9 @@ func (s *SQLiteState) AllContainers(loadState bool) ([]*Container, error) { ctrs = append(ctrs, ctr) } + if err := rows.Err(); err != nil { + return nil, err + } } else { rows, err := s.conn.Query("SELECT JSON FROM ContainerConfig;") if err != nil { @@ -794,6 +815,9 @@ func (s *SQLiteState) AllContainers(loadState bool) ([]*Container, error) { ctrs = append(ctrs, ctr) } + if err := rows.Err(); err != nil { + return nil, err + } } for _, ctr := range ctrs { @@ -1095,6 +1119,9 @@ func (s *SQLiteState) GetContainerExecSessions(ctr *Container) ([]string, error) } sessions = append(sessions, session) } + if err := rows.Err(); err != nil { + return nil, err + } return sessions, nil } @@ -1332,6 +1359,9 @@ func (s *SQLiteState) LookupPod(idOrName string) (*Pod, error) { } resCount++ } + if err := rows.Err(); err != nil { + return nil, err + } if !exactName { if resCount == 0 { return nil, fmt.Errorf("no pod with name or ID %s found: %w", idOrName, define.ErrNoSuchPod) @@ -1421,6 +1451,9 @@ func (s *SQLiteState) PodContainersByID(pod *Pod) ([]string, error) { ids = append(ids, id) } + if err := rows.Err(); err != nil { + return nil, err + } return ids, nil } @@ -1459,6 +1492,9 @@ func (s *SQLiteState) PodContainers(pod *Pod) ([]*Container, error) { ctrs = append(ctrs, ctr) } + if err := rows.Err(); err != nil { + return nil, err + } for _, ctr := range ctrs { if err := finalizeCtrSqlite(ctr); err != nil { @@ -1652,6 +1688,9 @@ func (s *SQLiteState) RemovePodContainers(pod *Pod) (defErr error) { return err } } + if err := rows.Err(); err != nil { + return err + } return nil } @@ -1804,6 +1843,9 @@ func (s *SQLiteState) AllPods() ([]*Pod, error) { pods = append(pods, pod) } + if err := rows.Err(); err != nil { + return nil, err + } return pods, nil } @@ -1910,6 +1952,9 @@ func (s *SQLiteState) RemoveVolume(volume *Volume) (defErr error) { } ctrs = append(ctrs, ctr) } + if err := rows.Err(); err != nil { + return err + } if len(ctrs) > 0 { return fmt.Errorf("volume %s is in use by containers %s: %w", volume.Name(), strings.Join(ctrs, ","), define.ErrVolumeBeingUsed) } @@ -2045,6 +2090,9 @@ func (s *SQLiteState) AllVolumes() ([]*Volume, error) { volumes = append(volumes, vol) } + if err := rows.Err(); err != nil { + return nil, err + } return volumes, nil } @@ -2113,6 +2161,9 @@ func (s *SQLiteState) LookupVolume(name string) (*Volume, error) { break } } + if err := rows.Err(); err != nil { + return nil, err + } if foundName == "" { return nil, fmt.Errorf("no volume with name %q found: %w", name, define.ErrNoSuchVolume) } @@ -2186,6 +2237,9 @@ func (s *SQLiteState) VolumeInUse(volume *Volume) ([]string, error) { } ctrs = append(ctrs, ctr) } + if err := rows.Err(); err != nil { + return nil, err + } return ctrs, nil } From aa453c4f113c77dd6b3a91d4badc7b2550df7350 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 19 Jun 2023 15:09:34 +0200 Subject: [PATCH 5/8] make lint: re-enable ginkgolinter To make sure the e2e tests are kept in order. Signed-off-by: Valentin Rothberg --- .golangci.yml | 1 - pkg/machine/e2e/inspect_test.go | 4 +--- pkg/machine/e2e/list_test.go | 4 ++-- test/e2e/create_test.go | 2 +- test/e2e/generate_kube_test.go | 2 +- test/e2e/images_test.go | 2 +- test/e2e/inspect_test.go | 4 ++-- test/e2e/network_test.go | 4 ++-- test/e2e/pause_test.go | 4 ++-- test/e2e/play_kube_test.go | 4 ++-- test/e2e/pod_create_test.go | 4 ++-- test/e2e/prune_test.go | 2 +- test/e2e/restart_test.go | 2 +- test/e2e/rm_test.go | 2 +- test/e2e/start_test.go | 8 ++++---- test/e2e/stop_test.go | 2 +- 16 files changed, 24 insertions(+), 27 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 13f87bdeb3..571fe0f246 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -17,7 +17,6 @@ linters: - revive - tagalign # useful hints that should be addressed - - ginkgolinter - nakedret - gosmopolitan # usage of time.Local in pkg/k8s.io - tagliatelle # too many JSON keys cannot be changed due to compat diff --git a/pkg/machine/e2e/inspect_test.go b/pkg/machine/e2e/inspect_test.go index a23d0950b1..308ec8e609 100644 --- a/pkg/machine/e2e/inspect_test.go +++ b/pkg/machine/e2e/inspect_test.go @@ -1,8 +1,6 @@ package e2e_test import ( - "strings" - "github.com/containers/podman/v4/pkg/machine" jsoniter "github.com/json-iterator/go" @@ -67,7 +65,7 @@ var _ = Describe("podman machine stop", func() { var inspectInfo []machine.InspectInfo err = jsoniter.Unmarshal(inspectSession.Bytes(), &inspectInfo) Expect(err).ToNot(HaveOccurred()) - Expect(strings.HasSuffix(inspectInfo[0].ConnectionInfo.PodmanSocket.GetPath(), "podman.sock")) + Expect(inspectInfo[0].ConnectionInfo.PodmanSocket.GetPath()).To(HaveSuffix("podman.sock")) inspect := new(inspectMachine) inspect = inspect.withFormat("{{.Name}}") diff --git a/pkg/machine/e2e/list_test.go b/pkg/machine/e2e/list_test.go index fae5a61cf0..4343fd3556 100644 --- a/pkg/machine/e2e/list_test.go +++ b/pkg/machine/e2e/list_test.go @@ -52,12 +52,12 @@ var _ = Describe("podman machine list", func() { firstList, err := mb.setCmd(list.withQuiet()).run() Expect(err).NotTo(HaveOccurred()) Expect(firstList).Should(Exit(0)) - Expect(firstList.outputToStringSlice()).To(HaveLen(0)) // No header with quiet + Expect(firstList.outputToStringSlice()).To(BeEmpty()) // No header with quiet noheaderSession, err := mb.setCmd(list.withNoHeading()).run() // noheader Expect(err).NotTo(HaveOccurred()) Expect(noheaderSession).Should(Exit(0)) - Expect(noheaderSession.outputToStringSlice()).To(HaveLen(0)) + Expect(noheaderSession.outputToStringSlice()).To(BeEmpty()) i := new(initMachine) session, err := mb.setName(name1).setCmd(i.withImagePath(mb.imagePath)).run() diff --git a/test/e2e/create_test.go b/test/e2e/create_test.go index 0af1e34216..3a9df1956e 100644 --- a/test/e2e/create_test.go +++ b/test/e2e/create_test.go @@ -315,7 +315,7 @@ var _ = Describe("Podman create", func() { It("podman create --authfile with nonexistent authfile", func() { session := podmanTest.Podman([]string{"create", "--authfile", "/tmp/nonexistent", "--name=foo", ALPINE}) session.WaitWithDefaultTimeout() - Expect(session).To(Not(Equal(0))) + Expect(session).ToNot(HaveValue(Equal(0))) }) It("podman create --signature-policy", func() { diff --git a/test/e2e/generate_kube_test.go b/test/e2e/generate_kube_test.go index d18c025ccc..decb547594 100644 --- a/test/e2e/generate_kube_test.go +++ b/test/e2e/generate_kube_test.go @@ -182,7 +182,7 @@ var _ = Describe("Podman kube generate", func() { err := yaml.Unmarshal(kube.Out.Contents(), pod) Expect(err).ToNot(HaveOccurred()) Expect(pod.Spec).To(HaveField("HostNetwork", false)) - Expect(pod.Annotations).To(HaveLen(0)) + Expect(pod.Annotations).To(BeEmpty()) numContainers := 0 for range pod.Spec.Containers { diff --git a/test/e2e/images_test.go b/test/e2e/images_test.go index 23e15defb9..7beca4c3e5 100644 --- a/test/e2e/images_test.go +++ b/test/e2e/images_test.go @@ -192,7 +192,7 @@ WORKDIR /test result := podmanTest.Podman([]string{"images", "-q", "-f", "dangling=true"}) result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0), "dangling image output: %q", result.OutputToString()) - Expect(result.OutputToStringArray()).Should(HaveLen(0), "dangling image output: %q", result.OutputToString()) + Expect(result.OutputToStringArray()).Should(BeEmpty(), "dangling image output: %q", result.OutputToString()) }) It("podman pull by digest and list --all", func() { diff --git a/test/e2e/inspect_test.go b/test/e2e/inspect_test.go index e37765da45..1f00eabc07 100644 --- a/test/e2e/inspect_test.go +++ b/test/e2e/inspect_test.go @@ -571,7 +571,7 @@ var _ = Describe("Podman inspect", func() { session = podmanTest.Podman([]string{"container", "inspect", cid, "-f", "{{ .State.Error }}"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) - Expect(session.OutputToString()).To(HaveLen(0)) + Expect(session.OutputToString()).To(BeEmpty()) session = podmanTest.Podman([]string{"start", cid}) session.WaitWithDefaultTimeout() @@ -579,7 +579,7 @@ var _ = Describe("Podman inspect", func() { session = podmanTest.Podman([]string{"container", "inspect", cid, "-f", "'{{ .State.Error }}"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) - Expect(session.OutputToString()).To(Not(HaveLen(0))) + Expect(session.OutputToString()).ToNot(BeEmpty()) }) }) diff --git a/test/e2e/network_test.go b/test/e2e/network_test.go index 2f207468ce..6fccf22950 100644 --- a/test/e2e/network_test.go +++ b/test/e2e/network_test.go @@ -569,7 +569,7 @@ var _ = Describe("Podman network", func() { Expect(result).To(HaveField("Driver", "macvlan")) Expect(result).To(HaveField("NetworkInterface", "lo")) Expect(result.IPAMOptions).To(HaveKeyWithValue("driver", "dhcp")) - Expect(result.Subnets).To(HaveLen(0)) + Expect(result.Subnets).To(BeEmpty()) nc = podmanTest.Podman([]string{"network", "rm", net}) nc.WaitWithDefaultTimeout() @@ -599,7 +599,7 @@ var _ = Describe("Podman network", func() { Expect(result).To(HaveField("Driver", "ipvlan")) Expect(result).To(HaveField("NetworkInterface", "lo")) Expect(result.IPAMOptions).To(HaveKeyWithValue("driver", "dhcp")) - Expect(result.Subnets).To(HaveLen(0)) + Expect(result.Subnets).To(BeEmpty()) nc = podmanTest.Podman([]string{"network", "rm", net}) nc.WaitWithDefaultTimeout() diff --git a/test/e2e/pause_test.go b/test/e2e/pause_test.go index b02d2c1e25..bcd31f347a 100644 --- a/test/e2e/pause_test.go +++ b/test/e2e/pause_test.go @@ -464,7 +464,7 @@ var _ = Describe("Podman pause", func() { session1 = podmanTest.Podman([]string{"pause", "-a", "--filter", fmt.Sprintf("id=%swrongid", shortCid3)}) session1.WaitWithDefaultTimeout() Expect(session1).Should(Exit(0)) - Expect(session1.OutputToString()).To(HaveLen(0)) + Expect(session1.OutputToString()).To(BeEmpty()) session1 = podmanTest.Podman([]string{"pause", "-a", "--filter", fmt.Sprintf("id=%s", shortCid3)}) session1.WaitWithDefaultTimeout() @@ -474,7 +474,7 @@ var _ = Describe("Podman pause", func() { session1 = podmanTest.Podman([]string{"unpause", "-a", "--filter", fmt.Sprintf("id=%swrongid", shortCid3)}) session1.WaitWithDefaultTimeout() Expect(session1).Should(Exit(0)) - Expect(session1.OutputToString()).To(HaveLen(0)) + Expect(session1.OutputToString()).To(BeEmpty()) session1 = podmanTest.Podman([]string{"unpause", "-a", "--filter", fmt.Sprintf("id=%s", shortCid3)}) session1.WaitWithDefaultTimeout() diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 4b9c2f7faa..4da73e9420 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -3665,7 +3665,7 @@ o: {{ .Options.o }}`}) kube := podmanTest.Podman([]string{"play", "kube", kubeYaml}) kube.WaitWithDefaultTimeout() if IsRemote() { - Expect(kube).Error() + Expect(kube).To(ExitWithError()) Expect(kube.ErrorToString()).To(ContainSubstring("importing volumes is not supported for remote requests")) return } @@ -4947,7 +4947,7 @@ spec: ps := podmanTest.Podman([]string{"pod", "ps", "-q"}) ps.WaitWithDefaultTimeout() Expect(ps).Should(Exit(0)) - Expect(ps.OutputToStringArray()).To(HaveLen(0)) + Expect(ps.OutputToStringArray()).To(BeEmpty()) }) It("podman play kube with named volume subpaths", func() { diff --git a/test/e2e/pod_create_test.go b/test/e2e/pod_create_test.go index e725eea1ca..40b4fc4dc5 100644 --- a/test/e2e/pod_create_test.go +++ b/test/e2e/pod_create_test.go @@ -1058,9 +1058,9 @@ ENTRYPOINT ["sleep","99999"] data := inspectPod.InspectPodToJSON() inspect := podmanTest.InspectContainer(ctrCreate.OutputToString()) - Expect(data.CgroupPath).To(HaveLen(0)) + Expect(data.CgroupPath).To(BeEmpty()) if podmanTest.CgroupManager == "cgroupfs" || !isRootless() { - Expect(inspect[0].HostConfig.CgroupParent).To(HaveLen(0)) + Expect(inspect[0].HostConfig.CgroupParent).To(BeEmpty()) } else if podmanTest.CgroupManager == "systemd" { Expect(inspect[0].HostConfig).To(HaveField("CgroupParent", "user.slice")) } diff --git a/test/e2e/prune_test.go b/test/e2e/prune_test.go index 913383fc80..6e7ec4a96c 100644 --- a/test/e2e/prune_test.go +++ b/test/e2e/prune_test.go @@ -261,7 +261,7 @@ var _ = Describe("Podman prune", func() { session = podmanTest.Podman([]string{"network", "ls", "-q", "--filter", "name=^test$"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) - Expect(session.OutputToStringArray()).To(HaveLen(0)) + Expect(session.OutputToStringArray()).To(BeEmpty()) // Create new network. session = podmanTest.Podman([]string{"network", "create", "test1", "--label", "foo"}) diff --git a/test/e2e/restart_test.go b/test/e2e/restart_test.go index 7c31f84fb8..3986476705 100644 --- a/test/e2e/restart_test.go +++ b/test/e2e/restart_test.go @@ -324,7 +324,7 @@ var _ = Describe("Podman restart", func() { session1 = podmanTest.Podman([]string{"restart", "-a", "--filter", fmt.Sprintf("id=%swrongid", shortCid3)}) session1.WaitWithDefaultTimeout() Expect(session1).Should(Exit(0)) - Expect(session1.OutputToString()).To(HaveLen(0)) + Expect(session1.OutputToString()).To(BeEmpty()) session1 = podmanTest.Podman([]string{"restart", "-a", "--filter", fmt.Sprintf("id=%s", shortCid3)}) session1.WaitWithDefaultTimeout() diff --git a/test/e2e/rm_test.go b/test/e2e/rm_test.go index 77c669d918..b4b5b17ef9 100644 --- a/test/e2e/rm_test.go +++ b/test/e2e/rm_test.go @@ -305,7 +305,7 @@ var _ = Describe("Podman rm", func() { session1 = podmanTest.Podman([]string{"rm", "-a", "-f", "--filter", fmt.Sprintf("id=%swrongid", shortCid3)}) session1.WaitWithDefaultTimeout() Expect(session1).Should(Exit(0)) - Expect(session1.OutputToString()).To(HaveLen(0)) + Expect(session1.OutputToString()).To(BeEmpty()) session1 = podmanTest.Podman([]string{"rm", "-a", "-f", "--filter", fmt.Sprintf("id=%s", shortCid3)}) session1.WaitWithDefaultTimeout() diff --git a/test/e2e/start_test.go b/test/e2e/start_test.go index 90df80656e..fe1b7ce963 100644 --- a/test/e2e/start_test.go +++ b/test/e2e/start_test.go @@ -145,7 +145,7 @@ var _ = Describe("Podman start", func() { wait.WaitWithDefaultTimeout() Expect(wait).To(ExitWithError()) - Eventually(podmanTest.NumberOfContainers(), defaultWaitTimeout, 3.0).Should(BeZero()) + Eventually(podmanTest.NumberOfContainers, defaultWaitTimeout, 3.0).Should(BeZero()) }) It("podman failed to start without --rm should NOT delete the container", func() { @@ -157,7 +157,7 @@ var _ = Describe("Podman start", func() { start.WaitWithDefaultTimeout() Expect(start).To(ExitWithError()) - Eventually(podmanTest.NumberOfContainers(), defaultWaitTimeout, 3.0).Should(Equal(1)) + Eventually(podmanTest.NumberOfContainers, defaultWaitTimeout, 3.0).Should(Equal(1)) }) It("podman start --sig-proxy should not work without --attach", func() { @@ -216,12 +216,12 @@ var _ = Describe("Podman start", func() { session1 = podmanTest.Podman([]string{"start", cid1, "-f", "status=running"}) session1.WaitWithDefaultTimeout() Expect(session1).Should(Exit(0)) - Expect(session1.OutputToString()).To(HaveLen(0)) + Expect(session1.OutputToString()).To(BeEmpty()) session1 = podmanTest.Podman([]string{"start", "--all", "--filter", fmt.Sprintf("id=%swrongid", shortCid3)}) session1.WaitWithDefaultTimeout() Expect(session1).Should(Exit(0)) - Expect(session1.OutputToString()).To(HaveLen(0)) + Expect(session1.OutputToString()).To(BeEmpty()) session1 = podmanTest.Podman([]string{"start", "--all", "--filter", fmt.Sprintf("id=%s", shortCid3)}) session1.WaitWithDefaultTimeout() diff --git a/test/e2e/stop_test.go b/test/e2e/stop_test.go index c707444072..38e38b9da0 100644 --- a/test/e2e/stop_test.go +++ b/test/e2e/stop_test.go @@ -398,7 +398,7 @@ var _ = Describe("Podman stop", func() { session1 = podmanTest.Podman([]string{"stop", "-a", "--filter", fmt.Sprintf("id=%swrongid", shortCid3)}) session1.WaitWithDefaultTimeout() Expect(session1).Should(Exit(0)) - Expect(session1.OutputToString()).To(HaveLen(0)) + Expect(session1.OutputToString()).To(BeEmpty()) session1 = podmanTest.Podman([]string{"stop", "-a", "--filter", fmt.Sprintf("id=%s", shortCid3)}) session1.WaitWithDefaultTimeout() From 84e42877ae52df2955eb6dd781104fb5eea1e057 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 19 Jun 2023 15:20:57 +0200 Subject: [PATCH 6/8] make lint: re-enable revive But disable the `unused-parameter` linter as there are just too many reports that I could handle. Also allow unused nolintlint reports. Signed-off-by: Valentin Rothberg --- .golangci.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 571fe0f246..5c34421231 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -14,7 +14,6 @@ linters: disable: # too many reports but requires attention - depguard - - revive - tagalign # useful hints that should be addressed - nakedret @@ -79,7 +78,12 @@ linters-settings: ignore: fmt:.* nolintlint: allow-leading-space: false + allow-unused: true require-specific: true + revive: + rules: + - name: unused-parameter + disabled: true issues: # Maximum issues count per one linter. From 574e00d32469701501b2850eeab11c7c827165e8 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 20 Jun 2023 08:54:09 +0200 Subject: [PATCH 7/8] e2e --authfile test: fix test condition Which revealed that absent --authfile's are ignored but shouldn't. The issue is now being tracked in #18938. Signed-off-by: Valentin Rothberg --- test/e2e/create_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/e2e/create_test.go b/test/e2e/create_test.go index 3a9df1956e..10ac0f1be5 100644 --- a/test/e2e/create_test.go +++ b/test/e2e/create_test.go @@ -313,9 +313,10 @@ var _ = Describe("Podman create", func() { }) It("podman create --authfile with nonexistent authfile", func() { + // FIXME (#18938): this test should fail but does not! session := podmanTest.Podman([]string{"create", "--authfile", "/tmp/nonexistent", "--name=foo", ALPINE}) session.WaitWithDefaultTimeout() - Expect(session).ToNot(HaveValue(Equal(0))) + Expect(session).Should(Exit(0)) }) It("podman create --signature-policy", func() { From ddcefc9b9f9f5b62d55bd643c2d95d9accb8a7ed Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 20 Jun 2023 08:58:48 +0200 Subject: [PATCH 8/8] e2e: kube test: specify expected exit code Let's make sure to always specify the expected exit codes, even in case of failure. Signed-off-by: Valentin Rothberg --- test/e2e/play_kube_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 4da73e9420..990ee0f342 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -3665,7 +3665,7 @@ o: {{ .Options.o }}`}) kube := podmanTest.Podman([]string{"play", "kube", kubeYaml}) kube.WaitWithDefaultTimeout() if IsRemote() { - Expect(kube).To(ExitWithError()) + Expect(kube).Should(Exit(125)) Expect(kube.ErrorToString()).To(ContainSubstring("importing volumes is not supported for remote requests")) return }