-
Notifications
You must be signed in to change notification settings - Fork 807
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
[Automate-Release-1] Add image-source-of-truth.yaml and update-truth-sidecars scripts #1791
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2d6814f
to
3a21c39
Compare
3a21c39
to
b87eea0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the purpose of this PR - Why are we creating a "source of truth" file to live in our git repository when the Helm chart is the actual source of truth?
If it's just going to be a convenient file for other scripts to read, it should probably be in the .gitignore so that maintainers doing a release don't update the file.
ROOT_DIRECTORY=${ROOT_DIRECTORY:=$(git rev-parse --show-toplevel)} | ||
TRUTH_FILEPATH=${TRUTH_FILEPATH:="$ROOT_DIRECTORY/hack/release-scripts/image-source-of-truth.yaml"} | ||
|
||
tmp_filename="tmp_$RANDOM.txt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use mktemp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like mktemp
suffers from some Linux/MacOS incompatibilities. I think the current way of creating a temporary file is more readable for this non-mission-critical use-case compared to mktemp fix.
@@ -268,3 +268,7 @@ generate-kustomize: bin/helm | |||
cd charts/aws-ebs-csi-driver && ../../bin/helm template kustomize . -s templates/serviceaccount-csi-node.yaml | sed -e "/namespace: /d" > ../../deploy/kubernetes/base/serviceaccount-csi-node.yaml | |||
cd charts/aws-ebs-csi-driver && ../../bin/helm template kustomize . -s templates/role-leases.yaml | sed -e "/namespace: /d" > ../../deploy/kubernetes/base/role-leases.yaml | |||
cd charts/aws-ebs-csi-driver && ../../bin/helm template kustomize . -s templates/rolebinding-leases.yaml | sed -e "/namespace: /d" > ../../deploy/kubernetes/base/rolebinding-leases.yaml | |||
|
|||
.PHONY: update-truth-sidecars | |||
update-truth-sidecars: hack/release-scripts/image-source-of-truth.yaml hack/release-scripts/update-truth-sidecars |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Targets should not depend on the files they update, remove hack/release-scripts/image-source-of-truth.yaml
Is this a bug fix or adding new feature?
Release automation
What is this PR about? / Why do we need it?
NOTE: This PR is the first in the series [Automate-Release].
The [Automate-Release] PRs add the directory
hack/release-scripts
to help automate release related activity.This PR adds:
image_source_of_truth.yaml
: Is the source of truth of images, tags, and manifest digests used in the rest of the repository. Scripts inhack/release-scripts
will update this file and propagating those updates to the rest of the repository.update-truth-sidecars
: Updates the image-source-of-truth.yaml with the latest tags and associated manifest digests for each sidecar image by usingcrane
.update-truth-sidecars
target for ourMakefile
.What testing is done?
Running make
update-truth-sidecars
will produce the following diff: