-
Notifications
You must be signed in to change notification settings - Fork 22
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
Use backstage-operator to install backstage #61
Conversation
@jianrongzhang89 can you have a look and see if this is the correct way to go? |
f7f64e4
to
b67dfa0
Compare
When the RHDH operator is released, it will be a certified Red Hat operator. It will NOT be released as an community operator. |
b67dfa0
to
f3f3d70
Compare
@masayag I tried to install the chart based on the instruction but got error: Error: INSTALLATION FAILED: no SonataFlowPlatform with the name "sonataflow-platform" found |
@jianrongzhang89 just to make sure I'm reproducing the same setup, did you create the DB before? |
f3f3d70
to
e8634f6
Compare
Yes the DB was created. Did you test the PR on a fresh cluster? |
not on a fresh cluster. but made sure to clean the resources. I'll try on a fresh one. |
If you test on an existing cluster, make sure you remove all related CRDs before rerun your test. Note that when an operator is undeployed, the CRDs remain in the cluster by default. |
Yes, you will need to create a RHDH Backstage custom resource (after the operator is deployed) in order to deploy Backstage. |
def42bf
to
6f63f86
Compare
With the updated version of the chart I'm able to get the backstage instance to start, but it fails to load the orchestrator and the notification plugins. I'll check this earliest next week. However, one thing I could bypass is the environment set always to production, no matter what type of method I tried to override it. |
8d7b0dd
to
47e3979
Compare
abe9656
to
55d8f74
Compare
59f322a
to
00cc8ef
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.
/lgtm
metadata: | ||
name: rhdh-operator | ||
namespace: {{ .Values.rhdhOperator.subscription.namespace }} | ||
spec: |
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 you want to simplify the template and the documentation and, at the same time, provide more flexibility so that new sub-fields can be added if needed, you can just have a single structured JSON object representing the full configuration and then install it as:
spec:
{{ toYaml .Values.subscription.spec | indent 2 }}
Then you can provide a default in your values.yaml with a sample data. (in the above example should be under rhdhOperator.subscription.spec
, of course you can name it as you prefer
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'm going to leave this one as is for now since the other operators follow the same method. We can re-work this in the future to have the same way to configure all of the components.
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.
lgtm, will approve after a quick validation on my cluster
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.
validated locally: apart from the notifications which are broken (as expected, IIUC), all looks good.
00cc8ef
to
3a2628b
Compare
New changes are detected. LGTM label has been removed. |
Signed-off-by: Moti Asayag <masayag@redhat.com>
3a2628b
to
6e3f5e0
Compare
The PR changes the installation method of Janus-IDP from helm-based to an operator-based.
The configuration for backstage is split between several configmaps according to the sections in app-config.