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

libpod: Podman info output more network information #18383

Merged
merged 1 commit into from
Jun 13, 2023

Conversation

sstosh
Copy link
Contributor

@sstosh sstosh commented Apr 28, 2023

podman info prints the network information about binary path,
package version, program version and DNS information.

Fixes: #18443

cni info
  networkBackendInfo:
    backend: cni
    dns:
      package: podman-plugins-4.5.0-1.fc38.x86_64
      path: /usr/libexec/cni/dnsname
      version: |-
        CNI dnsname plugin
        version: 1.3.1
        commit: unknown
    package: |-
      containernetworking-plugins-1.1.1-16.fc38.x86_64
      podman-plugins-4.5.0-1.fc38.x86_64
    path: /usr/libexec/cni
netavark info
  networkBackendInfo:
    backend: netavark
    dns:
      package: aardvark-dns-1.6.0-1.fc38.x86_64
      path: /usr/libexec/podman/aardvark-dns
      version: aardvark-dns 1.6.0
    package: netavark-1.6.0-2.fc38.x86_64
    path: /usr/libexec/podman/netavark
    version: netavark 1.6.0

Does this PR introduce a user-facing change?

Now podman info prints the network information about binary path, package version, program version and DNS information.

@openshift-ci openshift-ci bot added release-note do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Apr 28, 2023
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 28, 2023
@sstosh sstosh force-pushed the info-networkbackend branch from 0c3fcc7 to de54f2e Compare April 28, 2023 07:57
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, I really like the idea. However I think it would be much better to move the logic directly into the networkBackend in c/common and then add a function there called Info() to return the info you want. Alo I would rather have a proper struct with the info then a generic map.

You can move packageVersion and programVersion to some c/common util package if needed.

LogDriver string `json:"logDriver"`
MemFree int64 `json:"memFree"`
MemTotal int64 `json:"memTotal"`
NetworkBackend map[string]interface{} `json:"networkBackend"`
Copy link
Member

Choose a reason for hiding this comment

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

This is a breaking change and thus cannot be made, I suggest adding a new NetworkBackendInfo field.

@flouthoc
Copy link
Collaborator

flouthoc commented May 3, 2023

Just adding a note but totally upto @sstosh , once we have proper implementation in c/common as discussed above, I think PR could be easily extended to also include aardvark-dns details. Then this will close: #18443

@baude
Copy link
Member

baude commented May 8, 2023

@sstosh what say you?

@sstosh
Copy link
Contributor Author

sstosh commented May 9, 2023

Just adding a note but totally upto @sstosh , once we have proper implementation in c/common as discussed above, I think PR could be easily extended to also include aardvark-dns details. Then this will close: #18443

@flouthoc OK. I also will try to add the feature to show the details of aardvark-dns.

@sstosh sstosh force-pushed the info-networkbackend branch from de54f2e to e72699c Compare May 9, 2023 09:30
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 9, 2023
@sstosh sstosh force-pushed the info-networkbackend branch from e72699c to 2d84e30 Compare May 9, 2023 09:47
@sstosh sstosh changed the title [WIP] Podman info output more netavark information [WIP] libpod: Podman info output more network information May 9, 2023
@sstosh
Copy link
Contributor Author

sstosh commented May 9, 2023

The output is as shown below:

  networkBackend: netavark
  networkBackendInfo:
    aardvark-package: aardvark-dns-1.6.0-1.fc37.x86_64
    aardvark-path: /usr/libexec/podman/aardvark-dns
    aardvark-version: aardvark-dns 1.6.0
    netavark-package: netavark-1.6.0-2.fc37.x86_64
    netavark-path: /usr/libexec/podman/netavark
    netavark-version: netavark 1.6.0
  networkBackend: cni
  networkBackendInfo: null

@flouthoc
Copy link
Collaborator

@sstosh Thank you 😄 output LGTM

@sstosh sstosh force-pushed the info-networkbackend branch 5 times, most recently from 8e44954 to ea5a7e3 Compare May 16, 2023 05:24
@Luap99
Copy link
Member

Luap99 commented Jun 12, 2023

@sstosh Can you rebase, the new c/common is merged into main.

@sstosh sstosh force-pushed the info-networkbackend branch from ea5a7e3 to 3303fc6 Compare June 13, 2023 00:23
@sstosh sstosh changed the title [WIP] libpod: Podman info output more network information libpod: Podman info output more network information Jun 13, 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. label Jun 13, 2023
@sstosh sstosh force-pushed the info-networkbackend branch 3 times, most recently from 1b215e9 to 11f7f0a Compare June 13, 2023 02:09
@sstosh sstosh force-pushed the info-networkbackend branch from 11f7f0a to 6f82163 Compare June 13, 2023 02:18
podman info prints the network information about binary path,
package version, program version and DNS information.

Fixes: containers#18443

Signed-off-by: Toshiki Sonoda <[email protected]>
@sstosh
Copy link
Contributor Author

sstosh commented Jun 13, 2023

The review is ready.

Copy link
Collaborator

@flouthoc flouthoc left a comment

Choose a reason for hiding this comment

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

LGTM Thanks @sstosh

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 13, 2023
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 13, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, Luap99, sstosh

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:

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

@Luap99
Copy link
Member

Luap99 commented Jun 13, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 13, 2023
@openshift-merge-robot openshift-merge-robot merged commit 8d0fcd4 into containers:main Jun 13, 2023
@sstosh sstosh deleted the info-networkbackend branch June 13, 2023 08:55
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 12, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

podman info must return netavark and aardvark-dns version.
5 participants