Skip to content

Conversation

@nickytd
Copy link
Collaborator

@nickytd nickytd commented Feb 9, 2026

How to categorize this PR?
/kind enhancement
/area logging

What this PR does / why we need it:
This PR replaces the legacy shared informer implementation with controller-runtime for managing dynamic Kubernetes clients. The controller-runtime framework provides a more modern, maintainable, and robust reconciliation pattern for watching Cluster resources and managing corresponding log forwarding clients. This change improves concurrent safety through atomic operations and proper synchronization primitives while maintaining the existing functionality for dynamic host path routing.

Code changes:

  • Controller Layer Refactoring:

    • Removed legacy pkg/cluster/clientset and informer-based code generation packages
    • Introduced pkg/controller/manager.go implementing controller-runtime Manager with REST config handling (in-cluster and kubeconfig fallback)
    • Added pkg/controller/reconciler.go with ClusterReconciler implementing the Reconcile pattern for Cluster resources
    • Created pkg/controller/test_helpers.go with TestControllerManager and TestClusterReconciler for unit testing without real Kubernetes clusters
    • Refactored pkg/controller/controller.go to use sync.Map for concurrent-safe client storage and atomic.Bool for stopped state
    • Updated pkg/controller/client.go with improved locking patterns using LoadOrStore for race condition prevention
  • Plugin Integration:

    • Modified pkg/plugin/logging.go to accept a controller-runtime Controller interface instead of raw informer
    • Updated cmd/fluent-bit-output-plugin/kubernetes_client.go and output_plugin.go to initialize controller-runtime manager
    • Changed plugin initialization flow to use initControllerManager with proper context handling
  • Test Updates:

    • Refactored pkg/plugin/logging_test.go and tests/plugin/plugin_test.go to use fake controller-runtime clients
    • Updated all test cases to work with TestControllerManager instead of informers
    • Improved test reliability with proper context management and reconciliation loops
  • Configuration Fixes:

    • Fixed typos in pkg/config/config.go: corrected config key names (e.g., sendlogstoshootwhenisinhhibernatingstatesendlogstoshootwhenisinhibernatingstate)
  • Minor Improvements:

    • Removed verbose debug logging from pkg/client/dque_batch_processor.go

Additional context:
The migration to controller-runtime aligns with Kubernetes ecosystem best practices and provides better integration with other Gardener components. The new implementation maintains backward compatibility in terms of functionality while improving the internal architecture. All existing tests pass with the new implementation using fake clients.

Which issue(s) this PR fixes:
None specified

Special notes for your reviewer:

  • The PR includes significant refactoring of the controller layer but maintains the same external API
  • Test helpers have been introduced to facilitate testing without real Kubernetes clusters
  • The atomic operations and sync.Map usage improve concurrent safety but require careful review
  • The controller-runtime manager startup includes proper timeout handling and error propagation

Release note:

Migrated from shared informers to controller-runtime for managing dynamic log forwarding clients, improving maintainability and concurrent safety.
  • 🔄 Regenerate and Update Summary

📬 Subscribe to the Hyperspace PR Bot DL to get the latest announcements and pilot features!

PR Bot Information

Version: 1.17.53 | 📖 Documentation | 🚨 Create Incident | 💬 Feedback

  • Output Template: PR Template File
  • Summary Prompt: PR Prompt File
  • Event Trigger: pull_request.opened
  • LLM: anthropic--claude-4.5-sonnet
  • Correlation ID: f67b9230-05e1-11f1-8a98-aafc2cfffa4b

@gardener-prow gardener-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 9, 2026
@nickytd nickytd self-assigned this Feb 9, 2026
@gardener-prow gardener-prow bot added do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. labels Feb 9, 2026
@nickytd
Copy link
Collaborator Author

nickytd commented Feb 9, 2026

/ask review

@gardener-prow
Copy link

gardener-prow bot commented Feb 9, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from nickytd. 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:

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

@nickytd
Copy link
Collaborator Author

nickytd commented Feb 9, 2026

review feedback is addressed.

@nickytd nickytd force-pushed the with-controller-runtime branch from 0e9613a to 2beb85d Compare February 9, 2026 19:46
@gardener gardener deleted a comment from hyperspace-insights bot Feb 9, 2026
@nickytd nickytd closed this Feb 10, 2026
@nickytd nickytd deleted the with-controller-runtime branch February 10, 2026 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant