Generate CRD and RBAC yaml templates from submariner-operator #595
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tpantelis
requested review from
aswinsuryan,
dfarrell07,
maayanf24,
Oats87,
skitt,
sridhargaddam,
vthapar and
yboaron
as code owners
|
🤖 Created branch: z_pr595/tpantelis/generate_yamls |
tpantelis
force-pushed
the
generate_yamls
branch
from
5c2442a to
e575826
Compare
skitt
requested changes
Jan 21, 2025
Makefile
Outdated
Comment on lines
28
to
30
| local-helm-repo: generate-yamls $(CHART_PACKAGES) | ||
| mkdir -p $(HELM_REPO_LOCATION) | ||
| for archive in $^; do \ | ||
| for archive in $(CHART_PACKAGES); do \ |
There was a problem hiding this comment.
This doesn’t need to be updated, updated YAMLs are required to correctly populate the tarballs (see below).
There was a problem hiding this comment.
I didn't add it as a dependency to the %.tgz tag b/c there are multiple (2) tarballs and I thought generate-yamls would get executed each time. But that doesn't seem to be the case...
Makefile
Outdated
| @@ -25,15 +25,18 @@ REPO_URL=$(shell git config remote.origin.url) | |||
|
|
|||
| CHART_PACKAGES := submariner-k8s-broker-$(CHARTS_VERSION).tgz submariner-operator-$(CHARTS_VERSION).tgz | |||
|
|
|||
| local-helm-repo: $(CHART_PACKAGES) | |||
| local-helm-repo: generate-yamls $(CHART_PACKAGES) | |||
There was a problem hiding this comment.
Suggested change
| local-helm-repo: generate-yamls $(CHART_PACKAGES) | |
| local-helm-repo: $(CHART_PACKAGES) |
Makefile
Outdated
| mkdir -p $(HELM_REPO_LOCATION) | ||
| for archive in $^; do \ | ||
| for archive in $(CHART_PACKAGES); do \ |
There was a problem hiding this comment.
Suggested change
| for archive in $(CHART_PACKAGES); do \ | |
| for archive in $^; do \ |
Makefile
Outdated
| tar xzf $$archive -C $(HELM_REPO_LOCATION); \ | ||
| done | ||
|
|
||
| e2e: local-helm-repo | ||
| $(SCRIPTS_DIR)/e2e.sh | ||
|
|
||
| generate-yamls: | ||
| ./generate-yamls.sh $(BASE_BRANCH) | ||
|
|
||
| %.tgz: |
There was a problem hiding this comment.
Suggested change
| %.tgz: | |
| %.tgz: generate-yamls |
Makefile
Outdated
| @@ -52,7 +55,7 @@ helm-docs: | |||
| exit 1; \ | |||
| fi | |||
|
|
|||
| release: $(CHART_PACKAGES) | |||
| release: generate-yamls $(CHART_PACKAGES) | |||
There was a problem hiding this comment.
Suggested change
| release: generate-yamls $(CHART_PACKAGES) | |
| release: $(CHART_PACKAGES) |
The submariner-operator repo is the source of truth for the CRD and RBAC resource yaml used by subctl and the ACM add-on so we should use it for the helm charts as well. This will avoid having to duplicate changes from the submariner-operator repo. All the yaml is assembled in the pkg/embeddedyamls/yamls.go file in submariner-operator so download and extract the yaml into template files in the chart templates directories which can then be included in other manifest files. Signed-off-by: Tom Pantelis <[email protected]>
These all default to true and there doesn't seem to be any reason a user would want to set any to false as Submariner woild not work without these resources. Removing them simplifies the charts. Signed-off-by: Tom Pantelis <[email protected]>
tpantelis
force-pushed
the
generate_yamls
branch
from
e575826 to
58c42a4
Compare
skitt
approved these changes
Jan 21, 2025
dfarrell07
approved these changes
Jan 21, 2025
|
🤖 Closed branches: [z_pr595/tpantelis/generate_yamls] |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The submariner-operator repo is the source of truth for the CRD and RBAC resource yaml used by subctl and the ACM add-on so we should use it for the helm charts as well. This will avoid having to duplicate changes from the submariner-operator repo. All the yaml is assembled in the pkg/embeddedyamls/yamls.go file in submariner-operator so download and extract the yaml into files in the chart templates directories.
The second commit 5c2442a does some cleanup.