Skip to content

Commit c2c6875

Browse files
rhrmorhrmo
authored
OCPBUGS-2915: Updated info link in insights recommendations (#683)
* Updated info link in insights recommendations * updated AdvisorURI in insightsreport * unit tests update * updated code in case of GetClusterVersion() error * small fix in recommendation link * reworking how info link gets cluster ID * update of GetClusterVersion() in case of error * Updated where clusterID is being set, created interface for mocking insightsClient inside unit tests * updated interface name and noop-ed function in unit tests
1 parent 04a37c2 commit c2c6875

File tree

5 files changed

+79
-29
lines changed

5 files changed

+79
-29
lines changed

‎pkg/insights/insightsclient/insightsclient.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import (
2727
configv1 "github.com/openshift/api/config/v1"
2828
configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
2929
"github.com/openshift/insights-operator/pkg/insights"
30-
apierrors "k8s.io/apimachinery/pkg/api/errors"
3130
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3231
apimachineryversion "k8s.io/apimachinery/pkg/version"
3332
)
@@ -162,7 +161,7 @@ func userAgent(releaseVersionEnv string, v apimachineryversion.Info, cv *configv
162161
return fmt.Sprintf("insights-operator/%s cluster/%s", gitVersion, cv.Spec.ClusterID)
163162
}
164163

165-
func (c *Client) getClusterVersion() (*configv1.ClusterVersion, error) {
164+
func (c *Client) GetClusterVersion() (*configv1.ClusterVersion, error) {
166165
if c.clusterVersion != nil {
167166
return c.clusterVersion, nil
168167
}
@@ -174,9 +173,6 @@ func (c *Client) getClusterVersion() (*configv1.ClusterVersion, error) {
174173
}
175174

176175
cv, err := gatherConfigClient.ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
177-
if apierrors.IsNotFound(err) {
178-
return nil, nil
179-
}
180176
if err != nil {
181177
return nil, err
182178
}

‎pkg/insights/insightsclient/requests.go

+21-20
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,18 @@ import (
1212
"k8s.io/klog/v2"
1313

1414
"github.com/openshift/insights-operator/pkg/authorizer"
15+
apierrors "k8s.io/apimachinery/pkg/api/errors"
1516
)
1617

1718
// Send uploads archives to Ingress service
1819
func (c *Client) Send(ctx context.Context, endpoint string, source Source) error {
19-
cv, err := c.getClusterVersion()
20+
cv, err := c.GetClusterVersion()
21+
if apierrors.IsNotFound(err) {
22+
return ErrWaitingForVersion
23+
}
2024
if err != nil {
2125
return err
2226
}
23-
if cv == nil {
24-
return ErrWaitingForVersion
25-
}
2627

2728
req, err := c.prepareRequest(ctx, http.MethodPost, endpoint, cv)
2829
if err != nil {
@@ -87,13 +88,13 @@ func (c *Client) Send(ctx context.Context, endpoint string, source Source) error
8788

8889
// RecvReport performs a request to Insights Results Smart Proxy endpoint
8990
func (c *Client) RecvReport(ctx context.Context, endpoint string) (*http.Response, error) {
90-
cv, err := c.getClusterVersion()
91+
cv, err := c.GetClusterVersion()
92+
if apierrors.IsNotFound(err) {
93+
return nil, ErrWaitingForVersion
94+
}
9195
if err != nil {
9296
return nil, err
9397
}
94-
if cv == nil {
95-
return nil, ErrWaitingForVersion
96-
}
9798

9899
endpoint = fmt.Sprintf(endpoint, cv.Spec.ClusterID)
99100
klog.Infof("Retrieving report for cluster: %s", cv.Spec.ClusterID)
@@ -169,13 +170,13 @@ func (c *Client) RecvReport(ctx context.Context, endpoint string) (*http.Respons
169170
}
170171

171172
func (c *Client) RecvSCACerts(_ context.Context, endpoint string) ([]byte, error) {
172-
cv, err := c.getClusterVersion()
173+
cv, err := c.GetClusterVersion()
174+
if apierrors.IsNotFound(err) {
175+
return nil, ErrWaitingForVersion
176+
}
173177
if err != nil {
174178
return nil, err
175179
}
176-
if cv == nil {
177-
return nil, ErrWaitingForVersion
178-
}
179180
token, err := c.authorizer.Token()
180181
if err != nil {
181182
return nil, err
@@ -213,13 +214,13 @@ func (c *Client) RecvGatheringRules(ctx context.Context, endpoint string) ([]byt
213214
klog.Infof(
214215
`Preparing a request to Insights Operator Gathering Conditions Service at the endpoint "%v"`, endpoint,
215216
)
216-
cv, err := c.getClusterVersion()
217+
cv, err := c.GetClusterVersion()
218+
if apierrors.IsNotFound(err) {
219+
return nil, ErrWaitingForVersion
220+
}
217221
if err != nil {
218222
return nil, err
219223
}
220-
if cv == nil {
221-
return nil, ErrWaitingForVersion
222-
}
223224

224225
req, err := c.prepareRequest(ctx, http.MethodGet, endpoint, cv)
225226
if err != nil {
@@ -251,13 +252,13 @@ func (c *Client) RecvGatheringRules(ctx context.Context, endpoint string) ([]byt
251252
// It is a HTTP GET request with the `search` query parameter limiting the result
252253
// only for the one cluster and only for the `accepted` cluster transfers.
253254
func (c *Client) RecvClusterTransfer(endpoint string) ([]byte, error) {
254-
cv, err := c.getClusterVersion()
255+
cv, err := c.GetClusterVersion()
256+
if apierrors.IsNotFound(err) {
257+
return nil, ErrWaitingForVersion
258+
}
255259
if err != nil {
256260
return nil, err
257261
}
258-
if cv == nil {
259-
return nil, ErrWaitingForVersion
260-
}
261262
token, err := c.authorizer.Token()
262263
if err != nil {
263264
return nil, err

‎pkg/insights/insightsreport/insightsreport.go

+16-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"k8s.io/component-base/metrics"
1313
"k8s.io/klog/v2"
1414

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

3031
configurator configobserver.Configurator
31-
client *insightsclient.Client
32+
client insightsReportClient
3233
LastReport types.SmartProxyReport
3334
archiveUploadReporter <-chan struct{}
3435
insightsOperatorCLI operatorv1client.InsightsOperatorInterface
@@ -243,7 +244,6 @@ func (c *Controller) Run(ctx context.Context) {
243244
c.StatusController.UpdateStatus(controllerstatus.Summary{Healthy: true})
244245
klog.V(2).Info("Starting report retriever")
245246
klog.V(2).Infof("Initial config: %v", c.configurator.Config())
246-
247247
for {
248248
// always wait for new uploaded archive or insights-operator ends
249249
select {
@@ -261,6 +261,12 @@ type healthStatusCounts struct {
261261
critical, important, moderate, low, total int
262262
}
263263

264+
type insightsReportClient interface {
265+
RecvReport(ctx context.Context, endpoint string) (*http.Response, error)
266+
IncrementRecvReportMetric(statusCode int)
267+
GetClusterVersion() (*configv1.ClusterVersion, error)
268+
}
269+
264270
func (c *Controller) readInsightsReport(report types.SmartProxyReport) ([]types.InsightsRecommendation, healthStatusCounts, time.Time) {
265271
healthStatus := healthStatusCounts{}
266272
healthStatus.total = report.Meta.Count
@@ -291,6 +297,12 @@ func (c *Controller) readInsightsReport(report types.SmartProxyReport) ([]types.
291297
klog.Errorf("Unable to extract recommendation's error key: %v", err)
292298
continue
293299
}
300+
clusterVersion, err := c.client.GetClusterVersion()
301+
if err != nil {
302+
klog.Errorf("Unable to extract cluster version. Error: %v", err)
303+
continue
304+
}
305+
insights.RecommendationCollector.SetClusterID(clusterVersion.Spec.ClusterID)
294306

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

328340
updatedOperatorCR := insightsOperatorCR.DeepCopy()
329341
var healthChecks []v1.HealthCheck
342+
330343
for _, rule := range report.Data {
331344
errorKey, err := extractErrorKeyFromRuleData(rule)
332345
if err != nil {
@@ -338,7 +351,7 @@ func (c *Controller) updateOperatorStatusCR(report types.SmartProxyReport) error
338351
Description: rule.Description,
339352
TotalRisk: int32(rule.TotalRisk),
340353
State: v1.HealthCheckEnabled,
341-
AdvisorURI: fmt.Sprintf("https://console.redhat.com/openshift/insights/advisor/recommendations/%s%%7C%s", ruleIDStr, errorKey),
354+
AdvisorURI: fmt.Sprintf("https://console.redhat.com/openshift/insights/advisor/clusters/%s?first=%s|%s", insights.RecommendationCollector.ClusterID(), ruleIDStr, errorKey),
342355
}
343356

344357
if rule.Disabled {

‎pkg/insights/insightsreport/insightsreport_test.go

+30
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,26 @@
11
package insightsreport
22

33
import (
4+
"context"
45
"fmt"
6+
"net/http"
57
"testing"
68

9+
v1 "github.com/openshift/api/config/v1"
710
"github.com/openshift/insights-operator/pkg/config"
811
"github.com/openshift/insights-operator/pkg/insights/types"
912
"github.com/stretchr/testify/assert"
1013
)
1114

1215
func Test_readInsightsReport(t *testing.T) {
16+
client := mockInsightsClient{
17+
clusterVersion: &v1.ClusterVersion{
18+
Spec: v1.ClusterVersionSpec{
19+
ClusterID: v1.ClusterID("0000 0000 0000 0000"),
20+
},
21+
},
22+
metricsName: "yeet",
23+
}
1324
tests := []struct {
1425
name string
1526
testController *Controller
@@ -24,6 +35,7 @@ func Test_readInsightsReport(t *testing.T) {
2435
configurator: config.NewMockSecretConfigurator(&config.Controller{
2536
DisableInsightsAlerts: false,
2637
}),
38+
client: &client,
2739
},
2840
report: types.SmartProxyReport{
2941
Data: []types.RuleWithContentResponse{
@@ -111,6 +123,7 @@ func Test_readInsightsReport(t *testing.T) {
111123
configurator: config.NewMockSecretConfigurator(&config.Controller{
112124
DisableInsightsAlerts: false,
113125
}),
126+
client: &client,
114127
},
115128
report: types.SmartProxyReport{
116129
Data: []types.RuleWithContentResponse{
@@ -186,6 +199,7 @@ func Test_readInsightsReport(t *testing.T) {
186199
configurator: config.NewMockSecretConfigurator(&config.Controller{
187200
DisableInsightsAlerts: true,
188201
}),
202+
client: &client,
189203
},
190204
report: types.SmartProxyReport{
191205
Data: []types.RuleWithContentResponse{
@@ -295,3 +309,19 @@ func Test_extractErrorKeyFromRuleData(t *testing.T) {
295309
})
296310
}
297311
}
312+
313+
type mockInsightsClient struct {
314+
clusterVersion *v1.ClusterVersion
315+
metricsName string
316+
}
317+
318+
func (c *mockInsightsClient) GetClusterVersion() (*v1.ClusterVersion, error) {
319+
return c.clusterVersion, nil
320+
}
321+
322+
func (c *mockInsightsClient) IncrementRecvReportMetric(statusCode int) {
323+
}
324+
325+
func (c *mockInsightsClient) RecvReport(ctx context.Context, endpoint string) (*http.Response, error) {
326+
return nil, nil
327+
}

‎pkg/insights/metrics.go

+11-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"strings"
66

77
semver "github.com/blang/semver/v4"
8+
v1 "github.com/openshift/api/config/v1"
89
"github.com/openshift/insights-operator/pkg/insights/types"
910
"github.com/prometheus/client_golang/prometheus"
1011
"k8s.io/component-base/metrics"
@@ -37,6 +38,15 @@ func init() {
3738
type InsightsRecommendationCollector struct {
3839
activeRecommendations []types.InsightsRecommendation
3940
metricName string
41+
clusterID v1.ClusterID
42+
}
43+
44+
func (c *InsightsRecommendationCollector) SetClusterID(clusterID v1.ClusterID) {
45+
c.clusterID = clusterID
46+
}
47+
48+
func (c *InsightsRecommendationCollector) ClusterID() v1.ClusterID {
49+
return c.clusterID
4050
}
4151

4252
func (c *InsightsRecommendationCollector) SetActiveRecommendations(activeRecommendations []types.InsightsRecommendation) {
@@ -57,7 +67,7 @@ func (c *InsightsRecommendationCollector) Collect(ch chan<- prometheus.Metric) {
5767
prometheus.NewDesc(c.metricName, "", []string{}, prometheus.Labels{
5868
"description": rec.Description,
5969
"total_risk": totalRiskToStr(rec.TotalRisk),
60-
"info_link": fmt.Sprintf("https://console.redhat.com/openshift/insights/advisor/recommendations/%s%%7C%s", ruleIDStr, rec.ErrorKey),
70+
"info_link": fmt.Sprintf("https://console.redhat.com/openshift/insights/advisor/clusters/%s?first=%s|%s", c.clusterID, ruleIDStr, rec.ErrorKey),
6171
}),
6272
prometheus.GaugeValue,
6373
1,

0 commit comments

Comments
 (0)