Skip to content

Conversation

@manusa
Copy link
Member

@manusa manusa commented Oct 16, 2025

  • Removed Manager.IsInCluster method and created a function scoped to the runtime environment.
      As a method, the implementation was not correct.
  • Removed GetAPIServerHost method from Manager which is no used.
  • Temporarily added an inCluster field to the Manager struct but should be eventually removed since it doesn't really make sense to hava a Manager in-cluster or out-of-cluster in the multi-cluster scenario.
  • Provider resolution (resolveStrategy) is now clearer, added complete coverage for all scenarios.
  • Added additional coverage for provider and manager.
  • Removes Provider.newForContext(context string) method to simplify Manager creation.

The next follow-up would be to ideally remove the Manager parameter from the ProviderFactory function type.
It doesn't make sense for all scenarios to have a base Kubernetes Manager, Provider implementations should be responsible of creating their own Manager instances if needed, when needed.

/cc @Cali0707

…r detection

- Removed IsInCluster method from Manager and created function scoped to the runtime environment.
  As a method, the implementation was not correct.
  Removed GetAPIServerHost method from Manager which is no used.
- **Temporarily** added an `inCluster` field to the Manager struct but should be eventually removed since it doesn't really make sense to hava a Manager in-cluster or out-of-cluster in the multi-cluster scenario.
- Provider resolution (resolveStrategy) is now clearer, added complete coverage for all scenarios.
- Added additional coverage for provider and manager.

Signed-off-by: Marc Nuri <[email protected]>
…and simplify manager creation

- Removes Provider.newForContext(context string) method.

Signed-off-by: Marc Nuri <[email protected]>
@manusa manusa added this to the 0.1.0 milestone Oct 16, 2025
Copy link
Collaborator

@Cali0707 Cali0707 left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for continuing to refactor and clean up this code @manusa !

@Cali0707 Cali0707 merged commit 9da29f4 into containers:main Oct 16, 2025
6 checks passed
@manusa manusa deleted the refactor/provider branch October 17, 2025 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants