chore: change Info->Error log level related to declarative validation runtime tests and refactor panic wrapper names

Author: aaron-prindle

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

@@ -116,7 +116,7 @@ func CompareDeclarativeErrorsAndEmitMismatches(ctx context.Context, imperativeEr
 	mismatchDetails := gatherDeclarativeValidationMismatches(imperativeErrs, declarativeErrs, takeover)
 	for _, detail := range mismatchDetails {
 		// Log information about the mismatch using contextual logger
-		logger.Info(detail)
+		logger.Error(nil, detail)
 
 		// Increment the metric for the mismatch
 		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
 				*errs = append(*errs, field.InternalError(nil, fmt.Errorf(errorFmt, r)))
 			} else {
-				// if takeover not enabled, log the panic as an info message
-				logger.Info(fmt.Sprintf(errorFmt, r))
+				// if takeover not enabled, log the panic as an error message
+				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
 // and returns a function with the same signature.
 // The returned function will execute the wrapped function and handle any panics by
 // incrementing the panic metric, and logging an error message
 // 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,
 	takeover bool,
 ) 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
 // and returns a function with the same signature.
 // The returned function will execute the wrapped function and handle any panics by
 // incrementing the panic metric, and logging an error message
 // 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,
 	takeover bool,
 ) 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
 // takeover is true.
 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
@@ -315,5 +315,5 @@ func ValidateDeclarativelyWithRecovery(ctx context.Context, options sets.Set[str
 // conversion fails, or if a panic occurs during validation when
 // takeover is true.
 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)
 }

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

@@ -394,8 +394,8 @@ func TestCompareDeclarativeErrorsAndEmitMismatches(t *testing.T) {
 			declarativeErrs: field.ErrorList{errA},
 			takeover:        true,
 			expectLogs:      true,
-			// logs have a prefix of the form - I0309 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",
+			// logs have a prefix of the form - E0309 21:05:33.865030 1926106 validate.go:199]
+			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",
@@ -468,8 +468,8 @@ func TestWithRecover(t *testing.T) {
 			},
 			takeoverEnabled: false,
 			wantErrs:        nil,
-			// logs have a prefix of the form - I0309 21:05:33.865030 1926106 validate.go:199]
-			expectLogRegex: "I.*panic during declarative validation: test panic",
+			// logs have a prefix of the form - E0309 21:05:33.865030 1926106 validate.go:199]
+			expectLogRegex: "E.*panic during declarative validation: test panic",
 		},
 		{
 			name: "panic with takeover enabled",
@@ -500,16 +500,16 @@ func TestWithRecover(t *testing.T) {
 			klog.LogToStderr(false)
 			defer klog.LogToStderr(true)
 
-			// Pass the takeover flag to withRecover instead of relying on the feature gate
-			wrapped := withRecover(tc.validateFn, tc.takeoverEnabled)
+			// Pass the takeover flag to panicSafeValidateFunc instead of relying on the feature gate
+			wrapped := panicSafeValidateFunc(tc.validateFn, tc.takeoverEnabled)
 			gotErrs := wrapped(ctx, options, scheme, obj)
 
 			klog.Flush()
 			logOutput := buf.String()
 
 			// Compare gotErrs vs. 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
@@ -562,8 +562,8 @@ func TestWithRecoverUpdate(t *testing.T) {
 			},
 			takeoverEnabled: false,
 			wantErrs:        nil,
-			// logs have a prefix of the form - I0309 21:05:33.865030 1926106 validate.go:199]
-			expectLogRegex: "I.*panic during declarative validation: test update panic",
+			// logs have a prefix of the form - E0309 21:05:33.865030 1926106 validate.go:199]
+			expectLogRegex: "E.*panic during declarative validation: test update panic",
 		},
 		{
 			name: "panic with takeover enabled",
@@ -594,16 +594,16 @@ func TestWithRecoverUpdate(t *testing.T) {
 			klog.LogToStderr(false)
 			defer klog.LogToStderr(true)
 
-			// Pass the takeover flag to withRecoverUpdate instead of relying on the feature gate
-			wrapped := withRecoverUpdate(tc.validateFn, tc.takeoverEnabled)
+			// Pass the takeover flag to panicSafeValidateUpdateFunc instead of relying on the feature gate
+			wrapped := panicSafeValidateUpdateFunc(tc.validateFn, tc.takeoverEnabled)
 			gotErrs := wrapped(ctx, options, scheme, obj, oldObj)
 
 			klog.Flush()
 			logOutput := buf.String()
 
 			// Compare gotErrs with 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