Add Unknown State to Component.

[adisky]

What type of PR is this?

/kind bug

What does does this PR do / why we need it:
When odo is not connected to cluster, is should list out component from local config with state unknown.

Which issue(s) this PR fixes:
Fixes #2444

How to test changes / Special notes to the reviewer:
Run odo list --path <path> on a not logged in cluster, is should return state of all the component to unknown.

[adisky@localhost odo]$ odo list --path ~/
APP     NAME                    PROJECT         TYPE       STATE       CONTEXT
app     frontend                new-project     nodejs     Unknown     /home/adisky/multicompapp/frontend
app     nodejs-mycmp6-fijc      default         nodejs     Unknown     /home/adisky/mycmp6
app     nodejs-myname1-gewe     ezsnxrczvt      nodejs     Unknown     /home/adisky/myname1

[amitkrout]

@adisky It will be hard to add integration test particularly in this usecase to run on our CI infrastructure, so cover the test through UTs.

[adisky]

@adisky It will be hard to add integration test particularly in this usecase to run on our CI infrastructure, so cover the test through UTs.

@amitkrout sure, was stuck with something in UT, it is cleared now. will add

[codecov]

Codecov Report

Merging #2690 into master will decrease coverage by 0.02%.
The diff coverage is 44.00%.

@@            Coverage Diff             @@
##           master    #2690      +/-   ##
==========================================
- Coverage   43.67%   43.65%   -0.03%     
==========================================
  Files          95       95              
  Lines        8762     8780      +18     
==========================================
+ Hits         3827     3833       +6     
- Misses       4571     4584      +13     
+ Partials      364      363       -1     

Impacted Files	Coverage Δ
pkg/application/application.go	40.00% <0.00%> (-1.27%)	⬇️
pkg/odo/util/cmdutils.go	1.69% <0.00%> (ø)
pkg/util/util.go	65.40% <0.00%> (-1.70%)	⬇️
pkg/component/component.go	24.38% <58.33%> (+0.59%)	⬆️
pkg/occlient/occlient.go	52.73% <100.00%> (+0.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cef105b...5c5724e. Read the comment docs.

[adisky]

/retest

[dharmit]

Besides the requested change, I think we need to also add the definition of various states (Pushed, Not Pushed and Unkown) in the docs. Specially Unknown could confuse the users. From what I see in the docs right now, there's no mention of the states when user executed odo list. That's where this needs to be added. Ping @Preeticp @boczkowska can you help take this bit further?

@kadel should we also add documentation of the various states in odo list -h? I'm inclined towards adding it, but what do you think?

[kadel]

@adisky It will be hard to add integration test particularly in this usecase to run on our CI infrastructure, so cover the test through UTs.

@amitkrout Why it would be hard? You mean because you need to be disconnected from the cluster?
That should be easy to do, the test could just set KUBECONFIG to a new path. Or copy the old config new one, modify it and use it.

[kadel]

@adisky It will be hard to add integration test particularly in this usecase to run on our CI infrastructure, so cover the test through UTs.

@amitkrout Why it would be hard? You mean because you need to be disconnected from the cluster?
That should be easy to do, the test could just set KUBECONFIG to a new path. Or copy the old config new one, modify it and use it.

@kadel should we also add documentation of the various states in odo list -h? I'm inclined towards adding it, but what do you think?

yep, we definitely should. Docs need to include definition and descriptions of each state.

[yhontyk]

@kadel @amitkrout Thank you for bringing it up. I filed a separate docs issue to track.

[amitkrout]

@amitkrout Why it would be hard? You mean because you need to be disconnected from the cluster?
That should be easy to do, the test could just set KUBECONFIG to a new path. Or copy the old config new one, modify it and use it.

@kadel yup, it is doable. @adisky from https://github.com/openshift/odo/blob/master/tests/integration/cmd_pref_config_test.go#L308 you will get the reference to implement it.

[adisky]

@amitkrout Why it would be hard? You mean because you need to be disconnected from the cluster?
That should be easy to do, the test could just set KUBECONFIG to a new path. Or copy the old config new one, modify it and use it.

@kadel yup, it is doable. @adisky from https://github.com/openshift/odo/blob/master/tests/integration/cmd_pref_config_test.go#L308 you will get the reference to implement it.

@amitkrout Thanks, would add an integration test case

[adisky]

@amitkrout @kadel I tried setting KUBECONFIG to the new path for integration tests, it gives following error

✗  invalid configuration: no configuration has been provided
Please login to your server: 

odo login https://mycluster.mydomain.com

for most commands in odo, it creates a newContext[1], which in turn creates a kubeClient[2] and it is failing here, if setting wrong KUBECONFIG.

I have tested the PR manually with oc logout and it works, and issue says the same. I think readjusting context is out of scope for this PR. @amitkrout Please suggest any other way to add the integration test case.

[1]
https://github.com/openshift/odo/blob/791be7f00f4ebcd6fbffed8ccd66dda1aa8586a9/pkg/odo/genericclioptions/context.go#L280

[2]
https://github.com/openshift/odo/blob/791be7f00f4ebcd6fbffed8ccd66dda1aa8586a9/pkg/occlient/occlient.go#L240

[dgolovin]

@kadel @dharmit The list of states is missing one more state 'Locally Deleted'. This what happens for URL and should happening for Storage when it is deleted in component context, but not pushed to cluster yet.

[adisky]

@kadel @dharmit The list of states is missing one more state 'Locally Deleted'. This what happens for URL and should happening for Storage when it is deleted in component context, but not pushed to cluster yet.

@dgolovin Locally Deleted for Storage has been added here. https://github.com/openshift/odo/blob/master/pkg/storage/types.gohttps://github.com/openshift/odo/blob/master/pkg/storage/types.go#L24

[adisky]

/retest

[kadel]

/retest
/approve

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kadel

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 [kadel]

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

[girishramnani]

/retest

[adisky]

/retest

[adisky]

/retest

[dharmit]

/lgtm

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.