NO-JIRA: controller/servingcerts: Align log verbosity

[tchap]

Some informational statements are being logged on V(4), which makes then effectively invisible in production clusters. It would be handly to have them for analyzing cluster issues, though.

Summary by CodeRabbit

• Chores
  • Improved logging visibility for operational diagnostics. Key system messages are now more readily available in standard debug output, enhancing observability and simplifying troubleshooting without requiring verbose logging configurations.

[openshift-ci-robot]

@tchap: This pull request explicitly references no jira issue.

Details

In response to this:

Some informational statements are being logged on V(4), which makes then effectively invisible in production clusters. It would be handly to have them for analyzing cluster issues, though.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

Walkthrough

Log verbosity levels reduced from V(4) to V(2) across multiple informational messages in the serving certificate controller's secret creation logic. No functional changes to control flow or error handling.

Changes

Cohort / File(s)	Summary
Log Verbosity Reduction pkg/controller/servingcert/controller/secret_creating_controller.go	Reduced logging verbosity level from V(4) to V(2) in four distinct locations: certificate generation, certificate renewal, secret requirement checking, and service failure annotation updates. Changes affect only log level configuration with no impact on logic or behavior.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: reducing log verbosity levels from V(4) to V(2) in the servingcerts controller to make informational messages more visible in production.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Stable And Deterministic Test Names	✅ Passed	PR modifies only controller implementation file, not test files. No test names were added or changed.
Test Structure And Quality	✅ Passed	PR modifies only logging verbosity in production code; no test code changes, so custom check for test quality is not applicable.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tchap
Once this PR has been reviewed and has the lgtm label, please assign p0lyn0mial for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

🧹 Nitpick comments (1)

pkg/controller/servingcert/controller/secret_creating_controller.go (1)

242-248: Consider using klog.Warningf for warning messages.

The verbosity adjustment is appropriate. However, these messages are semantically warnings (they indicate parsing errors that trigger cert regeneration). Consider using klog.Warningf() instead of klog.V(2).Infof("warning: ...") for clearer log semantics and consistent filtering.

♻️ Optional: Use warning-level logging

 	certs, err := cert.ParseCertsPEM(secret.Data[corev1.TLSCertKey])
 	if err != nil {
-		klog.V(2).Infof("warning: error parsing certificate data in %s/%s during regeneration check: %v",
+		klog.Warningf("error parsing certificate data in %s/%s during regeneration check: %v",
 			secret.Namespace, secret.Name, err)
 		return true
 	}
 	if len(certs) == 0 || certs[0] == nil {
-		klog.V(2).Infof("warning: no certs returned from ParseCertsPEM during regeneration check")
+		klog.Warningf("no certs returned from ParseCertsPEM during regeneration check")
 		return true
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/controller/servingcert/controller/secret_creating_controller.go` around
lines 242 - 248, The two log lines in secret_creating_controller.go that
currently call klog.V(2).Infof with a "warning: ..." message should be changed
to use klog.Warningf to reflect true warning semantics; specifically replace the
klog.V(2).Infof call that logs the parsing error for
secret.Namespace/secret.Name (in the ParseCertsPEM regeneration-check path) and
the klog.V(2).Infof call that logs "no certs returned from ParseCertsPEM during
regeneration check" with klog.Warningf calls that keep the same message and
parameters so the warning level is used consistently.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/controller/servingcert/controller/secret_creating_controller.go`:
- Around line 242-248: The two log lines in secret_creating_controller.go that
currently call klog.V(2).Infof with a "warning: ..." message should be changed
to use klog.Warningf to reflect true warning semantics; specifically replace the
klog.V(2).Infof call that logs the parsing error for
secret.Namespace/secret.Name (in the ParseCertsPEM regeneration-check path) and
the klog.V(2).Infof call that logs "no certs returned from ParseCertsPEM during
regeneration check" with klog.Warningf calls that keep the same message and
parameters so the warning level is used consistently.

---

ℹ️ Review info

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 503e4f5 and 63ecf71.

📒 Files selected for processing (1)

• pkg/controller/servingcert/controller/secret_creating_controller.go

[tchap]

CI blocked by openshift/oc#2215

[openshift-ci]

@tchap: it appears that you have attempted to use some version of the payload command, but your comment was incorrectly formatted and cannot be acted upon. See the docs for usage info.

[openshift-ci]

@tchap: it appears that you have attempted to use some version of the payload command, but your comment was incorrectly formatted and cannot be acted upon. See the docs for usage info.

[openshift-ci]

@tchap: it appears that you have attempted to use some version of the payload command, but your comment was incorrectly formatted and cannot be acted upon. See the docs for usage info.

[openshift-ci]

@tchap: it appears that you have attempted to use some version of the payload command, but your comment was incorrectly formatted and cannot be acted upon. See the docs for usage info.

[tchap]

/retest

[openshift-ci]

@tchap: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-operator-serial-disruptive-ote	63ecf71	link	false	/test e2e-aws-operator-serial-disruptive-ote

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.