Merge pull request #132613 from gavinkflam/130656-duplicate-validation-errors-metric

feat: increment an internal metric when duplicate validation errors are found
This commit is contained in:
Kubernetes Prow Robot
2025-08-27 14:54:41 -07:00
committed by GitHub
5 changed files with 165 additions and 6 deletions

View File

@@ -153,6 +153,7 @@ func BeforeUpdate(strategy RESTUpdateStrategy, ctx context.Context, obj, old run
errs = append(errs, strategy.ValidateUpdate(ctx, obj, old)...)
if len(errs) > 0 {
RecordDuplicateValidationErrors(ctx, kind.GroupKind(), errs)
return errors.NewInvalid(kind.GroupKind(), objectMeta.GetName(), errs)
}

View File

@@ -19,6 +19,7 @@ package rest
import (
"context"
"fmt"
"slices"
"strings"
"k8s.io/apimachinery/pkg/api/operation"
@@ -320,3 +321,20 @@ func panicSafeValidateFunc(
return validateUpdateFunc(ctx, scheme, obj, oldObj, o)
}
}
// RecordDuplicateValidationErrors increments a metric and log the error when duplicate validation errors are found.
func RecordDuplicateValidationErrors(ctx context.Context, qualifiedKind schema.GroupKind, errs field.ErrorList) {
logger := klog.FromContext(ctx)
seenErrs := make([]string, 0, len(errs))
for _, err := range errs {
errStr := fmt.Sprintf("%v", err)
if slices.Contains(seenErrs, errStr) {
logger.Info("Found duplicate validation error", "kind", qualifiedKind.String(), "error", errStr)
validationmetrics.Metrics.IncDuplicateValidationErrorMetric()
} else {
seenErrs = append(seenErrs, errStr)
}
}
}

View File

@@ -34,6 +34,9 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/validation"
"k8s.io/component-base/metrics/legacyregistry"
"k8s.io/component-base/metrics/testutil"
"k8s.io/klog/v2"
)
@@ -678,6 +681,71 @@ func TestValidateUpdateDeclarativelyWithRecovery(t *testing.T) {
})
}
func TestRecordDuplicateValidationErrors(t *testing.T) {
ctx := context.Background()
testCases := []struct {
name string
qualifiedKind schema.GroupKind
errs field.ErrorList
expectedMetric string
}{
{
name: "detect duplicates and increment metric",
qualifiedKind: schema.GroupKind{Group: "apps", Kind: "ReplicaSet"},
errs: field.ErrorList{
field.Invalid(field.NewPath("spec").Child("replicas"), -1, "must be greater than or equal to 0").WithOrigin("minimum"),
field.Invalid(field.NewPath("spec").Child("replicas"), -1, "must be greater than or equal to 0").WithOrigin("minimum"),
field.Invalid(field.NewPath("spec").Child("selector"), &metav1.LabelSelector{MatchLabels: map[string]string{}, MatchExpressions: []metav1.LabelSelectorRequirement{}}, "empty selector is invalid for deployment"),
field.Invalid(field.NewPath("spec").Child("selector"), &metav1.LabelSelector{MatchLabels: map[string]string{}, MatchExpressions: []metav1.LabelSelectorRequirement{}}, "empty selector is invalid for deployment"),
},
expectedMetric: `
# HELP apiserver_validation_duplicate_validation_error_total [INTERNAL] Number of duplicate validation errors during validation.
# TYPE apiserver_validation_duplicate_validation_error_total counter
apiserver_validation_duplicate_validation_error_total 2
`,
},
{
name: "detect duplicates with all fields but origin being equal",
qualifiedKind: schema.GroupKind{Group: "apps", Kind: "ReplicaSet"},
errs: field.ErrorList{
field.Invalid(field.NewPath("spec").Child("replicas"), -1, "must be greater than or equal to 0").WithOrigin("minimum"),
field.Invalid(field.NewPath("spec").Child("replicas"), -1, "must be greater than or equal to 0").WithOrigin("min"),
},
expectedMetric: `
# HELP apiserver_validation_duplicate_validation_error_total [INTERNAL] Number of duplicate validation errors during validation.
# TYPE apiserver_validation_duplicate_validation_error_total counter
apiserver_validation_duplicate_validation_error_total 1
`,
},
{
name: "no duplicates",
qualifiedKind: schema.GroupKind{Group: "apps", Kind: "ReplicaSet"},
errs: field.ErrorList{
field.Invalid(field.NewPath("spec").Child("replicas"), -1, "must be greater than or equal to 0").WithOrigin("minimum"),
field.Invalid(field.NewPath("spec").Child("selector"), &metav1.LabelSelector{MatchLabels: map[string]string{}, MatchExpressions: []metav1.LabelSelectorRequirement{}}, "empty selector is invalid for deployment"),
},
expectedMetric: `
# HELP apiserver_validation_duplicate_validation_error_total [INTERNAL] Number of duplicate validation errors during validation.
# TYPE apiserver_validation_duplicate_validation_error_total counter
apiserver_validation_duplicate_validation_error_total 0
`,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
defer legacyregistry.Reset()
defer validation.ResetValidationMetricsInstance()
RecordDuplicateValidationErrors(ctx, tc.qualifiedKind, tc.errs)
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(tc.expectedMetric), "apiserver_validation_duplicate_validation_error_total"); err != nil {
t.Fatal(err)
}
})
}
}
func equalErrorLists(a, b field.ErrorList) bool {
// If both are nil, consider them equal
if a == nil && b == nil {

View File

@@ -30,6 +30,7 @@ const (
type ValidationMetrics interface {
IncDeclarativeValidationMismatchMetric()
IncDeclarativeValidationPanicMetric()
IncDuplicateValidationErrorMetric()
Reset()
}
@@ -52,6 +53,15 @@ var validationMetricsInstance = &validationMetrics{
StabilityLevel: metrics.BETA,
},
),
DuplicateValidationErrorCounter: metrics.NewCounter(
&metrics.CounterOpts{
Namespace: namespace,
Subsystem: subsystem,
Name: "duplicate_validation_error_total",
Help: "Number of duplicate validation errors during validation.",
StabilityLevel: metrics.INTERNAL,
},
),
}
// Metrics provides access to validation metrics.
@@ -60,17 +70,20 @@ var Metrics ValidationMetrics = validationMetricsInstance
func init() {
legacyregistry.MustRegister(validationMetricsInstance.DeclarativeValidationMismatchCounter)
legacyregistry.MustRegister(validationMetricsInstance.DeclarativeValidationPanicCounter)
legacyregistry.MustRegister(validationMetricsInstance.DuplicateValidationErrorCounter)
}
type validationMetrics struct {
DeclarativeValidationMismatchCounter *metrics.Counter
DeclarativeValidationPanicCounter *metrics.Counter
DuplicateValidationErrorCounter *metrics.Counter
}
// Reset resets the validation metrics.
func (m *validationMetrics) Reset() {
m.DeclarativeValidationMismatchCounter.Reset()
m.DeclarativeValidationPanicCounter.Reset()
m.DuplicateValidationErrorCounter.Reset()
}
// IncDeclarativeValidationMismatchMetric increments the counter for the declarative_validation_mismatch_total metric.
@@ -83,6 +96,11 @@ func (m *validationMetrics) IncDeclarativeValidationPanicMetric() {
m.DeclarativeValidationPanicCounter.Inc()
}
// IncDuplicateValidationErrorMetric increments the counter for the duplicate_validation_error_total metric.
func (m *validationMetrics) IncDuplicateValidationErrorMetric() {
m.DuplicateValidationErrorCounter.Inc()
}
func ResetValidationMetricsInstance() {
validationMetricsInstance.Reset()
}

View File

@@ -38,7 +38,7 @@ func TestDeclarativeValidationMismatchMetric(t *testing.T) {
apiserver_validation_declarative_validation_mismatch_total 1
`
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "declarative_validation_mismatch_total"); err != nil {
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "apiserver_validation_declarative_validation_mismatch_total"); err != nil {
t.Fatal(err)
}
}
@@ -57,7 +57,26 @@ func TestDeclarativeValidationPanicMetric(t *testing.T) {
apiserver_validation_declarative_validation_panic_total 1
`
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "declarative_validation_panic_total"); err != nil {
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "apiserver_validation_declarative_validation_panic_total"); err != nil {
t.Fatal(err)
}
}
// TestDuplicateValidationErrorMetric tests that the duplicate error metric correctly increments once
func TestDuplicateValidationErrorMetric(t *testing.T) {
defer legacyregistry.Reset()
defer ResetValidationMetricsInstance()
// Increment the metric once
Metrics.IncDuplicateValidationErrorMetric()
expected := `
# HELP apiserver_validation_duplicate_validation_error_total [INTERNAL] Number of duplicate validation errors during validation.
# TYPE apiserver_validation_duplicate_validation_error_total counter
apiserver_validation_duplicate_validation_error_total 1
`
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "apiserver_validation_duplicate_validation_error_total"); err != nil {
t.Fatal(err)
}
}
@@ -78,7 +97,7 @@ func TestDeclarativeValidationMismatchMetricMultiple(t *testing.T) {
apiserver_validation_declarative_validation_mismatch_total 3
`
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "declarative_validation_mismatch_total"); err != nil {
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "apiserver_validation_declarative_validation_mismatch_total"); err != nil {
t.Fatal(err)
}
}
@@ -99,7 +118,28 @@ func TestDeclarativeValidationPanicMetricMultiple(t *testing.T) {
apiserver_validation_declarative_validation_panic_total 3
`
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "declarative_validation_panic_total"); err != nil {
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "apiserver_validation_declarative_validation_panic_total"); err != nil {
t.Fatal(err)
}
}
// TestDuplicateValidationErrorMetricMultiple tests that the duplicate error metric correctly increments multiple times
func TestDuplicateValidationErrorMetricMultiple(t *testing.T) {
defer legacyregistry.Reset()
defer ResetValidationMetricsInstance()
// Increment the metric three times
Metrics.IncDuplicateValidationErrorMetric()
Metrics.IncDuplicateValidationErrorMetric()
Metrics.IncDuplicateValidationErrorMetric()
expected := `
# HELP apiserver_validation_duplicate_validation_error_total [INTERNAL] Number of duplicate validation errors during validation.
# TYPE apiserver_validation_duplicate_validation_error_total counter
apiserver_validation_duplicate_validation_error_total 3
`
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "apiserver_validation_duplicate_validation_error_total"); err != nil {
t.Fatal(err)
}
}
@@ -112,6 +152,7 @@ func TestDeclarativeValidationMetricsReset(t *testing.T) {
// Increment both metrics
Metrics.IncDeclarativeValidationMismatchMetric()
Metrics.IncDeclarativeValidationPanicMetric()
Metrics.IncDuplicateValidationErrorMetric()
// Reset the metrics
Metrics.Reset()
@@ -124,15 +165,22 @@ func TestDeclarativeValidationMetricsReset(t *testing.T) {
# HELP apiserver_validation_declarative_validation_panic_total [BETA] Number of times declarative validation has panicked during validation.
# TYPE apiserver_validation_declarative_validation_panic_total counter
apiserver_validation_declarative_validation_panic_total 0
# HELP apiserver_validation_duplicate_validation_error_total [INTERNAL] Number of duplicate validation errors during validation.
# TYPE apiserver_validation_duplicate_validation_error_total counter
apiserver_validation_duplicate_validation_error_total 0
`
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "declarative_validation_mismatch_total", "declarative_validation_panic_total"); err != nil {
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected),
"apiserver_validation_declarative_validation_mismatch_total",
"apiserver_validation_declarative_validation_panic_total",
"apiserver_validation_duplicate_validation_error_total"); err != nil {
t.Fatal(err)
}
// Increment the metrics again to ensure they're still functional
Metrics.IncDeclarativeValidationMismatchMetric()
Metrics.IncDeclarativeValidationPanicMetric()
Metrics.IncDuplicateValidationErrorMetric()
// Verify they've been incremented correctly
expected = `
@@ -142,9 +190,15 @@ func TestDeclarativeValidationMetricsReset(t *testing.T) {
# HELP apiserver_validation_declarative_validation_panic_total [BETA] Number of times declarative validation has panicked during validation.
# TYPE apiserver_validation_declarative_validation_panic_total counter
apiserver_validation_declarative_validation_panic_total 1
# HELP apiserver_validation_duplicate_validation_error_total [INTERNAL] Number of duplicate validation errors during validation.
# TYPE apiserver_validation_duplicate_validation_error_total counter
apiserver_validation_duplicate_validation_error_total 1
`
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "declarative_validation_mismatch_total", "declarative_validation_panic_total"); err != nil {
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected),
"apiserver_validation_declarative_validation_mismatch_total",
"apiserver_validation_declarative_validation_panic_total",
"apiserver_validation_duplicate_validation_error_total"); err != nil {
t.Fatal(err)
}
}