From 0de597a07fc824d7803c624625da3f4a13d38422 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Mon, 26 Feb 2024 13:30:07 +1100 Subject: [PATCH 1/5] reduce the amount of package names in the logs --- pkg/cluster/adminupdate_test.go | 2 +- pkg/cluster/gatherlogs.go | 2 +- pkg/cluster/install_test.go | 22 ++++++++--------- pkg/util/steps/action.go | 2 +- pkg/util/steps/condition.go | 4 +-- pkg/util/steps/refreshing.go | 2 +- pkg/util/steps/runner.go | 7 +++++- pkg/util/steps/runner_test.go | 44 ++++++++++++++++----------------- 8 files changed, 45 insertions(+), 40 deletions(-) diff --git a/pkg/cluster/adminupdate_test.go b/pkg/cluster/adminupdate_test.go index 5804ad11435..2b979098be3 100644 --- a/pkg/cluster/adminupdate_test.go +++ b/pkg/cluster/adminupdate_test.go @@ -215,7 +215,7 @@ func TestAdminUpdateSteps(t *testing.T) { var stepsToRun []string for _, s := range toRun { // make it a little nicer when defining the steps that should run, since they're all methods - o := strings.Replace(s.String(), "github.com/Azure/ARO-RP/pkg/cluster.(*manager).", "", -1) + o := strings.Replace(s.String(), "pkg/cluster.(*manager).", "", -1) stepsToRun = append(stepsToRun, o) } diff --git a/pkg/cluster/gatherlogs.go b/pkg/cluster/gatherlogs.go index 86afc11adb8..94c8bfed638 100644 --- a/pkg/cluster/gatherlogs.go +++ b/pkg/cluster/gatherlogs.go @@ -31,7 +31,7 @@ func (m *manager) gatherFailureLogs(ctx context.Context) { continue } - m.log.Printf("%s: %s", steps.FriendlyName(f), string(b)) + m.log.Printf("%s: %s", steps.ShortFriendlyName(f), string(b)) } } diff --git a/pkg/cluster/install_test.go b/pkg/cluster/install_test.go index 9d8507780e5..600cd556189 100644 --- a/pkg/cluster/install_test.go +++ b/pkg/cluster/install_test.go @@ -101,27 +101,27 @@ func TestStepRunnerWithInstaller(t *testing.T) { wantEntries: []map[string]types.GomegaMatcher{ { "level": gomega.Equal(logrus.InfoLevel), - "msg": gomega.Equal(`running step [Action github.com/Azure/ARO-RP/pkg/cluster.failingFunc]`), + "msg": gomega.Equal(`running step [Action pkg/cluster.failingFunc]`), }, { "level": gomega.Equal(logrus.ErrorLevel), - "msg": gomega.Equal("step [Action github.com/Azure/ARO-RP/pkg/cluster.failingFunc] encountered error: oh no!"), + "msg": gomega.Equal("step [Action pkg/cluster.failingFunc] encountered error: oh no!"), }, { "level": gomega.Equal(logrus.InfoLevel), - "msg": gomega.MatchRegexp(`(?s)github.com/Azure/ARO-RP/pkg/cluster.\(\*manager\).logClusterVersion\-fm:.*"name": "version"`), + "msg": gomega.MatchRegexp(`(?s)pkg/cluster.\(\*manager\).logClusterVersion:.*"name": "version"`), }, { "level": gomega.Equal(logrus.InfoLevel), - "msg": gomega.MatchRegexp(`(?s)github.com/Azure/ARO-RP/pkg/cluster.\(\*manager\).logNodes\-fm:.*"name": "node"`), + "msg": gomega.MatchRegexp(`(?s)pkg/cluster.\(\*manager\).logNodes:.*"name": "node"`), }, { "level": gomega.Equal(logrus.InfoLevel), - "msg": gomega.MatchRegexp(`(?s)github.com/Azure/ARO-RP/pkg/cluster.\(\*manager\).logClusterOperators\-fm:.*"name": "operator"`), + "msg": gomega.MatchRegexp(`(?s)pkg/cluster.\(\*manager\).logClusterOperators:.*"name": "operator"`), }, { "level": gomega.Equal(logrus.InfoLevel), - "msg": gomega.MatchRegexp(`(?s)github.com/Azure/ARO-RP/pkg/cluster.\(\*manager\).logIngressControllers\-fm:.*"name": "ingress-controller"`), + "msg": gomega.MatchRegexp(`(?s)pkg/cluster.\(\*manager\).logIngressControllers:.*"name": "ingress-controller"`), }, }, kubernetescli: fake.NewSimpleClientset(node), @@ -137,11 +137,11 @@ func TestStepRunnerWithInstaller(t *testing.T) { wantEntries: []map[string]types.GomegaMatcher{ { "level": gomega.Equal(logrus.InfoLevel), - "msg": gomega.Equal(`running step [Action github.com/Azure/ARO-RP/pkg/cluster.failingFunc]`), + "msg": gomega.Equal(`running step [Action pkg/cluster.failingFunc]`), }, { "level": gomega.Equal(logrus.ErrorLevel), - "msg": gomega.Equal("step [Action github.com/Azure/ARO-RP/pkg/cluster.failingFunc] encountered error: oh no!"), + "msg": gomega.Equal("step [Action pkg/cluster.failingFunc] encountered error: oh no!"), }, { "level": gomega.Equal(logrus.ErrorLevel), @@ -149,15 +149,15 @@ func TestStepRunnerWithInstaller(t *testing.T) { }, { "level": gomega.Equal(logrus.InfoLevel), - "msg": gomega.Equal(`github.com/Azure/ARO-RP/pkg/cluster.(*manager).logNodes-fm: null`), + "msg": gomega.Equal(`pkg/cluster.(*manager).logNodes: null`), }, { "level": gomega.Equal(logrus.InfoLevel), - "msg": gomega.Equal(`github.com/Azure/ARO-RP/pkg/cluster.(*manager).logClusterOperators-fm: null`), + "msg": gomega.Equal(`pkg/cluster.(*manager).logClusterOperators: null`), }, { "level": gomega.Equal(logrus.InfoLevel), - "msg": gomega.Equal(`github.com/Azure/ARO-RP/pkg/cluster.(*manager).logIngressControllers-fm: null`), + "msg": gomega.Equal(`pkg/cluster.(*manager).logIngressControllers: null`), }, }, kubernetescli: fake.NewSimpleClientset(), diff --git a/pkg/util/steps/action.go b/pkg/util/steps/action.go index 69e77a42d0f..040a6420b30 100644 --- a/pkg/util/steps/action.go +++ b/pkg/util/steps/action.go @@ -32,7 +32,7 @@ func (s actionStep) run(ctx context.Context, log *logrus.Entry) error { } func (s actionStep) String() string { - return fmt.Sprintf("[Action %s]", FriendlyName(s.f)) + return fmt.Sprintf("[Action %s]", ShortFriendlyName(s.f)) } func (s actionStep) metricsName() string { diff --git a/pkg/util/steps/condition.go b/pkg/util/steps/condition.go index d4af64ea77a..b0ce3dceae4 100644 --- a/pkg/util/steps/condition.go +++ b/pkg/util/steps/condition.go @@ -108,7 +108,7 @@ func (c conditionStep) run(ctx context.Context, log *logrus.Entry) error { // returns enriched error messages mentioned in timeoutConditionErrors func enrichConditionTimeoutError(f conditionFunction, originalErr error) error { funcNameParts := strings.Split(FriendlyName(f), ".") - funcName := strings.TrimSuffix(funcNameParts[len(funcNameParts)-1], "-fm") + funcName := funcNameParts[len(funcNameParts)-1] message, exists := timeoutConditionErrors[funcName] if !exists { @@ -122,7 +122,7 @@ func enrichConditionTimeoutError(f conditionFunction, originalErr error) error { } func (c conditionStep) String() string { - return fmt.Sprintf("[Condition %s, timeout %s]", FriendlyName(c.f), c.timeout) + return fmt.Sprintf("[Condition %s, timeout %s]", ShortFriendlyName(c.f), c.timeout) } func (c conditionStep) metricsName() string { diff --git a/pkg/util/steps/refreshing.go b/pkg/util/steps/refreshing.go index 8138ecda695..a291ccaf465 100644 --- a/pkg/util/steps/refreshing.go +++ b/pkg/util/steps/refreshing.go @@ -111,7 +111,7 @@ func (s *authorizationRefreshingActionStep) run(ctx context.Context, log *logrus } func (s *authorizationRefreshingActionStep) String() string { - return fmt.Sprintf("[AuthorizationRetryingAction %s]", FriendlyName(s.f)) + return fmt.Sprintf("[AuthorizationRetryingAction %s]", ShortFriendlyName(s.f)) } func (s *authorizationRefreshingActionStep) metricsName() string { diff --git a/pkg/util/steps/runner.go b/pkg/util/steps/runner.go index 99e75744921..8054740ac41 100644 --- a/pkg/util/steps/runner.go +++ b/pkg/util/steps/runner.go @@ -21,7 +21,12 @@ import ( // FriendlyName returns a "friendly" stringified name of the given func. func FriendlyName(f interface{}) string { - return runtime.FuncForPC(reflect.ValueOf(f).Pointer()).Name() + return strings.TrimSuffix(runtime.FuncForPC(reflect.ValueOf(f).Pointer()).Name(), "-fm") +} + +// FriendlyName returns a "friendly" stringified name of the given func. +func ShortFriendlyName(f interface{}) string { + return strings.TrimPrefix(FriendlyName(f), "github.com/Azure/ARO-RP/") } func shortName(fullName string) string { diff --git a/pkg/util/steps/runner_test.go b/pkg/util/steps/runner_test.go index 1d7367a2499..8ebe5b69add 100644 --- a/pkg/util/steps/runner_test.go +++ b/pkg/util/steps/runner_test.go @@ -56,15 +56,15 @@ func TestStepRunner(t *testing.T) { }, wantEntries: []map[string]types.GomegaMatcher{ { - "msg": gomega.Equal("running step [Action github.com/Azure/ARO-RP/pkg/util/steps.successfulFunc]"), + "msg": gomega.Equal("running step [Action pkg/util/steps.successfulFunc]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal("running step [Action github.com/Azure/ARO-RP/pkg/util/steps.successfulFunc]"), + "msg": gomega.Equal("running step [Action pkg/util/steps.successfulFunc]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal("running step [Action github.com/Azure/ARO-RP/pkg/util/steps.successfulFunc]"), + "msg": gomega.Equal("running step [Action pkg/util/steps.successfulFunc]"), "level": gomega.Equal(logrus.InfoLevel), }, }, @@ -105,15 +105,15 @@ func TestStepRunner(t *testing.T) { }, wantEntries: []map[string]types.GomegaMatcher{ { - "msg": gomega.Equal("running step [Action github.com/Azure/ARO-RP/pkg/util/steps.successfulFunc]"), + "msg": gomega.Equal("running step [Action pkg/util/steps.successfulFunc]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal("running step [Action github.com/Azure/ARO-RP/pkg/util/steps.failingFunc]"), + "msg": gomega.Equal("running step [Action pkg/util/steps.failingFunc]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal(`step [Action github.com/Azure/ARO-RP/pkg/util/steps.failingFunc] encountered error: oh no!`), + "msg": gomega.Equal(`step [Action pkg/util/steps.failingFunc] encountered error: oh no!`), "level": gomega.Equal(logrus.ErrorLevel), }, }, @@ -130,15 +130,15 @@ func TestStepRunner(t *testing.T) { }, wantEntries: []map[string]types.GomegaMatcher{ { - "msg": gomega.Equal("running step [Action github.com/Azure/ARO-RP/pkg/util/steps.successfulFunc]"), + "msg": gomega.Equal("running step [Action pkg/util/steps.successfulFunc]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal("running step [Condition github.com/Azure/ARO-RP/pkg/util/steps.alwaysTrueCondition, timeout 50ms]"), + "msg": gomega.Equal("running step [Condition pkg/util/steps.alwaysTrueCondition, timeout 50ms]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal("running step [Action github.com/Azure/ARO-RP/pkg/util/steps.successfulFunc]"), + "msg": gomega.Equal("running step [Action pkg/util/steps.successfulFunc]"), "level": gomega.Equal(logrus.InfoLevel), }, }, @@ -154,19 +154,19 @@ func TestStepRunner(t *testing.T) { }, wantEntries: []map[string]types.GomegaMatcher{ { - "msg": gomega.Equal("running step [Action github.com/Azure/ARO-RP/pkg/util/steps.successfulFunc]"), + "msg": gomega.Equal("running step [Action pkg/util/steps.successfulFunc]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal("running step [Condition github.com/Azure/ARO-RP/pkg/util/steps.alwaysFalseCondition, timeout 50ms]"), + "msg": gomega.Equal("running step [Condition pkg/util/steps.alwaysFalseCondition, timeout 50ms]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal("step [Condition github.com/Azure/ARO-RP/pkg/util/steps.alwaysFalseCondition, timeout 50ms] failed but has configured 'fail=false'. Continuing. Error: timed out waiting for the condition"), + "msg": gomega.Equal("step [Condition pkg/util/steps.alwaysFalseCondition, timeout 50ms] failed but has configured 'fail=false'. Continuing. Error: timed out waiting for the condition"), "level": gomega.Equal(logrus.WarnLevel), }, { - "msg": gomega.Equal("running step [Action github.com/Azure/ARO-RP/pkg/util/steps.successfulFunc]"), + "msg": gomega.Equal("running step [Action pkg/util/steps.successfulFunc]"), "level": gomega.Equal(logrus.InfoLevel), }, }, @@ -187,15 +187,15 @@ func TestStepRunner(t *testing.T) { }, wantEntries: []map[string]types.GomegaMatcher{ { - "msg": gomega.Equal("running step [Action github.com/Azure/ARO-RP/pkg/util/steps.successfulFunc]"), + "msg": gomega.Equal("running step [Action pkg/util/steps.successfulFunc]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal("running step [Condition github.com/Azure/ARO-RP/pkg/util/steps.timingOutCondition, timeout 50ms]"), + "msg": gomega.Equal("running step [Condition pkg/util/steps.timingOutCondition, timeout 50ms]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal("step [Condition github.com/Azure/ARO-RP/pkg/util/steps.timingOutCondition, timeout 50ms] encountered error: timed out waiting for the condition"), + "msg": gomega.Equal("step [Condition pkg/util/steps.timingOutCondition, timeout 50ms] encountered error: timed out waiting for the condition"), "level": gomega.Equal(logrus.ErrorLevel), }, }, @@ -217,15 +217,15 @@ func TestStepRunner(t *testing.T) { }, wantEntries: []map[string]types.GomegaMatcher{ { - "msg": gomega.Equal("running step [Action github.com/Azure/ARO-RP/pkg/util/steps.successfulFunc]"), + "msg": gomega.Equal("running step [Action pkg/util/steps.successfulFunc]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal("running step [Condition github.com/Azure/ARO-RP/pkg/util/steps.internalTimeoutCondition, timeout 50ms]"), + "msg": gomega.Equal("running step [Condition pkg/util/steps.internalTimeoutCondition, timeout 50ms]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal("step [Condition github.com/Azure/ARO-RP/pkg/util/steps.internalTimeoutCondition, timeout 50ms] encountered error: condition encountered internal timeout: timed out waiting for the condition"), + "msg": gomega.Equal("step [Condition pkg/util/steps.internalTimeoutCondition, timeout 50ms] encountered error: condition encountered internal timeout: timed out waiting for the condition"), "level": gomega.Equal(logrus.ErrorLevel), }, }, @@ -242,15 +242,15 @@ func TestStepRunner(t *testing.T) { }, wantEntries: []map[string]types.GomegaMatcher{ { - "msg": gomega.Equal("running step [Action github.com/Azure/ARO-RP/pkg/util/steps.successfulFunc]"), + "msg": gomega.Equal("running step [Action pkg/util/steps.successfulFunc]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal("running step [Condition github.com/Azure/ARO-RP/pkg/util/steps.alwaysFalseCondition, timeout 50ms]"), + "msg": gomega.Equal("running step [Condition pkg/util/steps.alwaysFalseCondition, timeout 50ms]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal("step [Condition github.com/Azure/ARO-RP/pkg/util/steps.alwaysFalseCondition, timeout 50ms] encountered error: timed out waiting for the condition"), + "msg": gomega.Equal("step [Condition pkg/util/steps.alwaysFalseCondition, timeout 50ms] encountered error: timed out waiting for the condition"), "level": gomega.Equal(logrus.ErrorLevel), }, }, From da4b09a2f16b57d3ecc58df0f161ddfdded68687 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Wed, 12 Jun 2024 17:15:41 +1000 Subject: [PATCH 2/5] fixes --- pkg/cluster/adminupdate_test.go | 80 ++++++++++++++++----------------- pkg/util/steps/runner_test.go | 6 +-- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/pkg/cluster/adminupdate_test.go b/pkg/cluster/adminupdate_test.go index 2b979098be3..70b16c34adf 100644 --- a/pkg/cluster/adminupdate_test.go +++ b/pkg/cluster/adminupdate_test.go @@ -33,60 +33,60 @@ func TestAdminUpdateSteps(t *testing.T) { } zerothSteps := []string{ - "[Action initializeKubernetesClients-fm]", - "[Action ensureBillingRecord-fm]", - "[Action ensureDefaults-fm]", - "[AuthorizationRetryingAction fixupClusterSPObjectID-fm]", - "[Action fixInfraID-fm]", + "[Action initializeKubernetesClients]", + "[Action ensureBillingRecord]", + "[Action ensureDefaults]", + "[AuthorizationRetryingAction fixupClusterSPObjectID]", + "[Action fixInfraID]", } generalFixesSteps := []string{ - "[Action ensureResourceGroup-fm]", - "[Action createOrUpdateDenyAssignment-fm]", - "[Action ensureServiceEndpoints-fm]", - "[Action populateRegistryStorageAccountName-fm]", - "[Action migrateStorageAccounts-fm]", - "[Action fixSSH-fm]", - "[Action startVMs-fm]", - "[Condition apiServersReady-fm, timeout 30m0s]", - "[Action fixSREKubeconfig-fm]", - "[Action fixUserAdminKubeconfig-fm]", - "[Action createOrUpdateRouterIPFromCluster-fm]", - "[Action ensureGatewayUpgrade-fm]", - "[Action rotateACRTokenPassword-fm]", - "[Action populateRegistryStorageAccountName-fm]", - "[Action ensureMTUSize-fm]", + "[Action ensureResourceGroup]", + "[Action createOrUpdateDenyAssignment]", + "[Action ensureServiceEndpoints]", + "[Action populateRegistryStorageAccountName]", + "[Action migrateStorageAccounts]", + "[Action fixSSH]", + "[Action startVMs]", + "[Condition apiServersReady, timeout 30m0s]", + "[Action fixSREKubeconfig]", + "[Action fixUserAdminKubeconfig]", + "[Action createOrUpdateRouterIPFromCluster]", + "[Action ensureGatewayUpgrade]", + "[Action rotateACRTokenPassword]", + "[Action populateRegistryStorageAccountName]", + "[Action ensureMTUSize]", } certificateRenewalSteps := []string{ - "[Action startVMs-fm]", - "[Condition apiServersReady-fm, timeout 30m0s]", - "[Action populateDatabaseIntIP-fm]", - "[Action fixMCSCert-fm]", - "[Action fixMCSUserData-fm]", - "[Action configureAPIServerCertificate-fm]", - "[Action configureIngressCertificate-fm]", - "[Action initializeOperatorDeployer-fm]", - "[Action renewMDSDCertificate-fm]", + "[Action startVMs]", + "[Condition apiServersReady, timeout 30m0s]", + "[Action populateDatabaseIntIP]", + "[Action fixMCSCert]", + "[Action fixMCSUserData]", + "[Action configureAPIServerCertificate]", + "[Action configureIngressCertificate]", + "[Action initializeOperatorDeployer]", + "[Action renewMDSDCertificate]", } operatorUpdateSteps := []string{ - "[Action startVMs-fm]", - "[Condition apiServersReady-fm, timeout 30m0s]", - "[Action initializeOperatorDeployer-fm]", - "[Action ensureAROOperator-fm]", - "[Condition aroDeploymentReady-fm, timeout 20m0s]", - "[Condition ensureAROOperatorRunningDesiredVersion-fm, timeout 5m0s]", + "[Action startVMs]", + "[Condition apiServersReady, timeout 30m0s]", + "[Action initializeOperatorDeployer]", + "[Action ensureAROOperator]", + "[Condition aroDeploymentReady, timeout 20m0s]", + "[Condition ensureAROOperatorRunningDesiredVersion, timeout 5m0s]", } hiveSteps := []string{ - "[Action hiveCreateNamespace-fm]", - "[Action hiveEnsureResources-fm]", - "[Condition hiveClusterDeploymentReady-fm, timeout 5m0s]", - "[Action hiveResetCorrelationData-fm]", + "[Action hiveCreateNamespace]", + "[Action hiveEnsureResources]", + "[Condition hiveClusterDeploymentReady, timeout 5m0s]", + "[Action hiveResetCorrelationData]", } - updateProvisionedBySteps := []string{"[Action updateProvisionedBy-fm]"} + updateProvisionedBySteps := []string{"[Action updateProvisionedBy]"} for _, tt := range []struct { name string diff --git a/pkg/util/steps/runner_test.go b/pkg/util/steps/runner_test.go index 8ebe5b69add..75e4dc2230a 100644 --- a/pkg/util/steps/runner_test.go +++ b/pkg/util/steps/runner_test.go @@ -80,15 +80,15 @@ func TestStepRunner(t *testing.T) { }, wantEntries: []map[string]types.GomegaMatcher{ { - "msg": gomega.Equal("running step [Action github.com/Azure/ARO-RP/pkg/util/steps.successfulFunc]"), + "msg": gomega.Equal("running step [Action pkg/util/steps.successfulFunc]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal("running step [Action github.com/Azure/ARO-RP/pkg/util/steps.failingAzureError]"), + "msg": gomega.Equal("running step [Action pkg/util/steps.failingAzureError]"), "level": gomega.Equal(logrus.InfoLevel), }, { - "msg": gomega.Equal("step [Action github.com/Azure/ARO-RP/pkg/util/steps.failingAzureError] encountered error: Status=403 Code=\"AuthorizationFailed\""), + "msg": gomega.Equal("step [Action pkg/util/steps.failingAzureError] encountered error: Status=403 Code=\"AuthorizationFailed\""), "level": gomega.Equal(logrus.ErrorLevel), }, }, From a33264f0050c70ebed5eace5697f2fd4e2465034 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Wed, 12 Jun 2024 17:29:37 +1000 Subject: [PATCH 3/5] use an instantiated struct for this test, like the real steps --- pkg/util/steps/condition_test.go | 52 ++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/pkg/util/steps/condition_test.go b/pkg/util/steps/condition_test.go index f6466b59b43..f0209cc322e 100644 --- a/pkg/util/steps/condition_test.go +++ b/pkg/util/steps/condition_test.go @@ -9,21 +9,29 @@ import ( "testing" ) +type teststruct struct{} + // functionnames that will be used in the conditionFunction below // All the keys of map timeoutConditionErrors -func attachNSGs(context.Context) (bool, error) { return false, nil } -func apiServersReady(context.Context) (bool, error) { return false, nil } -func minimumWorkerNodesReady(context.Context) (bool, error) { return false, nil } -func operatorConsoleExists(context.Context) (bool, error) { return false, nil } -func operatorConsoleReady(context.Context) (bool, error) { return false, nil } -func clusterVersionReady(context.Context) (bool, error) { return false, nil } -func ingressControllerReady(context.Context) (bool, error) { return false, nil } -func aroDeploymentReady(context.Context) (bool, error) { return false, nil } -func ensureAROOperatorRunningDesiredVersion(context.Context) (bool, error) { return false, nil } -func hiveClusterDeploymentReady(context.Context) (bool, error) { return false, nil } -func hiveClusterInstallationComplete(context.Context) (bool, error) { return false, nil } +func (n *teststruct) attachNSGs(context.Context) (bool, error) { return false, nil } +func (n *teststruct) apiServersReady(context.Context) (bool, error) { return false, nil } +func (n *teststruct) minimumWorkerNodesReady(context.Context) (bool, error) { return false, nil } +func (n *teststruct) operatorConsoleExists(context.Context) (bool, error) { return false, nil } +func (n *teststruct) operatorConsoleReady(context.Context) (bool, error) { return false, nil } +func (n *teststruct) clusterVersionReady(context.Context) (bool, error) { return false, nil } +func (n *teststruct) ingressControllerReady(context.Context) (bool, error) { return false, nil } +func (n *teststruct) aroDeploymentReady(context.Context) (bool, error) { return false, nil } +func (n *teststruct) ensureAROOperatorRunningDesiredVersion(context.Context) (bool, error) { + return false, nil +} +func (n *teststruct) hiveClusterDeploymentReady(context.Context) (bool, error) { return false, nil } +func (n *teststruct) hiveClusterInstallationComplete(context.Context) (bool, error) { + return false, nil +} func TestEnrichConditionTimeoutError(t *testing.T) { + s := &teststruct{} + for _, tt := range []struct { desc string function conditionFunction @@ -41,57 +49,57 @@ func TestEnrichConditionTimeoutError(t *testing.T) { }, { desc: "test conditionfail for func - attachNSGs", - function: attachNSGs, + function: s.attachNSGs, wantErr: "500: DeploymentFailed: : Failed to attach the ARO NSG to the cluster subnets. Please retry, and if the issue persists, raise an Azure support ticket", }, { desc: "test conditionfail for func - apiServersReady", - function: apiServersReady, + function: s.apiServersReady, wantErr: "500: DeploymentFailed: : Kube API has not initialised successfully and is unavailable. Please retry, and if the issue persists, raise an Azure support ticket", }, { desc: "test conditionfail for func - minimumWorkerNodesReady", - function: minimumWorkerNodesReady, + function: s.minimumWorkerNodesReady, wantErr: "500: DeploymentFailed: : Minimum number of worker nodes have not been successfully created. Please retry, and if the issue persists, raise an Azure support ticket", }, { desc: "test conditionfail for func - operatorConsoleExists", - function: operatorConsoleExists, + function: s.operatorConsoleExists, wantErr: "500: DeploymentFailed: : Console Cluster Operator has failed to initialize successfully. Please retry, and if the issue persists, raise an Azure support ticket", }, { desc: "test conditionfail for func - operatorConsoleReady", - function: operatorConsoleReady, + function: s.operatorConsoleReady, wantErr: "500: DeploymentFailed: : Console Cluster Operator has not started successfully. Please retry, and if the issue persists, raise an Azure support ticket", }, { desc: "test conditionfail for func - clusterVersionReady", - function: clusterVersionReady, + function: s.clusterVersionReady, wantErr: "500: DeploymentFailed: : Cluster Verion is not reporting status as ready. Please retry, and if the issue persists, raise an Azure support ticket", }, { desc: "test conditionfail for func - clusterVersionReady", - function: ingressControllerReady, + function: s.ingressControllerReady, wantErr: "500: DeploymentFailed: : Ingress Cluster Operator has not started successfully. Please retry, and if the issue persists, raise an Azure support ticket", }, { desc: "test conditionfail for func - aroDeploymentReady", - function: aroDeploymentReady, + function: s.aroDeploymentReady, wantErr: "500: DeploymentFailed: : ARO Cluster Operator has failed to initialize successfully. Please retry, and if the issue persists, raise an Azure support ticket", }, { desc: "test conditionfail for func - ensureAROOperatorRunningDesiredVersion", - function: ensureAROOperatorRunningDesiredVersion, + function: s.ensureAROOperatorRunningDesiredVersion, wantErr: "500: DeploymentFailed: : ARO Cluster Operator is not running desired version. Please retry, and if the issue persists, raise an Azure support ticket", }, { desc: "test conditionfail for func - hiveClusterDeploymentReady", - function: hiveClusterDeploymentReady, + function: s.hiveClusterDeploymentReady, wantErr: "500: DeploymentFailed: : Timed out waiting for the condition to be ready. Please retry, and if the issue persists, raise an Azure support ticket", }, { desc: "test conditionfail for func - hiveClusterInstallationComplete", - function: hiveClusterInstallationComplete, + function: s.hiveClusterInstallationComplete, wantErr: "500: DeploymentFailed: : Timed out waiting for the condition to complete. Please retry, and if the issue persists, raise an Azure support ticket", }, } { From ec12eb5f50f731308dbb169baf42dd903d4af8a1 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Wed, 12 Jun 2024 17:29:41 +1000 Subject: [PATCH 4/5] cleanups --- pkg/cluster/gatherlogs.go | 2 +- pkg/util/steps/action.go | 2 +- pkg/util/steps/condition.go | 2 +- pkg/util/steps/refreshing.go | 2 +- pkg/util/steps/runner.go | 7 +------ 5 files changed, 5 insertions(+), 10 deletions(-) diff --git a/pkg/cluster/gatherlogs.go b/pkg/cluster/gatherlogs.go index 94c8bfed638..86afc11adb8 100644 --- a/pkg/cluster/gatherlogs.go +++ b/pkg/cluster/gatherlogs.go @@ -31,7 +31,7 @@ func (m *manager) gatherFailureLogs(ctx context.Context) { continue } - m.log.Printf("%s: %s", steps.ShortFriendlyName(f), string(b)) + m.log.Printf("%s: %s", steps.FriendlyName(f), string(b)) } } diff --git a/pkg/util/steps/action.go b/pkg/util/steps/action.go index 040a6420b30..69e77a42d0f 100644 --- a/pkg/util/steps/action.go +++ b/pkg/util/steps/action.go @@ -32,7 +32,7 @@ func (s actionStep) run(ctx context.Context, log *logrus.Entry) error { } func (s actionStep) String() string { - return fmt.Sprintf("[Action %s]", ShortFriendlyName(s.f)) + return fmt.Sprintf("[Action %s]", FriendlyName(s.f)) } func (s actionStep) metricsName() string { diff --git a/pkg/util/steps/condition.go b/pkg/util/steps/condition.go index b0ce3dceae4..ca1a71b1e79 100644 --- a/pkg/util/steps/condition.go +++ b/pkg/util/steps/condition.go @@ -122,7 +122,7 @@ func enrichConditionTimeoutError(f conditionFunction, originalErr error) error { } func (c conditionStep) String() string { - return fmt.Sprintf("[Condition %s, timeout %s]", ShortFriendlyName(c.f), c.timeout) + return fmt.Sprintf("[Condition %s, timeout %s]", FriendlyName(c.f), c.timeout) } func (c conditionStep) metricsName() string { diff --git a/pkg/util/steps/refreshing.go b/pkg/util/steps/refreshing.go index a291ccaf465..8138ecda695 100644 --- a/pkg/util/steps/refreshing.go +++ b/pkg/util/steps/refreshing.go @@ -111,7 +111,7 @@ func (s *authorizationRefreshingActionStep) run(ctx context.Context, log *logrus } func (s *authorizationRefreshingActionStep) String() string { - return fmt.Sprintf("[AuthorizationRetryingAction %s]", ShortFriendlyName(s.f)) + return fmt.Sprintf("[AuthorizationRetryingAction %s]", FriendlyName(s.f)) } func (s *authorizationRefreshingActionStep) metricsName() string { diff --git a/pkg/util/steps/runner.go b/pkg/util/steps/runner.go index 8054740ac41..06894226c51 100644 --- a/pkg/util/steps/runner.go +++ b/pkg/util/steps/runner.go @@ -21,12 +21,7 @@ import ( // FriendlyName returns a "friendly" stringified name of the given func. func FriendlyName(f interface{}) string { - return strings.TrimSuffix(runtime.FuncForPC(reflect.ValueOf(f).Pointer()).Name(), "-fm") -} - -// FriendlyName returns a "friendly" stringified name of the given func. -func ShortFriendlyName(f interface{}) string { - return strings.TrimPrefix(FriendlyName(f), "github.com/Azure/ARO-RP/") + return strings.TrimPrefix(strings.TrimSuffix(runtime.FuncForPC(reflect.ValueOf(f).Pointer()).Name(), "-fm"), "github.com/Azure/ARO-RP/") } func shortName(fullName string) string { From e57930cfbda22046bc44156c959d2cb5b8dbac25 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Thu, 13 Jun 2024 14:53:36 +1000 Subject: [PATCH 5/5] add better comments --- pkg/util/steps/condition_test.go | 3 +++ pkg/util/steps/runner.go | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/util/steps/condition_test.go b/pkg/util/steps/condition_test.go index f0209cc322e..1476beb1e1e 100644 --- a/pkg/util/steps/condition_test.go +++ b/pkg/util/steps/condition_test.go @@ -30,6 +30,9 @@ func (n *teststruct) hiveClusterInstallationComplete(context.Context) (bool, err } func TestEnrichConditionTimeoutError(t *testing.T) { + // When stringifying a method on a struct, golang adds -fm -- this is not + // useful to us, so using a struct instance here will verify that it is not + // present when matching the timeout error strings s := &teststruct{} for _, tt := range []struct { diff --git a/pkg/util/steps/runner.go b/pkg/util/steps/runner.go index 06894226c51..30f4ee315ba 100644 --- a/pkg/util/steps/runner.go +++ b/pkg/util/steps/runner.go @@ -19,7 +19,10 @@ import ( msgraph_errors "github.com/Azure/ARO-RP/pkg/util/graph/graphsdk/models/odataerrors" ) -// FriendlyName returns a "friendly" stringified name of the given func. +// FriendlyName returns a "friendly" stringified name of the given func. This +// consists of removing the ARO base package name (so it produces pkg/foobar +// instead of github.com/Azure/ARO-RP/pkg/foobar) and removing the -fm suffix +// from Golang struct methods. func FriendlyName(f interface{}) string { return strings.TrimPrefix(strings.TrimSuffix(runtime.FuncForPC(reflect.ValueOf(f).Pointer()).Name(), "-fm"), "github.com/Azure/ARO-RP/") }