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.

Sign up for GitHub

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

Jump to bottom

podman artifact #25005

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

podman artifact #25005

wants to merge 1 commit into from

Conversation

baude
Copy link
Member

@baude baude commented Jan 13, 2025 •
edited
Loading

the podman artifact verb is used to manage OCI artifacts. the following verbs were added to podman artifact:

  • add
  • inspect
  • ls
  • pull
  • push
  • rm

Notable items with this PR:

  • all artifact commands and their output are subject to change. i.e. consider all of this tech preview
  • there is no way to add a file to an artifact that already exists in the store. you would need to delete and recreate the artifact.
  • all references to artifacts names should be fully qualified names in the form of repo/name:tag (i.e. quay.io/artifact/foobar:latest)
  • i understand that we will likely want to be able to attribute things like arch, etc to artifact files. this function is not available yet.

Many thanks to Paul Holzinger for autocompletion PRs and review PRs that fixed issues early on.

Also fix up some Args function to specify the correct number of args.

Does this PR introduce a user-facing change?

OCI artifacts: add, inspect, list, pull, push, rm for Linux client only.  Remote support and Windows/Mac are forthcoming.

Note: All of these commands are experimental. This includes inputs, outputs, command options, and documentation.

@baude baude added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 13, 2025
@openshift-ci openshift-ci bot added release-note and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jan 13, 2025
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Jan 13, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: baude

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 Jan 13, 2025
@baude baude added the bloat_approved Approve a PR in which binary file size grows by over 50k label Jan 13, 2025
@baude baude force-pushed the artifactrebase branch 2 times, most recently from 3cf0340 to cc7bd96 Compare January 14, 2025 18:43
ashley-cui
ashley-cui reviewed Jan 14, 2025
View reviewed changes
Copy link
Member

@ashley-cui ashley-cui left a comment

Choose a reason for hiding this comment

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

A quick CLI test and I found these. Still need to go through other parts but a few comments to start.

There's also a bunch of spacing issues in the docs, but they can be addressed later if needed.

cmd/podman/artifact/list.go Outdated Show resolved Hide resolved
cmd/podman/artifact/list.go Outdated Show resolved Hide resolved
cmd/podman/artifact/pull.go Outdated Show resolved Hide resolved
cmd/podman/artifact/push.go Outdated Show resolved Hide resolved
cmd/podman/artifact/push.go Outdated Show resolved Hide resolved
docs/source/markdown/podman-artifact-ls.1.md Outdated Show resolved Hide resolved
@baude baude force-pushed the artifactrebase branch 2 times, most recently from ef9b936 to 5577dcf Compare January 15, 2025 18:14
Honny1
Honny1 reviewed Jan 16, 2025
View reviewed changes
docs/source/markdown/podman-artifact-push.1.md Outdated Show resolved Hide resolved
docs/source/markdown/podman-artifact-pull.1.md Outdated Show resolved Hide resolved
cmd/podman/artifact/inspect.go Outdated Show resolved Hide resolved
nalind
nalind reviewed Jan 16, 2025
View reviewed changes
pkg/libartifact/store/store.go Outdated
// indexName is the name of the JSON file in root of the artifact store
// that describes the store's contents
indexName = "index.json"
emptyStanza = []byte("{}")
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 the same as specV1.DescriptorEmptyJSON.Data.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah but i need a []byte representation so I was saving yet another JSON marshal. still worth it?

Copy link
Member

Choose a reason for hiding this comment

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

That Data field is already a JSON object with no fields in it, so you'd be good there.

@baude
podman artifact
15911b8 
the podman artifact verb is used to manage OCI artifacts.  the following
verbs were added to `podman artifact`:

* add
* inspect
* ls
* pull
* push
* rm

Notable items with this PR:

* all artifact commands and their output are subject to change. i.e.
  consider all of this tech preview
* there is no way to add a file to an artifact that already exists in
  the store.  you would need to delete and recreate the artifact.
* all references to artifacts names should be fully qualified names in
  the form of repo/name:tag (i.e. quay.io/artifact/foobar:latest)
* i understand that we will likely want to be able to attribute things
  like arch, etc to artifact files.  this function is not available yet.

Many thanks to Paul Holzinger for autocompletion PRs and review PRs that
fixed issues early on.

Also fix up some Args function to specify the correct number of args.

Signed-off-by: Paul Holzinger <[email protected]>
Signed-off-by: Brent Baude <[email protected]>
nalind
nalind reviewed Jan 17, 2025
View reviewed changes
pkg/libartifact/store/store.go
return nil, err
}
for _, l := range lrs {
imgSrc, err := l.Reference.NewImageSource(ctx, as.SystemContext)
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be Close()d as soon as we're done with it.

@packit-as-a-service Packit-as-a-Service
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashley-cui ashley-cui ashley-cui left review comments

@Honny1 Honny1 Honny1 left review comments

@nalind nalind nalind left review comments

@giuseppe giuseppe giuseppe left review comments

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bloat_approved Approve a PR in which binary file size grows by over 50k release-note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@baude @giuseppe @nalind @Honny1 @ashley-cui