Skip to content

Log error when podman client cannot be initialized#6538

Merged
openshift-merge-robot merged 11 commits intoredhat-developer:mainfrom
valaparthvi:podman-log
Feb 3, 2023
Merged

Log error when podman client cannot be initialized#6538
openshift-merge-robot merged 11 commits intoredhat-developer:mainfrom
valaparthvi:podman-log

Conversation

@valaparthvi
Copy link
Contributor

@valaparthvi valaparthvi commented Jan 25, 2023

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

What type of PR is this:
/kind bug

What does this PR do / why we need it:

Which issue(s) this PR fixes:

Fixes #6501

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

  1. export PODMAN_CMD=echo
  2. odo dev --platform podman -v 2

@netlify
Copy link

netlify bot commented Jan 25, 2023 •
edited
Loading

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit 3b22e20
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/63dc9e7908287d00088dfe24

@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Jan 25, 2023
@openshift-ci openshift-ci bot requested review from kadel and rm3l January 25, 2023 12:06
@odo-robot
Copy link

odo-robot bot commented Jan 25, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Jan 25, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Jan 25, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Jan 25, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Jan 25, 2023 •
edited
Loading

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

@valaparthvi valaparthvi changed the title Log error when podman client cannot be initialized WIP: Log error when podman client cannot be initialized Jan 26, 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 Jan 26, 2023
@odo-robot
Copy link

odo-robot bot commented Jan 26, 2023 •
edited
Loading

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

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Jan 27, 2023
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Jan 31, 2023
@valaparthvi valaparthvi changed the title WIP: Log error when podman client cannot be initialized Log error when podman client cannot be initialized Jan 31, 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 Jan 31, 2023
@odo-robot
Copy link

odo-robot bot commented Jan 31, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Jan 31, 2023 •
edited
Loading

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

rm3l
rm3l requested changes Jan 31, 2023
View reviewed changes
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.

Another point: I find the error message (executable "echo" not found) to be misleading for the end-user and might not be accurate. The echo executable does exist in my PATH, but the output of the command is not the one expected by our Podman Client.

$ ODO_EXPERIMENTAL_MODE=t PODMAN_CMD=echo odo describe component --platform podman -v 3     
I0131 17:30:43.309683 3703568 version.go:28] executing [echo version --format json]
 ✗  unable to access podman. Do you have podman client installed? Cause: executable "echo" not found

For example, if I try to reproduce the steps provided in #6501 using a real Podman binary in my PATH:

$ which podman-v3     
/home/asoro/.local/bin/podman-v3

$ ODO_EXPERIMENTAL_MODE=t PODMAN_CMD=podman-v3 odo describe component --platform podman -v 3
I0131 17:31:21.622000 3705354 version.go:28] executing [podman-v3 version --format json]
 ✗  unable to access podman. Do you have podman client installed? Cause: executable "podman-v3" not found

But if I run the same Podman command to retrieve the version:

$ podman-v3 version --format json
Cannot connect to Podman. Please verify your connection to the Linux system using `podman system connection list`, or try `podman machine init` and `podman machine start` to manage a new Linux VM
Error: unable to connect to Podman socket: Get "http://d/v3.4.4/libpod/_ping": dial unix ///run/user/1000/podman/podman.sock: connect: no such file or directory

I think odo should display the full command output if there is an error with the command executed. What do you think?

rm3l
rm3l reviewed Feb 1, 2023
View reviewed changes
Copy link
Member

@rm3l rm3l left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think you raised good points, but I think they might be out of scope for this PR. The errors "executable %q not found" is raised when you initiliaze a new Podman Client. This PR is more about logging error when there is one. I'd be happy to work on it in a separate PR.

Okay, we can do that in a separate PR. But in this case, I'd leave the original issue (#6501) open, as it was more about odo reporting a wrong (and unhelpful) error.
We can say that this PR relates to #6501, but, to me, it does not close it yet.

@valaparthvi
Copy link
Contributor Author

valaparthvi commented Feb 1, 2023

/override Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests
/override Kubernetes-Integration-Tests/Kubernetes-Integration-Tests
Unrelated errors.

@openshift-ci
Copy link

openshift-ci bot commented Feb 1, 2023

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

Details

In response to this:

/override Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests
/override Kubernetes-Integration-Tests/Kubernetes-Integration-Tests
Unrelated errors.

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.

@valaparthvi valaparthvi requested a review from rm3l February 1, 2023 13:00
@valaparthvi
Copy link
Contributor Author

valaparthvi commented Feb 1, 2023

I think you raised good points, but I think they might be out of scope for this PR. The errors "executable %q not found" is raised when you initiliaze a new Podman Client. This PR is more about logging error when there is one. I'd be happy to work on it in a separate PR.

Okay, we can do that in a separate PR. But in this case, I'd leave the original issue (#6501) open, as it was more about odo reporting a wrong (and unhelpful) error. We can say that this PR relates to #6501, but, to me, it does not close it yet.

I've updated the NewPodmanCli to return the actual error. But, this will still not work for odo describe component where it logs warning because it simply checks if the podmanClient is initialized.

rm3l
rm3l reviewed Feb 2, 2023
View reviewed changes
rm3l
rm3l reviewed Feb 2, 2023
View reviewed changes
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.

Added a few comments.

valaparthvi and others added 3 commits February 3, 2023 10:37
@valaparthvi @rm3l
Update pkg/podman/podman.go
cff630c 
Co-authored-by: Armel Soro <[email protected]>
@valaparthvi @rm3l
Update pkg/podman/errors.go
5d246ff 
Co-authored-by: Armel Soro <[email protected]>
@valaparthvi
Fix integration test
3b22e20 
Signed-off-by: Parthvi Vala <[email protected]>
@sonarqubecloud
Copy link

sonarqubecloud 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 3 Code Smells

No Coverage information No Coverage information
0.3% 0.3% Duplication

@valaparthvi valaparthvi requested a review from rm3l February 3, 2023 05:52
rm3l
rm3l approved these changes Feb 3, 2023
View reviewed changes
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.

Thanks Parthvi!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Feb 3, 2023
@openshift-merge-robot openshift-merge-robot merged commit d1fa996 into redhat-developer:main Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rm3l rm3l rm3l approved these changes

@kadel kadel Awaiting requested review from kadel

Assignees

@rm3l rm3l

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

odo experimental feature --run-on podman fails with wrong error saying 'unable to access podman'

3 participants

@valaparthvi @rm3l @openshift-merge-robot