Skip to content

CNTRLPLANE-3384: Remove unconditional low-quality log messages from controllers - #8428

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
csrwng:cntrlplane-3384-remove-low-quality-logs
May 11, 2026
Merged

CNTRLPLANE-3384: Remove unconditional low-quality log messages from controllers#8428
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
csrwng:cntrlplane-3384-remove-low-quality-logs

Conversation

@csrwng

@csrwng csrwng commented May 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Remove unconditional log.Info("Reconciling"), log.Info("reconciliation complete"), and equivalent messages from 18 controllers across the CPO and HO
  • Clean up unused variables and imports resulting from the removal

Context

These log messages provide no actionable information and are emitted on every reconcile invocation, including frequent re-queues. controller-runtime already provides reconcile lifecycle tracing at debug verbosity. In management clusters running many HostedClusters (e.g., ARO HCP), this produces significant log noise that obscures meaningful events during incident investigation.

Identified during a debugging session investigating cluster deletion issues in the managed Azure service (ARO HCP).

Jira

https://issues.redhat.com/browse/CNTRLPLANE-3384

Test plan

  • Verify no meaningful or conditional log messages were removed
  • Verify unused variables and imports are cleaned up
  • make test passes
  • make build passes

Summary by CodeRabbit

  • Chores
    • Reduced verbose informational logging across multiple controllers and providers to declutter logs and make operational output more concise without changing behavior.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@openshift-ci

openshift-ci Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 5, 2026
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 5, 2026
@openshift-ci-robot

openshift-ci-robot commented May 5, 2026

Copy link
Copy Markdown

@csrwng: This pull request references CNTRLPLANE-3384 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

  • Remove unconditional log.Info("Reconciling"), log.Info("reconciliation complete"), and equivalent messages from 18 controllers across the CPO and HO
  • Clean up unused variables and imports resulting from the removal

Context

These log messages provide no actionable information and are emitted on every reconcile invocation, including frequent re-queues. controller-runtime already provides reconcile lifecycle tracing at debug verbosity. In management clusters running many HostedClusters (e.g., ARO HCP), this produces significant log noise that obscures meaningful events during incident investigation.

Identified during a debugging session investigating cluster deletion issues in the managed Azure service (ARO HCP).

Jira

https://issues.redhat.com/browse/CNTRLPLANE-3384

Test plan

  • Verify no meaningful or conditional log messages were removed
  • Verify unused variables and imports are cleaned up
  • make test passes
  • make build passes

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.

@openshift-ci openshift-ci Bot added do-not-merge/needs-area area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release area/platform/aws PR/issue for AWS (AWSPlatform) platform area/platform/azure PR/issue for Azure (AzurePlatform) platform area/platform/gcp PR/issue for GCP (GCPPlatform) platform approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed do-not-merge/needs-area labels May 5, 2026
@csrwng
csrwng force-pushed the cntrlplane-3384-remove-low-quality-logs branch from 1f358a6 to fc7aa60 Compare May 5, 2026 19:40
@coderabbitai

coderabbitai Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: e7599282-e436-427e-a013-dcad777ec1db

📥 Commits

Reviewing files that changed from the base of the PR and between fc7aa60 and eae566a.

📒 Files selected for processing (18)
  • control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go
  • control-plane-operator/controllers/gcpprivateserviceconnect/observer.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/drainer/drainer.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/hcpstatus/hcpstatus.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/machine/machine.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/node/node.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/nodecount/controller.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/resources/kas/admissionpolicies.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/resources/registry/admissionpolicies.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go
  • hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go
  • hypershift-operator/controllers/hostedclustersizing/hostedclustersizing_validation_controller.go
  • hypershift-operator/controllers/nodepool/nodepool_controller.go
  • hypershift-operator/controllers/platform/aws/controller.go
  • hypershift-operator/controllers/scheduler/aws/autoscaler.go
  • hypershift-operator/controllers/scheduler/azure/controller.go
💤 Files with no reviewable changes (18)
  • control-plane-operator/hostedclusterconfigoperator/controllers/node/node.go
  • hypershift-operator/controllers/nodepool/nodepool_controller.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/machine/machine.go
  • hypershift-operator/controllers/scheduler/aws/autoscaler.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/nodecount/controller.go
  • hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go
  • hypershift-operator/controllers/hostedclustersizing/hostedclustersizing_validation_controller.go
  • control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/drainer/drainer.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/resources/registry/admissionpolicies.go
  • hypershift-operator/controllers/scheduler/azure/controller.go
  • control-plane-operator/controllers/gcpprivateserviceconnect/observer.go
  • hypershift-operator/controllers/platform/aws/controller.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/hcpstatus/hcpstatus.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go
  • control-plane-operator/hostedclusterconfigoperator/controllers/resources/kas/admissionpolicies.go

📝 Walkthrough

Walkthrough

This pull request removes informational log statements from numerous Reconcile methods across the control-plane-operator and hypershift-operator codebases. Changes include deletion of startup logs (e.g., log.Info("Reconciling")), some completion logs, and a few controller-runtime logging imports; a small number of reconciler functions now obtain a logger via ctrl.LoggerFrom(ctx) or logr.FromContext(ctx) before use. No reconciliation logic, control flow, error handling, public APIs, or functional behavior were modified.

🚥 Pre-merge checks | ✅ 12
✅ Passed checks (12 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: removing unconditional low-quality log messages from controllers across the codebase.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed Custom check not applicable. PR only modifies controller files to remove log statements. No test files or Ginkgo test names are affected.
Test Structure And Quality ✅ Passed Custom check for Ginkgo test quality is not applicable. PR modifies 15 standard Go test files (using func Test...) but no Ginkgo test files. Only 7 Ginkgo tests exist in codebase, none modified.
Microshift Test Compatibility ✅ Passed PR does not add new Ginkgo e2e tests. It only removes log statements from existing controller implementations. The MicroShift check applies only to new tests.
Single Node Openshift (Sno) Test Compatibility ✅ Passed PR removes logging statements from 18 controller files. No new Ginkgo e2e tests are added—the check is not applicable.
Topology-Aware Scheduling Compatibility ✅ Passed PR only removes log messages and cleans up unused code. No scheduling constraints, affinity rules, or topology-related changes introduced.
Ote Binary Stdout Contract ✅ Passed All changes are in controller Reconcile() methods (not process-level). Log.Info() calls use logr+zap configured to write stderr, not stdout. No stdout contract violation.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed This PR does not add any new Ginkgo e2e tests. The custom check applies only to new e2e tests. The PR modifies only 18 controller files to remove log messages—no test files were modified.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@csrwng
csrwng marked this pull request as ready for review May 5, 2026 19:42
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 5, 2026
@openshift-ci
openshift-ci Bot requested review from enxebre and muraee May 5, 2026 19:43
@codecov

codecov Bot commented May 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 37.44%. Comparing base (e827093) to head (eae566a).
⚠️ Report is 67 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8428      +/-   ##
==========================================
- Coverage   37.44%   37.44%   -0.01%     
==========================================
  Files         751      751              
  Lines       91969    91922      -47     
==========================================
- Hits        34435    34417      -18     
+ Misses      54894    54865      -29     
  Partials     2640     2640              
Files with missing lines Coverage Δ
...ollers/awsprivatelink/awsprivatelink_controller.go 18.80% <ø> (-0.15%) ⬇️
...r/controllers/gcpprivateserviceconnect/observer.go 61.36% <ø> (-1.68%) ⬇️
...usterconfigoperator/controllers/drainer/drainer.go 0.00% <ø> (ø)
...terconfigoperator/controllers/globalps/globalps.go 86.88% <ø> (+0.66%) ⬆️
...rconfigoperator/controllers/hcpstatus/hcpstatus.go 0.00% <ø> (ø)
...tor/controllers/inplaceupgrader/inplaceupgrader.go 57.16% <ø> (+0.10%) ⬆️
...usterconfigoperator/controllers/machine/machine.go 67.00% <ø> (-0.17%) ⬇️
...stedclusterconfigoperator/controllers/node/node.go 38.70% <ø> (+0.41%) ⬆️
...configoperator/controllers/nodecount/controller.go 0.00% <ø> (ø)
...tor/controllers/resources/kas/admissionpolicies.go 9.25% <ø> (+0.16%) ⬆️
... and 8 more
Flag Coverage Δ
cmd-support 32.63% <ø> (ø)
cpo-hostedcontrolplane 36.48% <ø> (ø)
cpo-other 37.70% <ø> (-0.03%) ⬇️
hypershift-operator 47.95% <ø> (+0.01%) ⬆️
other 27.77% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

These log messages (e.g. log.Info("Reconciling"), log.Info("reconcile
complete")) provide no clear information and are emitted on every
reconcile invocation, leading to excessive log noise. controller-runtime
already provides reconcile lifecycle tracing at debug verbosity.

Remove entry/exit logs and sub-function announcement logs from 18
controllers across the CPO and HO. Clean up unused variables and
imports where applicable.

Signed-off-by: Cesar Wong <cewong@redhat.com>
Commit-Message-Assisted-by: Claude (via Claude Code)
@csrwng
csrwng force-pushed the cntrlplane-3384-remove-low-quality-logs branch from fc7aa60 to eae566a Compare May 6, 2026 19:27

@bryan-cox bryan-cox left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 7, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks-4-22
/test e2e-aws-4-22
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-azure-self-managed
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws

@openshift-ci

openshift-ci Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bryan-cox, csrwng

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@cwbotbot

cwbotbot commented May 7, 2026

Copy link
Copy Markdown

Test Results

e2e-aws

e2e-aks

@bryan-cox

Copy link
Copy Markdown
Member

/retest

@hypershift-jira-solve-ci

hypershift-jira-solve-ci Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

I now have all the evidence needed. Let me produce the final report.

Test Failure Analysis Complete

Job Information

Test Failure Analysis

Error

TestNodePool/HostedCluster0: hypershift_framework.go:501: failed to create cluster, tearing down:
failed to create infra: cannot create VPC S3 endpoint: operation error EC2: CreateVpcEndpoint,
exceeded maximum number of attempts, 11, https response error StatusCode: 503,
RequestID: aa575bd0-a432-4ba0-89e5-3202e9a25bfd, api error RequestLimitExceeded:
Request limit exceeded. Account 820196288204 has been throttled on ec2:CreateVpcEndpoint
because it exceeded its request rate limit.

Summary

This failure is an AWS EC2 API rate-limiting flake unrelated to the PR changes. The TestNodePool/HostedCluster0 test failed because the shared CI AWS account (820196288204) was throttled on the ec2:CreateVpcEndpoint API call. The test attempted to create a VPC S3 endpoint as part of cluster infrastructure setup, was rate-limited with HTTP 503, exhausted all 11 retry attempts, and failed. The PR (CNTRLPLANE-3384) exclusively removes low-quality log.Info("Reconciling") messages from 18 controllers and does not touch any AWS infrastructure, VPC, endpoint creation, or test framework code. All other 474 tests passed; the 12 hosted clusters created concurrently by other tests in the suite all succeeded, indicating this was a transient capacity issue on the specific API endpoint at the moment TestNodePool/HostedCluster0 made its call.

Root Cause

AWS EC2 API Rate Limiting (transient infrastructure flake)

The E2E test suite creates 12+ hosted clusters in parallel, each requiring AWS infrastructure including VPC S3 gateway endpoints. All clusters share the same CI AWS account 820196288204. When TestNodePool/HostedCluster0 attempted to call ec2:CreateVpcEndpoint, AWS returned HTTP 503 with error code RequestLimitExceeded, indicating the account had been throttled on this specific API.

The infrastructure creation code (cmd/infra/aws/ec2.go, CreateVPCS3Endpoint) does include retry logic, but it specifically retries only InvalidRouteTableId.NotFound errors — not RequestLimitExceeded. The AWS SDK's built-in retry mechanism exhausted all 11 attempts without success because the throttling persisted throughout the retry window.

This is a known class of flake in HyperShift CI caused by the high degree of parallelism (12 clusters being created simultaneously) hitting shared AWS account rate limits. The fact that 8 clusters created before and 4 clusters created after this failure all succeeded confirms the throttling was transient.

The PR changes (removing log.Info("Reconciling") messages from controllers) have zero connection to this failure. The changed files are all in control-plane-operator/ and hypershift-operator/ controller reconciliation paths. No AWS infrastructure code, VPC endpoint creation code, or test framework code was modified.

Recommendations
  1. Retry / retest the PR — This is a transient AWS rate-limiting flake. A /retest should pass.
  2. No code changes needed in PR CNTRLPLANE-3384: Remove unconditional low-quality log messages from controllers #8428 — The failure is completely unrelated to the log message cleanup changes.
  3. Long-term improvement (not blocking) — The CreateVPCS3Endpoint retry logic in cmd/infra/aws/ec2.go only retries InvalidRouteTableId.NotFound errors. Adding RequestLimitExceeded / Throttling to the retriable error set would make the infra creation more resilient to shared-account rate limiting.
Evidence
Evidence Detail
Failed test TestNodePool/HostedCluster0 (1 of 476 tests; parent TestNodePool fails as a consequence)
Error type AWS EC2 API RequestLimitExceeded on ec2:CreateVpcEndpoint
HTTP status 503 (Service Unavailable — throttled)
AWS account 820196288204 (shared CI account)
Retry attempts 11 (all exhausted)
Parallel clusters 12 hosted clusters created concurrently; 11 succeeded, 1 hit rate limit
PR files changed 18 controller files — all log.Info() removals, zero AWS/infra/VPC changes
PR touches infra code? No — grep for infra/aws/ec2/vpc/endpoint in changed files returns empty
Other tests affected None — 474 passed, 21 skipped, 2 failed (both are TestNodePool)
Failure source hypershift_framework.go:501cmd/infra/aws/ec2.go:CreateVPCS3Endpoint

@stevekuznetsov

Copy link
Copy Markdown
Contributor

/test ci/prow/e2e-aws-4-22

@stevekuznetsov

Copy link
Copy Markdown
Contributor

/retest

@bryan-cox

Copy link
Copy Markdown
Member

/verified by e2e

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label May 11, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@bryan-cox: This PR has been marked as verified by e2e.

Details

In response to this:

/verified by e2e

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.

@openshift-ci

openshift-ci Bot commented May 11, 2026

Copy link
Copy Markdown
Contributor

@csrwng: all tests passed!

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.

@openshift-merge-bot
openshift-merge-bot Bot merged commit 9b3ea5b into openshift:main May 11, 2026
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release area/platform/aws PR/issue for AWS (AWSPlatform) platform area/platform/azure PR/issue for Azure (AzurePlatform) platform area/platform/gcp PR/issue for GCP (GCPPlatform) platform jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants