-
Notifications
You must be signed in to change notification settings - Fork 27
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
[doc] Adds operator deployment steps in OpenShift #132
base: main
Are you sure you want to change the base?
Conversation
Angie and Irina would be the best ones to review this. |
Signed-off-by: Alberto Losada <[email protected]>
b6b5360
to
89f950d
Compare
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Comments addressed. PTAL. |
i have tried to test the CRs in this PR for instance the metadata Kind and also the api version seems wrong. |
enabled: false | ||
cloudId: c0332915-6bff-4d8d-8628-0ab3cc2c7e5e | ||
deploymentManagerServerConfig: | ||
backendToken: ${BACKEND_TOKEN} |
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 backendToken
is not required. The server will automatically use its serviceaccount token to access other services; therefore this line and line 163 are not necessary.
backendToken: ${BACKEND_TOKEN} | ||
backendType: regular-hub | ||
enabled: true | ||
image: quay.io/openshift-kni/oran-o2ims:4.16.0 |
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 image
is also not necessary since the controller will inherit the value passed into its IMAGE
environment variable which is part of the default deployment and bundled deployment.
cloudId: c0332915-6bff-4d8d-8628-0ab3cc2c7e5e | ||
deploymentManagerServerConfig: | ||
backendToken: ${BACKEND_TOKEN} | ||
backendType: regular-hub |
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.
This is the default value for backendType
so not really necessary.
oc get route -n open-cluster-management search-api -o json | | ||
jq -r '"https://" + .spec.host' | ||
) | ||
$ export BACKEND_TOKEN_RS=$( |
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.
Same comment as above regarding the backend token. It is not necessary to specify one since the server will use its serviceaccount token automatically.
namespace/oran-o2ims created | ||
``` | ||
|
||
#### 1.2.1. <a name='Metadataserver'></a>Metadata server |
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.
This document has been structured to start the servers one at a time, but in practice that is not typically how it would be done. The Inventory CR would be added with all of the fields set to true
in one operation. There is no need to do them one at a time -- it really only inflates how many documentation lines that need to be managed and the number of steps to be tested. IMHO, it would be sufficient/best to simply 1) add the Inventory CR, and 2) verify the endpoints
@alosadagrande - it would be great if you could update this PR to address the comments and to incorporate the latest changes to how we deploy the operator. |
This PR updates documentation:
Let me know your thoughts.