-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: Feast-operator Helm chart #5578
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
base: master
Are you sure you want to change the base?
feat: Feast-operator Helm chart #5578
Conversation
…and readme Signed-off-by: matt <[email protected]>
…deployment, helpers) Signed-off-by: matt <[email protected]>
…Binding for leader election) Signed-off-by: matt <[email protected]>
… (CRD-gated) Signed-off-by: matt <[email protected]>
Signed-off-by: matt <[email protected]>
Signed-off-by: matt <[email protected]>
3d96b9d
to
0d588d7
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.
If the community is on board with creating a helm chart for deploying the operator, it should definitely be dynamically built and not require manual changes as the operator evolves. The following tool looks promising and has documented how to integrate with our existing operator-sdk
implementation, it's called Helmify
-
https://github.com/arttor/helmify?tab=readme-ov-file#integrate-to-your-operator-sdkkubebuilder-project
feast/infra/feast-operator/Makefile
Lines 100 to 101 in acb97fb
manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects. | |
$(CONTROLLER_GEN) rbac:roleName=manager-role crd:maxDescLen=120 webhook paths="./..." output:crd:artifacts:config=config/crd/bases |
feast/infra/feast-operator/Makefile
Lines 244 to 246 in acb97fb
kustomize: $(KUSTOMIZE) ## Download kustomize locally if necessary. | |
$(KUSTOMIZE): $(LOCALBIN) | |
$(call go-install-tool,$(KUSTOMIZE),sigs.k8s.io/kustomize/kustomize/v5,$(KUSTOMIZE_VERSION)) |
That said, the "official" method for managing operator deployment/lifecycle is something called OLM and operator bundles (existing make bundle
command) -
https://github.com/operator-framework/operator-lifecycle-manager
https://operatorhub.io/
feast/infra/feast-operator/Makefile
Line 312 in acb97fb
bundle: manifests kustomize related-image-fs operator-sdk ## Generate bundle manifests and metadata, then validate generated files. |
However, i understand the convenience and familiarity of helm for folks so something like helmify
would also suffice.
infra/feast-operator/Makefile
Outdated
# Generate dynamic Helm chart from updated manifests | ||
$(MAKE) helm-generate |
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.
does it make more sense to run this here?
feast/infra/feast-operator/Makefile
Lines 190 to 193 in acb97fb
build-installer: manifests generate-ref kustomize related-image-fs ## Generate a consolidated YAML with CRDs and deployment. | |
mkdir -p dist | |
cd config/manager && $(KUSTOMIZE) edit set image controller=${IMG} | |
$(KUSTOMIZE) build config/default > dist/install.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.
it's worth noting that make build-installer
is executed with CI for each release, it also similarly already runs kustomize build
33b4f41
to
74720d6
Compare
…ate chart from kustomize; set chart version Signed-off-by: matt <[email protected]>
Signed-off-by: matt <[email protected]>
8fbc403
to
cb27e31
Compare
apiVersion: apiextensions.k8s.io/v1 | ||
kind: CustomResourceDefinition | ||
metadata: | ||
annotations: | ||
controller-gen.kubebuilder.io/version: v0.15.0 | ||
name: featurestores.feast.dev |
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.
remove this file, as its a duplicate of -
https://github.com/feast-dev/feast/blob/master/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml
@@ -0,0 +1,21 @@ | |||
apiVersion: v2 |
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.
move this and any helm related files to a new dir within the existing infra/charts
path -
https://github.com/feast-dev/feast/tree/master/infra/charts
@mkerkstra any update here? I was wondering if you want me to include this in the next release? |
What this PR does / why we need it:
Which issue(s) this PR fixes:
Misc