Skip to content

Conversation

@panslava
Copy link
Contributor

No description provided.

Replace factory-based informer creation with centralized InformerSet that:
- Creates base SharedIndexInformers directly without factories
- Manages lifecycle and synchronization in one place
- Provides filtered views for ProviderConfig-specific controllers
- Reduces boilerplate and improves maintainability

Using informers directly without factories simplifies the logic and
eliminates potential mistakes from unnecessary factory usage, such as
cidentally creating duplicate informers or incorrect factory scoping.
- Add manager_test.go covering:
  - Start adds NEG cleanup finalizer and is idempotent
  - Start failure and GCE client creation failure roll back finalizer
  - Stop closes controller channel and removes finalizer
  - Concurrent starts for same ProviderConfig are single-shot
- Improve manager.go:
  - Introduce test seam via package var startNEGController
  - Ensure finalizer is added before start; roll back on any startup error
  - Delete finalizer using latest ProviderConfig from API to avoid stale updates
  - Wrap errors with %w and add GoDoc comments
  - Rename exported ControllerSet to unexported controllerSet
- No expected behavior change in the happy path; robustness and testability improved.
The ProviderConfigControllersManager was using a single mutex that blocked
all 5 workers when one ProviderConfig's initialization took a long time
(e.g., GCE client creation with infinite retry loops). This prevented
concurrent processing of different ProviderConfigs.

Changes:
- Extract thread-safe ControllerMap with minimal lock scope
- Only hold lock during map operations (Get/Set/Delete)
- Expensive operations (finalizer, GCE client, NEG controller startup)
  now execute without holding the lock
- Add comprehensive unit tests for concurrent access patterns

Impact:
- Multiple ProviderConfigs can now initialize concurrently
- One stuck ProviderConfig no longer blocks others
- Workqueue guarantees prevent same-key concurrent processing
- All 5 workers can now operate in parallel on different configs
Extract common utilities into pkg/multiproject/common and create generic
controller framework in pkg/multiproject/framework. Reorganize NEG
implementation to use the new structure.

This refactoring enables reuse of controller patterns across different
multiproject implementations while maintaining clean separation of concerns.
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 23, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: panslava

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

The pull request process is described here

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

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 23, 2025
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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.

2 participants