address comments

pkg/monitor/cluster/maintenance.go

@@ -64,23 +64,19 @@ func (mon *Monitor) emitPucmState(ctx context.Context) error {
 }
 
 func getPucmState(clusterProperties api.OpenShiftClusterProperties) pucmState {
-	var state pucmState
-
 	if pucmOngoing(clusterProperties) {
 		if clusterProperties.PucmPending {
-			state = pucmPlannedOngoing
+			return pucmPlannedOngoing
 		} else {
-			state = pucmUnplannedOngoing
+			return pucmUnplannedOngoing
 		}
 	} else {
 		if clusterProperties.PucmPending {
-			state = pucmPending
-		} else {
-			state = pucmNone
+			return pucmPending
 		}
 	}
 
-	return state
+	return pucmNone
 }
 
 func pucmOngoing(clusterProperties api.OpenShiftClusterProperties) bool {

pkg/monitor/cluster/maintenance_test.go

@@ -7,13 +7,95 @@ import (
 	"context"
 	"testing"
 
-	"github.com/golang/mock/gomock"
-
 	"github.com/Azure/ARO-RP/pkg/api"
 	"github.com/Azure/ARO-RP/pkg/metrics"
 	mock_metrics "github.com/Azure/ARO-RP/pkg/util/mocks/metrics"
+	"github.com/golang/mock/gomock"
 )
 
+func TestEmitPucmState(t *testing.T) {
+	for _, tt := range []struct {
+		name              string
+		provisioningState api.ProvisioningState
+		pucmPending       bool
+		adminUpdateErr    string
+		expectedPucmState pucmState
+	}{
+		{
+			name:              "state none",
+			provisioningState: api.ProvisioningStateSucceeded,
+			pucmPending:       false,
+			adminUpdateErr:    "",
+			expectedPucmState: pucmNone,
+		},
+		{
+			name:              "state pending",
+			provisioningState: api.ProvisioningStateSucceeded,
+			pucmPending:       true,
+			adminUpdateErr:    "",
+			expectedPucmState: pucmPending,
+		},
+		{
+			name:              "state unplanned ongoing - admin updating in flight and no admin update error",
+			provisioningState: api.ProvisioningStateAdminUpdating,
+			pucmPending:       false,
+			adminUpdateErr:    "",
+			expectedPucmState: pucmUnplannedOngoing,
+		},
+		{
+			name:              "state planned ongoing - admin updating in flight and no admin update error",
+			provisioningState: api.ProvisioningStateAdminUpdating,
+			pucmPending:       true,
+			adminUpdateErr:    "",
+			expectedPucmState: pucmPlannedOngoing,
+		},
+		{
+			name:              "state unplanned ongoing - not admin updating but admin update error",
+			provisioningState: api.ProvisioningStateFailed,
+			pucmPending:       false,
+			adminUpdateErr:    "PUCM failed",
+			expectedPucmState: pucmUnplannedOngoing,
+		},
+		{
+			name:              "state planned ongoing - not admin updating but admin update error",
+			provisioningState: api.ProvisioningStateFailed,
+			pucmPending:       true,
+			adminUpdateErr:    "PUCM failed",
+			expectedPucmState: pucmPlannedOngoing,
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := context.Background()
+
+			controller := gomock.NewController(t)
+			defer controller.Finish()
+
+			m := mock_metrics.NewMockEmitter(controller)
+			oc := &api.OpenShiftCluster{
+				Properties: api.OpenShiftClusterProperties{
+					ProvisioningState:    tt.provisioningState,
+					PucmPending:          tt.pucmPending,
+					LastAdminUpdateError: tt.adminUpdateErr,
+				},
+			}
+			mon := &Monitor{
+				m:  m,
+				oc: oc,
+			}
+
+			m.EXPECT().EmitGauge("cluster.maintenance.pucm", int64(1), map[string]string{
+				"state": tt.expectedPucmState.String(),
+			})
+
+			err := mon.emitPucmState(ctx)
+			if err != nil {
+				t.Fatal(err)
+			}
+		})
+	}
+}
+
+/*
 func TestEmitPucmState(t *testing.T) {
 	ctx := context.Background()
 
@@ -88,6 +170,7 @@ func TestEmitPucmState(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+*/
 
 func getMonitor(oc *api.OpenShiftCluster, m metrics.Emitter) *Monitor {
 	return &Monitor{