From 1aa328c8cf52affbc0b0c799d512933f8c8fff36 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Wed, 12 Jul 2023 08:26:46 -0700 Subject: [PATCH 01/26] PUCM Maintenance Signals --- pkg/api/admin/openshiftcluster.go | 1 + pkg/api/openshiftcluster.go | 2 + pkg/backend/openshiftcluster.go | 4 ++ pkg/monitor/cluster/cluster.go | 1 + pkg/monitor/cluster/maintenance.go | 89 ++++++++++++++++++++++++ pkg/monitor/cluster/maintenance_test.go | 91 +++++++++++++++++++++++++ 6 files changed, 188 insertions(+) create mode 100644 pkg/monitor/cluster/maintenance.go create mode 100644 pkg/monitor/cluster/maintenance_test.go diff --git a/pkg/api/admin/openshiftcluster.go b/pkg/api/admin/openshiftcluster.go index d8e6fdb1f40..5cabe48947a 100644 --- a/pkg/api/admin/openshiftcluster.go +++ b/pkg/api/admin/openshiftcluster.go @@ -54,6 +54,7 @@ type OpenShiftClusterProperties struct { ImageRegistryStorageAccountName string `json:"imageRegistryStorageAccountName,omitempty"` InfraID string `json:"infraId,omitempty"` HiveProfile HiveProfile `json:"hiveProfile,omitempty"` + PucmPending bool `json:"pucmPending,omitempty"` } // ProvisioningState represents a provisioning state. diff --git a/pkg/api/openshiftcluster.go b/pkg/api/openshiftcluster.go index 4201c544f15..cd9e6353f34 100644 --- a/pkg/api/openshiftcluster.go +++ b/pkg/api/openshiftcluster.go @@ -157,6 +157,8 @@ type OpenShiftClusterProperties struct { RegistryProfiles []*RegistryProfile `json:"registryProfiles,omitempty"` HiveProfile HiveProfile `json:"hiveProfile,omitempty"` + + PucmPending bool `json:"pucmPending,omitempty"` } // ProvisioningState represents a provisioning state diff --git a/pkg/backend/openshiftcluster.go b/pkg/backend/openshiftcluster.go index 4c258699fe7..2e743ce0ed2 100644 --- a/pkg/backend/openshiftcluster.go +++ b/pkg/backend/openshiftcluster.go @@ -161,6 +161,7 @@ func (ocb *openShiftClusterBackend) handle(ctx context.Context, log *logrus.Entr if err != nil { return ocb.endLease(ctx, log, stop, doc, api.ProvisioningStateFailed, err) } + doc.OpenShiftCluster.Properties.PucmPending = false return ocb.endLease(ctx, log, stop, doc, api.ProvisioningStateSucceeded, nil) case api.ProvisioningStateUpdating: @@ -361,3 +362,6 @@ func (ocb *openShiftClusterBackend) emitMetrics(doc *api.OpenShiftClusterDocumen "newProvisioningState": string(provisioningState), }) } + +func (ocb *openShiftClusterBackend) updatePUCMPendingState(doc *api.OpenShiftClusterDocument) *api.OpenShiftClusterDocument { +} diff --git a/pkg/monitor/cluster/cluster.go b/pkg/monitor/cluster/cluster.go index 228405061b7..02db5af4448 100644 --- a/pkg/monitor/cluster/cluster.go +++ b/pkg/monitor/cluster/cluster.go @@ -153,6 +153,7 @@ func (mon *Monitor) Monitor(ctx context.Context) (errs []error) { return } for _, f := range []func(context.Context) error{ + mon.emitPucmState, mon.emitAroOperatorHeartbeat, mon.emitAroOperatorConditions, mon.emitNSGReconciliation, diff --git a/pkg/monitor/cluster/maintenance.go b/pkg/monitor/cluster/maintenance.go new file mode 100644 index 00000000000..5e59a7cbc0e --- /dev/null +++ b/pkg/monitor/cluster/maintenance.go @@ -0,0 +1,89 @@ +package cluster + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +import ( + "context" + + "github.com/Azure/ARO-RP/pkg/api" +) + +/************************************************************** + Possible PUCM states: + + (1) PUCM pending + - We will do PUCM, so emit a maintenance pending signal + - Conditions: + * Field pucmPending is true + * Don't meet below conditions for in progress maintenance + + (2) Planned PUCM in progress + - Emit a planned maintenance in progress signal. + - If first PUCM attempt fails, leave cluster in this state + because we will need to retry PUCM in at a later time. + - Conditions: + * Field pucmPending is true + * One of: (a) provisoning state AdminUpdate or (2) AdminUpdate err is not nil + + (3) Unplanned PUCM in progress + - Emit an unplanned maintenance in progress signal. + - If first PUCM attempt fails, leave cluster in this state + because we will need to retry PUCM in at a later time. + - Conditions: + * Field pucmPending is false + * One of: (a) provisoning state AdminUpdate or (2) AdminUpdate err is not nil + + (4) No ongoinig or scheduled PUCM + - Don't emit a signal + - Conditions: + * Field pucmPending is false + * Provisioning state is not AdminUpdate and AdminUpdate err is not nil +**************************************************************/ + +type pucmState string + +func (p pucmState) String() string { + return string(p) +} + +const ( + pucmNone pucmState = "none" + pucmPending pucmState = "pending" + pucmPlannedOngoing pucmState = "planned_ongoing" + pucmUnplannedOngoing pucmState = "unplanned_ongoing" +) + +func (mon *Monitor) emitPucmState(ctx context.Context) error { + state := getPucmState(mon.oc.Properties) + mon.emitGauge("cluster.maintenance.pucm", 1, map[string]string{ + "state": state.String(), + }) + + return nil +} + +func getPucmState(clusterProperties api.OpenShiftClusterProperties) pucmState { + var state pucmState + + if pucmOngoing(clusterProperties) { + if clusterProperties.PucmPending { + state = pucmPlannedOngoing + } else { + state = pucmUnplannedOngoing + } + } else { + if clusterProperties.PucmPending { + state = pucmPending + } else { + state = pucmNone + } + } + + return state +} + +func pucmOngoing(clusterProperties api.OpenShiftClusterProperties) bool { + return clusterProperties.ProvisioningState == api.ProvisioningStateAdminUpdating || + clusterProperties.LastAdminUpdateError != "" +} diff --git a/pkg/monitor/cluster/maintenance_test.go b/pkg/monitor/cluster/maintenance_test.go new file mode 100644 index 00000000000..eb240d51aaa --- /dev/null +++ b/pkg/monitor/cluster/maintenance_test.go @@ -0,0 +1,91 @@ +package cluster + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +import ( + "context" + "testing" + + "github.com/Azure/ARO-RP/pkg/api" + mock_metrics "github.com/Azure/ARO-RP/pkg/util/mocks/metrics" + "github.com/golang/mock/gomock" +) + +func TestEmitPucmState(t *testing.T) { + ctx := context.Background() + + controller := gomock.NewController(t) + defer controller.Finish() + + m := mock_metrics.NewMockEmitter(controller) + + // Unplanned ongoing + oc := &api.OpenShiftCluster{ + Properties: api.OpenShiftClusterProperties{ + ProvisioningState: api.ProvisioningStateAdminUpdating, + }, + } + mon := &Monitor{ + m: m, + oc: oc, + } + m.EXPECT().EmitGauge("cluster.maintenance.pucm", int64(1), map[string]string{ + "state": pucmUnplannedOngoing.String(), + }) + + err := mon.emitPucmState(ctx) + if err != nil { + t.Fatal(err) + } + + // Planned ongoing + oc = &api.OpenShiftCluster{ + Properties: api.OpenShiftClusterProperties{ + ProvisioningState: api.ProvisioningStateAdminUpdating, + PucmPending: true, + }, + } + + m.EXPECT().EmitGauge("cluster.maintenance.pucm", int64(1), map[string]string{ + "state": pucmPlannedOngoing.String(), + }) + + err = mon.emitPucmState(ctx) + if err != nil { + t.Fatal(err) + } + + // Pending + oc = &api.OpenShiftCluster{ + Properties: api.OpenShiftClusterProperties{ + ProvisioningState: api.ProvisioningStateSucceeded, + PucmPending: true, + }, + } + + m.EXPECT().EmitGauge("cluster.maintenance.pucm", int64(1), map[string]string{ + "state": pucmPending.String(), + }) + + err = mon.emitPucmState(ctx) + if err != nil { + t.Fatal(err) + } + + // None + oc = &api.OpenShiftCluster{ + Properties: api.OpenShiftClusterProperties{ + ProvisioningState: api.ProvisioningStateSucceeded, + }, + } + + m.EXPECT().EmitGauge("cluster.maintenance.pucm", int64(1), map[string]string{ + "state": pucmNone.String(), + }) + + err = mon.emitPucmState(ctx) + if err != nil { + t.Fatal(err) + } +} From 4eef64791fad5e329d806ff42873433826672ef2 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Wed, 12 Jul 2023 09:13:42 -0700 Subject: [PATCH 02/26] fiix --- pkg/backend/openshiftcluster.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/backend/openshiftcluster.go b/pkg/backend/openshiftcluster.go index 2e743ce0ed2..0a5a7fee31a 100644 --- a/pkg/backend/openshiftcluster.go +++ b/pkg/backend/openshiftcluster.go @@ -362,6 +362,3 @@ func (ocb *openShiftClusterBackend) emitMetrics(doc *api.OpenShiftClusterDocumen "newProvisioningState": string(provisioningState), }) } - -func (ocb *openShiftClusterBackend) updatePUCMPendingState(doc *api.OpenShiftClusterDocument) *api.OpenShiftClusterDocument { -} From 81c091260b91f645f88ebbb1c923d28e4c3061e7 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Wed, 12 Jul 2023 09:19:25 -0700 Subject: [PATCH 03/26] lint --- pkg/monitor/cluster/maintenance_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/monitor/cluster/maintenance_test.go b/pkg/monitor/cluster/maintenance_test.go index eb240d51aaa..40e42727f35 100644 --- a/pkg/monitor/cluster/maintenance_test.go +++ b/pkg/monitor/cluster/maintenance_test.go @@ -7,9 +7,9 @@ import ( "context" "testing" + "github.com/golang/mock/gomock" "github.com/Azure/ARO-RP/pkg/api" mock_metrics "github.com/Azure/ARO-RP/pkg/util/mocks/metrics" - "github.com/golang/mock/gomock" ) func TestEmitPucmState(t *testing.T) { From 87009552e583cb6e19711c0b8084f9a019c9392f Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Wed, 12 Jul 2023 09:31:32 -0700 Subject: [PATCH 04/26] gofmt --- pkg/monitor/cluster/maintenance_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/monitor/cluster/maintenance_test.go b/pkg/monitor/cluster/maintenance_test.go index 40e42727f35..eb240d51aaa 100644 --- a/pkg/monitor/cluster/maintenance_test.go +++ b/pkg/monitor/cluster/maintenance_test.go @@ -7,9 +7,9 @@ import ( "context" "testing" - "github.com/golang/mock/gomock" "github.com/Azure/ARO-RP/pkg/api" mock_metrics "github.com/Azure/ARO-RP/pkg/util/mocks/metrics" + "github.com/golang/mock/gomock" ) func TestEmitPucmState(t *testing.T) { From 0cd2c92d09868b1c2bce50d0aaa346882543bfb7 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Wed, 12 Jul 2023 09:32:48 -0700 Subject: [PATCH 05/26] more lint --- pkg/monitor/cluster/maintenance_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/monitor/cluster/maintenance_test.go b/pkg/monitor/cluster/maintenance_test.go index eb240d51aaa..06ed7e558da 100644 --- a/pkg/monitor/cluster/maintenance_test.go +++ b/pkg/monitor/cluster/maintenance_test.go @@ -7,9 +7,10 @@ import ( "context" "testing" + "github.com/golang/mock/gomock" + "github.com/Azure/ARO-RP/pkg/api" mock_metrics "github.com/Azure/ARO-RP/pkg/util/mocks/metrics" - "github.com/golang/mock/gomock" ) func TestEmitPucmState(t *testing.T) { From 0cddd2973eff3bf01dec8673547f31e1ba6354bb Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Wed, 12 Jul 2023 10:11:27 -0700 Subject: [PATCH 06/26] cleanup test --- pkg/monitor/cluster/maintenance_test.go | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/pkg/monitor/cluster/maintenance_test.go b/pkg/monitor/cluster/maintenance_test.go index 06ed7e558da..87f3207f06c 100644 --- a/pkg/monitor/cluster/maintenance_test.go +++ b/pkg/monitor/cluster/maintenance_test.go @@ -10,6 +10,7 @@ import ( "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" ) @@ -27,10 +28,7 @@ func TestEmitPucmState(t *testing.T) { ProvisioningState: api.ProvisioningStateAdminUpdating, }, } - mon := &Monitor{ - m: m, - oc: oc, - } + mon := getMonitor(oc, m) m.EXPECT().EmitGauge("cluster.maintenance.pucm", int64(1), map[string]string{ "state": pucmUnplannedOngoing.String(), }) @@ -47,7 +45,7 @@ func TestEmitPucmState(t *testing.T) { PucmPending: true, }, } - + mon = getMonitor(oc, m) m.EXPECT().EmitGauge("cluster.maintenance.pucm", int64(1), map[string]string{ "state": pucmPlannedOngoing.String(), }) @@ -64,7 +62,7 @@ func TestEmitPucmState(t *testing.T) { PucmPending: true, }, } - + mon = getMonitor(oc, m) m.EXPECT().EmitGauge("cluster.maintenance.pucm", int64(1), map[string]string{ "state": pucmPending.String(), }) @@ -80,7 +78,7 @@ func TestEmitPucmState(t *testing.T) { ProvisioningState: api.ProvisioningStateSucceeded, }, } - + mon = getMonitor(oc, m) m.EXPECT().EmitGauge("cluster.maintenance.pucm", int64(1), map[string]string{ "state": pucmNone.String(), }) @@ -90,3 +88,10 @@ func TestEmitPucmState(t *testing.T) { t.Fatal(err) } } + +func getMonitor(oc *api.OpenShiftCluster, m metrics.Emitter) *Monitor { + return &Monitor{ + m: m, + oc: oc, + } +} From b0302d61c6dbd961d6d647deb1a32fb27786103f Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Wed, 12 Jul 2023 13:25:46 -0700 Subject: [PATCH 07/26] address comments --- pkg/monitor/cluster/maintenance.go | 12 ++-- pkg/monitor/cluster/maintenance_test.go | 87 ++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 10 deletions(-) diff --git a/pkg/monitor/cluster/maintenance.go b/pkg/monitor/cluster/maintenance.go index 5e59a7cbc0e..117ae03a7b3 100644 --- a/pkg/monitor/cluster/maintenance.go +++ b/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 { diff --git a/pkg/monitor/cluster/maintenance_test.go b/pkg/monitor/cluster/maintenance_test.go index 87f3207f06c..c7f7e66fad4 100644 --- a/pkg/monitor/cluster/maintenance_test.go +++ b/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{ From e10d12189add128ab0c21d6fbe2d19b70b113639 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Wed, 12 Jul 2023 13:31:07 -0700 Subject: [PATCH 08/26] lint --- pkg/monitor/cluster/maintenance_test.go | 85 ------------------------- 1 file changed, 85 deletions(-) diff --git a/pkg/monitor/cluster/maintenance_test.go b/pkg/monitor/cluster/maintenance_test.go index c7f7e66fad4..e648364d46f 100644 --- a/pkg/monitor/cluster/maintenance_test.go +++ b/pkg/monitor/cluster/maintenance_test.go @@ -8,7 +8,6 @@ import ( "testing" "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" ) @@ -94,87 +93,3 @@ func TestEmitPucmState(t *testing.T) { }) } } - -/* -func TestEmitPucmState(t *testing.T) { - ctx := context.Background() - - controller := gomock.NewController(t) - defer controller.Finish() - - m := mock_metrics.NewMockEmitter(controller) - - // Unplanned ongoing - oc := &api.OpenShiftCluster{ - Properties: api.OpenShiftClusterProperties{ - ProvisioningState: api.ProvisioningStateAdminUpdating, - }, - } - mon := getMonitor(oc, m) - m.EXPECT().EmitGauge("cluster.maintenance.pucm", int64(1), map[string]string{ - "state": pucmUnplannedOngoing.String(), - }) - - err := mon.emitPucmState(ctx) - if err != nil { - t.Fatal(err) - } - - // Planned ongoing - oc = &api.OpenShiftCluster{ - Properties: api.OpenShiftClusterProperties{ - ProvisioningState: api.ProvisioningStateAdminUpdating, - PucmPending: true, - }, - } - mon = getMonitor(oc, m) - m.EXPECT().EmitGauge("cluster.maintenance.pucm", int64(1), map[string]string{ - "state": pucmPlannedOngoing.String(), - }) - - err = mon.emitPucmState(ctx) - if err != nil { - t.Fatal(err) - } - - // Pending - oc = &api.OpenShiftCluster{ - Properties: api.OpenShiftClusterProperties{ - ProvisioningState: api.ProvisioningStateSucceeded, - PucmPending: true, - }, - } - mon = getMonitor(oc, m) - m.EXPECT().EmitGauge("cluster.maintenance.pucm", int64(1), map[string]string{ - "state": pucmPending.String(), - }) - - err = mon.emitPucmState(ctx) - if err != nil { - t.Fatal(err) - } - - // None - oc = &api.OpenShiftCluster{ - Properties: api.OpenShiftClusterProperties{ - ProvisioningState: api.ProvisioningStateSucceeded, - }, - } - mon = getMonitor(oc, m) - m.EXPECT().EmitGauge("cluster.maintenance.pucm", int64(1), map[string]string{ - "state": pucmNone.String(), - }) - - err = mon.emitPucmState(ctx) - if err != nil { - t.Fatal(err) - } -} -*/ - -func getMonitor(oc *api.OpenShiftCluster, m metrics.Emitter) *Monitor { - return &Monitor{ - m: m, - oc: oc, - } -} From dc64d2c4d4db646dd0321e545630108f2664d194 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Wed, 12 Jul 2023 13:53:16 -0700 Subject: [PATCH 09/26] validate --- pkg/monitor/cluster/maintenance_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/monitor/cluster/maintenance_test.go b/pkg/monitor/cluster/maintenance_test.go index e648364d46f..5cfdde9f55d 100644 --- a/pkg/monitor/cluster/maintenance_test.go +++ b/pkg/monitor/cluster/maintenance_test.go @@ -7,9 +7,10 @@ import ( "context" "testing" + "github.com/golang/mock/gomock" + "github.com/Azure/ARO-RP/pkg/api" mock_metrics "github.com/Azure/ARO-RP/pkg/util/mocks/metrics" - "github.com/golang/mock/gomock" ) func TestEmitPucmState(t *testing.T) { From be9abca731467a106e93bbae5568afff9b4ee309 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Wed, 12 Jul 2023 14:50:07 -0700 Subject: [PATCH 10/26] add loggiing --- pkg/monitor/cluster/maintenance.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/monitor/cluster/maintenance.go b/pkg/monitor/cluster/maintenance.go index 117ae03a7b3..8e632540647 100644 --- a/pkg/monitor/cluster/maintenance.go +++ b/pkg/monitor/cluster/maintenance.go @@ -5,6 +5,7 @@ package cluster import ( "context" + "time" "github.com/Azure/ARO-RP/pkg/api" ) @@ -59,6 +60,7 @@ func (mon *Monitor) emitPucmState(ctx context.Context) error { mon.emitGauge("cluster.maintenance.pucm", 1, map[string]string{ "state": state.String(), }) + mon.log.Infof("Cluster %s with PUCM state %s and time %s", mon.oc.Name, state.String(), time.Now().String()) return nil } From 4d14dce81b9c4a6340a2f16a4a593decfc8430e1 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Thu, 13 Jul 2023 05:23:34 -0700 Subject: [PATCH 11/26] Add maintenance task for pucm pending --- pkg/api/admin/openshiftcluster.go | 7 +- .../admin/openshiftcluster_validatestatic.go | 6 +- .../openshiftcluster_validatestatic_test.go | 13 ++++ pkg/api/openshiftcluster.go | 7 +- pkg/frontend/openshiftcluster_putorpatch.go | 30 ++++++--- .../openshiftcluster_putorpatch_test.go | 64 +++++++++++++++++++ 6 files changed, 112 insertions(+), 15 deletions(-) diff --git a/pkg/api/admin/openshiftcluster.go b/pkg/api/admin/openshiftcluster.go index 5cabe48947a..5992258432f 100644 --- a/pkg/api/admin/openshiftcluster.go +++ b/pkg/api/admin/openshiftcluster.go @@ -82,9 +82,10 @@ const ( type MaintenanceTask string const ( - MaintenanceTaskEverything MaintenanceTask = "Everything" - MaintenanceTaskOperator MaintenanceTask = "OperatorUpdate" - MaintenanceTaskRenewCerts MaintenanceTask = "CertificatesRenewal" + MaintenanceTaskEverything MaintenanceTask = "Everything" + MaintenanceTaskOperator MaintenanceTask = "OperatorUpdate" + MaintenanceTaskRenewCerts MaintenanceTask = "CertificatesRenewal" + MaintenanceTaskPucmPending MaintenanceTask = "PucmPending" ) // Operator feature flags diff --git a/pkg/api/admin/openshiftcluster_validatestatic.go b/pkg/api/admin/openshiftcluster_validatestatic.go index cad10cbf2c9..0a9b0f771dc 100644 --- a/pkg/api/admin/openshiftcluster_validatestatic.go +++ b/pkg/api/admin/openshiftcluster_validatestatic.go @@ -29,7 +29,11 @@ func (sv openShiftClusterStaticValidator) validateDelta(oc, current *OpenShiftCl return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodePropertyChangeNotAllowed, err.Target, err.Message) } - if !(oc.Properties.MaintenanceTask == "" || oc.Properties.MaintenanceTask == MaintenanceTaskEverything || oc.Properties.MaintenanceTask == MaintenanceTaskOperator || oc.Properties.MaintenanceTask == MaintenanceTaskRenewCerts) { + if !(oc.Properties.MaintenanceTask == "" || + oc.Properties.MaintenanceTask == MaintenanceTaskEverything || + oc.Properties.MaintenanceTask == MaintenanceTaskOperator || + oc.Properties.MaintenanceTask == MaintenanceTaskRenewCerts || + oc.Properties.MaintenanceTask == MaintenanceTaskPucmPending) { return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "properties.maintenanceTask", "Invalid enum parameter.") } diff --git a/pkg/api/admin/openshiftcluster_validatestatic_test.go b/pkg/api/admin/openshiftcluster_validatestatic_test.go index a9ce5135655..0eff0ddbd52 100644 --- a/pkg/api/admin/openshiftcluster_validatestatic_test.go +++ b/pkg/api/admin/openshiftcluster_validatestatic_test.go @@ -676,6 +676,19 @@ func TestOpenShiftClusterStaticValidateDelta(t *testing.T) { oc.Properties.MaintenanceTask = "" }, }, + { + name: "maintenanceTask change to PucmPending allowed", + oc: func() *OpenShiftCluster { + return &OpenShiftCluster{ + Properties: OpenShiftClusterProperties{ + MaintenanceTask: MaintenanceTaskPucmPending, + }, + } + }, + modify: func(oc *OpenShiftCluster) { + oc.Properties.MaintenanceTask = "" + }, + }, { name: "maintenanceTask change to other values is disallowed", oc: func() *OpenShiftCluster { diff --git a/pkg/api/openshiftcluster.go b/pkg/api/openshiftcluster.go index cd9e6353f34..cb5e938cf28 100644 --- a/pkg/api/openshiftcluster.go +++ b/pkg/api/openshiftcluster.go @@ -177,9 +177,10 @@ const ( type MaintenanceTask string const ( - MaintenanceTaskEverything MaintenanceTask = "Everything" - MaintenanceTaskOperator MaintenanceTask = "OperatorUpdate" - MaintenanceTaskRenewCerts MaintenanceTask = "CertificatesRenewal" + MaintenanceTaskEverything MaintenanceTask = "Everything" + MaintenanceTaskOperator MaintenanceTask = "OperatorUpdate" + MaintenanceTaskRenewCerts MaintenanceTask = "CertificatesRenewal" + MaintenanceTaskPucmPending MaintenanceTask = "PucmPending" ) // Cluster-scoped flags diff --git a/pkg/frontend/openshiftcluster_putorpatch.go b/pkg/frontend/openshiftcluster_putorpatch.go index 047118cbb69..b9449b783b5 100644 --- a/pkg/frontend/openshiftcluster_putorpatch.go +++ b/pkg/frontend/openshiftcluster_putorpatch.go @@ -205,14 +205,7 @@ func (f *frontend) _putOrPatchOpenShiftCluster(ctx context.Context, log *logrus. } } else { doc.OpenShiftCluster.Properties.LastProvisioningState = doc.OpenShiftCluster.Properties.ProvisioningState - - // TODO: Get rid of the special case - if apiVersion == admin.APIVersion { - doc.OpenShiftCluster.Properties.ProvisioningState = api.ProvisioningStateAdminUpdating - doc.OpenShiftCluster.Properties.LastAdminUpdateError = "" - } else { - doc.OpenShiftCluster.Properties.ProvisioningState = api.ProvisioningStateUpdating - } + setUpdateProvisioningState(doc, apiVersion) doc.Dequeues = 0 } @@ -312,3 +305,24 @@ func (f *frontend) ValidateNewCluster(ctx context.Context, subscription *api.Sub return nil } + +// Sets either the admin update or update provisioning state +func setUpdateProvisioningState(doc *api.OpenShiftClusterDocument, apiVersion string) { + switch apiVersion { + case admin.APIVersion: + // For PUCM pending update, we don't want to set ProvisioningStateAdminUpdating + // The cluster monitoring stack uses that value to determine if PUCM is ongoing + if doc.OpenShiftCluster.Properties.MaintenanceTask != api.MaintenanceTaskPucmPending { + doc.OpenShiftCluster.Properties.ProvisioningState = api.ProvisioningStateAdminUpdating + doc.OpenShiftCluster.Properties.LastAdminUpdateError = "" + } else { + doc.OpenShiftCluster.Properties.PucmPending = true + doc.OpenShiftCluster.Properties.ProvisioningState = api.ProvisioningStateUpdating + } + default: + // Non-admin update (ex: customer cluster update) + doc.OpenShiftCluster.Properties.ProvisioningState = api.ProvisioningStateUpdating + } + + return +} diff --git a/pkg/frontend/openshiftcluster_putorpatch_test.go b/pkg/frontend/openshiftcluster_putorpatch_test.go index 02b329ff8dd..690df742dd2 100644 --- a/pkg/frontend/openshiftcluster_putorpatch_test.go +++ b/pkg/frontend/openshiftcluster_putorpatch_test.go @@ -557,6 +557,70 @@ func TestPutOrPatchOpenShiftClusterAdminAPI(t *testing.T) { wantStatusCode: http.StatusBadRequest, wantError: `400: PropertyChangeNotAllowed: properties.registryProfiles: Changing property 'properties.registryProfiles' is not allowed.`, }, + { + name: "patch a cluster with pucm pending request", + request: func(oc *admin.OpenShiftCluster) { + oc.Properties.MaintenanceTask = admin.MaintenanceTaskPucmPending + }, + isPatch: true, + fixture: func(f *testdatabase.Fixture) { + f.AddSubscriptionDocuments(&api.SubscriptionDocument{ + ID: mockSubID, + Subscription: &api.Subscription{ + State: api.SubscriptionStateRegistered, + }, + }) + f.AddOpenShiftClusterDocuments(&api.OpenShiftClusterDocument{ + Key: strings.ToLower(testdatabase.GetResourcePath(mockSubID, "resourceName")), + OpenShiftCluster: &api.OpenShiftCluster{ + ID: testdatabase.GetResourcePath(mockSubID, "resourceName"), + Type: "Microsoft.RedHatOpenShift/openShiftClusters", + Tags: map[string]string{"tag": "will-be-kept"}, + Properties: api.OpenShiftClusterProperties{ + ProvisioningState: api.ProvisioningStateSucceeded, + MaintenanceTask: "", + }, + }, + }) + }, + wantSystemDataEnriched: true, + wantEnriched: []string{testdatabase.GetResourcePath(mockSubID, "resourceName")}, + wantDocuments: func(c *testdatabase.Checker) { + c.AddAsyncOperationDocuments(&api.AsyncOperationDocument{ + OpenShiftClusterKey: strings.ToLower(testdatabase.GetResourcePath(mockSubID, "resourceName")), + AsyncOperation: &api.AsyncOperation{ + InitialProvisioningState: api.ProvisioningStateSucceeded, + ProvisioningState: api.ProvisioningStateUpdating, + }, + }) + c.AddOpenShiftClusterDocuments(&api.OpenShiftClusterDocument{ + Key: strings.ToLower(testdatabase.GetResourcePath(mockSubID, "resourceName")), + OpenShiftCluster: &api.OpenShiftCluster{ + ID: testdatabase.GetResourcePath(mockSubID, "resourceName"), + Type: "Microsoft.RedHatOpenShift/openShiftClusters", + Tags: map[string]string{"tag": "will-be-kept"}, + Properties: api.OpenShiftClusterProperties{ + ProvisioningState: api.ProvisioningStateUpdating, + LastProvisioningState: api.ProvisioningStateSucceeded, + ClusterProfile: api.ClusterProfile{ + FipsValidatedModules: api.FipsValidatedModulesDisabled, + }, + MaintenanceTask: "", + NetworkProfile: api.NetworkProfile{ + OutboundType: api.OutboundTypeLoadbalancer, + PreconfiguredNSG: api.PreconfiguredNSGDisabled, + }, + MasterProfile: api.MasterProfile{ + EncryptionAtHost: api.EncryptionAtHostDisabled, + }, + PucmPending: true, + }, + }, + }) + }, + wantAsync: true, + wantStatusCode: http.StatusOK, + }, } { t.Run(tt.name, func(t *testing.T) { ti := newTestInfra(t). From c4f88185ad6d5b078e6d1267fe97e63fc340562d Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Thu, 13 Jul 2023 05:30:11 -0700 Subject: [PATCH 12/26] lint --- pkg/frontend/openshiftcluster_putorpatch.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/frontend/openshiftcluster_putorpatch.go b/pkg/frontend/openshiftcluster_putorpatch.go index b9449b783b5..eca83bd0aef 100644 --- a/pkg/frontend/openshiftcluster_putorpatch.go +++ b/pkg/frontend/openshiftcluster_putorpatch.go @@ -323,6 +323,4 @@ func setUpdateProvisioningState(doc *api.OpenShiftClusterDocument, apiVersion st // Non-admin update (ex: customer cluster update) doc.OpenShiftCluster.Properties.ProvisioningState = api.ProvisioningStateUpdating } - - return } From 3d1bd6c315c432de2433413a535c808c0c472574 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Thu, 13 Jul 2023 06:03:03 -0700 Subject: [PATCH 13/26] unit test --- pkg/frontend/openshiftcluster_putorpatch_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/frontend/openshiftcluster_putorpatch_test.go b/pkg/frontend/openshiftcluster_putorpatch_test.go index 690df742dd2..89b10718cd7 100644 --- a/pkg/frontend/openshiftcluster_putorpatch_test.go +++ b/pkg/frontend/openshiftcluster_putorpatch_test.go @@ -589,7 +589,7 @@ func TestPutOrPatchOpenShiftClusterAdminAPI(t *testing.T) { c.AddAsyncOperationDocuments(&api.AsyncOperationDocument{ OpenShiftClusterKey: strings.ToLower(testdatabase.GetResourcePath(mockSubID, "resourceName")), AsyncOperation: &api.AsyncOperation{ - InitialProvisioningState: api.ProvisioningStateSucceeded, + InitialProvisioningState: api.ProvisioningStateUpdating, ProvisioningState: api.ProvisioningStateUpdating, }, }) @@ -605,7 +605,7 @@ func TestPutOrPatchOpenShiftClusterAdminAPI(t *testing.T) { ClusterProfile: api.ClusterProfile{ FipsValidatedModules: api.FipsValidatedModulesDisabled, }, - MaintenanceTask: "", + MaintenanceTask: api.MaintenanceTaskPucmPending, NetworkProfile: api.NetworkProfile{ OutboundType: api.OutboundTypeLoadbalancer, PreconfiguredNSG: api.PreconfiguredNSGDisabled, @@ -614,6 +614,7 @@ func TestPutOrPatchOpenShiftClusterAdminAPI(t *testing.T) { EncryptionAtHost: api.EncryptionAtHostDisabled, }, PucmPending: true, + OperatorFlags: api.DefaultOperatorFlags(), }, }, }) From dc19e21d4b5da982516b13f90c85e26ea36cf09f Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Thu, 13 Jul 2023 06:35:38 -0700 Subject: [PATCH 14/26] fix test --- .../openshiftcluster_putorpatch_test.go | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/pkg/frontend/openshiftcluster_putorpatch_test.go b/pkg/frontend/openshiftcluster_putorpatch_test.go index 89b10718cd7..0755554bff3 100644 --- a/pkg/frontend/openshiftcluster_putorpatch_test.go +++ b/pkg/frontend/openshiftcluster_putorpatch_test.go @@ -613,7 +613,7 @@ func TestPutOrPatchOpenShiftClusterAdminAPI(t *testing.T) { MasterProfile: api.MasterProfile{ EncryptionAtHost: api.EncryptionAtHostDisabled, }, - PucmPending: true, + PucmPending: true, OperatorFlags: api.DefaultOperatorFlags(), }, }, @@ -621,6 +621,26 @@ func TestPutOrPatchOpenShiftClusterAdminAPI(t *testing.T) { }, wantAsync: true, wantStatusCode: http.StatusOK, + wantResponse: &admin.OpenShiftCluster{ + ID: testdatabase.GetResourcePath(mockSubID, "resourceName"), + Type: "Microsoft.RedHatOpenShift/openShiftClusters", + Tags: map[string]string{"tag": "will-be-kept"}, + Properties: admin.OpenShiftClusterProperties{ + ProvisioningState: admin.ProvisioningStateUpdating, + LastProvisioningState: admin.ProvisioningStateSucceeded, + ClusterProfile: admin.ClusterProfile{ + FipsValidatedModules: admin.FipsValidatedModulesDisabled, + }, + MaintenanceTask: admin.MaintenanceTaskPucmPending, + NetworkProfile: admin.NetworkProfile{ + OutboundType: admin.OutboundTypeLoadbalancer, + }, + MasterProfile: admin.MasterProfile{ + EncryptionAtHost: admin.EncryptionAtHostDisabled, + }, + OperatorFlags: admin.OperatorFlags(api.DefaultOperatorFlags()), + }, + }, }, } { t.Run(tt.name, func(t *testing.T) { From 6ed96c3bb9778876a3dfd9945fe3be575c79eaa1 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Thu, 13 Jul 2023 07:04:35 -0700 Subject: [PATCH 15/26] add to bottom --- pkg/monitor/cluster/cluster.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/monitor/cluster/cluster.go b/pkg/monitor/cluster/cluster.go index 02db5af4448..209a550ead9 100644 --- a/pkg/monitor/cluster/cluster.go +++ b/pkg/monitor/cluster/cluster.go @@ -153,7 +153,6 @@ func (mon *Monitor) Monitor(ctx context.Context) (errs []error) { return } for _, f := range []func(context.Context) error{ - mon.emitPucmState, mon.emitAroOperatorHeartbeat, mon.emitAroOperatorConditions, mon.emitNSGReconciliation, @@ -175,6 +174,7 @@ func (mon *Monitor) Monitor(ctx context.Context) (errs []error) { mon.emitSummary, mon.emitHiveRegistrationStatus, mon.emitOperatorFlagsAndSupportBanner, + mon.emitPucmState, mon.emitPrometheusAlerts, // at the end for now because it's the slowest/least reliable } { err = f(ctx) From 037fcf1c33047d3556fdca3061540e396b1deca6 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Thu, 13 Jul 2023 07:14:05 -0700 Subject: [PATCH 16/26] update logging --- pkg/monitor/cluster/maintenance.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/monitor/cluster/maintenance.go b/pkg/monitor/cluster/maintenance.go index 8e632540647..6a614cd5b1e 100644 --- a/pkg/monitor/cluster/maintenance.go +++ b/pkg/monitor/cluster/maintenance.go @@ -60,7 +60,6 @@ func (mon *Monitor) emitPucmState(ctx context.Context) error { mon.emitGauge("cluster.maintenance.pucm", 1, map[string]string{ "state": state.String(), }) - mon.log.Infof("Cluster %s with PUCM state %s and time %s", mon.oc.Name, state.String(), time.Now().String()) return nil } From 9aec74358bc4eb17c689d92f6e2583887f481735 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Thu, 13 Jul 2023 07:33:31 -0700 Subject: [PATCH 17/26] set no pucm pending properly --- pkg/backend/openshiftcluster.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/backend/openshiftcluster.go b/pkg/backend/openshiftcluster.go index 0a5a7fee31a..591bab19e8b 100644 --- a/pkg/backend/openshiftcluster.go +++ b/pkg/backend/openshiftcluster.go @@ -161,7 +161,7 @@ func (ocb *openShiftClusterBackend) handle(ctx context.Context, log *logrus.Entr if err != nil { return ocb.endLease(ctx, log, stop, doc, api.ProvisioningStateFailed, err) } - doc.OpenShiftCluster.Properties.PucmPending = false + ocb.setNoPucmPending(ctx, doc) return ocb.endLease(ctx, log, stop, doc, api.ProvisioningStateSucceeded, nil) case api.ProvisioningStateUpdating: @@ -362,3 +362,10 @@ func (ocb *openShiftClusterBackend) emitMetrics(doc *api.OpenShiftClusterDocumen "newProvisioningState": string(provisioningState), }) } + +func (ocb *openShiftClusterBackend) setNoPucmPending(ctx context.Context, doc *api.OpenShiftClusterDocument) (*api.OpenShiftClusterDocument, error) { + return ocb.dbOpenShiftClusters.Patch(ctx, doc.Key, func(doc *api.OpenShiftClusterDocument) error { + doc.OpenShiftCluster.Properties.PucmPending = false + return nil + }) +} From 26ad167c649316820bca7f50c0c8ab6662867c47 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Thu, 13 Jul 2023 07:35:44 -0700 Subject: [PATCH 18/26] fix error checking --- pkg/backend/openshiftcluster.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/backend/openshiftcluster.go b/pkg/backend/openshiftcluster.go index 591bab19e8b..560b61c772c 100644 --- a/pkg/backend/openshiftcluster.go +++ b/pkg/backend/openshiftcluster.go @@ -161,7 +161,10 @@ func (ocb *openShiftClusterBackend) handle(ctx context.Context, log *logrus.Entr if err != nil { return ocb.endLease(ctx, log, stop, doc, api.ProvisioningStateFailed, err) } - ocb.setNoPucmPending(ctx, doc) + doc, err = ocb.setNoPucmPending(ctx, doc) + if err != nil { + return ocb.endLease(ctx, log, stop, doc, api.ProvisioningStateFailed, err) + } return ocb.endLease(ctx, log, stop, doc, api.ProvisioningStateSucceeded, nil) case api.ProvisioningStateUpdating: From 0a62e4ede7f27c8d2125105e13289295d839155b Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Thu, 13 Jul 2023 07:40:52 -0700 Subject: [PATCH 19/26] fix lint --- pkg/monitor/cluster/maintenance.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/monitor/cluster/maintenance.go b/pkg/monitor/cluster/maintenance.go index 6a614cd5b1e..0a1d4530d03 100644 --- a/pkg/monitor/cluster/maintenance.go +++ b/pkg/monitor/cluster/maintenance.go @@ -4,8 +4,7 @@ package cluster // Licensed under the Apache License 2.0. import ( - "context" - "time" + "context" "github.com/Azure/ARO-RP/pkg/api" ) From 66d7ea82c97e6149558be64c9c6d8b1e135b4e4c Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Thu, 13 Jul 2023 07:54:00 -0700 Subject: [PATCH 20/26] lint --- pkg/monitor/cluster/maintenance.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/monitor/cluster/maintenance.go b/pkg/monitor/cluster/maintenance.go index 0a1d4530d03..117ae03a7b3 100644 --- a/pkg/monitor/cluster/maintenance.go +++ b/pkg/monitor/cluster/maintenance.go @@ -4,7 +4,7 @@ package cluster // Licensed under the Apache License 2.0. import ( - "context" + "context" "github.com/Azure/ARO-RP/pkg/api" ) From 3775b362684123591a148413f2eca8c01c8ff56a Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Mon, 17 Jul 2023 09:55:38 -0700 Subject: [PATCH 21/26] nit fix for unit test --- pkg/monitor/cluster/maintenance_test.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/pkg/monitor/cluster/maintenance_test.go b/pkg/monitor/cluster/maintenance_test.go index 5cfdde9f55d..b91f8af9c23 100644 --- a/pkg/monitor/cluster/maintenance_test.go +++ b/pkg/monitor/cluster/maintenance_test.go @@ -24,35 +24,28 @@ func TestEmitPucmState(t *testing.T) { { 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, }, From 2cefc953cd4cb3ea65c496d1f4175f8696d39a85 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Mon, 17 Jul 2023 10:06:02 -0700 Subject: [PATCH 22/26] refactor getPucmState --- pkg/monitor/cluster/maintenance.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/pkg/monitor/cluster/maintenance.go b/pkg/monitor/cluster/maintenance.go index 117ae03a7b3..1e3731ffd70 100644 --- a/pkg/monitor/cluster/maintenance.go +++ b/pkg/monitor/cluster/maintenance.go @@ -67,13 +67,10 @@ func getPucmState(clusterProperties api.OpenShiftClusterProperties) pucmState { if pucmOngoing(clusterProperties) { if clusterProperties.PucmPending { return pucmPlannedOngoing - } else { - return pucmUnplannedOngoing - } - } else { - if clusterProperties.PucmPending { - return pucmPending } + return pucmUnplannedOngoing + } else if clusterProperties.PucmPending { + return pucmPending } return pucmNone From d8379b6deeb3bd1f7e9056b39acf451cfbe40662 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Mon, 17 Jul 2023 10:17:50 -0700 Subject: [PATCH 23/26] remove ongoing --- pkg/monitor/cluster/maintenance.go | 12 ++++++------ pkg/monitor/cluster/maintenance_test.go | 16 ++++++++-------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/monitor/cluster/maintenance.go b/pkg/monitor/cluster/maintenance.go index 1e3731ffd70..31d93086e44 100644 --- a/pkg/monitor/cluster/maintenance.go +++ b/pkg/monitor/cluster/maintenance.go @@ -48,10 +48,10 @@ func (p pucmState) String() string { } const ( - pucmNone pucmState = "none" - pucmPending pucmState = "pending" - pucmPlannedOngoing pucmState = "planned_ongoing" - pucmUnplannedOngoing pucmState = "unplanned_ongoing" + pucmNone pucmState = "none" + pucmPending pucmState = "pending" + pucmPlanned pucmState = "planned" + pucmUnplanned pucmState = "unplanned" ) func (mon *Monitor) emitPucmState(ctx context.Context) error { @@ -66,9 +66,9 @@ func (mon *Monitor) emitPucmState(ctx context.Context) error { func getPucmState(clusterProperties api.OpenShiftClusterProperties) pucmState { if pucmOngoing(clusterProperties) { if clusterProperties.PucmPending { - return pucmPlannedOngoing + return pucmPlanned } - return pucmUnplannedOngoing + return pucmUnplanned } else if clusterProperties.PucmPending { return pucmPending } diff --git a/pkg/monitor/cluster/maintenance_test.go b/pkg/monitor/cluster/maintenance_test.go index b91f8af9c23..d9f79ee1fe9 100644 --- a/pkg/monitor/cluster/maintenance_test.go +++ b/pkg/monitor/cluster/maintenance_test.go @@ -33,28 +33,28 @@ func TestEmitPucmState(t *testing.T) { expectedPucmState: pucmPending, }, { - name: "state unplanned ongoing - admin updating in flight and no admin update error", + name: "state unplanned - admin updating in flight and no admin update error", provisioningState: api.ProvisioningStateAdminUpdating, - expectedPucmState: pucmUnplannedOngoing, + expectedPucmState: pucmUnplanned, }, { - name: "state planned ongoing - admin updating in flight and no admin update error", + name: "state planned - admin updating in flight and no admin update error", provisioningState: api.ProvisioningStateAdminUpdating, pucmPending: true, - expectedPucmState: pucmPlannedOngoing, + expectedPucmState: pucmPlanned, }, { - name: "state unplanned ongoing - not admin updating but admin update error", + name: "state unplanned - not admin updating but admin update error", provisioningState: api.ProvisioningStateFailed, adminUpdateErr: "PUCM failed", - expectedPucmState: pucmUnplannedOngoing, + expectedPucmState: pucmUnplanned, }, { - name: "state planned ongoing - not admin updating but admin update error", + name: "state planned - not admin updating but admin update error", provisioningState: api.ProvisioningStateFailed, pucmPending: true, adminUpdateErr: "PUCM failed", - expectedPucmState: pucmPlannedOngoing, + expectedPucmState: pucmPlanned, }, } { t.Run(tt.name, func(t *testing.T) { From b13ddf95aed3589b1ddbfdba0a1dab1c9cda37b0 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Mon, 17 Jul 2023 11:08:50 -0700 Subject: [PATCH 24/26] validate static refactor --- pkg/api/admin/openshiftcluster_validatestatic.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/api/admin/openshiftcluster_validatestatic.go b/pkg/api/admin/openshiftcluster_validatestatic.go index 0a9b0f771dc..2b21bc125c3 100644 --- a/pkg/api/admin/openshiftcluster_validatestatic.go +++ b/pkg/api/admin/openshiftcluster_validatestatic.go @@ -29,11 +29,15 @@ func (sv openShiftClusterStaticValidator) validateDelta(oc, current *OpenShiftCl return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodePropertyChangeNotAllowed, err.Target, err.Message) } - if !(oc.Properties.MaintenanceTask == "" || - oc.Properties.MaintenanceTask == MaintenanceTaskEverything || - oc.Properties.MaintenanceTask == MaintenanceTaskOperator || - oc.Properties.MaintenanceTask == MaintenanceTaskRenewCerts || - oc.Properties.MaintenanceTask == MaintenanceTaskPucmPending) { + return validateMaintenanceTask(oc.Properties.MaintenanceTask) +} + +func validateMaintenanceTask(task MaintenanceTask) error { + if !(task == "" || + task == MaintenanceTaskEverything || + task == MaintenanceTaskOperator || + task == MaintenanceTaskRenewCerts || + task == MaintenanceTaskPucmPending) { return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "properties.maintenanceTask", "Invalid enum parameter.") } From 9cd12648ebc218aeb07245ee30c5fd92817452dc Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Mon, 17 Jul 2023 11:11:42 -0700 Subject: [PATCH 25/26] add comment for setUpdateProvisioningState --- pkg/frontend/openshiftcluster_putorpatch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/frontend/openshiftcluster_putorpatch.go b/pkg/frontend/openshiftcluster_putorpatch.go index eca83bd0aef..0ce1146f24e 100644 --- a/pkg/frontend/openshiftcluster_putorpatch.go +++ b/pkg/frontend/openshiftcluster_putorpatch.go @@ -306,7 +306,7 @@ func (f *frontend) ValidateNewCluster(ctx context.Context, subscription *api.Sub return nil } -// Sets either the admin update or update provisioning state +// setUpdateProvisioningState Sets either the admin update or update provisioning state func setUpdateProvisioningState(doc *api.OpenShiftClusterDocument, apiVersion string) { switch apiVersion { case admin.APIVersion: From caf98c7bf2b35d068d3f9ad0a48ca44b7d059474 Mon Sep 17 00:00:00 2001 From: Nicolas Ontiveros Date: Mon, 17 Jul 2023 11:18:15 -0700 Subject: [PATCH 26/26] minor refactor --- pkg/monitor/cluster/maintenance.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/monitor/cluster/maintenance.go b/pkg/monitor/cluster/maintenance.go index 31d93086e44..c70e1ed4662 100644 --- a/pkg/monitor/cluster/maintenance.go +++ b/pkg/monitor/cluster/maintenance.go @@ -69,7 +69,9 @@ func getPucmState(clusterProperties api.OpenShiftClusterProperties) pucmState { return pucmPlanned } return pucmUnplanned - } else if clusterProperties.PucmPending { + } + + if clusterProperties.PucmPending { return pucmPending }