Cleanup (Server) #1 - Tasks: A-2, 6, 7, 8#983
Cleanup (Server) #1 - Tasks: A-2, 6, 7, 8#983MukuFlash03 wants to merge 6 commits intoe-mission:masterfrom
Conversation
Refer to details in cleanup issue: Task A-6: e-mission/e-mission-docs#1082 (comment)
Refer to details in cleanup issue: Task A-2: e-mission/e-mission-docs#1082 (comment) Storing server tag as well so that artifacts are not needed. Can also remove image tag passed as input in Workflow dispatch POST request. Workflow input also removed in dashboard workflows For now not removing artifacts until the internal script is updated to handle this change. ---- Task A-8: Prefixing branch name to the docker tag along with the date. In the internal script we will not need to maintain the different branch lists as the images will be completely tagged in the external workflows themselves. We can simply use the tags without modifications then. For now, not prefixing the tag to the artifact since we will be removing the artifact anyways. And current internal script works with artifacts. Once I update the internal script, will come back and remove artifacts.
…ally Refer to issue comment for details: Task A-7: e-mission/e-mission-docs#1082 (comment) The certificates are relevant to our internal AWS configuration and not needed externally. They can be present externally too without having any major effect. But removing them helps keeping the base image clean. Additionally, anyone working with the code can customize with their own certificates if needed or adopt an approach which doesn't even need certificates in the first place.
Internal script updated as well. Internal PR must be merged as well once these external PR changes merged.
MukuFlash03
changed the title
shankari
force-pushed
the
master
branch
4 times, most recently
from
a2a9a44 to
e50e9f3
Compare
shankari
force-pushed
the
master
branch
3 times, most recently
from
cd45974 to
30c4273
Compare
There was a problem hiding this comment.
Pull request overview
Updates the server-side Docker image build/push GitHub Actions workflow to align with the redesign cleanup tasks by moving tag propagation to a checked-in .env file and modernizing how step outputs are set.
Changes:
- Reads/writes
SERVER_IMAGE_TAGvia a repository.envfile (instead of uploading artifacts). - Switches GitHub Actions step outputs to use
$GITHUB_OUTPUT(instead of deprecatedset-output/GITHUB_ENVpatterns). - Adjusts downstream workflow dispatch to no longer pass
docker_image_tagas an input.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
Dockerfile |
Adds maintainer metadata to the image build definition. |
.github/workflows/image_build_push.yml |
Updates tag handling/output mechanism; commits tag to .env; triggers downstream workflows. |
.env |
Introduces a tracked SERVER_IMAGE_TAG value used by CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # python 3 | ||
| FROM ubuntu:jammy-20240227 | ||
|
|
||
| MAINTAINER K. Shankari (k.shankari@nlr.gov) |
There was a problem hiding this comment.
The MAINTAINER Dockerfile instruction is deprecated and ignored by some tooling. Prefer a LABEL (e.g., OCI labels like org.opencontainers.image.authors or a maintainer label) so metadata is preserved consistently.
| MAINTAINER K. Shankari (k.shankari@nlr.gov) | |
| LABEL org.opencontainers.image.authors="K. Shankari (k.shankari@nlr.gov)" |
| - name: Get current date # get the date of the build | ||
| id: date | ||
| run: echo "::set-output name=date::$(date +'%Y-%m-%d--%M-%S')" | ||
| run: echo "date=$(date +'%Y-%m-%d--%M-%S')" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
The generated tag timestamp format uses %Y-%m-%d--%M-%S (minute/second) but omits the hour, which can cause tag collisions across different hours on the same day (e.g., 01:05:30 vs 02:05:30). Include the hour in the format (e.g., %H-%M-%S) to guarantee uniqueness as intended.
| run: echo "date=$(date +'%Y-%m-%d--%M-%S')" >> "$GITHUB_OUTPUT" | |
| run: echo "date=$(date +'%Y-%m-%d--%H-%M-%S')" >> "$GITHUB_OUTPUT" |
| - name: Update .env file | ||
| run: | | ||
| echo ${{ steps.date.outputs.date }} > tag_file.txt | ||
| echo "Created tag text file" | ||
| echo "SERVER_IMAGE_TAG=${GITHUB_REF##*/}_${{ steps.date.outputs.date }}" > .env | ||
|
|
||
| - name: Upload Artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: docker-image-tag | ||
| path: tag_file.txt | ||
| overwrite: true | ||
| - name: Add, Commit, Push changes to .env file | ||
| run: | | ||
| git config --local user.email "action@github.com" | ||
| git config --local user.name "Github Actions bot to update .env with latest tags" | ||
| if git diff --quiet; then | ||
| echo "Latest timestamp already present in .env file, no changes to commit" | ||
| else | ||
| git add .env | ||
| git commit -m "Updated docker image tags in .env file to the latest timestamp" | ||
| git push origin | ||
| fi |
There was a problem hiding this comment.
This workflow is triggered on push and then makes a commit and git push back to the same branch. That will re-trigger the workflow and can create an infinite loop of builds/commits. Add a guard (e.g., on.push.paths-ignore for .env, or an if: condition to skip when github.actor is the actions bot / commit message contains a skip marker) so the bot update doesn’t retrigger the pipeline.
| - name: Add, Commit, Push changes to .env file | ||
| run: | | ||
| git config --local user.email "action@github.com" | ||
| git config --local user.name "Github Actions bot to update .env with latest tags" | ||
| if git diff --quiet; then | ||
| echo "Latest timestamp already present in .env file, no changes to commit" | ||
| else | ||
| git add .env | ||
| git commit -m "Updated docker image tags in .env file to the latest timestamp" | ||
| git push origin | ||
| fi |
There was a problem hiding this comment.
The workflow pushes a commit with the default GITHUB_TOKEN, but the workflow does not declare permissions: contents: write. On repos with restricted default permissions this git push will fail. Explicitly set the needed workflow/job permissions and push the correct ref (e.g., HEAD:${GITHUB_REF_NAME}) to avoid failures due to detached HEAD or branch ambiguity.
| - name: Set docker image tags | ||
| id: set-tags | ||
| run: | | ||
| set -a; source .env; set +a | ||
| echo "SERVER_IMAGE_TAG=${SERVER_IMAGE_TAG}" >> "$GITHUB_OUTPUT" | ||
| echo "Current server image tag (push): ${SERVER_IMAGE_TAG}" | ||
|
|
There was a problem hiding this comment.
The Set docker image tags step writes SERVER_IMAGE_TAG to $GITHUB_OUTPUT, but that output is not referenced by any later step/job (the image tag is recomputed from GITHUB_REF and steps.date.outputs.date). Consider removing this step or wiring it into the tag computation to avoid dead/duplicated logic.
| - name: Set docker image tags | |
| id: set-tags | |
| run: | | |
| set -a; source .env; set +a | |
| echo "SERVER_IMAGE_TAG=${SERVER_IMAGE_TAG}" >> "$GITHUB_OUTPUT" | |
| echo "Current server image tag (push): ${SERVER_IMAGE_TAG}" | |
| @@ -0,0 +1 @@ | |||
| SERVER_IMAGE_TAG=2024-09-20--06-45 | |||
There was a problem hiding this comment.
The committed .env value (SERVER_IMAGE_TAG=2024-09-20--06-45) doesn’t match the new <branch>_<timestamp> format that the workflow writes. Consider updating the checked-in baseline to the same format to avoid confusion and to ensure source .env produces a realistic tag value.
First set of cleanup changes for the redesign cleanup issue
Includes changes for tasks:
Task A-2: Reading tags from .env files instead of uploading artifacts.
Task A-6: Switch to GITHUB_OUTPUT step outputs instead of using GITHUB_ENV.
Task A-7: Certificates moved to internal Dockerfiles.
Task A-8: Tags now stored as
<branch_name>_<timestamp>Note: Once these changes are merged, the internal PR with the updates to the internal script must also be merged since no longer uploading artifacts externally, hence internal script must also be updated to fetch tags from .env files instead.