Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added offscreen cicd & local cache #353

Merged
merged 7 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ jobs:
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Create cache directory
run: |
mkdir -p ./.buildx-cache/cache
mkdir ./.buildx-cache/cache-new
mkdir ./.buildx-cache/cache/test
mkdir ./.buildx-cache/cache/latest
mkdir ./.buildx-cache/cache-new/test
mkdir ./.buildx-cache/cache-new/latest
Comment on lines +45 to +52
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling and cleanup to cache directory creation.

The directory creation script should handle potential failures and clean up existing directories to prevent issues with stale cache data.

 - name: Create cache directory
   run: |
+    set -e  # Exit on any error
+    rm -rf ./.buildx-cache  # Clean up existing cache
     mkdir -p ./.buildx-cache/cache
     mkdir ./.buildx-cache/cache-new
     mkdir ./.buildx-cache/cache/test
     mkdir ./.buildx-cache/cache/latest
     mkdir ./.buildx-cache/cache-new/test
     mkdir ./.buildx-cache/cache-new/latest
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Create cache directory
run: |
mkdir -p ./.buildx-cache/cache
mkdir ./.buildx-cache/cache-new
mkdir ./.buildx-cache/cache/test
mkdir ./.buildx-cache/cache/latest
mkdir ./.buildx-cache/cache-new/test
mkdir ./.buildx-cache/cache-new/latest
- name: Create cache directory
run: |
set -e # Exit on any error
rm -rf ./.buildx-cache # Clean up existing cache
mkdir -p ./.buildx-cache/cache
mkdir ./.buildx-cache/cache-new
mkdir ./.buildx-cache/cache/test
mkdir ./.buildx-cache/cache/latest
mkdir ./.buildx-cache/cache-new/test
mkdir ./.buildx-cache/cache-new/latest


- name: Test build
if: github.event_name == 'pull_request'
uses: docker/build-push-action@v3
Expand All @@ -50,8 +59,8 @@ jobs:
file: ./build/docker/agent/Dockerfile
load: true
tags: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME}}:test
cache-from: type=gha
cache-to: type=gha,mode=max
cache-from: type=local,src=./.buildx-cache/cache/test/
cache-to: type=local,dest=./.buildx-cache/cache-new/test/,mode=max
build-args: |
USERNAME=paf
USER_UID=1000
Expand All @@ -67,8 +76,8 @@ jobs:
push: true
# tag 'latest' and version on push to main, otherwise use the commit hash
tags: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:latest
cache-from: type=gha
cache-to: type=gha,mode=max
cache-from: type=local,src=./.buildx-cache/cache/latest/
cache-to: type=local,dest=./.buildx-cache/cache-new/latest/,mode=max
build-args: |
USERNAME=paf
USER_UID=1000
Expand All @@ -85,4 +94,10 @@ jobs:
uses: actions/upload-artifact@v4
with:
name: pr_id
path: pr/
path: pr/
Comment on lines +97 to +98
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add PR ID validation.

The PR ID saving process should validate the input to prevent empty or invalid values from being stored.

 - name: Save PR ID
   env:
     PR_ID: ${{ github.event.number }}
   run: |
+    if [ -z "$PR_ID" ]; then
+      echo "Error: PR_ID is empty"
+      exit 1
+    fi
     mkdir -p ./pr
     echo $PR_ID > ./pr/pr_id

Committable suggestion was skipped due to low confidence.

retention-days: 1

- name: Clean up cache
run: |
rm -rf ./.buildx-cache
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix cache cleanup paths and add error handling.

The cache cleanup step has several issues:

  1. The destination path .buildx-cache-new doesn't match the previously created directory structure
  2. Missing error handling
  3. Missing newline at end of file

Apply this fix:

 - name: Clean up cache
   run: |
+    set -e  # Exit on any error
     rm -rf ./.buildx-cache
-    mv ./.buildx-cache-new /tmp/.buildx-cache
+    mv ./.buildx-cache/cache-new /tmp/.buildx-cache
+

Committable suggestion was skipped due to low confidence.

🧰 Tools
🪛 yamllint

[error] 103-103: no new line character at the end of file

(new-line-at-end-of-file)

mv ./.buildx-cache-new /tmp/.buildx-cache
19 changes: 17 additions & 2 deletions .github/workflows/drive.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ jobs:
let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => {
return artifact.name == "pr_id"
})[0];
if (!matchArtifact) {
core.setFailed('No pr_id artifact found from the build workflow');
return;
}
let download = await github.rest.actions.downloadArtifact({
owner: context.repo.owner,
repo: context.repo.repo,
Expand All @@ -42,12 +46,23 @@ jobs:
fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/pr_id.zip`, Buffer.from(download.data));

- name: 'Unzip artifact (PR ID)'
run: unzip pr_id.zip
run: |
unzip pr_id.zip

- name: Check PR ID
uses: actions/github-script@v6
with:
script: |
let issue_number = fs.readFileSync('./pr_id');
if (!issue_number || isNaN(Number(issue_number))) {
core.setFailed(`Invalid PR ID: ${prIdContent}`);
return;
}

- name: Run docker-compose
run: |
xhost +local:
USERNAME=$(whoami) USER_UID=$(id -u) USER_GID=$(id -g) docker compose up --quiet-pull --exit-code-from agent
USERNAME=$(whoami) USER_UID=$(id -u) USER_GID=$(id -g) RENDER_OFFSCREEN=-RenderOffScreen docker compose up --quiet-pull --exit-code-from agent
- name: Copy results
run: docker compose cp agent:/tmp/simulation_results.json .
- name: Stop docker-compose
Expand Down
2 changes: 1 addition & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"docker.commands.composeUp": [
{
"label": "Compose Up",
"template": "xhost +local: && USERNAME=$(whoami) USER_UID=$(id -u) USER_GID=$(id -g) ${composeCommand} ${configurationFile} up"
"template": "xhost +local: && USERNAME=$(whoami) USER_UID=$(id -u) USER_GID=$(id -g) RENDER_OFFSCREEN= ${composeCommand} ${configurationFile} up"
ll7 marked this conversation as resolved.
Show resolved Hide resolved
}
],
"workbench.iconTheme": "vscode-icons",
Expand Down
2 changes: 1 addition & 1 deletion build/docker-compose.carla-simulator.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
services:
# based on https://github.com/ll7/paf21-1/blob/master/scenarios/docker-carla-sim-compose.yml
carla-simulator:
command: /bin/bash CarlaUE4.sh -quality-level=Epic -world-port=2000 -resx=800 -resy=600 -nosound -carla-settings="/home/carla/CarlaUE4/Config/CustomCarlaSettings.ini"
command: /bin/bash CarlaUE4.sh -quality-level=Epic -world-port=2000 -resx=800 -resy=600 -nosound -carla-settings="/home/carla/CarlaUE4/Config/CustomCarlaSettings.ini" ${RENDER_OFFSCREEN}
image: ghcr.io/una-auxme/paf23:leaderboard-2.0
init: true
deploy:
Expand Down
Loading