-
Notifications
You must be signed in to change notification settings - Fork 66
🌱 OPRUN-4077: Remove the kustomize config #2213
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
🌱 OPRUN-4077: Remove the kustomize config #2213
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
/hold |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2213 +/- ##
=======================================
Coverage 71.87% 71.87%
=======================================
Files 86 86
Lines 8474 8474
=======================================
Hits 6091 6091
Misses 1982 1982
Partials 401 401
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CRD_DIFF_OPCON_SOURCE := config/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml | ||
CRD_DIFF_CATD_SOURCE := config/base/catalogd/crd/standard/olm.operatorframework.io_clustercatalogs.yaml | ||
CRD_DIFF_OPCON_SOURCE := helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml | ||
CRD_DIFF_CATD_SOURCE := helm/olmv1/base/catalogd/crd/standard/olm.operatorframework.io_clustercatalogs.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.
Why do we actually need the base?
It feels more like a kustomize-style layout.
Since everything is already managed under Helm, why wouldn’t those manifests live under templates instead?
Also, I noticed we have CRDs under templates — shouldn’t this be consistent?
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.
The base
directory is storing the output of the CRDs - these is the "raw" CRDs. This is kept outside of the templates
directory and is purposely not put into a crds
directory.
Because the CRDs are be conditionally included into the manifest (i.e. standard vs experimental), and because they are autogenerated, and because we don't want to go through a messy process of post-processing them to include Helm directives, they are put into that base
directory.
So, the CRDs are not under templates
directory, but instead those files in the templates/crds
directory conditionally include CRDs from the base
directory:
Lines 1 to 9 in 16d1089
{{- if .Values.options.catalogd.enabled }} | |
{{- if (eq .Values.options.featureSet "standard") }} | |
{{ tpl (.Files.Get "base/catalogd/crd/standard/olm.operatorframework.io_clustercatalogs.yaml") . }} | |
{{- else if (eq .Values.options.featureSet "experimental") }} | |
{{ tpl (.Files.Get "base/catalogd/crd/experimental/olm.operatorframework.io_clustercatalogs.yaml") . }} | |
{{- else }} | |
{{- fail "options.featureSet must be set to one of: {standard,experimental}" }} | |
{{- end }} | |
{{- end }} |
Lines 1 to 9 in 16d1089
{{- if .Values.options.operatorController.enabled }} | |
{{- if (eq .Values.options.featureSet "standard") }} | |
{{- /* Add when GA: tpl (.Files.Get "base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensionrevisionss.yaml") . */}} | |
{{- else if (eq .Values.options.featureSet "experimental") }} | |
{{ tpl (.Files.Get "base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml") . }} | |
{{- else }} | |
{{- fail "options.featureSet must be set to one of: {standard,experimental}" }} | |
{{- end }} | |
{{- end }} |
(i.e. We don't want to do any post-processing of the CRDs to include the above.)
This PR, because it's removing the config
directory, is merely changing the location of where the "raw" CRDs live. It is NOT changing the process of how they are generated nor included into the Helm manifest.
/unhold |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tmshort 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 |
Should we address these as well while we're at it?
These urls will need to be updated too so they don't break:
|
Add a helm/OWNERS file (copied from config/OWNERS) Update hack/tools/update-crds.sh to not reference config directory Signed-off-by: Todd Short <tshort@redhat.com>
Signed-off-by: Todd Short <tshort@redhat.com>
Signed-off-by: Todd Short <tshort@redhat.com>
/lgtm |
c6a2fed
into
operator-framework:main
Add a helm/OWNERS file (copied from config/OWNERS) Update hack/tools/update-crds.sh to not reference config directory
Description
Reviewer Checklist