public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Dashboard Migrations: Report metrics when migration fails (#111212)

Browse Source 
* return migration error when running a conversionFunc

* fix tests
This commit is contained in:
Haris Rozajac
2025-09-18 08:11:15 -06:00
committed by GitHub
parent ab00074734
commit 0aa5dee86a
3 changed files with 37 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/dashboard/pkg/migration/conversion/conversion_test.go
+34 -26
View File
@@ -249,7 +249,8 @@ func TestConversionMetrics(t *testing.T) {
name string
source metav1.Object
target metav1.Object
expectSuccess bool
expectAPISuccess bool
expectMetricsSuccess bool
expectedSourceAPI string
expectedTargetAPI string
expectedSourceSchema string
@@ -266,7 +267,8 @@ func TestConversionMetrics(t *testing.T) {
}},
},
target: &dashv1.Dashboard{},
expectSuccess: true,
expectAPISuccess: true,
expectMetricsSuccess: true,
expectedSourceAPI: dashv0.APIVERSION,
expectedTargetAPI: dashv1.APIVERSION,
expectedSourceSchema: "14",
@@ -282,7 +284,8 @@ func TestConversionMetrics(t *testing.T) {
}},
},
target: &dashv0.Dashboard{},
expectSuccess: true,
expectAPISuccess: true,
expectMetricsSuccess: true,
expectedSourceAPI: dashv1.APIVERSION,
expectedTargetAPI: dashv0.APIVERSION,
expectedSourceSchema: "42",
@@ -295,7 +298,8 @@ func TestConversionMetrics(t *testing.T) {
Spec: dashv2alpha1.DashboardSpec{Title: "test dashboard"},
},
target: &dashv2beta1.Dashboard{},
expectSuccess: true,
expectAPISuccess: true,
expectMetricsSuccess: true,
expectedSourceAPI: dashv2alpha1.APIVERSION,
expectedTargetAPI: dashv2beta1.APIVERSION,
expectedSourceSchema: "v2alpha1",
@@ -311,7 +315,8 @@ func TestConversionMetrics(t *testing.T) {
}},
},
target: &dashv1.Dashboard{},
expectSuccess: true, // Conversion succeeds but status indicates failure
expectAPISuccess: true, // Conversion succeeds but status indicates failure
expectMetricsSuccess: false,
expectedSourceAPI: dashv0.APIVERSION,
expectedTargetAPI: dashv1.APIVERSION,
expectedSourceSchema: "5",
@@ -329,7 +334,7 @@ func TestConversionMetrics(t *testing.T) {
err := scheme.Convert(tt.source, tt.target, nil)
// Check error expectation
if tt.expectSuccess {
if tt.expectAPISuccess {
require.NoError(t, err, "expected successful conversion")
} else {
require.Error(t, err, "expected conversion to fail")
@@ -352,7 +357,7 @@ func TestConversionMetrics(t *testing.T) {
}
}
if tt.expectSuccess {
if tt.expectAPISuccess && tt.expectMetricsSuccess {
require.Equal(t, float64(1), successTotal, "success metric should be incremented")
require.Equal(t, float64(0), failureTotal, "failure metric should not be incremented")
} else {
@@ -372,14 +377,15 @@ func TestConversionMetricsWrapper(t *testing.T) {
migration.RegisterMetrics(registry)
tests := []struct {
name string
source interface{}
target interface{}
conversionFunction func(a, b interface{}, scope conversion.Scope) error
expectSuccess bool
expectedSourceUID string
expectedSourceAPI string
expectedTargetAPI string
name string
source interface{}
target interface{}
conversionFunction func(a, b interface{}, scope conversion.Scope) error
expectAPISuccess bool
expectMetricsSuccess bool
expectedSourceUID string
expectedSourceAPI string
expectedTargetAPI string
}{
{
name: "successful conversion wrapper",
@@ -395,10 +401,11 @@ func TestConversionMetricsWrapper(t *testing.T) {
// Simulate successful conversion
return nil
},
expectSuccess: true,
expectedSourceUID: "test-wrapper-1",
expectedSourceAPI: dashv0.APIVERSION,
expectedTargetAPI: dashv1.APIVERSION,
expectAPISuccess: true,
expectMetricsSuccess: true,
expectedSourceUID: "test-wrapper-1",
expectedSourceAPI: dashv0.APIVERSION,
expectedTargetAPI: dashv1.APIVERSION,
},
{
name: "failed conversion wrapper",
@@ -414,10 +421,11 @@ func TestConversionMetricsWrapper(t *testing.T) {
// Simulate conversion failure
return fmt.Errorf("conversion failed")
},
expectSuccess: false,
expectedSourceUID: "test-wrapper-2",
expectedSourceAPI: dashv1.APIVERSION,
expectedTargetAPI: dashv0.APIVERSION,
expectAPISuccess: true,
expectMetricsSuccess: false,
expectedSourceUID: "test-wrapper-2",
expectedSourceAPI: dashv1.APIVERSION,
expectedTargetAPI: dashv0.APIVERSION,
},
}
@@ -434,7 +442,7 @@ func TestConversionMetricsWrapper(t *testing.T) {
err := wrappedFunc(tt.source, tt.target, nil)
// Check error expectation
if tt.expectSuccess {
if tt.expectAPISuccess {
require.NoError(t, err, "expected successful conversion")
} else {
require.Error(t, err, "expected conversion to fail")
@@ -457,7 +465,7 @@ func TestConversionMetricsWrapper(t *testing.T) {
}
}
if tt.expectSuccess {
if tt.expectAPISuccess && tt.expectMetricsSuccess {
require.Equal(t, float64(1), successTotal, "success metric should be incremented")
require.Equal(t, float64(0), failureTotal, "failure metric should not be incremented")
} else {
@@ -704,7 +712,7 @@ func TestConversionLogLevels(t *testing.T) {
target2 := &dashv0.Dashboard{}
err = failureWrapper(source2, target2, nil)
require.Error(t, err, "failed conversion should error")
require.NoError(t, err, "conversion wrapper should not error after recording logs")
// The logging code paths are executed in both cases above
// Success case logs at Debug level with fields:
apps/dashboard/pkg/migration/conversion/metrics.go
+1 -1
View File
@@ -209,6 +209,6 @@ func withConversionMetrics(sourceVersionAPI, targetVersionAPI string, conversion
logger.Debug("Dashboard conversion succeeded", successLogFields...)
}
return err
return nil
}
}
apps/dashboard/pkg/migration/conversion/v0.go
+2 -2
View File
@@ -36,7 +36,7 @@ func Convert_V0_to_V1(in *dashv0.Dashboard, out *dashv1.Dashboard, scope convers
if err != nil {
out.Status.Conversion.Failed = true
out.Status.Conversion.Error = ptr.To(err.Error())
return nil
return schemaversion.NewMigrationError(err.Error(), schemaversion.GetSchemaVersion(in.Spec.Object), schemaversion.LATEST_VERSION, "Convert_V0_to_V1")
}
// a background service identity is used here because the user who is reading the specific dashboard
@@ -47,7 +47,7 @@ func Convert_V0_to_V1(in *dashv0.Dashboard, out *dashv1.Dashboard, scope convers
if err := migration.Migrate(ctx, out.Spec.Object, schemaversion.LATEST_VERSION); err != nil {
out.Status.Conversion.Failed = true
out.Status.Conversion.Error = ptr.To(err.Error())
return nil
return schemaversion.NewMigrationError(err.Error(), schemaversion.GetSchemaVersion(in.Spec.Object), schemaversion.LATEST_VERSION, "Convert_V0_to_V1")
}
return nil