Skip to content
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

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

Merged
6 changes: 1 addition & 5 deletions pkg/insights/insightsclient/insightsclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
configv1 "github.com/openshift/api/config/v1"
configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
"github.com/openshift/insights-operator/pkg/insights"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
apimachineryversion "k8s.io/apimachinery/pkg/version"
)
Expand Down Expand Up @@ -162,7 +161,7 @@ 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) GetClusterVersion() (*configv1.ClusterVersion, error) {
if c.clusterVersion != nil {
return c.clusterVersion, nil
}
Expand All @@ -174,9 +173,6 @@ func (c *Client) getClusterVersion() (*configv1.ClusterVersion, error) {
}

cv, err := gatherConfigClient.ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
if apierrors.IsNotFound(err) {
return nil, nil
}
if err != nil {
return nil, err
}
Expand Down
41 changes: 21 additions & 20 deletions pkg/insights/insightsclient/requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,18 @@ import (
"k8s.io/klog/v2"

"github.com/openshift/insights-operator/pkg/authorizer"
apierrors "k8s.io/apimachinery/pkg/api/errors"
)

// 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 apierrors.IsNotFound(err) {
return ErrWaitingForVersion
}
if err != nil {
return err
}
if cv == nil {
return ErrWaitingForVersion
}

req, err := c.prepareRequest(ctx, http.MethodPost, endpoint, cv)
if err != nil {
Expand Down Expand Up @@ -87,13 +88,13 @@ 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 apierrors.IsNotFound(err) {
return nil, ErrWaitingForVersion
}
if err != nil {
return nil, err
}
if cv == nil {
return nil, ErrWaitingForVersion
}

endpoint = fmt.Sprintf(endpoint, cv.Spec.ClusterID)
klog.Infof("Retrieving report for cluster: %s", cv.Spec.ClusterID)
Expand Down Expand Up @@ -169,13 +170,13 @@ 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 apierrors.IsNotFound(err) {
return nil, ErrWaitingForVersion
}
if err != nil {
return nil, err
}
if cv == nil {
return nil, ErrWaitingForVersion
}
token, err := c.authorizer.Token()
if err != nil {
return nil, err
Expand Down Expand Up @@ -213,13 +214,13 @@ 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 apierrors.IsNotFound(err) {
return nil, ErrWaitingForVersion
}
if err != nil {
return nil, err
}
if cv == nil {
return nil, ErrWaitingForVersion
}

req, err := c.prepareRequest(ctx, http.MethodGet, endpoint, cv)
if err != nil {
Expand Down Expand Up @@ -251,13 +252,13 @@ 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 apierrors.IsNotFound(err) {
return nil, ErrWaitingForVersion
}
if err != nil {
return nil, err
}
if cv == nil {
return nil, ErrWaitingForVersion
}
token, err := c.authorizer.Token()
if err != nil {
return nil, err
Expand Down
19 changes: 16 additions & 3 deletions pkg/insights/insightsreport/insightsreport.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"k8s.io/component-base/metrics"
"k8s.io/klog/v2"

configv1 "github.com/openshift/api/config/v1"
v1 "github.com/openshift/api/operator/v1"
operatorv1client "github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1"
"github.com/openshift/insights-operator/pkg/authorizer"
Expand All @@ -28,7 +29,7 @@ type Controller struct {
controllerstatus.StatusController

configurator configobserver.Configurator
client *insightsclient.Client
client insightsReportClient
LastReport types.SmartProxyReport
archiveUploadReporter <-chan struct{}
insightsOperatorCLI operatorv1client.InsightsOperatorInterface
Expand Down Expand Up @@ -243,7 +244,6 @@ func (c *Controller) Run(ctx context.Context) {
c.StatusController.UpdateStatus(controllerstatus.Summary{Healthy: true})
klog.V(2).Info("Starting report retriever")
klog.V(2).Infof("Initial config: %v", c.configurator.Config())

for {
// always wait for new uploaded archive or insights-operator ends
select {
Expand All @@ -261,6 +261,12 @@ type healthStatusCounts struct {
critical, important, moderate, low, total int
}

type insightsReportClient interface {
RecvReport(ctx context.Context, endpoint string) (*http.Response, error)
IncrementRecvReportMetric(statusCode int)
GetClusterVersion() (*configv1.ClusterVersion, error)
}

func (c *Controller) readInsightsReport(report types.SmartProxyReport) ([]types.InsightsRecommendation, healthStatusCounts, time.Time) {
healthStatus := healthStatusCounts{}
healthStatus.total = report.Meta.Count
Expand Down Expand Up @@ -291,6 +297,12 @@ func (c *Controller) readInsightsReport(report types.SmartProxyReport) ([]types.
klog.Errorf("Unable to extract recommendation's error key: %v", err)
continue
}
clusterVersion, err := c.client.GetClusterVersion()
if err != nil {
klog.Errorf("Unable to extract cluster version. Error: %v", err)
continue
}
insights.RecommendationCollector.SetClusterID(clusterVersion.Spec.ClusterID)

activeRecommendations = append(activeRecommendations, types.InsightsRecommendation{
RuleID: rule.RuleID,
Expand Down Expand Up @@ -327,6 +339,7 @@ func (c *Controller) updateOperatorStatusCR(report types.SmartProxyReport) error

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

for _, rule := range report.Data {
errorKey, err := extractErrorKeyFromRuleData(rule)
if err != nil {
Expand All @@ -338,7 +351,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", insights.RecommendationCollector.ClusterID(), ruleIDStr, errorKey),
}

if rule.Disabled {
Expand Down
30 changes: 30 additions & 0 deletions pkg/insights/insightsreport/insightsreport_test.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,26 @@
package insightsreport

import (
"context"
"fmt"
"net/http"
"testing"

v1 "github.com/openshift/api/config/v1"
"github.com/openshift/insights-operator/pkg/config"
"github.com/openshift/insights-operator/pkg/insights/types"
"github.com/stretchr/testify/assert"
)

func Test_readInsightsReport(t *testing.T) {
client := mockInsightsClient{
clusterVersion: &v1.ClusterVersion{
Spec: v1.ClusterVersionSpec{
ClusterID: v1.ClusterID("0000 0000 0000 0000"),
},
},
metricsName: "yeet",
}
tests := []struct {
name string
testController *Controller
Expand All @@ -24,6 +35,7 @@ func Test_readInsightsReport(t *testing.T) {
configurator: config.NewMockSecretConfigurator(&config.Controller{
DisableInsightsAlerts: false,
}),
client: &client,
},
report: types.SmartProxyReport{
Data: []types.RuleWithContentResponse{
Expand Down Expand Up @@ -111,6 +123,7 @@ func Test_readInsightsReport(t *testing.T) {
configurator: config.NewMockSecretConfigurator(&config.Controller{
DisableInsightsAlerts: false,
}),
client: &client,
},
report: types.SmartProxyReport{
Data: []types.RuleWithContentResponse{
Expand Down Expand Up @@ -186,6 +199,7 @@ func Test_readInsightsReport(t *testing.T) {
configurator: config.NewMockSecretConfigurator(&config.Controller{
DisableInsightsAlerts: true,
}),
client: &client,
},
report: types.SmartProxyReport{
Data: []types.RuleWithContentResponse{
Expand Down Expand Up @@ -295,3 +309,19 @@ func Test_extractErrorKeyFromRuleData(t *testing.T) {
})
}
}

type mockInsightsClient struct {
clusterVersion *v1.ClusterVersion
metricsName string
}

func (c *mockInsightsClient) GetClusterVersion() (*v1.ClusterVersion, error) {
return c.clusterVersion, nil
}

func (c *mockInsightsClient) IncrementRecvReportMetric(statusCode int) {
}

func (c *mockInsightsClient) RecvReport(ctx context.Context, endpoint string) (*http.Response, error) {
return nil, nil
}
12 changes: 11 additions & 1 deletion pkg/insights/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"strings"

semver "github.com/blang/semver/v4"
v1 "github.com/openshift/api/config/v1"
"github.com/openshift/insights-operator/pkg/insights/types"
"github.com/prometheus/client_golang/prometheus"
"k8s.io/component-base/metrics"
Expand Down Expand Up @@ -37,6 +38,15 @@ func init() {
type InsightsRecommendationCollector struct {
activeRecommendations []types.InsightsRecommendation
metricName string
clusterID v1.ClusterID
}

func (c *InsightsRecommendationCollector) SetClusterID(clusterID v1.ClusterID) {
c.clusterID = clusterID
}

func (c *InsightsRecommendationCollector) ClusterID() v1.ClusterID {
return c.clusterID
}

func (c *InsightsRecommendationCollector) SetActiveRecommendations(activeRecommendations []types.InsightsRecommendation) {
Expand All @@ -57,7 +67,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", c.clusterID, ruleIDStr, rec.ErrorKey),
}),
prometheus.GaugeValue,
1,
Expand Down