Merge pull request #28262 from dgoodwin/cluster-operator-intervals

Port the clusteroperator intervals to new structured format

Author: openshift-merge-robot

pkg/monitor/monitorapi/construction.go

@@ -317,6 +317,12 @@ func (b *LocatorBuilder) E2ETest(testName string) Locator {
 	return b.Build()
 }
 
+func (b *LocatorBuilder) ClusterOperator(name string) Locator {
+	b.targetType = LocatorTypeClusterOperator
+	b.annotations[LocatorClusterOperatorKey] = name
+	return b.Build()
+}
+
 func (b *LocatorBuilder) Build() Locator {
 	ret := Locator{
 		Type: b.targetType,

pkg/monitor/monitorapi/identification_operator.go

@@ -1,46 +1,27 @@
 package monitorapi
 
 import (
-	"strings"
-
 	configv1 "github.com/openshift/api/config/v1"
 )
 
-// condition/Degraded status/True reason/DNSDegraded changed: DNS default is degraded
-func GetOperatorConditionStatus(message string) *configv1.ClusterOperatorStatusCondition {
-	if !strings.HasPrefix(message, "condition/") {
+// GetOperatorConditionStatus reconstructs a ClusterOperatorStatusCondition from an interval.
+func GetOperatorConditionStatus(interval Interval) *configv1.ClusterOperatorStatusCondition {
+	c, ok := interval.StructuredMessage.Annotations[AnnotationCondition]
+	if !ok {
 		return nil
 	}
-	stanzas := strings.Split(message, " ")
-	condition := &configv1.ClusterOperatorStatusCondition{}
-
-	for _, stanza := range stanzas {
-		keyValue := strings.SplitN(stanza, "/", 2)
-		if len(keyValue) != 2 {
-			continue
-		}
 
-		switch keyValue[0] {
-		case "condition":
-			if condition.Type == "" {
-				condition.Type = configv1.ClusterStatusConditionType(keyValue[1])
-			}
-		case "status":
-			if condition.Status == "" {
-				condition.Status = configv1.ConditionStatus(keyValue[1])
-			}
-		case "reason":
-			if condition.Reason == "" {
-				condition.Reason = keyValue[1]
-			}
-		}
+	condition := &configv1.ClusterOperatorStatusCondition{}
+	condition.Type = configv1.ClusterStatusConditionType(c)
+	s, ok := interval.StructuredMessage.Annotations[AnnotationStatus]
+	if ok {
+		condition.Status = configv1.ConditionStatus(s)
 	}
-
-	messages := strings.SplitN(message, ": ", 2)
-	if len(messages) < 2 {
-		return condition
+	r, ok := interval.StructuredMessage.Annotations[AnnotationReason]
+	if ok {
+		condition.Reason = r
 	}
-	condition.Message = messages[1]
 
+	condition.Message = interval.StructuredMessage.HumanMessage
 	return condition
 }

pkg/monitor/monitorapi/identification_operator_test.go

@@ -1,21 +1,34 @@
 package monitorapi
 
 import (
-	"reflect"
 	"testing"
 
 	configv1 "github.com/openshift/api/config/v1"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestGetOperatorConditionStatus(t *testing.T) {
 	tests := []struct {
-		name    string
-		message string
-		want    *configv1.ClusterOperatorStatusCondition
+		name     string
+		interval Interval
+		want     *configv1.ClusterOperatorStatusCondition
 	}{
 		{
-			name:    "simple",
-			message: "condition/Degraded status/True reason/DNSDegraded changed: DNS default is degraded",
+			name: "simple",
+			interval: Interval{
+				Source: SourceClusterOperatorMonitor,
+				Condition: Condition{
+					StructuredMessage: Message{
+						Reason:       "DNSDegraded",
+						HumanMessage: "DNS default is degraded",
+						Annotations: map[AnnotationKey]string{
+							AnnotationCondition: "Degraded",
+							AnnotationStatus:    "True",
+							AnnotationReason:    "DNSDegraded",
+						},
+					},
+				},
+			},
 			want: &configv1.ClusterOperatorStatusCondition{
 				Type:    configv1.OperatorDegraded,
 				Status:  configv1.ConditionTrue,
@@ -24,31 +37,33 @@ func TestGetOperatorConditionStatus(t *testing.T) {
 			},
 		},
 		{
-			name:    "unknown",
-			message: "condition/Upgradeable status/Unknown reason/NoData changed: blah blah",
+			name: "unknown",
+			interval: Interval{
+				Source: SourceClusterOperatorMonitor,
+				Condition: Condition{
+					StructuredMessage: Message{
+						Reason:       "NoData",
+						HumanMessage: "blah blah",
+						Annotations: map[AnnotationKey]string{
+							AnnotationCondition: "Upgradeable",
+							AnnotationStatus:    "Unknown",
+							AnnotationReason:    "NoData",
+						},
+					},
+				},
+			},
 			want: &configv1.ClusterOperatorStatusCondition{
 				Type:    configv1.OperatorUpgradeable,
 				Status:  configv1.ConditionUnknown,
 				Reason:  "NoData",
 				Message: "blah blah",
 			},
 		},
-		{
-			name:    "repeat reason",
-			message: "condition/Available status/True reason/AsExpected changed: reason/again",
-			want: &configv1.ClusterOperatorStatusCondition{
-				Type:    configv1.OperatorAvailable,
-				Status:  configv1.ConditionTrue,
-				Reason:  "AsExpected",
-				Message: "reason/again",
-			},
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := GetOperatorConditionStatus(tt.message); !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("GetOperatorConditionStatus() got = %v, want %v", got, tt.want)
-			}
+			got := GetOperatorConditionStatus(tt.interval)
+			assert.Equal(t, tt.want, got)
 		})
 	}
 }

pkg/monitor/monitorapi/types.go

@@ -243,6 +243,8 @@ const (
 	SourceKubeEvent               IntervalSource = "KubeEvent"
 	SourceTestData                IntervalSource = "TestData"                // some tests have no real source to assign
 	SourcePathologicalEventMarker IntervalSource = "PathologicalEventMarker" // not sure if this is really helpful since the events all have a different origin
+	SourceClusterOperatorMonitor  IntervalSource = "ClusterOperatorMonitor"
+	SourceOperatorState           IntervalSource = "OperatorState"
 )
 
 type Interval struct {
@@ -264,9 +266,20 @@ func (i Interval) String() string {
 	}
 	duration := i.To.Sub(i.From)
 	if duration < time.Second {
-		return fmt.Sprintf("%s.%03d - %-5s %s %s %s", i.From.Format("Jan 02 15:04:05"), i.From.Nanosecond()/int(time.Millisecond), strconv.Itoa(int(duration/time.Millisecond))+"ms", i.Level.String()[:1], i.Locator, strings.Replace(i.Message, "\n", "\\n", -1))
-	}
-	return fmt.Sprintf("%s.%03d - %-5s %s %s %s", i.From.Format("Jan 02 15:04:05"), i.From.Nanosecond()/int(time.Millisecond), strconv.Itoa(int(duration/time.Second))+"s", i.Level.String()[:1], i.Locator, strings.Replace(i.Message, "\n", "\\n", -1))
+		return fmt.Sprintf("%s.%03d - %-5s %s %s %s",
+			i.From.Format("Jan 02 15:04:05"),
+			i.From.Nanosecond()/int(time.Millisecond),
+			strconv.Itoa(int(duration/time.Millisecond))+"ms",
+			i.Level.String()[:1],
+			i.Locator,
+			strings.Replace(i.Message, "\n", "\\n", -1))
+	}
+	return fmt.Sprintf("%s.%03d - %-5s %s %s %s",
+		i.From.Format("Jan 02 15:04:05"),
+		i.From.Nanosecond()/int(time.Millisecond), strconv.Itoa(int(duration/time.Second))+"s",
+		i.Level.String()[:1],
+		i.Locator,
+		strings.Replace(i.Message, "\n", "\\n", -1))
 }
 
 func (i Message) OldMessage() string {

pkg/monitor/serialization/serialize.go

@@ -19,6 +19,10 @@ type EventInterval struct {
 	Locator string `json:"locator"`
 	Message string `json:"message"`
 
+	// TODO: Remove the omitempty, just here to keep from having to repeatedly updated the json
+	// files used in some new tests
+	Source string `json:"tempSource,omitempty"` // also temporary, unsure if this concept will survive
+
 	// TODO: we're hoping to move these to just locator/message when everything is ready.
 	StructuredLocator monitorapi.Locator `json:"tempStructuredLocator"`
 	StructuredMessage monitorapi.Message `json:"tempStructuredMessage"`
@@ -60,6 +64,7 @@ func EventsFromJSON(data []byte) (monitorapi.Intervals, error) {
 			return nil, err
 		}
 		events = append(events, monitorapi.Interval{
+			Source: monitorapi.IntervalSource(interval.Source),
 			Condition: monitorapi.Condition{
 				Level:   level,
 				Locator: interval.Locator,
@@ -84,10 +89,13 @@ func IntervalFromJSON(data []byte) (*monitorapi.Interval, error) {
 		return nil, err
 	}
 	return &monitorapi.Interval{
+		Source: monitorapi.IntervalSource(serializedInterval.Source),
 		Condition: monitorapi.Condition{
-			Level:   level,
-			Locator: serializedInterval.Locator,
-			Message: serializedInterval.Message,
+			Level:             level,
+			Locator:           serializedInterval.Locator,
+			Message:           serializedInterval.Message,
+			StructuredLocator: serializedInterval.StructuredLocator,
+			StructuredMessage: serializedInterval.StructuredMessage,
 		},
 
 		From: serializedInterval.From.Time,
@@ -150,6 +158,7 @@ func monitorEventIntervalToEventInterval(interval monitorapi.Interval) EventInte
 		Message:           interval.Message,
 		StructuredLocator: interval.StructuredLocator,
 		StructuredMessage: interval.StructuredMessage,
+		Source:            string(interval.Source),
 
 		From: metav1.Time{Time: interval.From},
 		To:   metav1.Time{Time: interval.To},

pkg/monitortestframework/panic.go

@@ -3,8 +3,10 @@ package monitortestframework
 import (
 	"context"
 	"fmt"
+	"runtime/debug"
 	"time"
 
+	"github.com/sirupsen/logrus"
 	"k8s.io/client-go/rest"
 
 	"github.com/openshift/origin/pkg/monitor/monitorapi"
@@ -15,6 +17,8 @@ func startCollectionWithPanicProtection(ctx context.Context, monitortest Monitor
 	defer func() {
 		if r := recover(); r != nil {
 			err = fmt.Errorf("caught panic: %v", r)
+			logrus.Error("recovering from panic")
+			fmt.Print(debug.Stack())
 		}
 	}()
 
@@ -26,6 +30,8 @@ func collectDataWithPanicProtection(ctx context.Context, monitortest MonitorTest
 	defer func() {
 		if r := recover(); r != nil {
 			err = fmt.Errorf("caught panic: %v", r)
+			logrus.Error("recovering from panic")
+			fmt.Print(debug.Stack())
 		}
 	}()
 
@@ -37,6 +43,8 @@ func constructComputedIntervalsWithPanicProtection(ctx context.Context, monitort
 	defer func() {
 		if r := recover(); r != nil {
 			err = fmt.Errorf("caught panic: %v", r)
+			logrus.Error("recovering from panic")
+			fmt.Print(debug.Stack())
 		}
 	}()
 
@@ -48,6 +56,8 @@ func evaluateTestsFromConstructedIntervalsWithPanicProtection(ctx context.Contex
 	defer func() {
 		if r := recover(); r != nil {
 			err = fmt.Errorf("caught panic: %v", r)
+			logrus.Error("recovering from panic")
+			fmt.Print(debug.Stack())
 		}
 	}()
 
@@ -59,6 +69,8 @@ func writeContentToStorageWithPanicProtection(ctx context.Context, monitortest M
 	defer func() {
 		if r := recover(); r != nil {
 			err = fmt.Errorf("caught panic: %v", r)
+			logrus.Error("recovering from panic")
+			fmt.Print(debug.Stack())
 		}
 	}()
 
@@ -70,6 +82,8 @@ func cleanupWithPanicProtection(ctx context.Context, monitortest MonitorTest) (e
 	defer func() {
 		if r := recover(); r != nil {
 			err = fmt.Errorf("caught panic: %v", r)
+			logrus.Error("recovering from panic")
+			fmt.Print(debug.Stack())
 		}
 	}()