-
Notifications
You must be signed in to change notification settings - Fork 14
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
Hello helm #137
Hello helm #137
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #137 +/- ##
==========================================
- Coverage 85.51% 84.70% -0.81%
==========================================
Files 19 19
Lines 994 994
==========================================
- Hits 850 842 -8
- Misses 94 98 +4
- Partials 50 54 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
25aebe0
to
f1b0b36
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.
looking good so far!
@ficap join forces here Kuadrant/installation#1 |
e06789c
to
c699002
Compare
220d0fd
to
b2c033b
Compare
b2c033b
to
055a24d
Compare
50aaf54
to
25f5aed
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.
overall looks good to me.
few comments dropped, though, for further discussion.
|
||
- name: Build the Helm Chart manifests | ||
run: | | ||
make helm-build \ |
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 feel like we should not be doing this. The git commit (or tag) we are releasing should have done previously make helm-build
. Any thought?
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 was thinking it should be part of the building manifests steps just before tagging... However, just a reminder that currently we do that manually before tagging and then let the CI to build the manifests again and publish them to Quay... meaning it's kind of the same in this case, we could run this target and commit it with the rest of the manifests when releasing... but having a separated workflow is convenient when you want to attach charts to other already released operators. What do you reckon?
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.
currently we do that manually before tagging and then let the CI to build the manifests again and publish them to Quay.
Isn't that redundant? What I would do, maybe, it is a test to run (on release event) make helm-build
and check that no files changed. Same we do with go generate
or make bundle
.
having a separated workflow is convenient when you want to attach charts to other already released operators
Not sure I follow.
Anyway, I was not asking for changes actually. Whatever you decide is ok for me on this regard.
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 agree with you, we'll need to address the redundancy in our workflows, part should be reflected in Kuadrant/architecture#41
Signed-off-by: dd di cesare <[email protected]>
Signed-off-by: dd di cesare <[email protected]>
* Installs helm binary Signed-off-by: dd di cesare <[email protected]>
Signed-off-by: dd di cesare <[email protected]>
Signed-off-by: dd di cesare <[email protected]>
Signed-off-by: dd di cesare <[email protected]>
Signed-off-by: dd di cesare <[email protected]>
Signed-off-by: dd di cesare <[email protected]>
Signed-off-by: dd di cesare <[email protected]>
* Instead of curl to endpoint * Removing makefile target previously used * Not passing asset_id to sync package target
9f7c141
to
a3649ad
Compare
This PR introduces a way to manage a Limitador Operator Helm Chart. This is not meant to replace the way we are building and delivering our manifests (Kustomize, OLM) but to provide an alternative (complementary) way of delivering the operator.
This early implementation uses Kustomize to create the chart template, instead of creating and maintaining new ones with Helm, to later customize the Helm only settings with its
values.yaml
NOTES
workflow_dispatch
it will change in the near future.TODO
Verification Steps
latest
) Limitador Operator chartkubectl get deployments/limitador-operator-controller-manager -n limitador-operator-system -o yaml | grep image:
it should return:
kubectl get deployments/limitador-operator-controller-manager -n limitador-operator-system -o yaml | grep image:
it should return: