Update

Author: tremes

manifests/03-clusterrole.yaml

@@ -62,7 +62,7 @@ rules:
   - apiGroups:
       - "operator.openshift.io"
     resources:
-      - insightsoperators
+      - insightsoperators/status
     verbs:
       - get
       - update

pkg/controller/periodic/periodic.go

@@ -26,8 +26,10 @@ const (
 	DataGatheredCondition = "DataGathered"
 	// NoDataGathered is a reason when there is no data gathered - e.g the resource is not in a cluster
 	NoDataGatheredReason = "NoData"
-	// GatherError is a reason when there is some error and no data gathered
+	// Error is a reason when there is some error and no data gathered
 	GatherErrorReason = "Error"
+	// Panic is a reason when there is some error and no data gathered
+	GatherPanicReason = "Panic"
 	// GatheredOK is a reason when data is gathered as expected
 	GatherOKReason = "GatheredOK"
 	// GatherWithError is a reason when data is gathered partially or with another error message
@@ -240,6 +242,13 @@ func createGathererStatus(gfr *gather.GathererFunctionReport) v1.GathererStatus
 	con := metav1.Condition{
 		Type:               DataGatheredCondition,
 		LastTransitionTime: metav1.Now(),
+		Status:             metav1.ConditionFalse,
+		Reason:             NoDataGatheredReason,
+	}
+
+	if gfr.Panic != nil {
+		con.Reason = GatherPanicReason
+		con.Message = gfr.Panic.(string)
 	}
 
 	if gfr.RecordsCount > 0 {
@@ -249,22 +258,20 @@ func createGathererStatus(gfr *gather.GathererFunctionReport) v1.GathererStatus
 
 		if len(gfr.Errors) > 0 {
 			con.Reason = GatherWithErrorReason
-			con.Message = fmt.Sprintf("%s Error is: %s", con.Message, strings.Join(gfr.Errors, ","))
+			con.Message = fmt.Sprintf("%s Error: %s", con.Message, strings.Join(gfr.Errors, ","))
 		}
 
 		gs.Conditions = append(gs.Conditions, con)
-	} else {
-		con.Status = metav1.ConditionFalse
-		con.Reason = NoDataGatheredReason
-
-		if len(gfr.Errors) > 0 {
-			con.Reason = GatherErrorReason
-			con.Message = strings.Join(gfr.Errors, ",")
-		}
+		return gs
+	}
 
-		gs.Conditions = append(gs.Conditions, con)
+	if len(gfr.Errors) > 0 {
+		con.Reason = GatherErrorReason
+		con.Message = strings.Join(gfr.Errors, ",")
 	}
 
+	gs.Conditions = append(gs.Conditions, con)
+
 	return gs
 }
 

pkg/controller/periodic/periodic_test.go

@@ -7,7 +7,9 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	v1 "github.com/openshift/api/operator/v1"
 	fakeOperatorCli "github.com/openshift/client-go/operator/clientset/versioned/fake"
 	"github.com/openshift/insights-operator/pkg/config"
 	"github.com/openshift/insights-operator/pkg/gather"
@@ -177,3 +179,138 @@ func getMocksForPeriodicTest(listGatherers []gatherers.Interface, interval time.
 	return New(&mockConfigurator, &mockRecorder, listGatherers, nil, fakeInsightsOperatorCli), &mockRecorder
 }
 
+func Test_createGathererStatus(t *testing.T) { //nolint: funlen
+	tests := []struct {
+		name       string
+		gfr        gather.GathererFunctionReport
+		expectedGs v1.GathererStatus
+	}{
+		{
+			name: "Data gathered OK",
+			gfr: gather.GathererFunctionReport{
+				FuncName:     "gatherer1/foo",
+				Duration:     115000,
+				RecordsCount: 5,
+			},
+			expectedGs: v1.GathererStatus{
+				Name: "gatherer1/foo",
+				LastGatherDuration: metav1.Duration{
+					Duration: 115000000000,
+				},
+				Conditions: []metav1.Condition{
+					{
+						Type:    DataGatheredCondition,
+						Status:  metav1.ConditionTrue,
+						Reason:  GatherOKReason,
+						Message: "Created 5 records in the archive.",
+					},
+				},
+			},
+		},
+		{
+			name: "No Data",
+			gfr: gather.GathererFunctionReport{
+				FuncName:     "gatherer2/baz",
+				Duration:     0,
+				RecordsCount: 0,
+			},
+			expectedGs: v1.GathererStatus{
+				Name: "gatherer2/baz",
+				LastGatherDuration: metav1.Duration{
+					Duration: 0,
+				},
+				Conditions: []metav1.Condition{
+					{
+						Type:   DataGatheredCondition,
+						Status: metav1.ConditionFalse,
+						Reason: NoDataGatheredReason,
+					},
+				},
+			},
+		},
+		{
+			name: "Gatherer Error",
+			gfr: gather.GathererFunctionReport{
+				FuncName:     "gatherer3/bar",
+				Duration:     0,
+				RecordsCount: 0,
+				Errors:       []string{"unable to read the data"},
+			},
+			expectedGs: v1.GathererStatus{
+				Name: "gatherer3/bar",
+				LastGatherDuration: metav1.Duration{
+					Duration: 0,
+				},
+				Conditions: []metav1.Condition{
+					{
+						Type:    DataGatheredCondition,
+						Status:  metav1.ConditionFalse,
+						Reason:  GatherErrorReason,
+						Message: "unable to read the data",
+					},
+				},
+			},
+		},
+		{
+			name: "Data gathered with an error",
+			gfr: gather.GathererFunctionReport{
+				FuncName:     "gatherer4/quz",
+				Duration:     9000,
+				RecordsCount: 2,
+				Errors:       []string{"didn't find xyz configmap"},
+			},
+			expectedGs: v1.GathererStatus{
+				Name: "gatherer4/quz",
+				LastGatherDuration: metav1.Duration{
+					Duration: 9000000000,
+				},
+				Conditions: []metav1.Condition{
+					{
+						Type:    DataGatheredCondition,
+						Status:  metav1.ConditionTrue,
+						Reason:  GatherWithErrorReason,
+						Message: "Created 2 records in the archive. Error: didn't find xyz configmap",
+					},
+				},
+			},
+		},
+		{
+			name: "Gatherer panicked",
+			gfr: gather.GathererFunctionReport{
+				FuncName:     "gatherer5/quz",
+				Duration:     0,
+				RecordsCount: 0,
+				Panic:        "quz gatherer panicked",
+			},
+			expectedGs: v1.GathererStatus{
+				Name: "gatherer5/quz",
+				LastGatherDuration: metav1.Duration{
+					Duration: 0,
+				},
+				Conditions: []metav1.Condition{
+					{
+						Type:    DataGatheredCondition,
+						Status:  metav1.ConditionFalse,
+						Reason:  GatherPanicReason,
+						Message: "quz gatherer panicked",
+					},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gathererStatus := createGathererStatus(&tt.gfr)
+			assert.Equal(t, tt.expectedGs.Name, gathererStatus.Name)
+			assert.Equal(t, tt.expectedGs.LastGatherDuration, gathererStatus.LastGatherDuration)
+
+			// more asserts since we can use simple equal because of the last transition time of the condition
+			assert.Len(t, gathererStatus.Conditions, 1)
+			assert.Equal(t, tt.expectedGs.Conditions[0].Type, gathererStatus.Conditions[0].Type)
+			assert.Equal(t, tt.expectedGs.Conditions[0].Reason, gathererStatus.Conditions[0].Reason)
+			assert.Equal(t, tt.expectedGs.Conditions[0].Status, gathererStatus.Conditions[0].Status)
+			assert.Equal(t, tt.expectedGs.Conditions[0].Message, gathererStatus.Conditions[0].Message)
+		})
+	}
+}

pkg/controller/status/controller.go

@@ -474,5 +474,6 @@ func relatedObjects(namespace string) []configv1.ObjectReference {
 		{Resource: "serviceaccounts", Namespace: namespace, Name: "operator"},
 		{Resource: "services", Namespace: namespace, Name: "metrics"},
 		{Resource: "configmaps", Namespace: namespace, Name: "service-ca-bundle"},
+		{Group: "operator.openshift.io", Resource: "insightsoperators", Name: "cluster"},
 	}
 }