Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

split kubernetes-client into an api and an impl #3645

Closed
shawkins opened this issue Dec 10, 2021 · 3 comments
Closed

split kubernetes-client into an api and an impl #3645

shawkins opened this issue Dec 10, 2021 · 3 comments

Comments

@shawkins
Copy link
Contributor

Is your task related to a problem? Please describe

Rather than just creating an api module specific to the http api, we can consider splitting the whole kubernetes-client into a separate api module.

Describe the solution you'd like

This is related to #3549 and #2829 - beyond CustomResource and CustomResourceList, it would be a good idea for the dsl, client package interfaces, etc. to be in their own api module. As much as possible the compile dependencies like common-*, generex, bouncy castle, slf4j, and ideally jackson would be eliminated there.

This would also somewhat alleviate the need for the internal package convention - only classes from the api would be intended for public use.

Ideally this will add some flexibility in terms of matching version across dependencies - if you depend on a module that uses fabric8 6.0.1 api, that should work with any fabric8 6.0.x impl, etc.

Describe alternatives you've considered

No response

Additional context

No response

@shawkins shawkins changed the title split kubernetes-client-project into an api and an impl split kubernetes-client into an api and an impl Dec 11, 2021
@shawkins
Copy link
Contributor Author

Changes (some breaking) that are needed for this:

  • separation of dsl and other interfaces from io.fabric8.kubernetes.client from *Client impls, *ExtensionAdapters, and other logic. This will require a new package to avoid a split package warning.
  • " io.fabric8.kubernetes.client.dsl.base
  • Some of the io.fabric8.kubernetes.client.internal is still needed on the api side with the current incarnation of the logic in Config and other classes - either they'll need to stay, or we'll have to do some refactoring
  • KubernetesClient.customResource returns a RawCustomResourceOperationsImpl - that needs to be an interface or removed
  • SharedInformerFactory needs to be an interface - ideally removing the deprecated exposure of OperationContext
  • some of io.fabric8.kubernetes.client.informers.cache will move to informers.impl - Cache.NAMESPACE_INDEX though is effectively part of the public api, so we may need to create a separate CacheImpl.
  • MetricAPIGroupDSL directly expose PodMetricOperationsImpl and NodeMetricOperationsImpl, that will require interfaces
  • RunOperations needs to be an interface and/or have a different way getting a PodOperationsImpl
  • Readiness based upon another issue should move to the api and out of an internal package - a deprecated/legacy version (just calling the new one) could be left in the original package if desired.
  • ManagedKubernetesClient - not sure where that needs to live
  • utils stuff would likely need to move to the api side, but things like PodOperationUtil would need to go to the impl side.

Based upon the above the only dependencies needed will be slf4j (which could be marked as provided) and jackson (which we could minimize somewhat. It would take quite a bit more work to refactor / break things to not have those.

@shawkins shawkins mentioned this issue Dec 16, 2021
11 tasks
@shawkins
Copy link
Contributor Author

shawkins commented Dec 16, 2021

Opened a pr #3654 to show what this module would look like - which addresses the interface related questions above, but does not yet make the kubernetes-client module use the new one, so split packages have not been addressed.

Additional thoughts after working through this:

  • also requires splitting the openshift module into a client and impl
  • a big sticking point is how clients are created - that requires direct access to Default*Client. So this initial incarnation could be used by api logic in projects like the operator sdk and strimzi, but any actual client usage would still need a compile dependency on kubernetes-client or opernshift-client. So we'll probably need to introduce factory methods and add a deprecation about directly creating clients. I don't see how to actually create the new module and not have split packages though - we'll have to have the io.fabric8.kubernetes.client package in both the api and the impl.
  • utils has not been minimized. The implication is that since they are in a non-internal package anyone could have been using that stuff. This however creates a pretty wide surface area that we don't really want to promote for general usage. I'd vote for moving it out of the api as much as possible.
  • There are things in utils and internal that are internally referenced with api still. If utils mostly moves to the impl side, Config is still a big offender - depending upon CertUtils, KubeConfigUtils, SSLUtils, IOHelpers, Utils, etc. We should consider refactoring to move any utility logic that doesn't belong in the api.

@manusa @metacosm @rohanKanojia if all of this seems like too much to bite off in a single release, we could start just with making the proposed interface changes prior to creating a new module.

@shawkins
Copy link
Contributor Author

Some things to move out of utils and other api packages:
WatcherToggle
SyncableStore
ResyncRunnable
ExponentialBackoffIntervalCalculator
DeleteAndCreateHelper
CreateOrReplaceHelper
SerialExecutor?

UnmatchedFieldTypeModule is effectively part of the public api.
The interceptors would be good to move out of the api (they also have dependencies on the internal utils), but would require additional refactoring. It would be straight-forward to just have them as package private.

It looks like most of the rest of utils would have to stay given their general nature.

shawkins added a commit to shawkins/kubernetes-client that referenced this issue Feb 11, 2022
this is currently not used
this is to avoid having the same class in multiple jars
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Feb 14, 2022
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Feb 14, 2022
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Feb 14, 2022
@manusa manusa closed this as completed in 71153e0 Feb 15, 2022
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

No branches or pull requests

1 participant