Merge pull request #132613 from gavinkflam/130656-duplicate-validation-errors-metric
feat: increment an internal metric when duplicate validation errors are found Kubernetes-commit: 310603902c51ff4efda99f8b11478e72303cc03c
This commit is contained in:
commit
51b01ad08b
4
go.mod
4
go.mod
|
@ -50,8 +50,8 @@ require (
|
|||
gopkg.in/natefinch/lumberjack.v2 v2.2.1
|
||||
k8s.io/api v0.0.0-20250816062245-fa01e40890d0
|
||||
k8s.io/apimachinery v0.0.0-20250816040907-f5dd29d6ada1
|
||||
k8s.io/client-go v0.0.0-20250816062719-0341f077c9d6
|
||||
k8s.io/component-base v0.0.0-20250816064138-bbf5f29946df
|
||||
k8s.io/client-go v0.0.0-20250827235241-01d95ed5589b
|
||||
k8s.io/component-base v0.0.0-20250828000726-422881f698ae
|
||||
k8s.io/klog/v2 v2.130.1
|
||||
k8s.io/kms v0.0.0-20250816064442-6cea1b3fd2d9
|
||||
k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b
|
||||
|
|
8
go.sum
8
go.sum
|
@ -300,10 +300,10 @@ k8s.io/api v0.0.0-20250816062245-fa01e40890d0 h1:WddRlAJwdWiTmGknuGqNHLxJ7RaF3bq
|
|||
k8s.io/api v0.0.0-20250816062245-fa01e40890d0/go.mod h1:PyEssxRzobRLFX/lEYzx5NDkS4JYE20SOKUZjTH0nvI=
|
||||
k8s.io/apimachinery v0.0.0-20250816040907-f5dd29d6ada1 h1:CyDLPRX8n0wju2WX8Bukq22Ucjz/XiXuS9WQvm/JBBI=
|
||||
k8s.io/apimachinery v0.0.0-20250816040907-f5dd29d6ada1/go.mod h1:/GwIlEcWuTX9zKIg2mbw0LRFIsXwrfoVxn+ef0X13lw=
|
||||
k8s.io/client-go v0.0.0-20250816062719-0341f077c9d6 h1:9LKKgpQKzMGFGHGTjKbbNdW7beOdDbMZNTkEXWRSbPs=
|
||||
k8s.io/client-go v0.0.0-20250816062719-0341f077c9d6/go.mod h1:7VxAeZExKZSEiiIaKgN5fzCEcBAeEk7K42u7LqPTESo=
|
||||
k8s.io/component-base v0.0.0-20250816064138-bbf5f29946df h1:sJe8s1VNOpaiyhfYzHjB7Ji3HdrgqCNBz3DGYoTm9kY=
|
||||
k8s.io/component-base v0.0.0-20250816064138-bbf5f29946df/go.mod h1:0QeGK6CVamDGXq+dvJFxmoGxkPuM2mdaiQCtCjGAuaY=
|
||||
k8s.io/client-go v0.0.0-20250827235241-01d95ed5589b h1:uf3dAf6INqCPRH45t2X7Zm/270VQEUPgMU6XIxK1I0c=
|
||||
k8s.io/client-go v0.0.0-20250827235241-01d95ed5589b/go.mod h1:7VxAeZExKZSEiiIaKgN5fzCEcBAeEk7K42u7LqPTESo=
|
||||
k8s.io/component-base v0.0.0-20250828000726-422881f698ae h1:ob6TeKTWpfGU3v1hYi9decYahOOW8Mm1hW3T+fOfhbM=
|
||||
k8s.io/component-base v0.0.0-20250828000726-422881f698ae/go.mod h1:aohEoE0FHN6tCIIet4G5za7u5plNmkpkY/AuDDnUhH4=
|
||||
k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk=
|
||||
k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE=
|
||||
k8s.io/kms v0.0.0-20250816064442-6cea1b3fd2d9 h1:phtfi+TAHz21WG6fDCmqdfaGtG72vS9nAdKxaieH+bI=
|
||||
|
|
|
@ -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)
|
||||
}
|
||||
|
||||
|
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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 {
|
||||
|
|
|
@ -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()
|
||||
}
|
||||
|
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue