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

odo logs: Do not panic when no access to cluster/podman #6561

Merged

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Feb 1, 2023

What type of PR is this:

/kind bug
What does this PR do / why we need it:

Which issue(s) this PR fixes:

Fixes #6555

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Feb 1, 2023
@netlify
Copy link

netlify bot commented Feb 1, 2023

Deploy Preview for odo-docusaurus-preview ready!

Name Link
🔨 Latest commit 15ffeea
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/63dcfb2809ced100086cc910
😎 Deploy Preview https://deploy-preview-6561--odo-docusaurus-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@feloy feloy removed the request for review from anandrkskd February 1, 2023 17:07
@feloy feloy assigned valaparthvi and unassigned valaparthvi Feb 1, 2023
@feloy feloy changed the title odo logs: Do not panic when no access to cluster/podman [wip] odo logs: Do not panic when no access to cluster/podman Feb 1, 2023
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Feb 1, 2023
@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

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

@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

OpenShift Unauthenticated Tests on commit 843e634 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

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

@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

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

@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

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

@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

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

@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

OpenShift Tests on commit 843e634 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

Kubernetes Docs Tests on commit bd0d2ef finished with errors.
View logs: TXT HTML

@feloy feloy force-pushed the bugfix-6555/odo-logs-panics branch from 5f9f9a6 to 63c7321 Compare February 2, 2023 15:29
@feloy feloy changed the title [wip] odo logs: Do not panic when no access to cluster/podman odo logs: Do not panic when no access to cluster/podman Feb 2, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Feb 2, 2023
Comment on lines 79 to 80
When("podman is not installed", func() {
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am assuming this needs to be removed?

Comment on lines 97 to 101
return errors.New("you need access to a cluster to run this command")
}
case commonflags.PlatformPodman:
if o.clientset.PodmanClient == nil {
return errors.New("you need access to podman to run this command")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rebase this on top of main and use the new kclient.NewNoConnectionError() function?
Same for podman, podman.NewPodmanNotFoundError().

@sonarcloud
Copy link

sonarcloud bot commented Feb 3, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after testing it.
@valaparthvi, since you initially reviewed it, please approve this PR if you are ok with the changes.

@valaparthvi
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Feb 6, 2023
@valaparthvi
Copy link
Member

/override Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests
Flake:


  user-guides/quickstart/docs-mdx/java/java_odo_dev_output.mdx
  Expected
      <string>:   (
        	"""
        	... // 10 identical lines
        	✓  Added storage m2 to component
        	âš   Pod is Pending
      - 	✓  Pod is Running
        	✓  Syncing files into the container [1s]
        	✓  Building your application in container (command: build) [1s]
        	... // 9 identical lines
        	[Ctrl+c] - Exit and delete resources from the cluster
        	[p] - Manually apply local changes to the application on the cluster
      + 	✓  Pod is Running
        	```
        	"""
        )
      
  to be empty
  In [It] at: /go/odo_1/tests/documentation/user-guides/doc_user_guides_quickstart_test.go:256

@openshift-ci
Copy link

openshift-ci bot commented Feb 6, 2023

@valaparthvi: Overrode contexts on behalf of valaparthvi: Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests

In response to this:

/override Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests
Flake:


 user-guides/quickstart/docs-mdx/java/java_odo_dev_output.mdx
 Expected
     <string>:   (
       	"""
       	... // 10 identical lines
       	✓  Added storage m2 to component
       	âš   Pod is Pending
     - 	✓  Pod is Running
       	✓  Syncing files into the container [1s]
       	✓  Building your application in container (command: build) [1s]
       	... // 9 identical lines
       	[Ctrl+c] - Exit and delete resources from the cluster
       	[p] - Manually apply local changes to the application on the cluster
     + 	✓  Pod is Running
       	```
       	"""
       )
     
 to be empty
 In [It] at: /go/odo_1/tests/documentation/user-guides/doc_user_guides_quickstart_test.go:256

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-merge-robot openshift-merge-robot merged commit 704ff2a into redhat-developer:main Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

odo logs panics if there is no Kubeconfig
4 participants