fix(dev-infrastructure): scope acr-replication/postgres-access/grafana-group-roles workingDir - #6398
Conversation
…r own script folder (AROSLSRE-1699) Point ocp-acr-replication, svc-acr-replication, cs-postgres-access, and maestro-postgres-access at their own scripts/<name> subfolder instead of the repo root, matching the pattern already used by housekeeping, upgrade-aks-cluster, oidc-storage-setup, cleanup-prometheus-pvc, and istio-upgrade. workingDir stays non-empty (required for EV2's scoped-package archiving, unsetting it breaks EV2), but the archive is now limited to the step's own script directory instead of the whole repo. This lets the incremental-rollout content-hash actually reflect that script's inputs, instead of effectively depending on the entire monorepo's content. command is updated to be relative to the new workingDir.
There was a problem hiding this comment.
🟢 Ready to approve
The updated workingDir and relative command paths align with existing pipeline patterns and remain consistent with where the prebuilt binaries are produced.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Scopes EV2 Shell step workingDir to each Go helper’s script directory so the archived/hashed step package matches the step’s actual runtime inputs (instead of content-hashing the entire repo), improving incremental-rollout cache correctness for regional ACR replication and Postgres access helpers.
Changes:
- Update
cs-postgres-accessandmaestro-postgres-accesssteps to run from./scripts/postgres-accesswith a relative./postgres-accesscommand. - Update
ocp-acr-replicationandsvc-acr-replicationsteps to run from./scripts/acr-replicationwith a relative./acr-replicationcommand.
File summaries
| File | Description |
|---|---|
| dev-infrastructure/svc-pipeline.yaml | Scopes Postgres access Shell steps to ./scripts/postgres-access and makes the command relative to that directory. |
| dev-infrastructure/region-pipeline.yaml | Scopes ACR replication Shell steps to ./scripts/acr-replication and makes the command relative to that directory. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
…own script folder (AROSLSRE-1699) Same fix as the acr-replication/postgres-access steps: point workingDir at scripts/grafana-group-roles instead of the repo root, so the EV2 package archive and incremental-rollout cache key actually reflect this step's own inputs.
|
/lgtm |
There was a problem hiding this comment.
🟢 Ready to approve
The updated workingDir/command values align with where each pipeline’s buildStep places the compiled binaries, with no other step inputs referenced from the repo root.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Rael Garcia (raelga)
changed the title
There was a problem hiding this comment.
🟢 Ready to approve
The changes are consistent across pipelines/config/schema, maintain prior behavior via per-region override, and include updated unit tests for the new env var parsing.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
a790739 to
830c365
Compare
Rael Garcia (raelga)
changed the title
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: raelga, roivaz 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 |
There was a problem hiding this comment.
🟢 Ready to approve
The updated workingDir values align with where the build steps emit the binaries and the commands are correctly adjusted to remain executable from the new directories.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
Rael Garcia (@raelga): The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
openshift-merge-bot
Bot
closed this pull request by merging all changes
into
Azure:main
in
ff3ae36
What
Scope
workingDirfor the acr-replication, postgres-access, and grafana-group-roles Shell steps to their ownscripts/<name>subfolder instead of the repo root (.), matching the existing pattern used byhousekeeping,upgrade-aks-cluster,oidc-storage-setup,cleanup-prometheus-pvc, andistio-upgrade(which all setworkingDir: ./scripts).Follow-up to AROSLSRE-1699 raised on the review of #6369.
Affected steps:
ocp-acr-replication/svc-acr-replicationindev-infrastructure/region-pipeline.yamlcs-postgres-access/maestro-postgres-accessindev-infrastructure/svc-pipeline.yamlgrafana-group-rolesindev-infrastructure/global-pipeline-stg.yamlworkingDirchanges from.to./scripts/acr-replication/./scripts/postgres-access/./scripts/grafana-group-roles, andcommandbecomes relative to that.Why
workingDirhas to stay non-empty, EV2 needs it set to build a scoped package archive for the step. Unsetting it falls back to the implicit whole-repo archive, which fails in EV2. WithworkingDir: .the package that gets archived and content-hashed is the entire repo, so the incremental-rollout cache key is effectively decoupled from what the step actually depends on. Scoping it down to the step's ownscripts/<name>folder keeps the same non-empty-workingDir requirement, but makes the archived/hashed content actually reflect that step's own inputs.Testing
templatize pipeline validate --topology-config-file topology.yaml --service-config-file config/config.yaml --dev-mode --dev-region westus3passes on all changed pipelines.main.goinscripts/acr-replication,scripts/postgres-access, andscripts/grafana-group-rolesfor any relative-path assumptions beyond the working directory itself, none found (they only read env vars).