Skip to content

gha: use sudo -E in some places to prevent dropping env-vars#4741

Merged
estesp merged 1 commit intocontainerd:masterfrom
thaJeztah:kudo_sudo
Apr 19, 2021
Merged

gha: use sudo -E in some places to prevent dropping env-vars#4741
estesp merged 1 commit intocontainerd:masterfrom
thaJeztah:kudo_sudo

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Nov 17, 2020 •
edited
Loading

Using -E preserves environment variables, except for PATH, so PATH has to be manually set to match the current environment.

I removed env-vars that were redundant (such as GOPATH=$GOPATH), which should be handled by -E.

@k8s-ci-robot
Copy link
Copy Markdown

k8s-ci-robot commented Nov 17, 2020

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Nov 17, 2020

Build succeeded.

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Nov 17, 2020

Hmm.. interesting

CONTAINERD_RUNTIME=$TEST_RUNTIME make cri-integration
  shell: /bin/bash -e {0}
  env:
    GOROOT: /opt/hostedtoolcache/go/1.15.5/x64
    GOPATH: /home/runner/work/containerd/containerd
    TEST_RUNTIME: io.containerd.runc.v2
+ bin/ctr
+ bin/containerd
+ bin/containerd-stress
+ bin/containerd-shim
+ bin/containerd-shim-runc-v1
+ bin/containerd-shim-runc-v2
+ binaries
+ bin/cri-integration.test
go test github.com/containerd/containerd/integration.test: copying /tmp/go-build334883615/b001/integration.test: open /home/runner/work/containerd/containerd/src/github.com/containerd/containerd/bin/cri-integration.test: permission denied
Error: Makefile:187: recipe for target 'bin/cri-integration.test' failed
make: *** [bin/cri-integration.test] Error 1
Error: Process completed with exit code 2.

estesp
estesp reviewed Nov 23, 2020
View reviewed changes
Comment thread .github/workflows/ci.yml Outdated
Copy link
Copy Markdown
Member

@estesp estesp Nov 23, 2020

Choose a reason for hiding this comment

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

make binaries doesn't need to be run as root/via sudo

@thaJeztah thaJeztah marked this pull request as ready for review April 19, 2021 11:02
@thaJeztah thaJeztah mentioned this pull request Apr 19, 2021
Closed
@thaJeztah
gha: use sudo -E in some places to prevent dropping env-vars
c1d1edb 
Using `-E` preserves environment variables, except for PATH, so PATH has to be
manually set to match the current environment.

I removed env-vars that were redundant (such as `GOPATH=$GOPATH`), which should
be handled by `-E`.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 19, 2021

Build succeeded.

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Apr 19, 2021

Updated; rebased, and removed the sudo for make binaries.

Looks like we may currently be running some tests on Go 1.15 currently; see #5362 (comment)

Hopefully this fixes that

@estesp @mxpv @fuweid ptal

estesp
estesp approved these changes Apr 19, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

dims
dims approved these changes Apr 19, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@dims dims left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 56512cc into containerd:master Apr 19, 2021
@thaJeztah thaJeztah deleted the kudo_sudo branch April 19, 2021 15:15
@thaJeztah thaJeztah mentioned this pull request Apr 19, 2021
Merged
@hakman hakman mentioned this pull request Apr 27, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dims dims dims approved these changes

@estesp estesp estesp approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@thaJeztah @k8s-ci-robot @dims @estesp