Added orchestrator-infra subchart - #108
Conversation
f15c87b to
aa0b58d
Compare
dcfb238 to
ef3a097
Compare
|
@masayag Updated the PR with all the comments |
|
@rm3l can you take a look pls? |
Sure - I'll take a look soon. /cc |
|
FYI /cc @durandom |
|
@ElaiShalevRH pls add also notes.txt file - https://helm.sh/docs/chart_template_guide/notes_files/ |
d2177d4 to
f8efde8
Compare
|
Hey @rm3l @masayag Should I open a MR for updating https://gitlab.cee.redhat.com/rhidp/rhdh/-/blob/rhdh-1-rhel-9/build/helm/prepare.sh to include the orchestrator-chart build? Also added NOTES.txt |
There was a problem hiding this comment.
Would it be possible to also add a test template? I think that would also be required for shipping the Chart into the OpenShift Charts repo.
@ElaiShalevRH That task was captured in https://issues.redhat.com/browse/RHIDP-6169. We (RHDH team) might be able to handle this, so you can focus on getting your PRs merged on GH. |
There was a problem hiding this comment.
@ElaiShalevRH Could you please rebase your PR branch? It should hopefully fix the CI errors: https://github.com/redhat-developer/rhdh-chart/actions/runs/13836562023/job/38726590327?pr=108#step:11:30. Thanks.
EDIT: Done
Thanks, that could be helpful |
9d70c23 to
dfd3821
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
d3c2ad5 to
7016d1f
Compare
|
Hey @rm3l, Thanks for the review :) Please let me know if there is more to be done that is required for shipping the Chart. |
rm3l
left a comment
rm3l
left a comment
There was a problem hiding this comment.
@ElaiShalevRH Thanks for the changes. A few comments below ;)
| subscription: | ||
| namespace: openshift-serverless-logic # namespace where the operator should be deployed | ||
| channel: alpha # channel of an operator package to subscribe to | ||
| installPlanApproval: Manual # whether the update should be installed automatically |
There was a problem hiding this comment.
| installPlanApproval: Manual # whether the update should be installed automatically | |
| installPlanApproval: Automatic # whether the update should be installed automatically |
Shouldn't this be Automatic by default? In your previous changes, it was hardcoded to Automatic in the template, IIRC. And I had noticed that as well in https://github.com/rhdhorchestrator/orchestrator-helm-operator/blob/main/helm-charts/orchestrator/templates/openshift-serverless.yaml#L57
There was a problem hiding this comment.
Having the installPlan be Automatic will cause it to automatically install updates as well. We want to avoid scenarios where the OpenshiftServerlessLogic operator version gets upgraded, but the newer version isn't supported by other orchestrator functionality.
There was a problem hiding this comment.
Ok, that makes sense. So there would be an extra step for cluster admins (approving the Install Plan) that should be added to the docs.
|
@rm3l Hey, added some more changes |
Okay, sounds like a false positive to me. I've dismissed it for now. Let's see how it goes now. |
|
rm3l
left a comment
rm3l
left a comment
There was a problem hiding this comment.
LGTM. Thanks for your patience, @ElaiShalevRH !! If you don't have anything else to add to this PR (noticed the "WIP" in the PR title), I am fine with merging this first version of the orchestrator infra chart.
elai-shalev
changed the title
4 participants
This PR will introduce the orchestrator-infra helm chart, that will be included as a dependency for RHDH chart for the RHDHxOrchestrator flavor of RHDH. This PR will introduce changes to the Chart Values: upon enabling orchestrator, the RHDH chart will install the orchestrator-infra chart before the backstage chart.
The orchestrator-infra chart will incluse template files for serverless operator (knative eventing, serving) and openshift-serverless operator (sonataflow) that are pre-requisite infrastructure for Orchestrator to work.
Tests and documentation are TBA.
Relates to this epic: https://issues.redhat.com/browse/RHIDP-6159
Chart.yamlaccording to semver.values.yamland added to the README.md. The pre-commit utility can be used to generate the necessary content. Usepre-commit run -ato apply changes.pre-commithook.ct lintcommand.