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

Changed printing of simulation results #397

Merged
merged 8 commits into from
Nov 5, 2024
73 changes: 48 additions & 25 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ env:
jobs:
build-and-push-image:
name: Build and push image
runs-on: self-hosted
runs-on: [self-hosted, build]
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Missing documentation for self-hosted runner configuration

The build label is only used in the build.yml workflow file and there is no documentation explaining the runner setup, required labels, or infrastructure configuration. This poses a maintainability risk as other developers won't know how to set up runners with the correct labels.

  • Add documentation about self-hosted runner setup and required labels
  • Consider creating an actionlint.yaml to suppress warnings for custom labels
  • Document the purpose of the build label and any specific requirements for runners using this label
🔗 Analysis chain

Document custom runner label configuration.

The build label appears to be a custom label for self-hosted runners. While this is a valid practice, it should be properly documented.

Run this script to check for runner setup documentation:

Consider:

  1. Adding documentation about the required runner labels
  2. Creating an actionlint.yaml to configure custom labels and suppress warnings

Let me gather more information about the runner configuration by checking other workflow files and documentation.


Let me check if there are any infrastructure-related files or documentation in other common locations that might contain runner setup information.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for runner setup documentation
fd -t f "README|CONTRIBUTING|SETUP|INSTALL" -X rg -i "runner|build.*label"

Length of output: 175


Script:

#!/bin/bash
# Check all workflow files for runner labels and configurations
fd -e yml -e yaml . .github/workflows -X rg -i "runs-on.*build"

# Look for any runner setup or infrastructure documentation
fd -t f -e md . .github/docs .github -X rg -i "self.hosted.*runner|runner.*setup|runner.*label"

# Check if there's an actionlint configuration
fd "actionlint" -X cat {}

Length of output: 312


Script:

#!/bin/bash
# Check for infrastructure or deployment related files
fd -t f "infrastructure|deploy|runner|setup" -X rg -i "self.hosted.*runner|runner.*setup|runner.*label"

# Check root level documentation
fd -t f -d 1 "README|CONTRIBUTING|SETUP|INSTALL" -X rg -i "github.*action|runner|workflow"

# Check if build label is used in any shell scripts or configuration files
fd -e sh -e conf -e yaml -e yml -X rg -i "build.*label|runner.*label"

Length of output: 270

🧰 Tools
🪛 actionlint

18-18: label "build" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)

outputs:
version: ${{ steps.build.version.outputs.version }}
permissions:
Expand All @@ -35,22 +35,25 @@ jobs:
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v2

- name: Cache Docker layers
uses: actions/cache@v4
with:
path: |
/tmp/.buildx-cache/cache/test
/tmp/.buildx-cache/cache/latest
/tmp/.buildx-cache/cache-new/test
/tmp/.buildx-cache/cache-new/latest
key: ${{ runner.os }}-buildx-${{ github.sha }}
restore-keys: |
${{ runner.os }}-buildx-

- name: Log in to the Container registry
uses: docker/login-action@v2
with:
registry: ${{ env.REGISTRY }}
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

- name: Test build
if: github.event_name == 'pull_request'
uses: docker/build-push-action@v3
Expand All @@ -59,8 +62,8 @@ jobs:
file: ./build/docker/agent/Dockerfile
load: true
tags: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME}}:test
cache-from: type=local,src=./.buildx-cache/cache/test/
cache-to: type=local,dest=./.buildx-cache/cache-new/test/,mode=max
cache-from: type=local,src=/tmp/.buildx-cache/cache/test/
cache-to: type=local,dest=/tmp/.buildx-cache/cache-new/test/,mode=max
build-args: |
USERNAME=paf
USER_UID=1000
Expand All @@ -76,31 +79,51 @@ 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=local,src=./.buildx-cache/cache/latest/
cache-to: type=local,dest=./.buildx-cache/cache-new/latest/,mode=max
cache-from: type=local,src=/tmp/.buildx-cache/cache/latest/
cache-to: type=local,dest=/tmp/.buildx-cache/cache-new/latest/,mode=max
build-args: |
USERNAME=paf
USER_UID=1000
USER_GID=1000

- name: Save PR ID
- name: Save pull request artifact
if: github.event_name == 'pull_request'
env:
PR_ID: ${{ github.event.number }}
run: |
mkdir -p ./pr
echo $PR_ID > ./pr/pr_id
mkdir -p ./artifact
printf '{
"is_pr": true,
"pr_id": $PR_ID
}' >> ./artifact/artifact.json
Comment on lines +89 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.

⚠️ Potential issue

Fix shell script to properly expand environment variable.

The logic is correct, but there's a potential issue with variable expansion due to single quotes.

Apply this fix to ensure proper variable expansion:

       - name: Save pull request artifact
         if: github.event_name == 'pull_request'
         env:
           PR_ID: ${{ github.event.number }}
         run: |
           mkdir -p ./artifact
-          printf '{
-            "is_pr": true,
-            "pr_id": $PR_ID
-          }' >> ./artifact/artifact.json
+          cat << EOF > ./artifact/artifact.json
+          {
+            "is_pr": true,
+            "pr_id": $PR_ID
+          }
+          EOF
📝 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: Save pull request artifact
if: github.event_name == 'pull_request'
env:
PR_ID: ${{ github.event.number }}
run: |
mkdir -p ./pr
echo $PR_ID > ./pr/pr_id
mkdir -p ./artifact
printf '{
"is_pr": true,
"pr_id": $PR_ID
}' >> ./artifact/artifact.json
- name: Save pull request artifact
if: github.event_name == 'pull_request'
env:
PR_ID: ${{ github.event.number }}
run: |
mkdir -p ./artifact
cat << EOF > ./artifact/artifact.json
{
"is_pr": true,
"pr_id": $PR_ID
}
EOF
🧰 Tools
🪛 actionlint

99-99: shellcheck reported issue in this script: SC2016:info:2:8: Expressions don't expand in single quotes, use double quotes for that

(shellcheck)


- name: Upload PR ID
if: github.event_name == 'pull_request'
- name: Save merge artifact
if: github.event_name != 'pull_request'
run: |
mkdir -p ./artifact
printf '{
"is_pr": false,
"pr_id": -1
}' >> ./artifact/artifact.json

- name: Upload artifact
uses: actions/upload-artifact@v4
with:
name: pr_id
path: pr/
name: artifact.json
path: ./artifact/
retention-days: 1

- name: Clean up cache
- name: Clean up PR cache
if: github.event_name == 'pull_request'
run: |
rm -rf /tmp/.buildx-cache/cache/test
mv /tmp/.buildx-cache/cache-new/test /tmp/.buildx-cache/cache/test

- name: Clean up merge cache
if: github.event_name != 'pull_request'
run: |
rm -rf ./.buildx-cache/cache/test
rm -rf ./.buildx-cache/cache/latest
mv ./.buildx-cache/cache-new ./.buildx-cache/cache
rm -rf /tmp/.buildx-cache/cache/latest
mv /tmp/.buildx-cache/cache-new/latest /tmp/.buildx-cache/cache/latest

- name: Prune all images older than 1 days from self-hosted runner
run: docker image prune -a -f --filter "until=24h"
55 changes: 31 additions & 24 deletions .github/workflows/drive.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ on:

jobs:
drive:
runs-on: self-hosted
runs-on: [self-hosted, drive]
env:
AGENT_VERSION: latest
COMPOSE_FILE: ./build/docker-compose.cicd.yaml
Expand All @@ -20,8 +20,7 @@ jobs:
run: |
echo "AGENT_VERSION=${AGENT_VERSION}"
echo "COMPOSE_FILE=${COMPOSE_FILE}"
- name: 'Download artifact (PR ID)'
if: github.event_name == 'pull_request'
- name: Download artifact
uses: actions/github-script@v6
with:
script: |
Expand All @@ -31,10 +30,10 @@ jobs:
run_id: context.payload.workflow_run.id,
});
let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => {
return artifact.name == "pr_id"
return artifact.name == "artifact.json"
})[0];
if (!matchArtifact) {
core.setFailed('No pr_id artifact found from the build workflow');
core.setFailed('No artifact found from the build workflow');
return;
}
let download = await github.rest.actions.downloadArtifact({
Expand All @@ -44,23 +43,20 @@ jobs:
archive_format: 'zip',
});
let fs = require('fs');
fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/pr_id.zip`, Buffer.from(download.data));
fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/artifact.zip`, Buffer.from(download.data));

- name: 'Unzip artifact (PR ID)'
if: github.event_name == 'pull_request'
- name: Unzip artifact
run: |
unzip pr_id.zip
unzip artifact.zip

- name: Check PR ID
if: github.event_name == 'pull_request'
- name: Return artifact JSON
id: return-artifact-json
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;
}
let fs = require('fs');
let data = JSON.parse(fs.readFileSync('./artifacts/artifacts.json'));
return data;

- name: Run docker-compose
run: |
Expand All @@ -73,12 +69,10 @@ jobs:
if: always()
run: docker compose down -v
# add rendered JSON as comment to the pull request
- name: Add simulation results as comment
if: github.event_name == 'pull_request'
- name: Create simulation results table
id: simulation-results
uses: actions/github-script@v6
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
# this script reads the simulation_results.json and creates a comment on the pull request with the results.
script: |
const fs = require('fs');
// read the simulation results
Expand All @@ -94,13 +88,26 @@ jobs:
let resultsTableDivider = `| --- | --- |`;
// add everything to the resultsTable
resultsTable = resultsTableHeader + '\n' + resultsTableDivider + '\n' + resultsTable.join('\n');
return resultsTable;
result-encoding: string
- name: Print simulation results
run: |
echo "Simulation results\n"
echo "${{ steps.simulation-results.outputs.result }}"
Comment on lines +94 to +96
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 echo command to properly handle escape sequences.

The current echo command may not properly expand the newline character.

Update the command to use printf:

-  echo "Simulation results\n"
-  echo "${{ steps.simulation-results.outputs.result }}"
+  printf "Simulation results\n\n%s\n" "${{ steps.simulation-results.outputs.result }}"
📝 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
run: |
echo "Simulation results\n"
echo "${{ steps.simulation-results.outputs.result }}"
run: |
printf "Simulation results\n\n%s\n" "${{ steps.simulation-results.outputs.result }}"
🧰 Tools
🪛 actionlint

94-94: shellcheck reported issue in this script: SC2028:info:1:6: echo may not expand escape sequences. Use printf

(shellcheck)

- name: Add simulation results as comment
if: ${{ steps.return-artifact-json.outputs.result.is_pr }}
uses: actions/github-script@v6
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
# this script reads the simulation_results.json and creates a comment on the pull request with the results.
script: |
let issue_number = Number(${{ steps.return-artifact-json.outputs.pr_id }});
// add the results as a comment to the pull request
let issue_number = Number(fs.readFileSync('./pr_id'));
github.rest.issues.createComment({
issue_number: issue_number,
owner: context.repo.owner,
repo: context.repo.repo,
body: "## Simulation results\n" + resultsTable
body: "## Simulation results\n" + ${{ steps.simulation-results.outputs.result }}
});
- name: Prune all images older than 30 days from self-hosted runner
run: docker image prune -a --force --filter "until=720h"
- name: Prune all images older than 1 days from self-hosted runner
run: docker image prune -a --filter "until=24h"
Loading