Add prometheus service to console plugin cr #181
Conversation
aruniiird
force-pushed
the
add-prometheus-service-to-console-plugin-cr
branch
2 times, most recently
from
7a30e6d to
9543092
Compare
aruniiird
changed the title
|
Rebased on top of latest main branch, which has PR#175 merged. |
controllers/managedocs_controller.go
Outdated
| @@ -1752,6 +1759,41 @@ func (r *ManagedOCSReconciler) updateMCGCSV(csv *opv1a1.ClusterServiceVersion) e | |||
| return nil | |||
| } | |||
|
|
|||
| func (r *ManagedOCSReconciler) reconcileConsolePlugin() error { | |||
There was a problem hiding this comment.
Do we need to run this reconcile func in all deployment modes? (converged, consumer, provider).
If not, please add an if at the start of the function to immediately exist if the deployment type does not match (see other reconcile funcs for reference)
There was a problem hiding this comment.
Will check and update
There was a problem hiding this comment.
Console UI plugin is only used in provider / consumer type deployment. The only difference (AFAIU) is in consumer mode both SRE and customers are able to view the dashboard and in provider deployment type only SREs are able to access the console UI. So added the check for the above deployment-types in the beginning of the function.
There was a problem hiding this comment.
-SREs are not logging into the OCP console so this is not relevant for provider mode as well.
Please add an early exit if not in consumer mode (you can find examples in other existing reconcile functions)
aruniiird
force-pushed
the
add-prometheus-service-to-console-plugin-cr
branch
from
9543092 to
8dae6ad
Compare
|
@aruniiird Please rename the template to be more specific and match the casing convention we have for templates File/Directory name casing is important when some contributed are developing on an OS/FS that is case-sensitive |
|
@aruniiird Can we even merge this PR? what will happen if we run this on an OCP 4.10/4.9/4.8 clusters? |
aruniiird
force-pushed
the
add-prometheus-service-to-console-plugin-cr
branch
from
8dae6ad to
572e521
Compare
aruniiird
force-pushed
the
add-prometheus-service-to-console-plugin-cr
branch
from
572e521 to
35b09fa
Compare
aruniiird
force-pushed
the
add-prometheus-service-to-console-plugin-cr
branch
5 times, most recently
from
389121e to
536cdef
Compare
templates/ocsconsoleplugin.go
Outdated
| ) | ||
|
|
||
| const ( | ||
| ocpConsoleProxyAlias = "consoleproxy" |
There was a problem hiding this comment.
| ocpConsoleProxyAlias = "consoleproxy" | |
| ocpConsoleProxyAlias = "odf-managed-service-prom" |
Please use this. This is something we discussed before and is used in the UI as well.
aruniiird
force-pushed
the
add-prometheus-service-to-console-plugin-cr
branch
2 times, most recently
from
cc2a01b to
77452f6
Compare
|
hey @aruniiird / @SanjalKatiyar is this PR related to https://issues.redhat.com/browse/ODFMS-281? we are about to start work on this task. cc: @rchikatw |
hey... |
Signed-off-by: Arun Kumar Mohan <[email protected]>
Signed-off-by: Arun Kumar Mohan <[email protected]>
Signed-off-by: Arun Kumar Mohan <[email protected]>
aruniiird
force-pushed
the
add-prometheus-service-to-console-plugin-cr
branch
from
77452f6 to
e58d9b2
Compare
|
/hold |
openshift-merge-robot
added
the
needs-rebase
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
This PR patches ConsolePlugin CR, which is generated by ODF operator, with additional ConsolePluginProxy details of Prometheus service, to which console has to connect.
This PR depends on PR #175