Make personal-dev-env creation endure repo changes#6183
Conversation
|
Hi @tmstff. Thanks for your PR. I'm waiting for a Azure member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
tmstff
changed the title
|
/ok-to-test |
There was a problem hiding this comment.
Pull request overview
This PR aims to make make personal-dev-env resilient to concurrent repo commits/changes by freezing the image tag/revision inputs used across the multi-service build/push workflow, so all services consistently reference the same tag during a single run.
Changes:
- Compute a single
IMAGE_TAGat the repo root (forDEPLOY_ENV=pers|swft) and propagate it into sub-make invocations. - Update multiple service Makefiles to use simply-expanded (
:=) image tag variables and prefer the sharedIMAGE_TAGwhen provided. - Apply the same tag-freezing behavior to the
tooling/aro-hcp-exporterimage build.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| Makefile | Introduces a root-level IMAGE_TAG and passes it into service build/push and override-recording targets. |
| backend/Makefile | Freezes image tag evaluation and prefers shared IMAGE_TAG to keep backend image tagging consistent. |
| frontend/Makefile | Same as backend: uses shared IMAGE_TAG / freezes tag evaluation for frontend image builds. |
| admin/Makefile | Same pattern applied to admin image tagging to ensure consistency during a run. |
| sessiongate/Makefile | Same pattern applied to sessiongate image tagging to ensure consistency during a run. |
| mgmt-agent/Makefile | Same pattern applied to mgmt-agent image tagging to ensure consistency during a run. |
| kube-applier/Makefile | Same pattern applied to kube-applier image tagging to ensure consistency during a run. |
| fleet/Makefile | Same pattern applied to fleet image tagging to ensure consistency during a run. |
| tooling/aro-hcp-exporter/Makefile | Freezes exporter image tag evaluation and prefers shared IMAGE_TAG when present. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (1)
Makefile:482
IMAGE_TAGandARO_HCP_REVISIONare frozen via two separate$(shell ...)calls. If a new commit lands between those calls, the pushed image tag can still end up referring to a different commit than the revision embedded into binaries/images (the original failure mode this PR is addressing).
$(eval IMAGE_TAG := $(shell DETECT_DIRTY_GIT_WORKTREE=${DETECT_DIRTY_GIT_WORKTREE} DEPLOY_ENV=${DEPLOY_ENV} ./generate-tag.sh))
$(eval ARO_HCP_REVISION := $(shell git rev-parse HEAD))
$(eval export IMAGE_TAG ARO_HCP_REVISION)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
Makefile:482
personal-dev-envfreezesIMAGE_TAGandARO_HCP_REVISION, but they’re computed via two separategitreads (one insidegenerate-tag.sh, one viagit rev-parse HEAD). If a commit happens between those two evaluations, the image tag can refer to a different revision than the embeddedCommitSHA/build-arg revision, which undermines the “freeze revisions” goal.
Consider computing both values from the same revision source (e.g., capture ARO_HCP_REVISION once, then have generate-tag.sh accept a revision input / env var so it can derive the tag from that fixed SHA).
$(eval IMAGE_TAG := $(shell DETECT_DIRTY_GIT_WORKTREE=${DETECT_DIRTY_GIT_WORKTREE} DEPLOY_ENV=${DEPLOY_ENV} ./generate-tag.sh))
$(eval ARO_HCP_REVISION := $(shell git rev-parse HEAD))
$(eval export IMAGE_TAG ARO_HCP_REVISION)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
Makefile:482
- In
personal-dev-envthe recipe unconditionally assignsIMAGE_TAGandARO_HCP_REVISIONvia$(eval ...), which overrides values provided via the environment (e.g.,export IMAGE_TAG=...). This makes it hard to intentionally pin/reuse a tag or revision when runningmake personal-dev-envand is inconsistent with the per-service Makefiles which explicitly honor these overrides.
$(eval IMAGE_TAG := $(shell DETECT_DIRTY_GIT_WORKTREE=${DETECT_DIRTY_GIT_WORKTREE} DEPLOY_ENV=${DEPLOY_ENV} ./generate-tag.sh))
$(eval ARO_HCP_REVISION := $(shell git rev-parse HEAD))
$(eval export IMAGE_TAG ARO_HCP_REVISION)
0f911cb to
9989e9b
Compare
Without this fix, each sub-make phase (build, record, deploy) called generate-tag.sh independently. If a commit happened between phases, the recorded image digest would reference a tag that was never pushed, breaking the deployment. Fix: compute IMAGE_TAG and ARO_HCP_REVISION once at the start of the personal-dev-env recipe via $(eval), then export both so all child make processes inherit them as environment variables. Service Makefiles use ?= with a $(or $(IMAGE_TAG),...) fallback so they read the exported value when set and compute locally when invoked standalone. ARO_HCP_COMMIT_TIME and *_IMAGE_TAG in service Makefiles are also changed from := (parse-time) to ?= (lazy) so that git show and generate-tag.sh only run when actually needed for image builds, not on every make invocation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
9989e9b to
588e665
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
Makefile:482
personal-dev-envfreezesIMAGE_TAGandARO_HCP_REVISIONvia two separate git reads (./generate-tag.shusesHEAD, thengit rev-parse HEAD). If a commit happens between these two lines (the scenario this PR is addressing), the pushed image tag can refer to a different commit than what gets embedded via-X ...CommitSHA=${ARO_HCP_REVISION}and passed as Docker build-arg. To fully “freeze revisions during the build”, derive both values from the same captured revision (e.g., captureARO_HCP_REVISIONonce and pass it intogenerate-tag.sh, or teachgenerate-tag.shto accept an override revision and use that).
$(eval IMAGE_TAG := $(shell DETECT_DIRTY_GIT_WORKTREE=${DETECT_DIRTY_GIT_WORKTREE} DEPLOY_ENV=${DEPLOY_ENV} ./generate-tag.sh))
$(eval ARO_HCP_REVISION := $(shell git rev-parse HEAD))
$(eval export IMAGE_TAG ARO_HCP_REVISION)
|
Didn't test but looks sane to me. /lgtm |
|
Thanks for addressing this, alternatively you could try copy the repo into a temp directory. That would solve this problem and other scenarios ie editing code while a deployment is running. Probably a bit heavy weight though. |
Right, can also be a possibility for some cases. But especially if you change something where you need to modify the code and see if it runs when using "make personal-dev-env" it can become unhandy quite easily. With this lightweight fix you don't have to worry about this :-) Another point is: you have to know about this behavior. Took me a while to figure out why make personal-dev-env was failing often. You just have to do a commit in the background and it may fail. Might save some other newbies some time :-) |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: janboll, mbarnes, tmstff The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
https://redhat.atlassian.net/browse/ARO-28516
What
fix for the following bug:
if you change files in ARO-HCP and commit them while
make personal-dev-envis running, the latter fails (because certain image hashs can not be found). This is fixed by freezing revisions during the build.Why
To save time by having fewer re-runs when continuing development while creating a presonal-dev-env.
Testing
make personal-dev-envwith concurrent changes ✅Special notes for your reviewer
GNU Make variable assignment semantics
Two assignment operators behave differently:
?= (conditional recursive) — like =. The RHS is stored as a recipe, not a value. Make re-evaluates it every time
the variable is expanded. $(shell ...) re-runs each expansion.
:= (simply-expanded) — RHS is evaluated once, at parse time, when make first reads the Makefile line. Result is
stored as a plain string. No re-evaluation ever.
What went wrong
FLEET_IMAGE_TAG ?= $(shell ... generate-tag.sh) # stores the recipe, not the value
-> if changes occur during build time, variables like FLEET_IMAGE_TAG become inconsistent
-> made sure consistent tagging is used for images.
PR Checklist
If E2E tests are included:
demonstrate that the test is able to detect a defect/error and fail with
proper error message and logs which communicates nature of the problem.