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

actually update branched image tag in all jobs, not just parent config #5592

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mikemorris
Copy link
Member

@mikemorris mikemorris commented Jan 31, 2025

This appears to be an oversight where the image was not being updated to the branched tag in all jobs, only in the parent config. I believe this should replace the need for the following manual part of Step 3 as described in istio/istio#54707

In the job configs prow/**/*-1.25.yaml, replace any image build-tools(-centos|-proxy):master with build-tools(-centos|-proxy):release-1.25. This is easy to do in VSCode, like this:

Additionally, filters out duplicate env vars in all jobs to replace the need for the following other manual part of Step 3,
refs https://pkg.go.dev/k8s.io/api/core/v1#EnvVar

Remove duplicated env entries of the following in the configs (you can use a similar search in VSCode as above, but beware, it'll only match the indentation level that you specify if you have strict matching enabled.):

- name: BUILD_WITH_CONTAINER
  value: "0"

Finally, adds logging for commands needed to retag images when running with --skip-gar-tagging and updates some minor deprecated filesystem calls.

/cc @kfaseela @dhawton

@mikemorris mikemorris requested a review from a team as a code owner January 31, 2025 20:04
@istio-testing istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 31, 2025
@mikemorris
Copy link
Member Author

/hold

Going to add the duplicate env removal on top of this since it's now actually iterating over all jobs.

@istio-testing istio-testing added the do-not-merge/hold Block automatic merging of a PR. label Jan 31, 2025
@istio-testing istio-testing added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 31, 2025
@mikemorris
Copy link
Member Author

/unhold

Tested locally and this appears to be working as intended now.

@istio-testing istio-testing removed the do-not-merge/hold Block automatic merging of a PR. label Jan 31, 2025
mikemorris added a commit that referenced this pull request Jan 31, 2025
I believe this is the last part of step 3 (aside from Google Artifact Registry image tagging) that must still be done manually after updates in #5592 and istio/release-builder#2026

I'm guessing an updated version of the automation PR should be merged before this.

/hold
@kfaseela kfaseela added the do-not-merge Block automatic merging of a PR. label Jan 31, 2025
@kfaseela
Copy link
Member

@mikemorris : this LGTM. I just added a hold in case you have anything else to verify or want @howardjohn to do a review before merging. Feel free to remove the dnm when you want to merge it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge Block automatic merging of a PR. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants