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

Add BuildOrigin field to podman info #24781

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ashley-cui
Copy link
Member

@ashley-cui ashley-cui commented Dec 6, 2024

BuiltOrigin is a field that can be set at build time by packagers. This helps us trace how and where the binary was built and installed from, allowing us to see if the issue is due to a specfic installation or a general podman bug. This field shows up in podman version and in podman info when populated.

Automatically set the BuildOrigin field when building the macOS pkginstaller to pkginstaller.

Usage: make podman-remote BUILD_ORIGIN="mypackaging"

Does this PR introduce a user-facing change?

Packagers can now set the `BUILT_FOR` environment variable when building podman from the makefile. This data shows up in podman version and in podman info, which helps us trace how and where the binary was built and installed from.

Copy link
Contributor

openshift-ci bot commented Dec 6, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ashley-cui

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 6, 2024
@ashley-cui ashley-cui added the No New Tests Allow PR to proceed without adding regression tests label Dec 6, 2024
@ashley-cui
Copy link
Member Author

No new tests, as this is a build-time variable:

acui@Ashleys-Laptop podman % make podman-remote BUILT_FOR=testing

When the VM is running:

acui@Ashleys-Laptop podman % ./bin/darwin/podman version      
Client:       Podman Engine
Version:      5.4.0-dev
API Version:  5.4.0-dev
Go Version:   go1.23.2
Git Commit:   fde039dd88425ff65367dd5bcc00eac74a11e6c0
Built:        Fri Dec  6 00:13:30 2024
Built For:    testing
OS/Arch:      darwin/arm64

Server:       Podman Engine
Version:      5.3.0-dev-da8995658
API Version:  5.3.0-dev-da8995658
Go Version:   go1.22.7
Built:        Mon Nov 11 19:00:00 2024
OS/Arch:      linux/arm64

When the VM is stopped, this is also shown in podman info:

acui@Ashleys-Laptop podman % ./bin/darwin/podman info         
OS: darwin/arm64
builtFor: testing
provider: applehv
version: 5.4.0-dev

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: failed to connect: dial tcp 127.0.0.1:64348: connect: connection refused

We also need to add this to winmake and the windows installer, but that requires re-structuring of winmake, so a followup PR makes more sense.

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.

I know naming is hard and bikeshedding over the name is not so useful but if find BuiltFor a bit confusing. Maybe BuiltName sounds better?

Also we should likely set the same for the windows installer and rpm spec then.

@ashley-cui
Copy link
Member Author

ashley-cui commented Dec 6, 2024

I know naming is hard and bikeshedding over the name is not so useful but if find BuiltFor a bit confusing. Maybe BuiltName sounds better?

I think both end up equally vague? Other options I considered: Build Source, Packager
@baude any preferences?

Also we should likely set the same for the windows installer and rpm spec then.

I'll do that in a followup PR, that requires some changes in winmake. Though if anyone is more familiar with the RPM spec change, I'll likely need help there.

@baude
Copy link
Member

baude commented Dec 6, 2024

I had thought of Origin but maybe that is just as vague as the next one. I am frankly fine with either of the recommendations from @ashley-cui and @Luap99

@ashley-cui
Copy link
Member Author

ashley-cui commented Dec 6, 2024

Changing to origin, since origin seems more specific than name or for

BuiltOrigin is a field that can be set at build time by packagers. This helps us trace how and where the binary was built and installed from, allowing us to see if the issue is due to a specfic installation or a general podman bug. This field shows up in podman version and in podman info when populated.

Automatically set the BuildOrigin field when building the macOS pkginstaller to pkginstaller.

Usage: make podman-remote BUILD_ORIGIN="mypackaging"

Signed-off-by: Ashley Cui <acui@redhat.com>
@ashley-cui ashley-cui changed the title Add BuiltFor field to podman info Add BuildOrigin field to podman info Dec 6, 2024
@ashley-cui
Copy link
Member Author

@baude @Luap99 PTAL again, thanks!

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.

podman info actually shows the remote BUILD_ORIGIN not the local one so it seem pretty much useless for what we need as all the mac reports would show the remote server origin not the brew client...

The local one is only shown when the info remote connection fails.

For podman-remote version the BUILD_ORIGIN output is never shown (only the local podman version shows it). In the remote case it should be able to show the origin for both the server and client.

OSArch string `json:"OS"`
Provider string `json:"provider"`
Version string `json:"version"`
BuildOrigin string `json:"buildOrigin,omitempty" yaml:",omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Why has this a yaml tag as only field?

Copy link
Member Author

Choose a reason for hiding this comment

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

We yaml marshall these on output, and buildOrigin is the only omitempty field we need. For consistency's sake, I can add it to all the fields.

@ashley-cui
Copy link
Member Author

@Luap99

podman info actually shows the remote BUILD_ORIGIN not the local one so it seem pretty much useless for what we need as all the mac reports would show the remote server origin not the brew client...

I'd argue that this is the correct behavior. All the podman info output is service side info, and outputting client info in podman info without adding a specific client field would be misleading. We could add a section for the client, I'd say that would be a breaking change, as we have to update the json to "hostInfo" and "clientInfo". This is why I chose to do it in podman version, since it already has both fields.

For podman-remote version the BUILD_ORIGIN output is never shown (only the local podman version shows it). In the remote case it should be able to show the origin for both the server and client.

For me the build origin output is shown? It should show for both the server and the client, but the server isn't building with the tag yet so we might not see it for in the server version, but this is what the output looks like for me:

acui@Ashleys-Laptop podman % ./bin/darwin/podman version            
Client:        Podman Engine
Version:       5.4.0-dev
API Version:   5.4.0-dev
Go Version:    go1.23.2
Git Commit:    b760b0bcfa4d36db7dc100f4c4f774f66fa31feb
Built:         Mon Dec  9 12:14:36 2024
Build Origin:  asdf
OS/Arch:       darwin/arm64

Server:       Podman Engine
Version:      5.3.0-dev-da8995658
API Version:  5.3.0-dev-da8995658
Go Version:   go1.22.7
Built:        Mon Nov 11 19:00:00 2024
OS/Arch:      linux/arm64

@@ -108,6 +109,7 @@ API Version:\t{{.APIVersion}}
Go Version:\t{{.GoVersion}}
{{if .GitCommit -}}Git Commit:\t{{.GitCommit}}\n{{end -}}
Built:\t{{.BuiltTime}}
{{if .BuildOrigin -}}Built Origin:\t{{.BuildOrigin}}\n{{end -}}
Copy link
Member

Choose a reason for hiding this comment

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

Built intentional, or should it be Build?

@mheon
Copy link
Member

mheon commented Dec 9, 2024

Feels like this should be in the packaging doc, I imagine it's relevant to distro packagers

@Luap99
Copy link
Member

Luap99 commented Dec 10, 2024

@Luap99

podman info actually shows the remote BUILD_ORIGIN not the local one so it seem pretty much useless for what we need as all the mac reports would show the remote server origin not the brew client...

I'd argue that this is the correct behavior. All the podman info output is service side info, and outputting client info in podman info without adding a specific client field would be misleading. We could add a section for the client, I'd say that would be a breaking change, as we have to update the json to "hostInfo" and "clientInfo". This is why I chose to do it in podman version, since it already has both fields.

I get what you mean but then this whole PR is mostly pointless considering the reason why @baude brought this up. Our issue template asks for podman info and if macos installer just says "fedora rpm" or nothing then we are no step closer to knowing if users installed via brew or our own installer or something else...
We can certainly ask for podman version in the template as well then but there we have the same issue that versions just errors without a working connection.

For podman-remote version the BUILD_ORIGIN output is never shown (only the local podman version shows it). In the remote case it should be able to show the origin for both the server and client.

For me the build origin output is shown? It should show for both the server and the client, but the server isn't building with the tag yet so we might not see it for in the server version, but this is what the output looks like for me:

acui@Ashleys-Laptop podman % ./bin/darwin/podman version            
Client:        Podman Engine
Version:       5.4.0-dev
API Version:   5.4.0-dev
Go Version:    go1.23.2
Git Commit:    b760b0bcfa4d36db7dc100f4c4f774f66fa31feb
Built:         Mon Dec  9 12:14:36 2024
Build Origin:  asdf
OS/Arch:       darwin/arm64

Server:       Podman Engine
Version:      5.3.0-dev-da8995658
API Version:  5.3.0-dev-da8995658
Go Version:   go1.22.7
Built:        Mon Nov 11 19:00:00 2024
OS/Arch:      linux/arm64

It doesn't show the BUILD_ORIGIN for the server as we do not have a proper version libpod API endpoint. This is all send via the docker API AFAIK.

This is with podman (same binary running the service) complied with test2 and podman-remote compiled with test1.

$ bin/podman-remote version
Client:        Podman Engine
Version:       5.4.0-dev
API Version:   5.4.0-dev
Go Version:    go1.22.9
Git Commit:    b760b0bcfa4d36db7dc100f4c4f774f66fa31feb
Built:         Tue Dec 10 18:01:14 2024
Build Origin:  test1
OS/Arch:       linux/amd64

Server:       Podman Engine
Version:      5.4.0-dev
API Version:  5.4.0-dev
Go Version:   go1.22.9
Git Commit:   b760b0bcfa4d36db7dc100f4c4f774f66fa31feb
Built:        Tue Dec 10 18:03:21 2024
OS/Arch:      linux/amd64


$ bin/podman version
Client:        Podman Engine
Version:       5.4.0-dev
API Version:   5.4.0-dev
Go Version:    go1.22.9
Git Commit:    b760b0bcfa4d36db7dc100f4c4f774f66fa31feb
Built:         Tue Dec 10 18:03:21 2024
Build Origin:  test2
OS/Arch:       linux/amd64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. No New Tests Allow PR to proceed without adding regression tests release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants