From 24a0ff60a782e88e249fefd07206ffb570e940e0 Mon Sep 17 00:00:00 2001 From: Arris Li Date: Thu, 11 Aug 2022 10:11:20 +1200 Subject: [PATCH 1/5] test --- pkg/cluster/install.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cluster/install.go b/pkg/cluster/install.go index 82c63f4df..a034a07df 100644 --- a/pkg/cluster/install.go +++ b/pkg/cluster/install.go @@ -186,7 +186,7 @@ func (m *manager) Install(ctx context.Context) error { steps.Action(m.incrInstallPhase), }, api.InstallPhaseRemoveBootstrap: { - steps.Action(m.initializeKubernetesClients), + steps.Action(m.initializeKubernetesClients, "asdasdasdas_dasdasdasdas_asdasdasd"), steps.Action(m.initializeOperatorDeployer), // depends on kube clients steps.Action(m.removeBootstrap), steps.Action(m.removeBootstrapIgnition), From d2324ececfec01a663c307e4621b30920c121485 Mon Sep 17 00:00:00 2001 From: Arris Li Date: Thu, 11 Aug 2022 10:15:39 +1200 Subject: [PATCH 2/5] test --- pkg/cluster/install.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cluster/install.go b/pkg/cluster/install.go index a034a07df..82c63f4df 100644 --- a/pkg/cluster/install.go +++ b/pkg/cluster/install.go @@ -186,7 +186,7 @@ func (m *manager) Install(ctx context.Context) error { steps.Action(m.incrInstallPhase), }, api.InstallPhaseRemoveBootstrap: { - steps.Action(m.initializeKubernetesClients, "asdasdasdas_dasdasdasdas_asdasdasd"), + steps.Action(m.initializeKubernetesClients), steps.Action(m.initializeOperatorDeployer), // depends on kube clients steps.Action(m.removeBootstrap), steps.Action(m.removeBootstrapIgnition), From 4bbc054d4d188f863e15ee8520f0b1612cc17c88 Mon Sep 17 00:00:00 2001 From: Arris Li Date: Fri, 2 Dec 2022 13:40:49 +1300 Subject: [PATCH 3/5] enable metrics topic for different cluster actions --- pkg/cluster/install.go | 16 ++++++++-------- pkg/cluster/install_test.go | 12 ++++++------ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/cluster/install.go b/pkg/cluster/install.go index 66a7dce8c..127bcde2d 100644 --- a/pkg/cluster/install.go +++ b/pkg/cluster/install.go @@ -30,7 +30,7 @@ import ( // AdminUpdate performs an admin update of an ARO cluster func (m *manager) AdminUpdate(ctx context.Context) error { toRun := m.adminUpdate() - return m.runSteps(ctx, toRun, false) + return m.runSteps(ctx, toRun, "adminUpdate") } func (m *manager) adminUpdate() []steps.Step { @@ -179,7 +179,7 @@ func (m *manager) Update(ctx context.Context) error { ) } - return m.runSteps(ctx, s, false) + return m.runSteps(ctx, s, "update") } func (m *manager) runIntegratedInstaller(ctx context.Context) error { @@ -314,21 +314,21 @@ func (m *manager) Install(ctx context.Context) error { return fmt.Errorf("unrecognised phase %s", m.doc.OpenShiftCluster.Properties.Install.Phase) } m.log.Printf("starting phase %s", m.doc.OpenShiftCluster.Properties.Install.Phase) - return m.runSteps(ctx, steps[m.doc.OpenShiftCluster.Properties.Install.Phase], true) + return m.runSteps(ctx, steps[m.doc.OpenShiftCluster.Properties.Install.Phase], "install") } -func (m *manager) runSteps(ctx context.Context, s []steps.Step, emitMetrics bool) error { +func (m *manager) runSteps(ctx context.Context, s []steps.Step, metricsTopic string) error { var err error - if emitMetrics { + if metricsTopic != "" { var stepsTimeRun map[string]int64 stepsTimeRun, err = steps.Run(ctx, m.log, 10*time.Second, s, m.now) if err == nil { var totalInstallTime int64 - for topic, duration := range stepsTimeRun { - m.metricsEmitter.EmitGauge(fmt.Sprintf("backend.openshiftcluster.installtime.%s", topic), duration, nil) + for stepName, duration := range stepsTimeRun { + m.metricsEmitter.EmitGauge(fmt.Sprintf("backend.openshiftcluster.%s.%s.time", metricsTopic, stepName), duration, nil) totalInstallTime += duration } - m.metricsEmitter.EmitGauge("backend.openshiftcluster.installtime.total", totalInstallTime, nil) + m.metricsEmitter.EmitGauge(fmt.Sprintf("backend.openshiftcluster.%s.time.total", metricsTopic), totalInstallTime, nil) } } else { _, err = steps.Run(ctx, m.log, 10*time.Second, s, nil) diff --git a/pkg/cluster/install_test.go b/pkg/cluster/install_test.go index ae4a8d729..263a43331 100644 --- a/pkg/cluster/install_test.go +++ b/pkg/cluster/install_test.go @@ -175,7 +175,7 @@ func TestStepRunnerWithInstaller(t *testing.T) { now: func() time.Time { return time.Now() }, } - err := m.runSteps(ctx, tt.steps, false) + err := m.runSteps(ctx, tt.steps, "") if err != nil && err.Error() != tt.wantErr || err == nil && tt.wantErr != "" { t.Error(err) @@ -257,10 +257,10 @@ func TestInstallationTimeMetrics(t *testing.T) { steps.AuthorizationRefreshingAction(nil, steps.Action(successfulActionStep)), }, wantedMetrics: map[string]int64{ - "backend.openshiftcluster.installtime.total": 6, - "backend.openshiftcluster.installtime.action.successfulActionStep": 2, - "backend.openshiftcluster.installtime.condition.successfulConditionStep": 2, - "backend.openshiftcluster.installtime.refreshing.successfulActionStep": 2, + "backend.openshiftcluster.install.time.total": 6, + "backend.openshiftcluster.install.action.successfulActionStep.time": 2, + "backend.openshiftcluster.install.condition.successfulConditionStep.time": 2, + "backend.openshiftcluster.install.refreshing.successfulActionStep.time": 2, }, }, } { @@ -272,7 +272,7 @@ func TestInstallationTimeMetrics(t *testing.T) { now: func() time.Time { return time.Now().Add(2 * time.Second) }, } - err := m.runSteps(ctx, tt.steps, true) + err := m.runSteps(ctx, tt.steps, "install") if err != nil { if len(fm.Metrics) != 0 { t.Error("fake metrics obj should be empty when run steps failed") From 6ecd8312fb94f69b9244e2f998b6497cd19bd814 Mon Sep 17 00:00:00 2001 From: Arris Li Date: Thu, 9 Mar 2023 15:49:11 +1300 Subject: [PATCH 4/5] unit test cases update --- pkg/cluster/install_test.go | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/pkg/cluster/install_test.go b/pkg/cluster/install_test.go index d214049f3..f2f75521a 100644 --- a/pkg/cluster/install_test.go +++ b/pkg/cluster/install_test.go @@ -239,18 +239,23 @@ func TestInstallationTimeMetrics(t *testing.T) { for _, tt := range []struct { name string + metricsTopic string + timePerStep int64 steps []steps.Step wantedMetrics map[string]int64 }{ { - name: "Failed step run will not generate any install time metrics", + name: "Failed step run will not generate any install time metrics", + metricsTopic: "install", steps: []steps.Step{ steps.Action(successfulActionStep), steps.Action(failingFunc), }, }, { - name: "Succeeded step run will generate a valid install time metrics", + name: "Succeeded step run for cluster installation will generate a valid install time metrics", + metricsTopic: "install", + timePerStep: 2, steps: []steps.Step{ steps.Action(successfulActionStep), steps.Condition(successfulConditionStep, 30*time.Minute, true), @@ -263,16 +268,32 @@ func TestInstallationTimeMetrics(t *testing.T) { "backend.openshiftcluster.install.refreshing.successfulActionStep.time": 2, }, }, + { + name: "Succeeded step run for cluster update will generate a valid install time metrics", + metricsTopic: "update", + timePerStep: 3, + steps: []steps.Step{ + steps.Action(successfulActionStep), + steps.Condition(successfulConditionStep, 30*time.Minute, true), + steps.AuthorizationRefreshingAction(nil, steps.Action(successfulActionStep)), + }, + wantedMetrics: map[string]int64{ + "backend.openshiftcluster.update.time.total": 9, + "backend.openshiftcluster.update.action.successfulActionStep.time": 3, + "backend.openshiftcluster.update.condition.successfulConditionStep.time": 3, + "backend.openshiftcluster.update.refreshing.successfulActionStep.time": 3, + }, + }, } { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() m := &manager{ log: log, metricsEmitter: fm, - now: func() time.Time { return time.Now().Add(2 * time.Second) }, + now: func() time.Time { return time.Now().Add(time.Duration(tt.timePerStep) * time.Second) }, } - err := m.runSteps(ctx, tt.steps, "install") + err := m.runSteps(ctx, tt.steps, tt.metricsTopic) if err != nil { if len(fm.Metrics) != 0 { t.Error("fake metrics obj should be empty when run steps failed") @@ -282,10 +303,10 @@ func TestInstallationTimeMetrics(t *testing.T) { for k, v := range tt.wantedMetrics { time, ok := fm.Metrics[k] if !ok { - t.Errorf("unexpected metrics topic: %s", k) + t.Errorf("unexpected metrics key: %s", k) } if time != v { - t.Errorf("incorrect fake metrics obj, want: %d, got: %d", v, time) + t.Errorf("incorrect fake metrics value, want: %d, got: %d", v, time) } } } From 8734325163eb8a88a34dcebd7822dbcf3a1827f5 Mon Sep 17 00:00:00 2001 From: Arris Li Date: Fri, 10 Mar 2023 11:03:49 +1300 Subject: [PATCH 5/5] metrics topic modification --- pkg/cluster/install.go | 4 ++-- pkg/cluster/install_test.go | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/cluster/install.go b/pkg/cluster/install.go index c191eaf07..0290cd191 100644 --- a/pkg/cluster/install.go +++ b/pkg/cluster/install.go @@ -348,10 +348,10 @@ func (m *manager) runSteps(ctx context.Context, s []steps.Step, metricsTopic str if err == nil { var totalInstallTime int64 for stepName, duration := range stepsTimeRun { - m.metricsEmitter.EmitGauge(fmt.Sprintf("backend.openshiftcluster.%s.%s.time", metricsTopic, stepName), duration, nil) + m.metricsEmitter.EmitGauge(fmt.Sprintf("backend.openshiftcluster.%s.%s.duration.seconds", metricsTopic, stepName), duration, nil) totalInstallTime += duration } - m.metricsEmitter.EmitGauge(fmt.Sprintf("backend.openshiftcluster.%s.time.total", metricsTopic), totalInstallTime, nil) + m.metricsEmitter.EmitGauge(fmt.Sprintf("backend.openshiftcluster.%s.duration.total.seconds", metricsTopic), totalInstallTime, nil) } } else { _, err = steps.Run(ctx, m.log, 10*time.Second, s, nil) diff --git a/pkg/cluster/install_test.go b/pkg/cluster/install_test.go index f2f75521a..00414f664 100644 --- a/pkg/cluster/install_test.go +++ b/pkg/cluster/install_test.go @@ -262,10 +262,10 @@ func TestInstallationTimeMetrics(t *testing.T) { steps.AuthorizationRefreshingAction(nil, steps.Action(successfulActionStep)), }, wantedMetrics: map[string]int64{ - "backend.openshiftcluster.install.time.total": 6, - "backend.openshiftcluster.install.action.successfulActionStep.time": 2, - "backend.openshiftcluster.install.condition.successfulConditionStep.time": 2, - "backend.openshiftcluster.install.refreshing.successfulActionStep.time": 2, + "backend.openshiftcluster.install.duration.total.seconds": 6, + "backend.openshiftcluster.install.action.successfulActionStep.duration.seconds": 2, + "backend.openshiftcluster.install.condition.successfulConditionStep.duration.seconds": 2, + "backend.openshiftcluster.install.refreshing.successfulActionStep.duration.seconds": 2, }, }, { @@ -278,10 +278,10 @@ func TestInstallationTimeMetrics(t *testing.T) { steps.AuthorizationRefreshingAction(nil, steps.Action(successfulActionStep)), }, wantedMetrics: map[string]int64{ - "backend.openshiftcluster.update.time.total": 9, - "backend.openshiftcluster.update.action.successfulActionStep.time": 3, - "backend.openshiftcluster.update.condition.successfulConditionStep.time": 3, - "backend.openshiftcluster.update.refreshing.successfulActionStep.time": 3, + "backend.openshiftcluster.update.duration.total.seconds": 9, + "backend.openshiftcluster.update.action.successfulActionStep.duration.seconds": 3, + "backend.openshiftcluster.update.condition.successfulConditionStep.duration.seconds": 3, + "backend.openshiftcluster.update.refreshing.successfulActionStep.duration.seconds": 3, }, }, } {