Fix: Panic when running odo list namespaces without an active Kubernetes context #6367

valaparthvi · 2022-12-01T13:33:04Z

What type of PR is this:
/kind bug

What does this PR do / why we need it:

Which issue(s) this PR fixes:

PR acceptance criteria:

Unit test
Integration test
Documentation

How to test changes / Special notes to the reviewer:
odo list namespace no longer panics, instead throws an appropriate error.

KUBECONFIG=/dev/null odo list namespace

$ KUBECONFIG=/dev/null odo list namespace
 ✗  
Please ensure you have an active kubernetes context to your cluster. 
Consult your Kubernetes distribution's documentation for more details.
Error: invalid configuration: no configuration has been provided, try setting KUBERNETES_MASTER environment variable

odo remove binding still works when the cluster is inaccessible. The test for this was failing when a new REMOVE_BINDING dep was not added. Commit f542b22 fixes that.

KUBECONFIG=/dev/null odo remove binding --name something

$ KUBECONFIG=/dev/null odo remove binding --name something
 ✗  Service Binding "something" not found in the devfile. Available Service Bind
ings:

netlify · 2022-12-01T13:33:09Z

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name	Link
🔨 Latest commit	`48bd771`
🔍 Latest deploy log	https://app.netlify.com/sites/odo-docusaurus-preview/deploys/638dcf0b6fea380008fc762b

odo-robot · 2022-12-01T13:36:38Z

Windows Tests (OCP) on commit db79bae finished successfully.
View logs: TXT HTML

odo-robot · 2022-12-01T13:39:27Z

NoCluster Tests on commit db79bae finished successfully.
View logs: TXT HTML

odo-robot · 2022-12-01T13:40:53Z

Unit Tests on commit db79bae finished successfully.
View logs: TXT HTML

rm3l

LGTM, but can you add a simple integration test case for this in cmd_namespace_test?

I think this should be do-able now that we can put a nocluster label on Ginkgo tests. Labelling the test spec with this label should make the spec run without an active Kubernetes context.

odo-robot · 2022-12-01T13:50:28Z

Validate Tests on commit db79bae finished successfully.
View logs: TXT HTML

odo-robot · 2022-12-01T13:53:52Z

Kubernetes Tests on commit db79bae finished successfully.
View logs: TXT HTML

odo-robot · 2022-12-01T13:58:59Z

OpenShift Tests on commit db79bae finished with errors.
View logs: TXT HTML

…tes context

Signed-off-by: Parthvi Vala <[email protected]>

…works even when cluster is inaccessible

valaparthvi · 2022-12-05T06:43:48Z

/retest

feloy · 2022-12-05T07:58:13Z

pkg/odo/cli/remove/binding/binding.go

@@ -86,7 +86,7 @@ func NewCmdBinding(name, fullName string) *cobra.Command {
 		},
 	}
 	bindingCmd.Flags().String(backend.FLAG_NAME, "", "Name of the Binding to create")
-	clientset.Add(bindingCmd, clientset.BINDING, clientset.FILESYSTEM)
+	clientset.Add(bindingCmd, clientset.REMOVE_BINDING, clientset.FILESYSTEM)


I'm not comfortable with this solution. If the problem is to define if KUBERNETES must be nullable or not for a specific command, we need to find a way to specify this directly

Intuitively, I think KUBERNETES and KUBERNETES_NULLABLE should be mutually exclusive, but in cases where both are needed, we could only return an error if KUBERNETES_NULLABLE was not defined. WDYT?

I've tried an alternate solution here which seems to be working fine: 3a88ffe

@valaparthvi Yes, that seems a very good idea to me

valaparthvi · 2022-12-05T10:22:39Z

/retest
cluster setup failure.

Signed-off-by: Parthvi Vala <[email protected]>

sonarqubecloud · 2022-12-05T11:01:08Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.3% Duplication

feloy · 2022-12-05T12:37:12Z

/override OpenShift-Integration-tests/OpenShift-Integration-tests
Flaky e2e test

/go/odo_1/tests/e2escenarios/e2e_test.go:19
  starting with non-empty Directory add Binding
  /go/odo_1/tests/e2escenarios/e2e_test.go:279
    [It] should verify developer workflow of using binding as env in innerloop
    /go/odo_1/tests/e2escenarios/e2e_test.go:321

openshift-ci · 2022-12-05T12:37:16Z

@feloy: Overrode contexts on behalf of feloy: OpenShift-Integration-tests/OpenShift-Integration-tests

In response to this:

/override OpenShift-Integration-tests/OpenShift-Integration-tests
Flaky e2e test

/go/odo_1/tests/e2escenarios/e2e_test.go:19
 starting with non-empty Directory add Binding
 /go/odo_1/tests/e2escenarios/e2e_test.go:279
   [It] should verify developer workflow of using binding as env in innerloop
   /go/odo_1/tests/e2escenarios/e2e_test.go:321

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.

feloy · 2022-12-05T13:31:04Z

/override ci/prow/v4.11-integration-e2e
Tests pass on IBM Cloud

openshift-ci · 2022-12-05T13:31:09Z

@feloy: Overrode contexts on behalf of feloy: ci/prow/v4.11-integration-e2e

In response to this:

/override ci/prow/v4.11-integration-e2e
Tests pass on IBM Cloud

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.

openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Dec 1, 2022

openshift-ci bot requested review from kadel and rm3l December 1, 2022 13:33

rm3l requested changes Dec 1, 2022

View reviewed changes

openshift-ci bot assigned rm3l Dec 1, 2022

valaparthvi closed this Dec 5, 2022

valaparthvi reopened this Dec 5, 2022

valaparthvi added 2 commits December 5, 2022 10:31

Fix: Panic when running odo list namespaces without an active Kuberne…

b6ca740

…tes context

Add integration test for odo list namespaces

c71eb51

Signed-off-by: Parthvi Vala <[email protected]>

valaparthvi force-pushed the 6316-fix-list-namespace branch from 62044cb to c71eb51 Compare December 5, 2022 05:02

Add a new REMOVE_BINDING dependency to ensure remove binding command …

f542b22

…works even when cluster is inaccessible

feloy suggested changes Dec 5, 2022

View reviewed changes

openshift-ci bot assigned feloy Dec 5, 2022

valaparthvi requested a review from feloy December 5, 2022 09:02

valaparthvi force-pushed the 6316-fix-list-namespace branch from 3a88ffe to 7269c0a Compare December 5, 2022 09:56

Revert REMOVE_BINDING change and use an alternate solution

48bd771

Signed-off-by: Parthvi Vala <[email protected]>

valaparthvi force-pushed the 6316-fix-list-namespace branch from 7269c0a to 48bd771 Compare December 5, 2022 10:59

feloy approved these changes Dec 5, 2022

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Dec 5, 2022

openshift-merge-robot merged commit d6371a4 into redhat-developer:main Dec 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Panic when running odo list namespaces without an active Kubernetes context #6367

Fix: Panic when running odo list namespaces without an active Kubernetes context #6367

valaparthvi commented Dec 1, 2022 •

edited

Loading

netlify bot commented Dec 1, 2022 •

edited

Loading

odo-robot bot commented Dec 1, 2022 •

edited

Loading

odo-robot bot commented Dec 1, 2022 •

edited

Loading

odo-robot bot commented Dec 1, 2022 •

edited

Loading

rm3l left a comment

odo-robot bot commented Dec 1, 2022 •

edited

Loading

odo-robot bot commented Dec 1, 2022 •

edited

Loading

odo-robot bot commented Dec 1, 2022 •

edited

Loading

valaparthvi commented Dec 5, 2022

feloy Dec 5, 2022

valaparthvi Dec 5, 2022 •

edited

Loading

feloy Dec 5, 2022

valaparthvi commented Dec 5, 2022

sonarqubecloud bot commented Dec 5, 2022

feloy commented Dec 5, 2022

openshift-ci bot commented Dec 5, 2022

feloy commented Dec 5, 2022

openshift-ci bot commented Dec 5, 2022

Fix: Panic when running odo list namespaces without an active Kubernetes context #6367

Fix: Panic when running odo list namespaces without an active Kubernetes context #6367

Conversation

valaparthvi commented Dec 1, 2022 • edited Loading

netlify bot commented Dec 1, 2022 • edited Loading

✅ Deploy Preview for odo-docusaurus-preview canceled.

odo-robot bot commented Dec 1, 2022 • edited Loading

odo-robot bot commented Dec 1, 2022 • edited Loading

odo-robot bot commented Dec 1, 2022 • edited Loading

rm3l left a comment

Choose a reason for hiding this comment

odo-robot bot commented Dec 1, 2022 • edited Loading

odo-robot bot commented Dec 1, 2022 • edited Loading

odo-robot bot commented Dec 1, 2022 • edited Loading

valaparthvi commented Dec 5, 2022

feloy Dec 5, 2022

Choose a reason for hiding this comment

valaparthvi Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

feloy Dec 5, 2022

Choose a reason for hiding this comment

valaparthvi commented Dec 5, 2022

sonarqubecloud bot commented Dec 5, 2022

feloy commented Dec 5, 2022

openshift-ci bot commented Dec 5, 2022

feloy commented Dec 5, 2022

openshift-ci bot commented Dec 5, 2022

valaparthvi commented Dec 1, 2022 •

edited

Loading

netlify bot commented Dec 1, 2022 •

edited

Loading

odo-robot bot commented Dec 1, 2022 •

edited

Loading

odo-robot bot commented Dec 1, 2022 •

edited

Loading

odo-robot bot commented Dec 1, 2022 •

edited

Loading

odo-robot bot commented Dec 1, 2022 •

edited

Loading

odo-robot bot commented Dec 1, 2022 •

edited

Loading

odo-robot bot commented Dec 1, 2022 •

edited

Loading

valaparthvi Dec 5, 2022 •

edited

Loading