-
Notifications
You must be signed in to change notification settings - Fork 243
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
chore: create a script for updating the version #2736
chore: create a script for updating the version #2736
Conversation
c5844ff
to
65b7323
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.
👍
kind: Kustomization | ||
|
||
resources: | ||
- ../release-bundle/install-bundle-namespaced/per-namespace-components.yaml |
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.
Ideally we want to put the single file under this directory, so that any changes to the per-namespace-components
file can be aware of and better traced
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 think that would be great, but I think we should do that separately!
resources: | ||
- ../release-bundle/install-bundle-autopilot-namespaced/0-cnrm-system.yaml | ||
|
||
patchesJson6902: |
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.
super nit: patches
contains patchesJson6902
and patchesStrategicMerge
. I saw this PR mixes the usage of patches
and patchesJson6902
, may be better to align them two.
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.
Yes, I tackled the ones that I thought were critical but there are still some to go.
Once we aren't running kustomize as part of the release itself (I.e. once we are committing the output of kustomize, as we are doing here), then I think we should feel much more comfortable about making bigger changes to our kustomizations.
# limitations under the License. | ||
|
||
- op: remove | ||
path: /spec/template/spec/containers/0/ports/0/hostPort |
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.
Mind adding some comment to explain what problem this solves? I have a vague impression that the Pod API has some issue about the defaulting values around port. But I cannot remember the details. I guess future maintainers may have the same question.
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.
This is copying existing patch scripts around to try to make them follow the normal kustomize structure...
/lgtm |
/hold |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yuwenma The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
65b7323
to
0029684
Compare
This script will update the manifests in the repository for a new release. We also avoid using the update-kcc-manifest tool, it is a wrapper around kustomize and it is easier to just use kustomize.
0029684
to
8de716f
Compare
/lgtm |
/hold cancel |
ea08b6f
into
GoogleCloudPlatform:master
This script will update the manifests in the repository for a new release.