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

Provide support in odo version for podman #6806

Closed
mohitsuman opened this issue May 10, 2023 · 7 comments · Fixed by #6913
Closed

Provide support in odo version for podman #6806

mohitsuman opened this issue May 10, 2023 · 7 comments · Fixed by #6913
Assignees
Milestone

Comments

@mohitsuman
Copy link

Currently when we run odo commands on podman, we should also understand if podman is running or not in the user system.

The odo version command can verify that and list down in the podman version (if running). It should send this information as json so the IDE extension can leverage it and make an informed decision beforehand that whether the system has podman started.

@github-actions github-actions bot added the needs-triage Indicates an issue or PR lacks a `triage/*` and requires one. label May 10, 2023
@rm3l
Copy link
Member

rm3l commented Jun 1, 2023

Grooming (2023-06-01)

If Podman does not respond after the timeout (1s currently), it should report that Podman is not available..

@rm3l rm3l removed the needs-triage Indicates an issue or PR lacks a `triage/*` and requires one. label Jun 1, 2023
@rm3l rm3l added this to the v3.12.0 🚀 milestone Jun 14, 2023
@valaparthvi
Copy link
Contributor

Apart from the podman server version, is there any other information you need? @mohitsuman

@rm3l
Copy link
Member

rm3l commented Jun 19, 2023

I was just thinking that we would also need to implement JSON output for odo version, to make it easy for IDE extensions to consume this feature.
I just realized there was no JSON output for odo version:

$ odo version -o json
 ✗  Machine readable output is not yet implemented for this command

@mohitsuman
Copy link
Author

mohitsuman commented Jun 19, 2023

@valaparthvi so the podman server version is shown only if the podman is running, or it also detects from the system whether user has podman binary installed or not ?

And with @rm3l comment, it would be great to have JSON output for odo version.

@valaparthvi
Copy link
Contributor

valaparthvi commented Jun 19, 2023

@rm3l

I was just thinking that we would also need to implement JSON output for odo version, to make it easy for IDE extensions to consume this feature. I just realized there was no JSON output for odo version:

$ odo version -o json
 ✗  Machine readable output is not yet implemented for this command

True. I am working on that. I guess we can print the complete podman Client information that we have, instead of just the version. WDYT?

$ odo version
odo v3.11.0 (0acf1a5af)
Server: https://kubernetes.docker.internal:6443
Kubernetes: v1.25.9
Podman (Client): 4.5.1

$ odo version -o json
{
	"version": "odo v3.11.0 (0acf1a5af)",
	"serverURL": "https://kubernetes.docker.internal:6443",
	"kubernetesVersion": "v1.25.9",
	"openShiftVersion": "",
	"podmanVersion": "4.5.1"
}

@mohitsuman

@valaparthvi so the podman server version is shown only if the podman is running, or it also detects from the system whether user has podman binary installed or not ?

I am still not sure I completely understand the difference between podman client, and server and whether only checking one is sufficient.
Currently we only show Podman Client information, this is the information odo relies on while determining if podman is running on the system. For odo, it makes sense to check the client, because we use podman CLI to run things on podman, I assume that will also be the case for IDE plugins.

TLDR; odo requires podman CLI on the system, so we use client information; if the server is down, odo will error out, returning the error it received from podman client.

There is more information that we can display about Podman in odo version -o json if there is a need for it.

type Version struct {
	APIVersion string
	Version    string
	GoVersion  string
	GitCommit  string
	BuiltTime  string
	Built      int64
	OsArch     string
	Os         string
}
$ podman version
Client:       Podman Engine
Version:      4.5.1
API Version:  4.5.1
Go Version:   go1.20.4
Git Commit:   9eef30051c83f62816a1772a743e5f1271b196d7
Built:        Fri May 26 20:40:12 2023
OS/Arch:      darwin/arm64

[...]

@rm3l
Copy link
Member

rm3l commented Jun 19, 2023

@rm3l

I was just thinking that we would also need to implement JSON output for odo version, to make it easy for IDE extensions to consume this feature. I just realized there was no JSON output for odo version:

$ odo version -o json
 ✗  Machine readable output is not yet implemented for this command

True. I am working on that. I guess we can print the complete podman Client information that we have, instead of just the version. WDYT?

$ odo version
odo v3.11.0 (0acf1a5af)
Server: https://kubernetes.docker.internal:6443
Kubernetes: v1.25.9
Podman (Client): 4.5.1

$ odo version -o json
{
	"version": "odo v3.11.0 (0acf1a5af)",
	"serverURL": "https://kubernetes.docker.internal:6443",
	"kubernetesVersion": "v1.25.9",
	"openShiftVersion": "",
	"podmanVersion": "4.5.1"
}

I'd advocate for returning only the bare minimum for now. Otherwise, it might be difficult to change it later once the fields are there. So only the podman client version for now, I think.
However, I'd suggest structuring the JSON output in a way that allows for future extensibility. For example, something like:

{
  "version": "v3.11.0",
  "gitCommit": "0acf1a5af",
  "cluster": {
    "serverURL": "https://kubernetes.docker.internal:6443",
    "kubernetes": {
      "version": "v1.25.9"
    },
    "openshift": {
      "version": "4.13.0"
    }
  },
  "podman": {
    "client": {
      "version": "4.5.1"
    }
  }
}

This way, I think it will be simpler to extend in the future.. What do you think?

@mohitsuman

@valaparthvi so the podman server version is shown only if the podman is running, or it also detects from the system whether user has podman binary installed or not ?

I am still not sure I completely understand the difference between podman client, and server and whether only checking one is sufficient. Currently we only show Podman Client information, this is the information odo relies on while determining if podman is running on the system. For odo, it makes sense to check the client, because we use podman CLI to run things on podman, I assume that will also be the case for IDE plugins.

TLDR; odo requires podman CLI on the system, so we use client information; if the server is down, odo will error out, returning the error it received from podman client.

I think the client uses a server if there is a "system connection" configured (e.g., case of a Podman Machine started locally or remotely). The client version is not returned by Podman if there is an error connecting to the server. So I think it should be okay for us to only check the client version reported:

$ podman --connection podman-machine-invalid-port version -f json
Error: failed to connect: dial tcp 127.0.0.1:42902: connect: connection refused

Also, if the call to get the client version does not return in a timely manner, odo assumes Podman is not available/ready, so I think that should also be okay for IDE extensions, because odo would anyways error out if we try to run it against Podman.

There is more information that we can display about Podman in odo version -o json if there is a need for it.

type Version struct {
	APIVersion string
	Version    string
	GoVersion  string
	GitCommit  string
	BuiltTime  string
	Built      int64
	OsArch     string
	Os         string
}
$ podman version
Client:       Podman Engine
Version:      4.5.1
API Version:  4.5.1
Go Version:   go1.20.4
Git Commit:   9eef30051c83f62816a1772a743e5f1271b196d7
Built:        Fri May 26 20:40:12 2023
OS/Arch:      darwin/arm64

[...]

+1

@valaparthvi
Copy link
Contributor

I'd advocate for returning only the bare minimum for now. Otherwise, it might be difficult to change it later once the fields are there. So only the podman client version for now, I think.
However, I'd suggest structuring the JSON output in a way that allows for future extensibility. For example, something like:

I like that idea, it's much much better! Let's do that.

@rm3l rm3l moved this to In Progress 🚧 in odo Project Jun 20, 2023
@rm3l rm3l moved this from In Progress 🚧 to In Review 👀 in odo Project Jun 21, 2023
@github-project-automation github-project-automation bot moved this from In Review 👀 to Done ✅ in odo Project Jun 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants