Skip to content

OCPBUGS-2915: Updated info link in insights recommendations #683

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

Merged
Merged
7 changes: 6 additions & 1 deletion pkg/insights/insightsclient/insightsclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,12 @@ func userAgent(releaseVersionEnv string, v apimachineryversion.Info, cv *configv
return fmt.Sprintf("insights-operator/%s cluster/%s", gitVersion, cv.Spec.ClusterID)
}

func (c *Client) getClusterVersion() (*configv1.ClusterVersion, error) {
func (c *Client) SetClusterVersion(cv *configv1.ClusterVersion) *Client {
c.clusterVersion = cv
return c
}

func (c *Client) GetClusterVersion() (*configv1.ClusterVersion, error) {
if c.clusterVersion != nil {
return c.clusterVersion, nil
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/insights/insightsclient/requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (

// Send uploads archives to Ingress service
func (c *Client) Send(ctx context.Context, endpoint string, source Source) error {
cv, err := c.getClusterVersion()
cv, err := c.GetClusterVersion()
if err != nil {
return err
}
Expand Down Expand Up @@ -87,7 +87,7 @@ func (c *Client) Send(ctx context.Context, endpoint string, source Source) error

// RecvReport performs a request to Insights Results Smart Proxy endpoint
func (c *Client) RecvReport(ctx context.Context, endpoint string) (*http.Response, error) {
cv, err := c.getClusterVersion()
cv, err := c.GetClusterVersion()
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -169,7 +169,7 @@ func (c *Client) RecvReport(ctx context.Context, endpoint string) (*http.Respons
}

func (c *Client) RecvSCACerts(_ context.Context, endpoint string) ([]byte, error) {
cv, err := c.getClusterVersion()
cv, err := c.GetClusterVersion()
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -213,7 +213,7 @@ func (c *Client) RecvGatheringRules(ctx context.Context, endpoint string) ([]byt
klog.Infof(
`Preparing a request to Insights Operator Gathering Conditions Service at the endpoint "%v"`, endpoint,
)
cv, err := c.getClusterVersion()
cv, err := c.GetClusterVersion()
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -251,7 +251,7 @@ func (c *Client) RecvGatheringRules(ctx context.Context, endpoint string) ([]byt
// It is a HTTP GET request with the `search` query parameter limiting the result
// only for the one cluster and only for the `accepted` cluster transfers.
func (c *Client) RecvClusterTransfer(endpoint string) ([]byte, error) {
cv, err := c.getClusterVersion()
cv, err := c.GetClusterVersion()
if err != nil {
return nil, err
}
Expand Down
25 changes: 21 additions & 4 deletions pkg/insights/insightsreport/insightsreport.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,11 @@ func (c *Controller) PullSmartProxy() (bool, error) {
return true, fmt.Errorf("report not updated")
}

recommendations, healthStatus, gatherTime := c.readInsightsReport(reportResponse.Report)
recommendations, healthStatus, gatherTime, err := c.readInsightsReport(reportResponse.Report)
if err != nil {
klog.Errorf("failed to read the Insights Operator Report")
return true, err
}
updateInsightsMetrics(recommendations, healthStatus, gatherTime)
err = c.updateOperatorStatusCR(reportResponse.Report)
if err != nil {
Expand Down Expand Up @@ -261,11 +265,17 @@ type healthStatusCounts struct {
critical, important, moderate, low, total int
}

func (c *Controller) readInsightsReport(report types.SmartProxyReport) ([]types.InsightsRecommendation, healthStatusCounts, time.Time) {
func (c *Controller) readInsightsReport(report types.SmartProxyReport) ([]types.InsightsRecommendation, healthStatusCounts, time.Time, error) {
healthStatus := healthStatusCounts{}
healthStatus.total = report.Meta.Count
activeRecommendations := []types.InsightsRecommendation{}

clusterVersion, err := c.client.GetClusterVersion()
if err != nil {
klog.Errorf("Unable to extract cluster version: %v", err)
return nil, healthStatus, time.Time{}, err
}

for _, rule := range report.Data {
if rule.Disabled {
// total also includes disabled rules
Expand Down Expand Up @@ -297,14 +307,15 @@ func (c *Controller) readInsightsReport(report types.SmartProxyReport) ([]types.
ErrorKey: errorKeyStr,
Description: rule.Description,
TotalRisk: rule.TotalRisk,
ClusterID: clusterVersion.Spec.ClusterID,
})
}

t, err := time.Parse(time.RFC3339, string(report.Meta.GatheredAt))
if err != nil {
klog.Errorf("Metric %s not updated. Failed to parse time: %v", insightsLastGatherTimeName, err)
}
return activeRecommendations, healthStatus, t
return activeRecommendations, healthStatus, t, nil
}

// updateInsightsMetrics update the Prometheus metrics from a report
Expand All @@ -327,6 +338,12 @@ func (c *Controller) updateOperatorStatusCR(report types.SmartProxyReport) error

updatedOperatorCR := insightsOperatorCR.DeepCopy()
var healthChecks []v1.HealthCheck

clusterVersion, err := c.client.GetClusterVersion()
if err != nil {
return err
}

for _, rule := range report.Data {
errorKey, err := extractErrorKeyFromRuleData(rule)
if err != nil {
Expand All @@ -338,7 +355,7 @@ func (c *Controller) updateOperatorStatusCR(report types.SmartProxyReport) error
Description: rule.Description,
TotalRisk: int32(rule.TotalRisk),
State: v1.HealthCheckEnabled,
AdvisorURI: fmt.Sprintf("https://console.redhat.com/openshift/insights/advisor/recommendations/%s%%7C%s", ruleIDStr, errorKey),
AdvisorURI: fmt.Sprintf("https://console.redhat.com/openshift/insights/advisor/clusters/%s?first=%s|%s", clusterVersion.Spec.ClusterID, ruleIDStr, errorKey),
}

if rule.Disabled {
Expand Down
26 changes: 25 additions & 1 deletion pkg/insights/insightsreport/insightsreport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,27 @@ import (
"fmt"
"testing"

v1 "github.com/openshift/api/config/v1"
"github.com/openshift/insights-operator/pkg/config"
"github.com/openshift/insights-operator/pkg/insights/insightsclient"
"github.com/openshift/insights-operator/pkg/insights/types"
"github.com/stretchr/testify/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func Test_readInsightsReport(t *testing.T) {
tclient := &insightsclient.Client{}
tclient.SetClusterVersion(
&v1.ClusterVersion{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{},
Spec: v1.ClusterVersionSpec{
ClusterID: v1.ClusterID("0000 0000 0000 0000"),
},
Status: v1.ClusterVersionStatus{},
},
)

tests := []struct {
name string
testController *Controller
Expand All @@ -24,6 +39,7 @@ func Test_readInsightsReport(t *testing.T) {
configurator: config.NewMockSecretConfigurator(&config.Controller{
DisableInsightsAlerts: false,
}),
client: tclient,
},
report: types.SmartProxyReport{
Data: []types.RuleWithContentResponse{
Expand Down Expand Up @@ -76,24 +92,28 @@ func Test_readInsightsReport(t *testing.T) {
ErrorKey: "test error key 1",
Description: "test rule description 1",
TotalRisk: 2,
ClusterID: v1.ClusterID("0000 0000 0000 0000"),
},
{
RuleID: "ccx.dev.super.recommendation",
ErrorKey: "test error key 2",
Description: "test rule description 2",
TotalRisk: 1,
ClusterID: v1.ClusterID("0000 0000 0000 0000"),
},
{
RuleID: "ccx.dev.cool.recommendation",
ErrorKey: "test error key 3",
Description: "test rule description 3",
TotalRisk: 3,
ClusterID: v1.ClusterID("0000 0000 0000 0000"),
},
{
RuleID: "ccx.dev.ultra.recommendation",
ErrorKey: "test error key 4",
Description: "test rule description 4",
TotalRisk: 1,
ClusterID: v1.ClusterID("0000 0000 0000 0000"),
},
},
expectedHealthStatus: healthStatusCounts{
Expand All @@ -111,6 +131,7 @@ func Test_readInsightsReport(t *testing.T) {
configurator: config.NewMockSecretConfigurator(&config.Controller{
DisableInsightsAlerts: false,
}),
client: tclient,
},
report: types.SmartProxyReport{
Data: []types.RuleWithContentResponse{
Expand Down Expand Up @@ -163,12 +184,14 @@ func Test_readInsightsReport(t *testing.T) {
ErrorKey: "test error key 1",
Description: "test rule description 1",
TotalRisk: 2,
ClusterID: v1.ClusterID("0000 0000 0000 0000"),
},
{
RuleID: "ccx.dev.cool.recommendation",
ErrorKey: "test error key 3",
Description: "test rule description 3",
TotalRisk: 3,
ClusterID: v1.ClusterID("0000 0000 0000 0000"),
},
},
expectedHealthStatus: healthStatusCounts{
Expand All @@ -186,6 +209,7 @@ func Test_readInsightsReport(t *testing.T) {
configurator: config.NewMockSecretConfigurator(&config.Controller{
DisableInsightsAlerts: true,
}),
client: tclient,
},
report: types.SmartProxyReport{
Data: []types.RuleWithContentResponse{
Expand Down Expand Up @@ -228,7 +252,7 @@ func Test_readInsightsReport(t *testing.T) {

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
activeRecommendations, healthStatus, gatherTime := tc.testController.readInsightsReport(tc.report)
activeRecommendations, healthStatus, gatherTime, _ := tc.testController.readInsightsReport(tc.report)
assert.Equal(t, tc.expectedActiveRecommendations, activeRecommendations)
assert.Equal(t, tc.expectedHealthStatus, healthStatus)
assert.Equal(t, tc.expectedGatherTime, gatherTime.String())
Expand Down
2 changes: 1 addition & 1 deletion pkg/insights/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (c *InsightsRecommendationCollector) Collect(ch chan<- prometheus.Metric) {
prometheus.NewDesc(c.metricName, "", []string{}, prometheus.Labels{
"description": rec.Description,
"total_risk": totalRiskToStr(rec.TotalRisk),
"info_link": fmt.Sprintf("https://console.redhat.com/openshift/insights/advisor/recommendations/%s%%7C%s", ruleIDStr, rec.ErrorKey),
"info_link": fmt.Sprintf("https://console.redhat.com/openshift/insights/advisor/clusters/%s?first=%s|%s", rec.ClusterID, ruleIDStr, rec.ErrorKey),
}),
prometheus.GaugeValue,
1,
Expand Down
3 changes: 3 additions & 0 deletions pkg/insights/types/types.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package types

import v1 "github.com/openshift/api/config/v1"

// Timestamp represents any timestamp in a form gathered from database
type Timestamp string

Expand Down Expand Up @@ -56,4 +58,5 @@ type InsightsRecommendation struct {
ErrorKey string
Description string
TotalRisk int
ClusterID v1.ClusterID
}