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

Process image manifest if platform not specified #997

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

bcrochet
Copy link
Contributor

@bcrochet bcrochet commented Jun 5, 2023

If --platform is not given on 'check container', preflight will now process an manifest list if that's what the URL points to. If platform is specified, only process that platform. It would be the equivalent of running preflight on each of the images in the manifest, with the 'arch' specified.

Fixes #955

@openshift-ci openshift-ci bot requested review from jomkz and komish June 5, 2023 21:06
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 5, 2023
Copy link
Contributor

@acornett21 acornett21 left a comment

Choose a reason for hiding this comment

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

LGTM...Just one comment needed for clarity.

cmd/preflight/cmd/check_container.go Outdated Show resolved Hide resolved
@bcrochet bcrochet added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 12, 2023
@bcrochet bcrochet marked this pull request as draft June 14, 2023 20:36
@bcrochet
Copy link
Contributor Author

/retest

@tkrishtop
Copy link
Contributor

tkrishtop commented Jul 5, 2023

The error in the DCI job occurred because the JUnit file was not generated by preflight. It seems like preflight did not generate the JUnit file because it did not catch the errors generated by HasNoProhibitedPackages:

time=\"2023-07-05T15:13:27Z\" level=info 
msg=\"check completed\" 
check=HasNoProhibitedPackages 
err=\"unable to get a list of all packages in the image: 
could not get rpm list: stat /tmp/preflight-779142393/fs/var/lib/rpm/Packages: no such file or directory\" 
result=ERROR

@bcrochet bcrochet marked this pull request as ready for review July 6, 2023 00:16
@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 Jul 6, 2023
@openshift-ci openshift-ci bot requested review from komish and skattoju July 6, 2023 00:16
@vikasmulaje
Copy link
Contributor

@bcrochet Can we add Architecture details in log so it will be easy to identify which arch image failed or have issues?
Example run: http://pastebin.test.redhat.com/1105130

@tonyskapunk
Copy link

This failure is because the DCI agent is expecting the results-junit.xml in the /assets/ directory, but this change is now placing them in /assets/<ARCH>/ when --platform is not used.

I could add that in the agent, but I was wondering if would it make sense to infer the arch from the system running the check. The reasoning behind that is that most of the container clients do that when pulling, running, copying a manifest list, unless explicitly requested to do all archs.

Examples:

Thoughts?

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 4, 2023
@tonyskapunk
Copy link

Before merging, would you mind allowing me to run one more test ? I'd like to validate a change to store all the files generated per arch on our side.

@tonyskapunk We are in no rush to merge this right now (soon, but not right now) so test away, we have an internal issue that QE also filed, and they probably won't re-test till tomorrow. Thanks for all the help on this!

At this point we're completed validations on our side, and we will be ready when this change will be merged.

You may see some FAILURE messages coming from @dcibot and that is OK, it all depends how fast we merge our changes so the checked jobs start using them.

Thanks!

@dcibot
Copy link

dcibot commented Aug 4, 2023

Starting dci-check-change job.

@dcibot
Copy link

dcibot commented Aug 4, 2023

@dcibot
Copy link

dcibot commented Aug 4, 2023

Starting dci-check-change job.

@dcibot
Copy link

dcibot commented Aug 4, 2023

@dcibot
Copy link

dcibot commented Aug 4, 2023

Starting dci-check-change job.

@dcibot
Copy link

dcibot commented Aug 4, 2023

  • dci-check-change preparation : FAILURE tested on libvirt:virt

@dcibot
Copy link

dcibot commented Aug 4, 2023

Starting dci-check-change job.

@dcibot
Copy link

dcibot commented Aug 5, 2023

sf-project-io pushed a commit to redhat-cip/dci-openshift-app-agent that referenced this pull request Aug 5, 2023
Current role only copies the artifacts generated by amd64 arch, when
multiple architectures are checked by preflight the other artifacts are
left behind. Now we look recursively for more artifacts and store them
in the logs

Also add globbing support to verify-test role to analyze multiple tests
with the same expected results

build-depends: redhat-openshift-ecosystem/openshift-preflight#997
Change-Id: Ia5baadeca536e8487fccf5d819fbb7a78cebb410
@acornett21
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 7, 2023
@gautamkrishnar
Copy link
Contributor

gautamkrishnar commented Aug 8, 2023

@bcrochet @acornett21 i tried running the code on my machine I got the following message:

>  ./preflight check container quay.io/gakrishn/test-multiarch:1.0 --pyxis-api-token <TOKEN> --certification-project-id <PROJECT-ID> --pyxis-host catalog.qa.redhat.com/api/containers --submit -d /run/user/1000/containers/auth.json
time="2023-08-08T19:14:00-04:00" level=info msg="certification library version" version="0.0.0 <commit: d3d26a96d46f99a89d8882ab7fc93c0337b01f56>"
Error: cannot process image manifest of different arch without platform override
Usage:
  preflight check container [flags]

Examples:
  preflight check container quay.io/repo-name/container-name:version

Flags:
      --certification-project-id string   Certification Project ID from connect.redhat.com/projects/{certification-project-id}/overview
                                          URL paramater. This value may differ from the PID on the overview page. (env: PFLT_CERTIFICATION_PROJECT_ID)
  -h, --help                              help for container
      --insecure                          Use insecure protocol for the registry. Default is False. Cannot be used with submit.
      --offline                           Intended to be used in disconnected environments and will tar artifacts used for submission,
                                          enabling other Red Hat managed tools/process to submit these artifacts at a later time.
                                          Cannot be used with submit.
      --platform string                   Architecture of image to pull. Defaults to runtime platform. (default "amd64")
      --pyxis-api-token string            API token for Pyxis authentication (env: PFLT_PYXIS_API_TOKEN)
      --pyxis-env string                  Env to use for Pyxis submissions. (default "prod")
      --pyxis-host string                 Host to use for Pyxis submissions. This will override Pyxis Env. Only set this if you know what you are doing.
                                          If you do set it, it should include just the host, and the URI path. (env: PFLT_PYXIS_HOST)
  -s, --submit                            submit check container results to Red Hat

Global Flags:
      --artifacts string       Where check-specific artifacts will be written. (env: PFLT_ARTIFACTS)
  -d, --docker-config string   Path to docker config.json file. This value is optional for publicly accessible images.
                               However, it is strongly encouraged for public Docker Hub images,
                               due to the rate limit imposed for unauthenticated requests. (env: PFLT_DOCKERCONFIG)
      --logfile string         Where the execution logfile will be written. (env: PFLT_LOGFILE)
      --loglevel string        The verbosity of the preflight tool itself. Ex. warn, debug, trace, info, error. (env: PFLT_LOGLEVEL)

2023/08/08 19:14:03 cannot process image manifest of different arch without platform override

Image is a multi-arch image:

> skopeo inspect docker://quay.io/gakrishn/test-multiarch:1.0
{
    "Name": "quay.io/gakrishn/test-multiarch",
    "Digest": "sha256:0080bfa42af0d2c7162585a3c3271e52883e1c044d6f750ed2a7aa6650e58438",
    "RepoTags": [
        "1.0"
    ],
    "Created": "2023-08-08T22:35:39.652341334Z",
    "DockerVersion": "",
    "Labels": {
        "architecture": "aarch64",
        "build-date": "2023-07-26T14:47:43",
        "com.redhat.component": "ubi9-container",
        "com.redhat.license_terms": "https://www.redhat.com/en/about/red-hat-end-user-license-agreements#UBI",
        "description": "a different sample description1",
        "distribution-scope": "public",
        "io.buildah.version": "1.26.2",
        "io.k8s.description": "The Universal Base Image is designed and engineered to be the base layer for all of your containerized applications, middleware and utilities. This base image is freely redistributable, but Red Hat only supports Red Hat technologies through subscriptions for Red Hat products. This image is maintained by Red Hat and updated regularly.",
        "io.k8s.display-name": "Red Hat Universal Base Image 9",
        "io.openshift.expose-services": "",
        "io.openshift.tags": "base rhel9",
        "maintainer": "Red Hat, Inc.",
        "name": "Gakrishn Test Image",
        "release": "1",
        "run": "docker run -d -p 8080:80 --name=happyweb happywebserver",
        "summary": "something different",
        "url": "https://access.redhat.com/containers/#/registry.access.redhat.com/ubi9/images/9.2-722",
        "vcs-ref": "6b5892a11894993e819f9a93ee1d7aaa80dc3a17",
        "vcs-type": "git",
        "vendor": "Redhat Test",
        "version": "1.0"
    },
    "Architecture": "arm64",
    "Os": "linux",
    "Layers": [
        "sha256:f35f46a0bf419d695399facd3df7e6384acc43ba9e099e3e5fb16882c8f65a50",
        "sha256:d3821f31c6974b008ec4cd98e661ecfa190b637176053478f17f0e19d8e39ed4",
        "sha256:952e27d9b3ced91c518d288d119bb0712c658780b29cc4d8293afdb91f9feec9",
        "sha256:26c6437aad911e534104a5ec17cd61aa3aec49bdfd833b9c7f99c1b97aaa3a90",
        "sha256:b58f1a444f7999661cfbded796a7548d4b637b73fec0cc524446a88e75b6164d",
        "sha256:40142f722cca2a4da05b0ba6547d3e9419192bcfcc575924d476110587e58eeb",
        "sha256:ae9ca8a1929c9894bd13642a8d495bafb72fe6ccb6965553b29b1d7dd2550513",
        "sha256:677ec9617679ca7668bc8989e4b3f3289f779ad869c22b867d0053db48632174"
    ],
    "Env": [
        "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
        "container=oci"
    ]
}

@bcrochet
Copy link
Contributor Author

bcrochet commented Aug 9, 2023

❯ crane manifest quay.io/gakrishn/test-multiarch:1.0 | jq .
{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.manifest.v1+json",
  "config": {
    "mediaType": "application/vnd.oci.image.config.v1+json",
    "digest": "sha256:0b1e5207c31e005c1d986bb7466792bfd58c166bc2682241aae62740d77fbcd2",
    "size": 9575
  },
<snip>

That is not a multiarch manifest. That is a single arch manifest, and I'm guessing that since it is arm64, that it is also not the same as your runtime platform. Therefore, --platform override would be warranted here. That means it's working as designed.

@gautamkrishnar
Copy link
Contributor

@bcrochet thanks for letting me know. I had retested it using the right commands its working fine.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 9, 2023
If --platform is not given on 'check container', preflight will now
process an manifest list if that's what the URL points to. If platform
is specified, only process that platform. It will not do anything
different yet for submission. It would be the equivalent of running
preflight on each of the images in the manifest, with the 'arch'
specified.

This fixes the reported issue. However, more will be implemented when
Pyxis and all other systems are ready to assosciate the images with
an actual manifest list.

Fixes redhat-openshift-ecosystem#955

Signed-off-by: Brad P. Crochet <[email protected]>
@dcibot
Copy link

dcibot commented Aug 9, 2023

@acornett21
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 9, 2023
@acornett21 acornett21 merged commit fe9d181 into redhat-openshift-ecosystem:main Aug 16, 2023
1 check passed
nocturnalastro pushed a commit to nocturnalastro/ansible-collection-redhatci-ocp that referenced this pull request Nov 9, 2023
- Execution outputs are now based on architectures: amd64,  arm64,  ppc64le, s390x. So, processing artifacts from amd64 directory
- At this time we are only processing amd64 in the results, we will need to add the proper post validations for other archs in a separate change

build-depends: redhat-openshift-ecosystem/openshift-preflight#997

Change-Id: Icf34ad6764a0a6cbf161c00ef0f932abd62d60ae
nocturnalastro pushed a commit to nocturnalastro/ansible-collection-redhatci-ocp that referenced this pull request Nov 9, 2023
Current role only copies the artifacts generated by amd64 arch, when
multiple architectures are checked by preflight the other artifacts are
left behind. Now we look recursively for more artifacts and store them
in the logs

Also add globbing support to verify-test role to analyze multiple tests
with the same expected results

build-depends: redhat-openshift-ecosystem/openshift-preflight#997
Change-Id: Ia5baadeca536e8487fccf5d819fbb7a78cebb410
@coveralls
Copy link

Coverage Status

coverage: 72.393% (-9.0%) from 81.388%
when pulling d781c79 on bcrochet:manifest-list
into 089e1b9 on redhat-openshift-ecosystem:main.

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. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support running preflight with manifest list
9 participants