Remove need to be connected to a cluster to create a component

[metacosm]

What type of PR is this?
/kind bug

What does does this PR do / why we need it:
This PR delays the need to connect to a cluster until it's actually needed. This is accomplished by:

1. delay resolution of project/namespace until needed
2. avoid creating a client (and thus requiring a valid kube config) until needed
3. remove requirement to provide a namespace when creating a devfile component (the namespace is resolved when pushing)
4. update the env file to properly reflect the namespace / name changes when the component is pushed

This is required for odo create to work without being connected to a cluster.

Which issue(s) this PR fixes:

Fixes #3811

PR acceptance criteria:

• Unit test

• Integration test

• Documentation

• I have read the test guidelines

How to test changes / Special notes to the reviewer:
odo create would fail if not connected to a cluster before this PR. odo create should work even when not connected after.

[cdrage]

Dont see anything with the code.

However, the tests seem to have cancelled by user?

[cdrage]

/approve

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cdrage

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:

• OWNERS [cdrage]

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

[metacosm]

/retest

[mik-dass]

Worked for me on a openshift cluster even after logging out. But in case of minikube, after shutdown, I faced this problem

[mrinaldas@localhost project]$ odo create nodejs 
 ✗  invalid configuration: no configuration has been provided
Please login to your server: 

odo login https://mycluster.mydomain.com

[metacosm]

Worked for me on a openshift cluster even after logging out. But in case of minikube, after shutdown, I faced this problem

[mrinaldas@localhost project]$ odo create nodejs 
 ✗  invalid configuration: no configuration has been provided
Please login to your server: 

odo login https://mycluster.mydomain.com

I've tested it without problem with kind…

[mik-dass]

I've tested it without problem with kind…

These are the steps that I ran

[mrinaldas@localhost ~]$ ./minikube start
😄  minikube v1.12.3 on Fedora 30
✨  Using the kvm2 driver based on existing profile
👍  Starting control plane node minikube in cluster minikube
🔄  Restarting existing kvm2 VM for "minikube" ...
🐳  Preparing Kubernetes v1.18.3 on Docker 19.03.12 ...
🔎  Verifying Kubernetes components...
🌟  Enabled addons: default-storageclass, storage-provisioner
🏄  Done! kubectl is now configured to use "minikube"
[mrinaldas@localhost ~]$ odo create nodejs
Validation
 ✓  Checking devfile existence [93573ns]
 ✓  Checking devfile compatibility [146947ns]
 ✓  Creating a devfile component from registry: odo-V2-Registry [173612ns]
 ✓  Validating devfile component [372911ns]

Please use `odo push` command to create the component with source deployed
[mrinaldas@localhost ~]$ odo delete -f --all

Gathering information for component nodejs
 ✗  Checking status for component [5ms]
 ⚠  pod not found for the selector: component=nodejs

Deleting local config
 ✓  Successfully deleted env file
 ✓  Successfully deleted devfile.yaml file
[mrinaldas@localhost ~]$ ./minikube stop
✋  Stopping node "minikube"  ...
🛑  1 nodes stopped.
[mrinaldas@localhost ~]$ odo create nodejs
 ✗  invalid configuration: no configuration has been provided
Please login to your server: 

odo login https://mycluster.mydomain.com

[metacosm]

I've tested it without problem with kind…

These are the steps that I ran

[mrinaldas@localhost ~]$ ./minikube start
😄  minikube v1.12.3 on Fedora 30
✨  Using the kvm2 driver based on existing profile
👍  Starting control plane node minikube in cluster minikube
🔄  Restarting existing kvm2 VM for "minikube" ...
🐳  Preparing Kubernetes v1.18.3 on Docker 19.03.12 ...
🔎  Verifying Kubernetes components...
🌟  Enabled addons: default-storageclass, storage-provisioner
🏄  Done! kubectl is now configured to use "minikube"
[mrinaldas@localhost ~]$ odo create nodejs
Validation
 ✓  Checking devfile existence [93573ns]
 ✓  Checking devfile compatibility [146947ns]
 ✓  Creating a devfile component from registry: odo-V2-Registry [173612ns]
 ✓  Validating devfile component [372911ns]

Please use `odo push` command to create the component with source deployed
[mrinaldas@localhost ~]$ odo delete -f --all

Gathering information for component nodejs
 ✗  Checking status for component [5ms]
 ⚠  pod not found for the selector: component=nodejs

Deleting local config
 ✓  Successfully deleted env file
 ✓  Successfully deleted devfile.yaml file
[mrinaldas@localhost ~]$ ./minikube stop
✋  Stopping node "minikube"  ...
🛑  1 nodes stopped.
[mrinaldas@localhost ~]$ odo create nodejs
 ✗  invalid configuration: no configuration has been provided
Please login to your server: 

odo login https://mycluster.mydomain.com

Going to check.

[metacosm]

@mik-dass I've looked into it and this has not much to do with the change, this is due to changes made for #3938: link command now checks if CSV is supported when the command is created which, in my opinion, is wrong because creating commands should only be about initializing the hierarchy of commands and nothing more. The consequence of that change is that a completely unrelated command (link) makes create fail. Generally speaking, we should keep to the Complete/Validate/Run pattern as much as possible and complete the commands' options in Complete instead of doing it either when creating the Options or even before. /cc @dharmit @kadel

[dharmit]

First off, thank you for a PR to make odo create fast. I found it to be really slow when working on a remote cluster and was hoping we could eventually make it faster.

link command now checks if CSV is supported when the command is created which, in my opinion, is wrong because creating commands should only be about initializing the hierarchy of commands and nothing more.

Ah, my change has lead to other troubles. However, I didn't find a better place for the change I made in that PR. 😞

Generally speaking, we should keep to the Complete/Validate/Run pattern as much as possible and complete the commands' options in Complete instead of doing it either when creating the Options or even before.

But in this particular case, we are setting the cobra Command's Use, Example and Long attributes which are set at the time of creation of Command. I could be wrong in understanding your suggestion, but I'm not sure how we could print a help message (here odo link -h), with a different approach. I'm open to change the implementation for odo link -h if someone can recommend an alternate approach.

Besides that, you're suggesting changing the implementation for all the commands, right? I'm talking about the suggestion to complete the commands' options in Complete part of your suggestion.

[metacosm]

First off, thank you for a PR to make odo create fast. I found it to be really slow when working on a remote cluster and was hoping we could eventually make it faster.

Faster is only a side-effect: the primary goal is to be able to work with odo without needing a cluster access until I actually want to push the code and see it running on the cluster.

Generally speaking, we should keep to the Complete/Validate/Run pattern as much as possible and complete the commands' options in Complete instead of doing it either when creating the Options or even before.

But in this particular case, we are setting the cobra Command's Use, Example and Long attributes which are set at the time of creation of Command. I could be wrong in understanding your suggestion, but I'm not sure how we could print a help message (here odo link -h), with a different approach. I'm open to change the implementation for odo link -h if someone can recommend an alternate approach.

Yes, I understand why it was made but I'm not sure it should made… 😄
First, I don't think that the documentation should change based on the backend because that might be confusing. Instead, the documentation should explain the options in all cases. From a UX point of view, it makes some sense to only provide the available information at any given point but I worry that it makes things less discoverable (i.e. if you connect to a cluster that doesn't support operator hub, how will you know (only using the tool, that is) that odo can also deal with it, and how? Options disappearing from commands is also confusing, in my opinion.

From an implementation perspective, it's a bad idea as evidenced with this issue: an unrelated command (in this case, link) affects the behavior of create and that shouldn't happen, which is why the Complete/Validate/Run pattern should be used to start with.

Besides that, you're suggesting changing the implementation for all the commands, right? I'm talking about the suggestion to complete the commands' options in Complete part of your suggestion.

Ultimately, yes. Tactically, I need at least link to change at this point to not make any cluster access until the command is actually run (i.e. not before Complete is called). Other commands might also need to change but at this point, link is the blocking one.

[dharmit]

Faster is only a side-effect: the primary goal is to be able to work with odo without needing a cluster access until I actually want to push the code and see it running on the cluster.

+1. Avoiding connecting to cluster is exactly what I was thinking when I encountered slowness of odo create. But I'm more glad about this side effect, tbh. 👍

First, I don't think that the documentation should change based on the backend because that might be confusing. Instead, the documentation should explain the options in all cases.

Makes sense, actually. We could elaborate on the web based docs about different use cases supported by the command.

From a UX point of view, it makes some sense to only provide the available information at any given point but I worry that it makes things less discoverable (i.e. if you connect to a cluster that doesn't support operator hub, how will you know (only using the tool, that is) that odo can also deal with it, and how? Options disappearing from commands is also confusing, in my opinion.

+1. That's a bad UX choice made by me. In my defence, I did what I did because odo link -h has some long examples based on Service Catalog and it seems impossible to me (when I think from user's perspective) to keep UX sane if I append more examples to it for Operator Hub. But we can come up with something because Operator Hub is going to be preferred option going further vs. Service Catalog. I'll open an issue for this and come up with a middle ground as far as how much and what info we should provide in odo link -h.

Ultimately, yes. Tactically, I need at least link to change at this point to not make any cluster access until the command is actually run (i.e. not before Complete is called). Other commands might also need to change but at this point, link is the blocking one.

For what's currently blocking you, as I mentioned, I'll open an issue and try to address this soon so that it unblocks you. But for the suggested change in overall implementation/design, I request you to open an issue so that it doesn't get lost in the discussion for just this PR.

[metacosm]

+1. That's a bad UX choice made by me. In my defence, I did what I did because odo link -h has some long examples based on Service Catalog and it seems impossible to me (when I think from user's perspective) to keep UX sane if I append more examples to it for Operator Hub. But we can come up with something because Operator Hub is going to be preferred option going further vs. Service Catalog. I'll open an issue for this and come up with a middle ground as far as how much and what info we should provide in odo link -h.

Inline help shouldn't be a substitute for documentation. I don't think that overloading the inline help would help. Maybe we'd be better served creating man pages? We should put the emphasis on the most common options and point to the documentation for the rest (if possible with a direct link).

For what's currently blocking you, as I mentioned, I'll open an issue and try to address this soon so that it unblocks you. But for the suggested change in overall implementation/design, I request you to open an issue so that it doesn't get lost in the discussion for just this PR.

👍

[dharmit]

Inline help shouldn't be a substitute for documentation.

We're not trying to do this on purpose. But looking at where things are with few commands, and especially odo link, I agree with you.

@metacosm I was stepping over the code based on the steps mentioned in #3825 (comment). What I found is that the failure is attributed to missing kubeconfig context. If a user does odo login/oc login to some server, this would work.

I don't find a call to the CSV part of #3938 while doing odo create? What am I missing here?

[metacosm]

@metacosm I was stepping over the code based on the steps mentioned in #3825 (comment). What I found is that the failure is attributed to missing kubeconfig context. If a user does odo login/oc login to some server, this would work.

I know. The issue is that create (and most odo commands) should work without requiring a cluster access. I should be able to start my development workflow with odo without having to be connected to a cluster. The only time I need a cluster access is when I'm ready to see my app run on a cluster. The point here is that I shouldn't need to log into a cluster to be able to create a component and start working.

I don't find a call to the CSV part of #3938 while doing odo create? What am I missing here?

The call happens when the component command is created because NewCmdComponent calls NewCmdLink, which in turns checks is CSV is supported. That's the reason why NewCmd* functions should only build the command structure and not perform business logic until actually called because as you can see, even if there is nothing CSV-related in create, the command still fails because of it.

[openshift-ci-robot]

@metacosm: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/v4.5-integration-e2e	fa67e5a	link	/test v4.5-integration-e2e

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[mik-dass]

/lgtm

[mik-dass]

rebase needed
/hold

[metacosm]

The current code is not going to work as it is because there are too many parts that rely on an exposed kubernetes client and quite frankly I don't feel like fixing all of them at this point, at least, not before a true effort is made to fix the architecture. Nor do I want to introduce more if/else code to handle all the edge cases. /cc @kadel

[girishramnani]

so should we close this PR and try to resolve this issue systematically by first considering the architecture?

[metacosm]

so should we close this PR and try to resolve this issue systematically by first considering the architecture?

It depends on how important the underlying issue is (my opinion: it's very important from a middleware perspective). Fixing the architecture will be a significant undertaking… My problem with this issue is that I don't quite understand why there are 2 clients (oc and plain k8s) instead of just one. I tried to address that but apparently client code won't accommodate a single client because the logic is different depending on which client is initialized, which just doesn't make any sense to me… So I don't feel I'm qualified to fix the remaining issues in a reasonable amount of time.

[devang-gaur]

Hi @metacosm , can you look into the failing tests and rebase with master ? Let's get this merged and then decide what's to be done #3935

[metacosm]

Hi @metacosm , can you look into the failing tests and rebase with master ? Let's get this merged and then decide what's to be done #3935

@dev-gaur See #3825 (comment) for my perspective on this

[metacosm]

See #4057 for more details on what I think should be done, architecture-wise.

[girishramnani]

should we close this PR as it seems solving this issue would take an architectural overhaul ( which is happening right now )?

[girishramnani]

closing this PR as a major architectural overhaul is underway