Merged
Conversation
Contributor
Reviewer's GuideUpdates the GitLab CI image check and build pipeline to use commit-SHA–aware retagging and watched-path diffs to decide when to rebuild Docker images, introduces retag pipelines per component, and removes obsolete build-only pipeline configs. Sequence diagram for updated .image_check image rebuild decisionsequenceDiagram
participant GitLabCI
participant image_check_job
participant HarborRegistry
participant GitRepository
GitLabCI->>image_check_job: Start job with COMPONENT, PROJECT, WATCHED_PATHS
image_check_job->>HarborRegistry: docker login REGISTRY
image_check_job->>HarborRegistry: docker pull IMAGE_PATH:latest
HarborRegistry-->>image_check_job: Success or failure
alt Image does not exist
image_check_job->>image_check_job: Set FORCE_BUILD = TRUE
else Image exists
image_check_job->>HarborRegistry: GET /api/v2.0/projects/PROJECT/repositories/REPO_NAME/artifacts
HarborRegistry-->>image_check_job: Harbor API response
alt Harbor API error or empty
image_check_job->>image_check_job: Set FORCE_BUILD = TRUE
else Harbor API ok
image_check_job->>image_check_job: Extract LATEST_IMAGE_SHA from tags
alt No SHA tag found
image_check_job->>image_check_job: Set FORCE_BUILD = TRUE
else SHA tag found
alt LATEST_IMAGE_SHA == CI_COMMIT_SHA
image_check_job->>image_check_job: No rebuild needed
else Different commit
image_check_job->>GitRepository: git fetch history
GitRepository-->>image_check_job: Updated history
image_check_job->>GitRepository: git cat-file -e LATEST_IMAGE_SHA
GitRepository-->>image_check_job: Exists or missing
alt Commit not in history
image_check_job->>image_check_job: Set FORCE_BUILD = TRUE
else Commit exists
image_check_job->>GitRepository: git diff --name-only LATEST_IMAGE_SHA CI_COMMIT_SHA -- WATCHED_PATHS
GitRepository-->>image_check_job: DIFF_OUTPUT
alt DIFF_OUTPUT not empty
image_check_job->>image_check_job: Set FORCE_BUILD = TRUE
else No watched changes
image_check_job->>image_check_job: Keep FORCE_BUILD = FALSE
end
end
end
end
end
end
alt BUILD_INTERMEDIATE == TRUE and FORCE_BUILD == FALSE
image_check_job->>HarborRegistry: docker pull COMPONENT-INTERMEDIATE_LAYER_NAME-BRANCH:latest
HarborRegistry-->>image_check_job: Success or failure
alt Intermediate missing
image_check_job->>image_check_job: Set FORCE_BUILD = TRUE
end
end
image_check_job->>image_check_job: FILE_COMPONENT = COMPONENT with dashes to underscores
alt FORCE_BUILD == TRUE
image_check_job->>image_check_job: cp .gitlab/build/force_build_FILE_COMPONENT_image.yml FILE_COMPONENT_image.yml
else FORCE_BUILD == FALSE
image_check_job->>image_check_job: cp .gitlab/build/retag_FILE_COMPONENT_image.yml FILE_COMPONENT_image.yml
end
image_check_job->>GitLabCI: Export build.env and artifact FILE_COMPONENT_image.yml
Flow diagram for FORCE_BUILD decision and file selection in .image_checkflowchart TD
A[Start .image_check script] --> B[Set BRANCH_LOWER<br>Set REPO_NAME<br>Set IMAGE_PATH]
B --> C[docker pull IMAGE_PATH:latest]
C --> D{Pull succeeded?}
D -- No --> E[Set FORCE_BUILD = TRUE]
D -- Yes --> F[Call Harbor API for latest artifacts]
F --> G{API ok and response present?}
G -- No --> E
G -- Yes --> H[Extract LATEST_IMAGE_SHA tag]
H --> I{LATEST_IMAGE_SHA present?}
I -- No --> E
I -- Yes --> J{LATEST_IMAGE_SHA == CI_COMMIT_SHA?}
J -- Yes --> K[Keep FORCE_BUILD = FALSE<br>No rebuild needed]
J -- No --> L[git fetch history]
L --> M[git cat-file -e LATEST_IMAGE_SHA]
M --> N{Commit exists in history?}
N -- No --> E
N -- Yes --> O{WATCHED_PATHS defined?}
O -- No --> P[Exit with error]
O -- Yes --> Q[git diff --name-only<br>LATEST_IMAGE_SHA CI_COMMIT_SHA -- WATCHED_PATHS]
Q --> R{DIFF_OUTPUT non-empty?}
R -- Yes --> E
R -- No --> K
E --> S[FORCE_BUILD may be TRUE]
K --> S
S --> T{BUILD_INTERMEDIATE == TRUE<br>and FORCE_BUILD == FALSE?}
T -- Yes --> U[docker pull intermediate image]
U --> V{Intermediate exists?}
V -- No --> E
V -- Yes --> W[Keep FORCE_BUILD as FALSE]
T -- No --> W
W --> X[FILE_COMPONENT = COMPONENT with dashes to underscores]
X --> Y{FORCE_BUILD == TRUE?}
Y -- Yes --> Z[cp .gitlab/build/force_build_FILE_COMPONENT_image.yml<br>FILE_COMPONENT_image.yml]
Y -- No --> AA[cp .gitlab/build/retag_FILE_COMPONENT_image.yml<br>FILE_COMPONENT_image.yml]
Z --> AB[Write build.env<br>export variables]
AA --> AB
AB --> AC[End .image_check script]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
.image_checkscript introduces several external dependencies (curl,jq,git) but does not validate their availability or handle failures ofjq/JSON parsing specifically; consider adding explicit checks and clearer failure messages around those calls so failures don’t silently fall back toFORCE_BUILDwithout context. - The
WATCHED_PATHShandling is spread between the job definitions and inline checks in the script; to avoid misconfiguration and hard‑to‑debug failures, consider centralizing default paths or adding a stricter validation step (e.g., fail fast when it’s unset for components that require it) instead of only checking for empty in the Harbor‑SHA flow. - The mapping from
COMPONENTtoFILE_COMPONENT(hyphen to underscore) and the newretag_*YAML files replaces the oldbuild_*files, but the retag YAMLs are currently empty; ensure they contain the equivalent job definitions/logic that the deletedbuild_*files had so that thecp .gitlab/build/retag_${FILE_COMPONENT}_image.ymlcalls result in a usable pipeline fragment.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `.image_check` script introduces several external dependencies (`curl`, `jq`, `git`) but does not validate their availability or handle failures of `jq`/JSON parsing specifically; consider adding explicit checks and clearer failure messages around those calls so failures don’t silently fall back to `FORCE_BUILD` without context.
- The `WATCHED_PATHS` handling is spread between the job definitions and inline checks in the script; to avoid misconfiguration and hard‑to‑debug failures, consider centralizing default paths or adding a stricter validation step (e.g., fail fast when it’s unset for components that require it) instead of only checking for empty in the Harbor‑SHA flow.
- The mapping from `COMPONENT` to `FILE_COMPONENT` (hyphen to underscore) and the new `retag_*` YAML files replaces the old `build_*` files, but the retag YAMLs are currently empty; ensure they contain the equivalent job definitions/logic that the deleted `build_*` files had so that the `cp .gitlab/build/retag_${FILE_COMPONENT}_image.yml` calls result in a usable pipeline fragment.
## Individual Comments
### Comment 1
<location> `.gitlab/common.yml:82-85` </location>
<code_context>
+ echo "Image exists, checking for changes in watched files"
+
+ # Get commit SHA from latest image via Harbor API
+ HARBOR_API_RESPONSE=$(curl -sf -u "${HARBOR_USER}:${HARBOR_DATAFED_GITLAB_CI_REGISTRY_TOKEN}" \
+ "https://${REGISTRY}/api/v2.0/projects/${PROJECT}/repositories/${REPO_NAME}/artifacts?page_size=1&sort=-push_time")
+
+ if [ $? -ne 0 ] || [ -z "$HARBOR_API_RESPONSE" ]; then
+ echo "WARNING: Harbor API request failed, forcing build"
+ FORCE_BUILD="TRUE"
</code_context>
<issue_to_address>
**issue (bug_risk):** The `curl` failure handling is ineffective under `set -e` and will cause the script to abort instead of falling back to `FORCE_BUILD="TRUE"`.
With `set -e` enabled, a non‑zero exit from `curl -sf` will exit the script before the `[ $? -ne 0 ]` check runs, so transient Harbor/API errors will fail the job instead of setting `FORCE_BUILD="TRUE"`.
To preserve the intended fallback, either temporarily disable `set -e` around `curl` (like the `docker pull` logic) or capture the status via `||`:
```bash
set +e
HARBOR_API_RESPONSE=$(curl -sf -u "${HARBOR_USER}:${HARBOR_DATAFED_GITLAB_CI_REGISTRY_TOKEN}" \
"https://${REGISTRY}/api/v2.0/projects/${PROJECT}/repositories/${REPO_NAME}/artifacts?page_size=1&sort=-push_time")
CURL_STATUS=$?
set -e
if [ $CURL_STATUS -ne 0 ] || [ -z "$HARBOR_API_RESPONSE" ]; then
echo "WARNING: Harbor API request failed, forcing build"
FORCE_BUILD="TRUE"
else
...
fi
```
This ensures Harbor API issues trigger a forced build rather than a hard pipeline failure.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+82
to
+85
| HARBOR_API_RESPONSE=$(curl -sf -u "${HARBOR_USER}:${HARBOR_DATAFED_GITLAB_CI_REGISTRY_TOKEN}" \ | ||
| "https://${REGISTRY}/api/v2.0/projects/${PROJECT}/repositories/${REPO_NAME}/artifacts?page_size=1&sort=-push_time") | ||
|
|
||
| if [ $? -ne 0 ] || [ -z "$HARBOR_API_RESPONSE" ]; then |
Contributor
There was a problem hiding this comment.
issue (bug_risk): The curl failure handling is ineffective under set -e and will cause the script to abort instead of falling back to FORCE_BUILD="TRUE".
With set -e enabled, a non‑zero exit from curl -sf will exit the script before the [ $? -ne 0 ] check runs, so transient Harbor/API errors will fail the job instead of setting FORCE_BUILD="TRUE".
To preserve the intended fallback, either temporarily disable set -e around curl (like the docker pull logic) or capture the status via ||:
set +e
HARBOR_API_RESPONSE=$(curl -sf -u "${HARBOR_USER}:${HARBOR_DATAFED_GITLAB_CI_REGISTRY_TOKEN}" \
"https://${REGISTRY}/api/v2.0/projects/${PROJECT}/repositories/${REPO_NAME}/artifacts?page_size=1&sort=-push_time")
CURL_STATUS=$?
set -e
if [ $CURL_STATUS -ne 0 ] || [ -z "$HARBOR_API_RESPONSE" ]; then
echo "WARNING: Harbor API request failed, forcing build"
FORCE_BUILD="TRUE"
else
...
fiThis ensures Harbor API issues trigger a forced build rather than a hard pipeline failure.
…submodule with updated openssl (#1826)
…d into 1839-DAPS-fix-ci-image-retag
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Ticket
Description
How Has This Been Tested?
Artifacts (if appropriate):
Tasks
Summary by Sourcery
Update CI Docker image check and tagging logic to avoid unnecessary rebuilds and switch from build pipelines to retag pipelines based on watched-path changes.
New Features:
Enhancements:
CI: