Skip to content

pkg: switch to golang native error wrapping#14839

Merged
openshift-ci[bot] merged 1 commit intocontainers:mainfrom
saschagrunert:errors-pkg
Jul 8, 2022
Merged

pkg: switch to golang native error wrapping#14839
openshift-ci[bot] merged 1 commit intocontainers:mainfrom
saschagrunert:errors-pkg

Conversation

@saschagrunert
Copy link
Copy Markdown
Member

We now use the golang error wrapping format specifier %w instead of the deprecated github.com/pkg/errors package.

Fixes #14784

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added release-note-none approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 6, 2022
@saschagrunert saschagrunert force-pushed the errors-pkg branch 2 times, most recently from 5303eee to e73a4e5 Compare July 6, 2022 07:56
go.mod Outdated
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥳

Copy link
Copy Markdown
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the mini nits, LGTM

Nice work, Sascha! Thanks a lot

@saschagrunert
Copy link
Copy Markdown
Member Author

Hm, the CI agent seems to not respond, I'll retrigger the tests for now.

Copy link
Copy Markdown
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 6, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 6, 2022
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Jul 6, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [giuseppe,saschagrunert]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 6, 2022
@saschagrunert
Copy link
Copy Markdown
Member Author

Need a rebase, I'll wait for the CI result to be green until doing that.

@mheon
Copy link
Copy Markdown
Member

mheon commented Jul 6, 2022

LGTM. You have a few integration failures, and they all look like the same test - probably a real regression

@TomSweeneyRedHat
Copy link
Copy Markdown
Member

LGTM
once the last tests are cleaned up

@saschagrunert saschagrunert changed the title pkg: switch to golang native error wrapping WIP: pkg: switch to golang native error wrapping Jul 7, 2022
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 7, 2022
@saschagrunert
Copy link
Copy Markdown
Member Author

Cannot reproduce the failure locally, I have to bisect the changes unfortunately.

Failing test

[+2529s] Podman pod create 
           podman pod create --uts test
           /var/tmp/go/src/github.com[/containers/podman/test/e2e/pod_create_test.go:1140](https://github.com/containers/podman/blob/41c8a00f3ceb982da0514e4252caa1330f90a3b9/test/e2e/pod_create_test.go#L1140)
         
         [BeforeEach] Podman pod create
           /var/tmp/go/src/github.com[/containers/podman/test/e2e/pod_create_test.go:32](https://github.com/containers/podman/blob/41c8a00f3ceb982da0514e4252caa1330f90a3b9/test/e2e/pod_create_test.go#L32)
         # podman [options] system service --time 0 unix:/run/podman/podman-1cbf35f4661cf1e08df17c94eb078d280b32b540cb33669a73b0ac6120d65382.sock
         # podman-remote [options] info
         host:
           arch: amd64
           buildahVersion: 1.27.0-dev
           cgroupControllers:
           - cpuset
           - cpu
           - io
           - memory
           - hugetlb
           - pids
           - misc
           cgroupManager: systemd
           cgroupVersion: v2
           conmon:
             package: conmon-2.1.0-2.fc36.x86_64
             path: /usr/bin/conmon
             version: 'conmon version 2.1.0, commit: '
           cpuUtilization:
             idlePercent: 4.37
             systemPercent: 32.3
             userPercent: 63.33
           cpus: 2
           distribution:
             distribution: fedora
             variant: cloud
             version: "36"
           eventLogger: journald
           hostname: cirrus-task-4956476354592768
           idMappings:
             gidmap: null
             uidmap: null
           kernel: 5.17.4-300.fc36.x86_64
           linkmode: dynamic
           logDriver: journald
           memFree: 288309248
           memTotal: 4109578240
           networkBackend: netavark
           ociRuntime:
             name: crun
             package: crun-1.4.4-1.fc36.x86_64
             path: /usr/bin/crun
             version: |-
               crun version 1.4.4
               commit: 6521fcc5806f20f6187eb933f9f45130c86da230
               spec: 1.0.0
               +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +CRIU +YAJL
           os: linux
           remoteSocket:
             exists: true
             path: unix:///run/podman/podman-1cbf35f4661cf1e08df17c94eb078d280b32b540cb33669a73b0ac6120d65382.sock
           security:
             apparmorEnabled: false
             capabilities: CAP_CHOWN,CAP_DAC_OVERRIDE,CAP_FOWNER,CAP_FSETID,CAP_KILL,CAP_NET_BIND_SERVICE,CAP_SETFCAP,CAP_SETGID,CAP_SETPCAP,CAP_SETUID,CAP_SYS_CHROOT
             rootless: false
             seccompEnabled: true
             seccompProfilePath: /usr/share/containers/seccomp.json
             selinuxEnabled: true
           serviceIsRemote: true
           slirp4netns:
             executable: /usr/bin/slirp4netns
             package: slirp4netns-1.2.0-0.2.beta.0.fc36.x86_64
             version: |-
               slirp4netns version 1.2.0-beta.0
               commit: 477db14a24ff1a3de3a705e51ca2c4c1fe3dda64
               libslirp: 4.6.1
               SLIRP_CONFIG_VERSION_MAX: 3
               libseccomp: 2.5.3
           swapFree: 4094160896
           swapTotal: 4109365248
           uptime: 0h 44m 40.00s
         plugins:
           authorization: null
           log:
           - k8s-file
           - none
           - passthrough
           - journald
           network:
           - bridge
           - macvlan
           volume:
           - local
         registries:
           docker.io:
             Blocked: false
             Insecure: false
             Location: mirror.gcr.io
             MirrorByDigestOnly: false
             Mirrors: null
             Prefix: docker.io
             PullFromMirror: ""
           docker.io/library:
             Blocked: false
             Insecure: false
             Location: quay.io/libpod
             MirrorByDigestOnly: false
             Mirrors: null
             Prefix: docker.io/library
             PullFromMirror: ""
           localhost:5000:
             Blocked: false
             Insecure: true
             Location: localhost:5000
             MirrorByDigestOnly: false
             Mirrors: null
             Prefix: localhost:5000
             PullFromMirror: ""
           search:
           - docker.io
           - quay.io
           - registry.fedoraproject.org
         store:
           configFile: /usr/share/containers/storage.conf
           containerStore:
             number: 0
             paused: 0
             running: 0
             stopped: 0
           graphDriverName: vfs
           graphOptions:
             vfs.imagestore: /tmp/podman/imagecachedir
           graphRoot: /tmp/podman_test2548615178/root
           graphRootAllocated: 213588619264
           graphRootUsed: 4088438784
           graphStatus: {}
           imageCopyTmpDir: /var/tmp
           imageStore:
             number: 12
           runRoot: /tmp/podman_test2548615178/runroot
           volumePath: /tmp/podman_test2548615178/root/volumes
         version:
           APIVersion: 4.2.0-dev
           Built: 1657120641
           BuiltTime: Wed Jul  6 10:17:21 2022
           GitCommit: 41c8a00f3ceb982da0514e4252caa1330f90a3b9
           GoVersion: go1.18
           Os: linux
           OsArch: linux/amd64
           Version: 4.2.0-dev
         
         [It] podman pod create --uts test
           /var/tmp/go/src/github.com[/containers/podman/test/e2e/pod_create_test.go:1140](https://github.com/containers/podman/blob/41c8a00f3ceb982da0514e4252caa1330f90a3b9/test/e2e/pod_create_test.go#L1140)
         # podman-remote [options] pod create --uts host
         ba0a2f786fd11c0b80178aef449b57febf8459a8787502b46d67dd448521fd48
         # podman-remote [options] run -it --pod ba0a2f786fd11c0b80178aef449b57febf8459a8787502b46d67dd448521fd48 quay.io/libpod/alpine:latest printenv HOSTNAME
         time="2022-07-06T12:36:42-05:00" level=warning msg="The input device is not a TTY. The --tty and --interactive flags might not work properly"
         gallant_mahavira
         # podman-remote [options] pod create --uts ns:/proc/self/ns/ --name utsPod --share uts
         ce6b7bc7e77f13267b439132172d34bfd5b72cec41df82715f26788d4ae98387
         # podman-remote [options] pod inspect utsPod
         {
              "Id": "ce6b7bc7e77f13267b439132172d34bfd5b72cec41df82715f26788d4ae98387",
              "Name": "utsPod",
              "Created": "2022-07-06T12:36:43.5213503-05:00",
              "CreateCommand": [
                   "/var/tmp/go/src/github.com/containers/podman/bin/podman-remote",
                   "--remote",
                   "--url",
                   "unix:/run/podman/podman-1cbf35f4661cf1e08df17c94eb078d280b32b540cb33669a73b0ac6120d65382.sock",
                   "pod",
                   "create",
                   "--uts",
                   "ns:/proc/self/ns/",
                   "--name",
                   "utsPod",
                   "--share",
                   "uts"
              ],
              "ExitPolicy": "continue",
              "State": "Created",
              "Hostname": "",
              "CreateCgroup": true,
              "CgroupParent": "machine.slice",
              "CgroupPath": "machine.slice/machine-libpod_pod_ce6b7bc7e77f13267b439132172d34bfd5b72cec41df82715f26788d4ae98387.slice",
              "CreateInfra": true,
              "InfraContainerID": "3dd9b328389593c5372739fa829a70884bf150b79038d9f6694b6e6cd81d4598",
              "InfraConfig": {
                   "PortBindings": {
                        
                   },
                   "HostNetwork": false,
                   "StaticIP": "",
                   "StaticMAC": "",
                   "NoManageResolvConf": false,
                   "DNSServer": null,
                   "DNSSearch": null,
                   "DNSOption": null,
                   "NoManageHosts": false,
                   "HostAdd": null,
                   "Networks": [
                        "podman"
                   ],
                   "NetworkOptions": null,
                   "pid_ns": "private",
                   "userns": "host",
                   "uts_ns": "private"
              },
              "SharedNamespaces": [
                   "uts"
              ],
              "NumContainers": 1,
              "Containers": [
                   {
                        "Id": "3dd9b328389593c5372739fa829a70884bf150b79038d9f6694b6e6cd81d4598",
                        "Name": "ce6b7bc7e77f-infra",
                        "State": "created"
                   }
              ]
         }
         [AfterEach] Podman pod create
           /var/tmp/go/src/github.com[/containers/podman/test/e2e/pod_create_test.go:41](https://github.com/containers/podman/blob/41c8a00f3ceb982da0514e4252caa1330f90a3b9/test/e2e/pod_create_test.go#L41)
         # podman-remote [options] pod rm -fa -t 0
         ba0a2f786fd11c0b80178aef449b57febf8459a8787502b46d67dd448521fd48
         ce6b7bc7e77f13267b439132172d34bfd5b72cec41df82715f26788d4ae98387
         # podman-remote [options] rm -fa -t 0
         
         

         • Failure [4.553 seconds]
         Podman pod create
         /var/tmp/go/src/github.com[/containers/podman/test/e2e/pod_create_test.go:24](https://github.com/containers/podman/blob/41c8a00f3ceb982da0514e4252caa1330f90a3b9/test/e2e/pod_create_test.go#L24)
           podman pod create --uts test [It]
           /var/tmp/go/src/github.com[/containers/podman/test/e2e/pod_create_test.go:1140](https://github.com/containers/podman/blob/41c8a00f3ceb982da0514e4252caa1330f90a3b9/test/e2e/pod_create_test.go#L1140)
         
           Value for field 'UtsNS' failed to satisfy matcher.
           Expected
               <string>: private
           to equal
               <string>: ns:/proc/self/ns/
         
           /var/tmp/go/src/github.com[/containers/podman/test/e2e/pod_create_test.go:1164](https://github.com/containers/podman/blob/41c8a00f3ceb982da0514e4252caa1330f90a3b9/test/e2e/pod_create_test.go#L1164)
         
           Full Stack Trace
           github.com/containers/podman/v4/test/e2e.glob..func49.58()
           	/var/tmp/go/src/github.com[/containers/podman/test/e2e/pod_create_test.go:1164](https://github.com/containers/podman/blob/41c8a00f3ceb982da0514e4252caa1330f90a3b9/test/e2e/pod_create_test.go#L1164) +0x691
           github.com/containers/podman/v4/test/e2e.TestLibpod(0x40c2f9?)
           	/var/tmp/go/src/github.com[/containers/podman/test/e2e/common_test.go:102](https://github.com/containers/podman/blob/41c8a00f3ceb982da0514e4252caa1330f90a3b9/test/e2e/common_test.go#L102) +0xbc
           testing.tRunner(0xc000104820, 0x140eef8)
           	/usr/lib/golang/src/testing/testing.go:1439 +0x102
           created by testing.(*T).Run
           	/usr/lib/golang/src/testing/testing.go:1486 +0x35f

@vrothberg
Copy link
Copy Markdown
Member

@cdoern, can you take a look at the flake?

@cdoern
Copy link
Copy Markdown
Contributor

cdoern commented Jul 7, 2022

@vrothberg I found the issue and will be issuing a patch soon for the flake. This also will fix #14847

@saschagrunert saschagrunert changed the title WIP: pkg: switch to golang native error wrapping pkg: switch to golang native error wrapping Jul 7, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 7, 2022
We now use the golang error wrapping format specifier `%w` instead of
the deprecated github.com/pkg/errors package.

[NO NEW TESTS NEEDED]

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
@saschagrunert
Copy link
Copy Markdown
Member Author

@containers/podman-maintainers CI will probably be green since we passed the remote integration tests now. May you give this another look?

@vrothberg
Copy link
Copy Markdown
Member

Restarted the flake.

Copy link
Copy Markdown
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/hold

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 8, 2022
@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Jul 8, 2022

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 8, 2022
@openshift-ci openshift-ci bot merged commit 6087fb2 into containers:main Jul 8, 2022
@saschagrunert saschagrunert deleted the errors-pkg branch July 8, 2022 09:27
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note-none

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace github.com/pkg/errors dependency with native error wrapping

7 participants