DRAFT: Add Perses feature flag for the monitoring-console-plugin by zhuje · Pull Request #655 · rhobs/observability-operator

zhuje · 2025-01-09T23:48:16Z

/hold

Hold for COO 1.1.0, NOT to be released with COO 1.0.0

JIRA Issue

https://issues.redhat.com/browse/OU-571

Description

. Remove required properties of thanosQuerier and Alertmanager (because we want Perses to be able to be used without ACM)

bundle/manifests/observability.openshift.io_uiplugins.yam and
deploy/crds/common/observability.openshift.io_uiplugins.yaml

Add Perses-dashboards to the compatibility-matrix, which enables feature flags based on the conditions of the cluster. Perses-dashboards will only be available in OCP 4.19+.

pkg/controllers/uiplugin/compatibility_matrix.go
pkg/controllers/uiplugin/compatibility_matrix_test.go

Adjust Proxies to include Perses-dashboards.

pkg/controllers/uiplugin/monitoring.go
pkg/controllers/uiplugin/monitoring_test.go

Other supporting files

pkg/apis/uiplugin/v1alpha1/types.go
pkg/controllers/uiplugin/utils.go

OPEN QUESTIONS:

Where are we getting the perses-dashboard service endpoint? I need this for monitoring.go

if persesDashboardsFeatureEnabled && config.PersesDashboards.Url == "" {
		return nil, fmt.Errorf("PersesDashboards location can not be empty for plugin type %s", plugin.Spec.Type)
}
...
pluginInfo.ExtraArgs = append(pluginInfo.ExtraArgs, fmt.Sprintf("-perses-dashboards=%s", config.PersesDashboards.Url))

…rtManager. This is because we want to be able to have the options of creating a Perses proxy only and not include thanosQuerier and the alertManager.

QUESTION : where is the perses dashboard proxy URL coming from, we may need to adjust the code because right now it assumes it is in plugin.Spec.Montioring

openshift-ci-robot · 2025-01-09T23:48:20Z

@zhuje: This pull request references OU-571 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.19.0" version, but no target version was set.

Details

In response to this:

JIRA Issue

https://issues.redhat.com/browse/OU-571

Description

. Remove required properties of thanosQuerier and Alertmanager (because we want Perses to be able to be used without ACM)

bundle/manifests/observability.openshift.io_uiplugins.yaml and

deploy/crds/common/observability.openshift.io_uiplugins.yaml

Add Perses-dashboards to the compatibility-matrix, which enables feature flags based on the conditions of the cluster. Perses-dashboards will only be available in OCP 4.19+.

pkg/controllers/uiplugin/compatibility_matrix.go

pkg/controllers/uiplugin/compatibility_matrix_test.go

Adjust Proxies to include Perses-dashboards.

pkg/controllers/uiplugin/monitoring.go

pkg/controllers/uiplugin/monitoring_test.go

OPEN QUESTIONS:

Where are we getting the perses-dashboard service endpoint? I need this for monitoring.go
if persesDashboardsFeatureEnabled && config.PersesDashboards.Url == "" {
  	return nil, fmt.Errorf("PersesDashboards location can not be empty for plugin type %s", plugin.Spec.Type)
}
...
pluginInfo.ExtraArgs = append(pluginInfo.ExtraArgs, fmt.Sprintf("-perses-dashboards=%s", config.PersesDashboards.Url))
Other supporting files

pkg/apis/uiplugin/v1alpha1/types.go

pkg/controllers/uiplugin/utils.go

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2025-01-09T23:48:28Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zhuje
Once this PR has been reviewed and has the lgtm label, please assign jan--f for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2025-01-09T23:48:55Z

@zhuje: This pull request references OU-571 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.19.0" version, but no target version was set.

Details

In response to this:

/hold

Hold for COO 1.1.0, NOT to be released with COO 1.0.0

JIRA Issue

https://issues.redhat.com/browse/OU-571

Description

. Remove required properties of thanosQuerier and Alertmanager (because we want Perses to be able to be used without ACM)

bundle/manifests/observability.openshift.io_uiplugins.yaml and

deploy/crds/common/observability.openshift.io_uiplugins.yaml

Add Perses-dashboards to the compatibility-matrix, which enables feature flags based on the conditions of the cluster. Perses-dashboards will only be available in OCP 4.19+.

pkg/controllers/uiplugin/compatibility_matrix.go

pkg/controllers/uiplugin/compatibility_matrix_test.go

Adjust Proxies to include Perses-dashboards.

pkg/controllers/uiplugin/monitoring.go

pkg/controllers/uiplugin/monitoring_test.go

OPEN QUESTIONS:

Where are we getting the perses-dashboard service endpoint? I need this for monitoring.go
if persesDashboardsFeatureEnabled && config.PersesDashboards.Url == "" {
  	return nil, fmt.Errorf("PersesDashboards location can not be empty for plugin type %s", plugin.Spec.Type)
}
...
pluginInfo.ExtraArgs = append(pluginInfo.ExtraArgs, fmt.Sprintf("-perses-dashboards=%s", config.PersesDashboards.Url))
Other supporting files

pkg/apis/uiplugin/v1alpha1/types.go

pkg/controllers/uiplugin/utils.go

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2025-01-09T23:49:12Z

@zhuje: This pull request references OU-571 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.19.0" version, but no target version was set.

Details

In response to this:

/hold

Hold for COO 1.1.0, NOT to be released with COO 1.0.0

JIRA Issue

https://issues.redhat.com/browse/OU-571

Description

. Remove required properties of thanosQuerier and Alertmanager (because we want Perses to be able to be used without ACM)

bundle/manifests/observability.openshift.io_uiplugins.yaml and

deploy/crds/common/observability.openshift.io_uiplugins.yaml

Add Perses-dashboards to the compatibility-matrix, which enables feature flags based on the conditions of the cluster. Perses-dashboards will only be available in OCP 4.19+.

pkg/controllers/uiplugin/compatibility_matrix.go

pkg/controllers/uiplugin/compatibility_matrix_test.go

Adjust Proxies to include Perses-dashboards.

pkg/controllers/uiplugin/monitoring.go

pkg/controllers/uiplugin/monitoring_test.go

Other supporting files

pkg/apis/uiplugin/v1alpha1/types.go

pkg/controllers/uiplugin/utils.go

OPEN QUESTIONS:

Where are we getting the perses-dashboard service endpoint? I need this for monitoring.go
if persesDashboardsFeatureEnabled && config.PersesDashboards.Url == "" {
  	return nil, fmt.Errorf("PersesDashboards location can not be empty for plugin type %s", plugin.Spec.Type)
}
...
pluginInfo.ExtraArgs = append(pluginInfo.ExtraArgs, fmt.Sprintf("-perses-dashboards=%s", config.PersesDashboards.Url))

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2025-01-09T23:56:54Z

@zhuje: This pull request references OU-571 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.19.0" version, but no target version was set.

Details

In response to this:

/hold

Hold for COO 1.1.0, NOT to be released with COO 1.0.0

JIRA Issue

https://issues.redhat.com/browse/OU-571

Description

. Remove required properties of thanosQuerier and Alertmanager (because we want Perses to be able to be used without ACM)

bundle/manifests/observability.openshift.io_uiplugins.yam and

deploy/crds/common/observability.openshift.io_uiplugins.yaml

Add Perses-dashboards to the compatibility-matrix, which enables feature flags based on the conditions of the cluster. Perses-dashboards will only be available in OCP 4.19+.

pkg/controllers/uiplugin/compatibility_matrix.go

pkg/controllers/uiplugin/compatibility_matrix_test.go

Adjust Proxies to include Perses-dashboards.

pkg/controllers/uiplugin/monitoring.go

pkg/controllers/uiplugin/monitoring_test.go

Other supporting files

pkg/apis/uiplugin/v1alpha1/types.go

pkg/controllers/uiplugin/utils.go

OPEN QUESTIONS:

Where are we getting the perses-dashboard service endpoint? I need this for monitoring.go
if persesDashboardsFeatureEnabled && config.PersesDashboards.Url == "" {
  	return nil, fmt.Errorf("PersesDashboards location can not be empty for plugin type %s", plugin.Spec.Type)
}
...
pluginInfo.ExtraArgs = append(pluginInfo.ExtraArgs, fmt.Sprintf("-perses-dashboards=%s", config.PersesDashboards.Url))

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 openshift-eng/jira-lifecycle-plugin repository.

PeterYurkovich · 2025-01-10T01:39:33Z

pkg/apis/uiplugin/v1alpha1/types.go

+
+	// PersesDashboards points to the perses-dashboards service of which it should create a proxy to.
+	//
+	// +kubebuilder:validation:Required


In general when working with kubebuilder configs you should try to make adjustments in the the go file and then run the make generate command to create the yaml for you. Here you will need to remove the required validations in this file for the monitoring options.

Updated to // +kubebuilder:validation:Optional then ran make generate and make bundle to update the uiplugin CRDs

PeterYurkovich · 2025-01-10T01:45:04Z

pkg/controllers/uiplugin/monitoring.go

+	if persesDashboardsFeatureEnabled {
+		// JZ OPEN QUESTION: Need to align on PORT number, name, and namespace...with Perses Operator?
+		proxy := osv1.ConsolePluginProxy{
+			Alias:         "perses-dashboards-proxy",


Proxy name should just be perses based on how the monitoring-plugin links are structured (link)

Updated the Alias: "perses" instead of "perses-dashboard-proxy".

PeterYurkovich · 2025-01-10T01:48:12Z

pkg/controllers/uiplugin/monitoring.go

+			},
+		}
+		pluginInfo.ExtraArgs = append(pluginInfo.ExtraArgs, fmt.Sprintf("-perses-dashboards=%s", config.PersesDashboards.Url))
+		pluginInfo.Proxies = append(pluginInfo.Proxies, proxy)


NIT: Rather than assigning to a variable can we just directly put the definition in the append call.

What do you mean? Can you write an example?

pluginInfo.ExtraArgs = append(pluginInfo.ExtraArgs, fmt.Sprintf("-perses-dashboards=%s", config.PersesDashboards.Url)) pluginInfo.Proxies = append(pluginInfo.Proxies, osv1.ConsolePluginProxy{ Alias: "perses-dashboards-proxy", Authorization: "UserToken", Endpoint: osv1.ConsolePluginProxyEndpoint{ Type: osv1.ProxyTypeService, Service: &osv1.ConsolePluginProxyServiceConfig{ Name: name, Namespace: namespace, Port: 9446, }, }, }) pluginInfo.LegacyProxies = append(pluginInfo.LegacyProxies, osv1alpha1.ConsolePluginProxy{ Type: "Service", Alias: "perses-dashboards-proxy", Authorize: true, Service: osv1alpha1.ConsolePluginProxyServiceConfig{ Name: name, Namespace: namespace, Port: 9446, }, })

Makes sense, ty! Updated created the objects directly in the append() call

PeterYurkovich · 2025-01-10T01:50:29Z

pkg/controllers/uiplugin/utils.go

@@ -0,0 +1,10 @@
+package uiplugin
+
+func contains(array []string, value string) bool {


As of version 1.18, go now contains a slices.contains method so we no longer need to write one ourselves

ty, updated!

…ersesDashboards are not required attributes. Then run `make generate` to update CRDs (rather than manual editing).

…lity.openshift.io_uiplugins.yaml

…" instead of "perses-dashboard-proxy".

… rather than a URL. This because COO controls Perses Operator so COO just needs the ServiceName and Namespace to fetch the Perses Service. Requiring the exact endpoint is less flexible.

…d the name, namespace, and port to connect to the Perses Service.

zhuje · 2025-01-13T21:15:06Z

pkg/controllers/uiplugin/monitoring.go

+			Endpoint: osv1.ConsolePluginProxyEndpoint{
+				Type: osv1.ProxyTypeService,
+				Service: &osv1.ConsolePluginProxyServiceConfig{
+					Name:      name,


@jgbernalp @PeterYurkovich Is it necessary to require the PersesDashboardsReference > serviceName in the types.go? I think Service name and namespace are already passed in the props here...

Those are the name and namespace of the monitoring-plugin, not the perses service

Ahhhh okay.

…wn contains() function.

…traArgs' in the PluginInfo

…viceName instead of PersesDashboards.Url should be present in UIPlugin CR

…urce. Update createMonitoringPluginInfo to include name (of service) and namespace. Update CRD (custom resource defintiion) to reflect this too.

openshift-ci-robot · 2025-01-30T01:57:06Z

@zhuje: No Jira issue is referenced in the title of this pull request.
To reference a jira issue, add 'XYZ-NNN:' to the title of this pull request and request another refresh with /jira refresh.

Details

In response to this:

/hold

Hold for COO 1.1.0, NOT to be released with COO 1.0.0

JIRA Issue

https://issues.redhat.com/browse/OU-571

Description

. Remove required properties of thanosQuerier and Alertmanager (because we want Perses to be able to be used without ACM)

bundle/manifests/observability.openshift.io_uiplugins.yam and

deploy/crds/common/observability.openshift.io_uiplugins.yaml

Add Perses-dashboards to the compatibility-matrix, which enables feature flags based on the conditions of the cluster. Perses-dashboards will only be available in OCP 4.19+.

pkg/controllers/uiplugin/compatibility_matrix.go

pkg/controllers/uiplugin/compatibility_matrix_test.go

Adjust Proxies to include Perses-dashboards.

pkg/controllers/uiplugin/monitoring.go

pkg/controllers/uiplugin/monitoring_test.go

Other supporting files

pkg/apis/uiplugin/v1alpha1/types.go

pkg/controllers/uiplugin/utils.go

OPEN QUESTIONS:

Where are we getting the perses-dashboard service endpoint? I need this for monitoring.go
if persesDashboardsFeatureEnabled && config.PersesDashboards.Url == "" {
  	return nil, fmt.Errorf("PersesDashboards location can not be empty for plugin type %s", plugin.Spec.Type)
}
...
pluginInfo.ExtraArgs = append(pluginInfo.ExtraArgs, fmt.Sprintf("-perses-dashboards=%s", config.PersesDashboards.Url))

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 openshift-eng/jira-lifecycle-plugin repository.

zhuje · 2025-01-30T02:30:50Z

Closed because moved to new PR : #664

zhuje added 3 commits January 7, 2025 15:54

Remove Requirements for UIPlugin > monitoring > thanosQuerier and ale…

d4c52f4

…rtManager. This is because we want to be able to have the options of creating a Perses proxy only and not include thanosQuerier and the alertManager.

Add perses-dashboards to compatbility matrix and test

4055b0e

Add Perses Dashboards Proxy

5738f0c

QUESTION : where is the perses dashboard proxy URL coming from, we may need to adjust the code because right now it assumes it is in plugin.Spec.Montioring

zhuje requested a review from a team as a code owner January 9, 2025 23:48

zhuje requested review from jan--f and sthaha and removed request for a team January 9, 2025 23:48

openshift-ci-robot added the jira/valid-reference label Jan 9, 2025

openshift-ci bot requested review from jgbernalp and marioferh January 9, 2025 23:48

zhuje marked this pull request as draft January 9, 2025 23:48

openshift-ci bot added the do-not-merge/work-in-progress label Jan 9, 2025

PeterYurkovich requested changes Jan 10, 2025

View reviewed changes

openshift-ci bot assigned PeterYurkovich Jan 10, 2025

zhuje added 6 commits January 13, 2025 11:10

Adjust kubebuilder configs so that Alertmanager, Thanos Querier and P…

ea5d2a2

…ersesDashboards are not required attributes. Then run `make generate` to update CRDs (rather than manual editing).

Run make bundle to update uiplugins CRD /bundle/manifests/observabi…

cacd545

…lity.openshift.io_uiplugins.yaml

Update UIPlugin controller to use Service and UserToken Alias:"perses…

0a435b9

…" instead of "perses-dashboard-proxy".

Update UIPlugin CRDs so that PersesDashboards require a 'serviceName'…

d97b6a5

… rather than a URL. This because COO controls Perses Operator so COO just needs the ServiceName and Namespace to fetch the Perses Service. Requiring the exact endpoint is less flexible.

Remove the need for "config.PersesDashboards.Url" because we only nee…

fc32261

…d the name, namespace, and port to connect to the Perses Service.

Update monitoring_test.go to align with Alias name change "perses".

75dce84

zhuje commented Jan 13, 2025

View reviewed changes

zhuje added 4 commits January 13, 2025 16:28

Utilize library "slices" for "slices.Contains" instead of using our o…

d06ca79

…wn contains() function.

Update monitoring.go to append PersesDashboards.ServiceName as an 'Ex…

1bd6d05

…traArgs' in the PluginInfo

Update montiroing_test.go to align with changes, PersesDashboards.Ser…

cc65187

…viceName instead of PersesDashboards.Url should be present in UIPlugin CR

observability-ui-plugins.md to include Perses in UIPlugin Custom Reso…

db53dc1

…urce. Update createMonitoringPluginInfo to include name (of service) and namespace. Update CRD (custom resource defintiion) to reflect this too.

BROKEN add log statements -- won't work on cluster

c3938bd

zhuje changed the title ~~DRAFT: OU-571: Add Perses feature flag for the monitoring-console-plugin~~ DRAFT: Add Perses feature flag for the monitoring-console-plugin Jan 30, 2025

openshift-ci-robot removed the jira/valid-reference label Jan 30, 2025

zhuje closed this Jan 30, 2025

		@@ -0,0 +1,10 @@
		package uiplugin

		func contains(array []string, value string) bool {

Conversation

zhuje commented Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

JIRA Issue

Description

Uh oh!

openshift-ci-robot commented Jan 9, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

JIRA Issue

Description

Uh oh!

openshift-ci bot commented Jan 9, 2025

Uh oh!

openshift-ci-robot commented Jan 9, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

JIRA Issue

Description

Uh oh!

openshift-ci-robot commented Jan 9, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

JIRA Issue

Description

Uh oh!

openshift-ci-robot commented Jan 9, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

JIRA Issue

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PeterYurkovich Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhuje Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhuje Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Jan 30, 2025

JIRA Issue

Description

Uh oh!

zhuje commented Jan 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zhuje commented Jan 9, 2025 •

edited

Loading

openshift-ci-robot commented Jan 9, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Jan 9, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Jan 9, 2025 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Jan 9, 2025 •

edited by openshift-ci bot

Loading

PeterYurkovich Jan 13, 2025 •

edited

Loading

zhuje Jan 13, 2025 •

edited

Loading

zhuje Jan 13, 2025 •

edited

Loading