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

List logically bound images #871

Merged
merged 1 commit into from
Nov 19, 2024
Merged

List logically bound images #871

merged 1 commit into from
Nov 19, 2024

Conversation

omertuc
Copy link
Contributor

@omertuc omertuc commented Nov 5, 2024

Solves the second part of #846

The (hidden) image list command now has a --type flag to allow users to list only logical images, only host images, or all images.

Also the command has been adjusted so that it can run even when not booted off of a bootc system. In that case, it will only list logical images. If a user tries to list host images without a booted system, an error will be thrown.

The command also has a --format flag to allow users to choose between a human-readable table format and a JSON format.

Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

This looks sane to me to start!

I suspect though to really meet the goal we need to think about machine-readable output, which raises its own set of questions.

Also should be straightforward to change test-logically-bound-install.nu to cover this right?

@omertuc
Copy link
Contributor Author

omertuc commented Nov 5, 2024

I suspect though to really meet the goal we need to think about machine-readable output, which raises its own set of questions.

I've played around with formats, none of it is final so feel free to suggest changes:

[omer@hal9000 ~]$ sudo bootc image list
 IMAGE TYPE   IMAGE
 host         docker://quay.io/otuchfel/bootc:listss
 logical      quay.io/curl/curl-base:latest
[omer@hal9000 ~]$ sudo bootc image list --help
List fetched images stored in the bootc storage.

Note that these are distinct from images stored via e.g. `podman`.

Usage: bootc image list [OPTIONS]

Options:
      --list-type <LIST_TYPE>
          Type of image to list

          [default: all]

          Possible values:
          - all:     List all images
          - logical: List only logically bound images
          - host:    List only host images

      --list-format <LIST_FORMAT>
          [default: table]

          Possible values:
          - table: Human readable table format
          - json:  JSON format
          - csv:   CSV format

  -h, --help
          Print help (see a summary with '-h')
[omer@hal9000 ~]$ sudo bootc image list --list-format json
[
  {
    "image_type": "Host",
    "image": "docker://quay.io/otuchfel/bootc:listss"
  },
  {
    "image_type": "Logical",
    "image": "quay.io/curl/curl-base:latest"
  }
]
[omer@hal9000 ~]$ sudo bootc image list --list-format csv
image_type,image
Host,docker://quay.io/otuchfel/bootc:listss
Logical,quay.io/curl/curl-base:latest
[omer@hal9000 ~]$ sudo bootc image list --list-format json --list-type logical
[
  {
    "image_type": "Logical",
    "image": "quay.io/curl/curl-base:latest"
  }
]
[omer@hal9000 ~]$ sudo bootc image list --list-format json --list-type logical | jq '.[].image' -r
quay.io/curl/curl-base:latest

@omertuc
Copy link
Contributor Author

omertuc commented Nov 6, 2024

Also should be straightforward to change test-logically-bound-install.nu to cover this right?

Seems so, I'm trying to get at-least one successful run of the tmt tests on a clean main branch, then I'll see how I can extend the .nu script

@cgwalters
Copy link
Collaborator

I don't see the need to have both csv and json, let's pick json.

This overlaps a bit with #522 in that in theory we could expose this via varlink too? But eh, json seems like a sane baseline.

@omertuc
Copy link
Contributor Author

omertuc commented Nov 7, 2024

I don't see the need to have both csv and json, let's pick json.

Removed CSV (it was trivial to add and to remove so no harm done), it was mostly just an example of how we can add formats easily anyway

@omertuc
Copy link
Contributor Author

omertuc commented Nov 7, 2024

Seems so, I'm trying to get at-least one successful run of the tmt tests on a clean main branch, then I'll see how I can extend the .nu script

Extending a bootc image with the following commands seems to be enough to prepare it , once booted on a VM, for cloning the bootc repo and running make test-tmt successfully:

FROM localhost/bootc

RUN dnf install -y --setopt=install_weak_deps=False cargo git genisoimage libvirt python-pip qemu-kvm rsync && \
    dnf clean all && \
    rm -rf /var/cache/dnf /var/log/dnf* /var/log/yum.*

RUN python3 -m pip install testcloud tmt && \
    rm -rf ~/.cache/pip

RUN systemctl enable virtqemud.socket

Is anything like this documented anywhere? Am I even doing it right or am I missing some obvious simpler way?

@cgwalters
Copy link
Collaborator

Is anything like this documented anywhere? Am I even doing it right or am I missing some obvious simpler way?

I run most things from a toolbox container personally where I have this stuff already installed. What you're doing there is "recursive" which is cool, but I am unsure I would have that be a primary target.

Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

A few nits but looking sane to me!

lib/Cargo.toml Outdated Show resolved Hide resolved
lib/src/image.rs Outdated Show resolved Hide resolved
lib/src/image.rs Outdated Show resolved Hide resolved
lib/src/image.rs Outdated Show resolved Hide resolved
lib/src/image.rs Outdated Show resolved Hide resolved
@omertuc omertuc force-pushed the imagelist branch 7 times, most recently from 6cc374d to 916f296 Compare November 9, 2024 01:04
@omertuc omertuc force-pushed the imagelist branch 2 times, most recently from 2b53d68 to 165b676 Compare November 19, 2024 18:56
@omertuc omertuc changed the title DRAFT: List logically bound images List logically bound images Nov 19, 2024
@omertuc omertuc marked this pull request as ready for review November 19, 2024 18:57
@omertuc omertuc requested a review from cgwalters November 19, 2024 18:57
lib/src/image.rs Outdated Show resolved Hide resolved
lib/src/image.rs Show resolved Hide resolved
tests-integration/src/container.rs Outdated Show resolved Hide resolved
lib/src/image.rs Outdated Show resolved Hide resolved
lib/src/image.rs Show resolved Hide resolved
Solves the second part of containers#846

The (hidden) image list command now has a `--type` flag to allow users
to list only logical images, only host images, or all images.

Also the command has been adjusted so that it can run even when not
booted off of a bootc system. In that case, it will only list logical
images. If a user tries to list host images without a booted system, an
error will be thrown.

The command also has a `--format` flag to allow users to choose between
a human-readable table format and a JSON format.

Signed-off-by: Omer Tuchfeld <[email protected]>
}

async fn list_images(list_type: ImageListType) -> Result<Vec<ImageOutput>> {
let rootfs = cap_std::fs::Dir::open_ambient_dir("/", cap_std::ambient_authority())
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is OK as is but generally so far I've been trying to do things like this closer to cli.rs, keeping functions in other places expecting ambient state.

@cgwalters cgwalters enabled auto-merge November 19, 2024 22:11
@cgwalters cgwalters merged commit 139db98 into containers:main Nov 19, 2024
8 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants