github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-29 06:27:05 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #130754 from aaron-prindle/validation-gen-add-metric-and-runtime-verification-review-comments-upstream

Browse Source 
[Declarative Validation] chore: change Info->Error log level related to declarative validation runtime tests and refactor panic wrapper names
This commit is contained in:
Kubernetes Prow Robot 2025-03-13 13:28:02 -07:00 committed by GitHub
commit 020c4b7c65
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 21 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go
View File

@ -116,7 +116,7 @@ func CompareDeclarativeErrorsAndEmitMismatches(ctx context.Context, imperativeEr
mismatchDetails := gatherDeclarativeValidationMismatches(imperativeErrs, declarativeErrs, takeover) mismatchDetails := gatherDeclarativeValidationMismatches(imperativeErrs, declarativeErrs, takeover)
for _, detail := range mismatchDetails { for _, detail := range mismatchDetails {
// Log information about the mismatch using contextual logger // Log information about the mismatch using contextual logger
logger.Info(detail) logger.Error(nil, detail)
// Increment the metric for the mismatch // Increment the metric for the mismatch
validationmetrics.Metrics.IncDeclarativeValidationMismatchMetric() validationmetrics.Metrics.IncDeclarativeValidationMismatchMetric()
@ -231,20 +231,20 @@ func createDeclarativeValidationPanicHandler(ctx context.Context, errs *field.Er
// If takeover is enabled, output as a validation error as authoritative validator panicked and validation should error // If takeover is enabled, output as a validation error as authoritative validator panicked and validation should error
*errs = append(*errs, field.InternalError(nil, fmt.Errorf(errorFmt, r))) *errs = append(*errs, field.InternalError(nil, fmt.Errorf(errorFmt, r)))
} else { } else {
// if takeover not enabled, log the panic as an info message // if takeover not enabled, log the panic as an error message
logger.Info(fmt.Sprintf(errorFmt, r)) logger.Error(nil, fmt.Sprintf(errorFmt, r))
} }
} }
} }
} }
// withRecover wraps a validation function with panic recovery logic. // panicSafeValidateFunc wraps a validation function with panic recovery logic.
// It takes a validation function with the ValidateDeclaratively signature // It takes a validation function with the ValidateDeclaratively signature
// and returns a function with the same signature. // and returns a function with the same signature.
// The returned function will execute the wrapped function and handle any panics by // The returned function will execute the wrapped function and handle any panics by
// incrementing the panic metric, and logging an error message // incrementing the panic metric, and logging an error message
// if takeover=false, and adding a validation error if takeover=true. // if takeover=false, and adding a validation error if takeover=true.
func withRecover( func panicSafeValidateFunc(
validateFunc func(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj runtime.Object) field.ErrorList, validateFunc func(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj runtime.Object) field.ErrorList,
takeover bool, takeover bool,
) func(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj runtime.Object) field.ErrorList { ) func(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj runtime.Object) field.ErrorList {
@ -255,13 +255,13 @@ func withRecover(
} }
} }
// withRecoverUpdate wraps an update validation function with panic recovery logic. // panicSafeValidateUpdateFunc wraps an update validation function with panic recovery logic.
// It takes a validation function with the ValidateUpdateDeclaratively signature // It takes a validation function with the ValidateUpdateDeclaratively signature
// and returns a function with the same signature. // and returns a function with the same signature.
// The returned function will execute the wrapped function and handle any panics by // The returned function will execute the wrapped function and handle any panics by
// incrementing the panic metric, and logging an error message // incrementing the panic metric, and logging an error message
// if takeover=false, and adding a validation error if takeover=true. // if takeover=false, and adding a validation error if takeover=true.
func withRecoverUpdate( func panicSafeValidateUpdateFunc(
validateUpdateFunc func(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj, oldObj runtime.Object) field.ErrorList, validateUpdateFunc func(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj, oldObj runtime.Object) field.ErrorList,
takeover bool, takeover bool,
) func(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj, oldObj runtime.Object) field.ErrorList { ) func(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj, oldObj runtime.Object) field.ErrorList {
@ -292,7 +292,7 @@ func withRecoverUpdate(
// conversion fails, or if a panic occurs during validation when // conversion fails, or if a panic occurs during validation when
// takeover is true. // takeover is true.
func ValidateDeclarativelyWithRecovery(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj runtime.Object, takeover bool) field.ErrorList { func ValidateDeclarativelyWithRecovery(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj runtime.Object, takeover bool) field.ErrorList {
return withRecover(ValidateDeclaratively, takeover)(ctx, options, scheme, obj) return panicSafeValidateFunc(ValidateDeclaratively, takeover)(ctx, options, scheme, obj)
} }
// ValidateUpdateDeclarativelyWithRecovery validates obj and oldObj against declarative // ValidateUpdateDeclarativelyWithRecovery validates obj and oldObj against declarative
@ -315,5 +315,5 @@ func ValidateDeclarativelyWithRecovery(ctx context.Context, options sets.Set[str
// conversion fails, or if a panic occurs during validation when // conversion fails, or if a panic occurs during validation when
// takeover is true. // takeover is true.
func ValidateUpdateDeclarativelyWithRecovery(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj, oldObj runtime.Object, takeover bool) field.ErrorList { func ValidateUpdateDeclarativelyWithRecovery(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj, oldObj runtime.Object, takeover bool) field.ErrorList {
return withRecoverUpdate(ValidateUpdateDeclaratively, takeover)(ctx, options, scheme, obj, oldObj) return panicSafeValidateUpdateFunc(ValidateUpdateDeclaratively, takeover)(ctx, options, scheme, obj, oldObj)
} }

24
staging/src/k8s.io/apiserver/pkg/registry/rest/validate_test.go
View File

@ -394,8 +394,8 @@ func TestCompareDeclarativeErrorsAndEmitMismatches(t *testing.T) {
declarativeErrs: field.ErrorList{errA}, declarativeErrs: field.ErrorList{errA},
takeover: true, takeover: true,
expectLogs: true, expectLogs: true,
// logs have a prefix of the form - I0309 21:05:33.865030 1926106 validate.go:199] // logs have a prefix of the form - E0309 21:05:33.865030 1926106 validate.go:199]
expectedRegex: "I.*Unexpected difference between hand written validation and declarative validation error results.*Consider disabling the DeclarativeValidationTakeover feature gate to keep data persisted in etcd consistent with prior versions of Kubernetes", expectedRegex: "E.*Unexpected difference between hand written validation and declarative validation error results.*Consider disabling the DeclarativeValidationTakeover feature gate to keep data persisted in etcd consistent with prior versions of Kubernetes",
}, },
{ {
name: "matching errors, don't log info", name: "matching errors, don't log info",
@ -468,8 +468,8 @@ func TestWithRecover(t *testing.T) {
}, },
takeoverEnabled: false, takeoverEnabled: false,
wantErrs: nil, wantErrs: nil,
// logs have a prefix of the form - I0309 21:05:33.865030 1926106 validate.go:199] // logs have a prefix of the form - E0309 21:05:33.865030 1926106 validate.go:199]
expectLogRegex: "I.*panic during declarative validation: test panic", expectLogRegex: "E.*panic during declarative validation: test panic",
}, },
{ {
name: "panic with takeover enabled", name: "panic with takeover enabled",
@ -500,8 +500,8 @@ func TestWithRecover(t *testing.T) {
klog.LogToStderr(false) klog.LogToStderr(false)
defer klog.LogToStderr(true) defer klog.LogToStderr(true)
// Pass the takeover flag to withRecover instead of relying on the feature gate // Pass the takeover flag to panicSafeValidateFunc instead of relying on the feature gate
wrapped := withRecover(tc.validateFn, tc.takeoverEnabled) wrapped := panicSafeValidateFunc(tc.validateFn, tc.takeoverEnabled)
gotErrs := wrapped(ctx, options, scheme, obj) gotErrs := wrapped(ctx, options, scheme, obj)
klog.Flush() klog.Flush()
@ -509,7 +509,7 @@ func TestWithRecover(t *testing.T) {
// Compare gotErrs vs. tc.wantErrs // Compare gotErrs vs. tc.wantErrs
if !equalErrorLists(gotErrs, tc.wantErrs) { if !equalErrorLists(gotErrs, tc.wantErrs) {
t.Errorf("withRecover() gotErrs = %#v, want %#v", gotErrs, tc.wantErrs) t.Errorf("panicSafeValidateFunc() gotErrs = %#v, want %#v", gotErrs, tc.wantErrs)
} }
// Check logs if needed // Check logs if needed
@ -562,8 +562,8 @@ func TestWithRecoverUpdate(t *testing.T) {
}, },
takeoverEnabled: false, takeoverEnabled: false,
wantErrs: nil, wantErrs: nil,
// logs have a prefix of the form - I0309 21:05:33.865030 1926106 validate.go:199] // logs have a prefix of the form - E0309 21:05:33.865030 1926106 validate.go:199]
expectLogRegex: "I.*panic during declarative validation: test update panic", expectLogRegex: "E.*panic during declarative validation: test update panic",
}, },
{ {
name: "panic with takeover enabled", name: "panic with takeover enabled",
@ -594,8 +594,8 @@ func TestWithRecoverUpdate(t *testing.T) {
klog.LogToStderr(false) klog.LogToStderr(false)
defer klog.LogToStderr(true) defer klog.LogToStderr(true)
// Pass the takeover flag to withRecoverUpdate instead of relying on the feature gate // Pass the takeover flag to panicSafeValidateUpdateFunc instead of relying on the feature gate
wrapped := withRecoverUpdate(tc.validateFn, tc.takeoverEnabled) wrapped := panicSafeValidateUpdateFunc(tc.validateFn, tc.takeoverEnabled)
gotErrs := wrapped(ctx, options, scheme, obj, oldObj) gotErrs := wrapped(ctx, options, scheme, obj, oldObj)
klog.Flush() klog.Flush()
@ -603,7 +603,7 @@ func TestWithRecoverUpdate(t *testing.T) {
// Compare gotErrs with wantErrs // Compare gotErrs with wantErrs
if !equalErrorLists(gotErrs, tc.wantErrs) { if !equalErrorLists(gotErrs, tc.wantErrs) {
t.Errorf("withRecoverUpdate() gotErrs = %#v, want %#v", gotErrs, tc.wantErrs) t.Errorf("panicSafeValidateUpdateFunc() gotErrs = %#v, want %#v", gotErrs, tc.wantErrs)
} }
// Verify log output // Verify log output