Refactor occlient and kclient by unifying them into one

[adisky]

/kind code-refactoring

Acceptance Criteria

• oc client and kclient should have the same interface and based on what odo encounters in the context, the client needs to be decided in the context.
• Unify the occlient and kclient into one client called as kclient. Remove the duplications and do everything through kclient. Interfaces to be added later.

[girishramnani]

We need to primarily bring back the original 3 layered architecture that odo had -

• cli packages
• odo specific packages
• cluster communication packages

Currently things are bleading into each other.

[mik-dass]

It might be good idea to merge the two clients into a single client which handles both openshift and kubenetes clusters. We already are using the kclient inside the occlient
https://github.com/openshift/odo/blob/49853dc69d4f45d2f54cd4bad80e601bd2b4e715/pkg/occlient/occlient.go#L200

This will help reduce duplicate code between the two clients. Also a single interface for both the clients might be tricky as certain resources like routes etc are not in the kclient.

[dharmit]

It might be good idea to merge the two clients into a single client

I remember that we started kclient with the intention of being able to deprecate occlient at some point. So this has to be our end goal, IMO.

[mik-dass]

I remember that we started kclient with the intention of being able to deprecate occlient at some point. So this has to be our end goal, IMO.

We will just remove the unnecessary functions when that deprecation happens from the new merged client. Also creation of routes instead of ingress does need a occlient currently and we need to support s2i for some time.

[metacosm]

The two clients should be indeed be merged. It shouldn't matter to the client code which kind of cluster odo is connected to. I don't think that this can be done without first isolating how components / etc. are realized on the cluster from code calling it: i.e. code calling the client shouldn't need to know how a component is implemented on the cluster. More precisely, the client should expose something like GetComponent *Component and not GetDeployment *v1.Deployment.

[mik-dass]

More precisely, the client should expose something like GetComponent *Component and not GetDeployment *v1.Deployment.

We were thinking of doing this on the odo specific packages. The current idea is to have interfaces for storage, URL etc and s2i/devfile will have their own implementations of those functions. The client won't have any idea about odo specific things.

[mik-dass]

I have added a document regarding the refactor here https://docs.google.com/document/d/1izxse1fBKqWG1822zvSsZwZHTDj57nlxlZllLxpDfzI/edit?usp=sharing. @metacosm @kadel @girishramnani @adisky @dharmit Please have a look and share your feedback.

[metacosm]

@mik-dass I've added some comments, please also see: #4057 (comment)

[mik-dass]

Resources to be moved and cleaned up from the occlient and kclient

• ImageStreams (Refactors and moves buildConfigs, imageStreams and some other resources #4261)
• Secrets (Refactors, cleans and moves some resources to the new files. #4250)
• Service (Refactors, cleans and moves some resources to the new files. #4250)
• BuildConfigs (Refactors and moves buildConfigs, imageStreams and some other resources #4261)
• DeploymentConfigs (Refactors, cleans and moves some resources to the new files. #4250)
• Pods (Refactors, cleans and moves some resources to the new files. #4250)
• Events (Refactors, cleans and moves some resources to the new files. #4250)
• ServiceCatalog (Refactors service catalog and discovery client code #4272)
• Operator (Refactors service catalog and discovery client code #4272)

Other tasks include clean up files, clean up interfaces like ExecClient to reduce import cycles and remove duplicate functions.

[dharmit]

@mik-dass can you move this into the issue description and check off the ones you have already got merged? Add a note to those that have a PR open. Just want to get an idea about its status before we consider it for the next sprint.

And, if all the resources have been taken care, feel free to close this!

[mik-dass]

Add a note to those that have a PR open. Just want to get an idea about its status before we consider it for the next sprint.

I have checked off the resources which are done.

Other tasks include clean up files, clean up interfaces like ExecClient to reduce import cycles and remove duplicate functions.

We need to finish this part and unify the two clients.

[dharmit]

Other tasks include clean up files, clean up interfaces like ExecClient to reduce import cycles and remove duplicate functions.

We need to finish this part and unify the two clients.

So now we're supposed to make changes that we discussed in #4057? I remember you were working on url package to make it more interface-y. Are you planning to open a PR for it or have you abondoned that code?

For things in "odo service" and "odo catalog list services", I intend to address those via #4242.

[mik-dass]

So now we're supposed to make changes that we discussed in #4057?

Yes, the client related changes discussed in #4057.

I remember you were working on url package to make it more interface-y. Are you planning to open a PR for it or have you abandoned that code?

I need to clean it up as a part of #4115.

[dharmit]

@kadel @mik-dass do you think this issue should be closed?

[mik-dass]

@kadel @mik-dass do you think this issue should be closed?

Yes, I think we can close this issue.