fix(ci) fixing the file permission issue for CI runner #8311
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
fyi @joshua-goldstein @dshekhar95 we should have fixed it this way. We should try to avoid adding too many cleanups to the CI steps (as they should mimic & control out test flow, NOT infrastructure cleaning or setup). I have put the correct fix towards the runner infrastructure. We can discuss next week - but pls do look to understand these changes. |
skrdgraph
commented
Sep 17, 2022
Comment on lines
-12
to
-19
| #TODO: WE NEED TO REMOVE THIS CLEANUP, and handle it more elegantly, it makes us incompatible with gh-hosted-runners | ||
| cleanup: | ||
| runs-on: self-hosted | ||
| container: | ||
| image: ubuntu:latest | ||
| steps: | ||
| - name: Cleaning up the $GITHUB_WORKSPACE as root from a Docker image | ||
| run: find /__w/${{ github.event.repository.name }}/${{ github.event.repository.name }}/. -name . -o -prune -exec rm -rf -- {} + || true |
There was a problem hiding this comment.
@joshua-goldstein & @dshekhar95 this won't be needed anymore because of the fix in the infrastructure layer
skrdgraph
commented
Sep 17, 2022
Comment on lines
-12
to
-19
| #TODO: WE NEED TO REMOVE THIS CLEANUP, and handle it more elegantly, it makes us incompatible with gh-hosted-runners | ||
| cleanup: | ||
| runs-on: self-hosted | ||
| container: | ||
| image: ubuntu:latest | ||
| steps: | ||
| - name: Cleaning up the $GITHUB_WORKSPACE as root from a Docker image | ||
| run: find /__w/${{ github.event.repository.name }}/${{ github.event.repository.name }}/. -name . -o -prune -exec rm -rf -- {} + || true |
There was a problem hiding this comment.
@joshua-goldstein & @dshekhar95 this won't be needed anymore because of the fix in the infrastructure layer
skrdgraph
commented
Sep 17, 2022
| @@ -26,6 +26,7 @@ curl -o actions-runner-linux-x64-2.296.2.tar.gz -L https://github.com/actions/ru | |||
| echo "34a8f34956cdacd2156d4c658cce8dd54c5aef316a16bbbc95eb3ca4fd76429a actions-runner-linux-x64-2.296.2.tar.gz" | shasum -a 256 -c | |||
| tar xzf ./actions-runner-linux-x64-2.296.2.tar.gz | |||
| ./config.sh --url https://github.com/dgraph-io/dgraph --token $TOKEN | |||
| sudo chown -R $USER:$USER /home/ubuntu/actions-runner/_work # fixes the test cruft cleanup issue | |||
There was a problem hiding this comment.
@joshua-goldstein @dshekhar95 this is the correct way to fix it at the infrastructure layer.
skrdgraph
requested review from
darkn3rd,
akon-dey,
gcxml and
meghalims
as code owners
skrdgraph
changed the title
dshekhar95
pushed a commit
that referenced
this pull request
Sep 19, 2022
* undo changes to dgraph tests * undo changes to load tests * fixing the runner user, to handle cruft cleanup issue * fix test cruft issue * fix test cleanup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
The previous fixes were introducing a cleanup step in our CI, which is not ideal & it is not consistent either - because any files created from within the container cannot be cleaned up by other steps because of root user issues. Docker runs as root.
Solution
The correct fix is to have the runner run as root, which is the ideal way of dealing with this. There are security concerns here, but I have looked further to ensure we are not impacted. This change is made in this PR. The earlier PRs #8307 #8301 & issues actions/checkout#211 Gamify-IT/issues#60 talk about the fixes.