search

confidential-containers / cloud-api-adaptor

Ability to create Kata pods using cloud provider APIs aka the peer-pods approach
Apache License 2.0
44 stars 71 forks source link

cloud-api-adaptor: Support podman in Build.sh #1798

Open davidhadas opened 2 months ago

davidhadas commented 2 months ago

Update build.sh to support podman podman does not support the --push flag

davidhadas commented 2 months ago

Please test with docker before merging. Was tested with podman.

beraldoleal commented 2 months ago

Please, do not merge this PR before 0.8.2 release is cut.

davidhadas commented 2 months ago

Hi @davidhadas this lgtm, but we still have one test failing:

https://github.com/confidential-containers/cloud-api-adaptor/actions/runs/8900345375/job/24441704047?pr=1798#step:10:123

Do you mind taking a look?

Can we run the tests again to see if this is a transient test issue or somehow related to the change we made? For some reason I do not see a way to do that.

beraldoleal commented 2 months ago

Hi @davidhadas this lgtm, but we still have one test failing: https://github.com/confidential-containers/cloud-api-adaptor/actions/runs/8900345375/job/24441704047?pr=1798#step:10:123 Do you mind taking a look?

Can we run the tests again to see if this is a transient test issue or somehow related to the change we made? For some reason I do not see a way to do that.

It seems to be related:

~/work/cloud-api-adaptor/cloud-api-adaptor/src ~/work/cloud-api-adaptor/cloud-api-adaptor/src/cloud-api-adaptor
ERROR: docker exporter does not currently support exporting manifest lists

In any case I re triggered the failed jobs.

stevenhorsman commented 2 months ago

Yes - I seems related as the PR adds in the --load command that seems linked to the error: https://github.com/docker/buildx/issues/59

davidhadas commented 2 months ago

Does it make sense to keep the changes for single arch and revert it to the multi-arch? Can I assume that multi-arch is typically done for release purposes? If so, this can allow developers use podman and at the same time will still require that docker be used instead when doing a release. (Until such time that an equivalent podman support of --push is added)

If this does not make sense, maybe we should drop this PR for now.

stevenhorsman commented 2 months ago

Does it make sense to keep the changes for single arch and revert it to the multi-arch? Can I assume that multi-arch is typically done for release purposes? If so, this can allow developers use podman and at the same time will still require that docker be used instead when doing a release. (Until such time that an equivalent podman support of --push is added)

I think it is fair to say that developers are most likely to build single arch (an example case in point being that I did exactly that when I tested this) and the CI process does multi-arch. I'm personally a little bit reluctant to add complexity in adding two different ways, but if it can be done fairly simply then I'd be open to it.

davidhadas commented 2 months ago

Tests still fail - not sure why.

stevenhorsman commented 2 months ago

Tests still fail - not sure why.

I was able to reproduce this locally using your branch:

# cd src/cloud-api-adaptor && ARCHES=linux/amd64,linux/s390x,linux/ppc64le RELEASE_BUILD=false DEV_TAGS=test make image registry=quay.io/stevenhorsman
bash: line 1: go: command not found
COMMIT=ae85b24817aca5aae31dadf83f99bd64d9ad7e68 VERSION=v0.8.1-alpha.1-dev YQ_VERSION=v4.35.1 YQ_CHECKSUM="sha256:bd695a6513f1196aeda17b174a15e9c351843fb1cef5f9be0af170f2dd744f08" hack/build.sh -i
~/go/src/github.com/confidential-containers/cloud-api-adaptor/src ~/go/src/github.com/confidential-containers/cloud-api-adaptor/src/cloud-api-adaptor
[+] Building 0.0s (0/0)                                                                                                             docker:default
ERROR: docker exporter does not currently support exporting manifest lists
make: *** [Makefile:113: image] Error 1
davidhadas commented 2 months ago

I was able to reproduce this locally using your branch:

# cd src/cloud-api-adaptor && ARCHES=linux/amd64,linux/s390x,linux/ppc64le RELEASE_BUILD=false DEV_TAGS=test make image registry=quay.io/stevenhorsman
bash: line 1: go: command not found
COMMIT=ae85b24817aca5aae31dadf83f99bd64d9ad7e68 VERSION=v0.8.1-alpha.1-dev YQ_VERSION=v4.35.1 YQ_CHECKSUM="sha256:bd695a6513f1196aeda17b174a15e9c351843fb1cef5f9be0af170f2dd744f08" hack/build.sh -i
~/go/src/github.com/confidential-containers/cloud-api-adaptor/src ~/go/src/github.com/confidential-containers/cloud-api-adaptor/src/cloud-api-adaptor
[+] Building 0.0s (0/0)                                                                                                             docker:default
ERROR: docker exporter does not currently support exporting manifest lists
make: *** [Makefile:113: image] Error 1

Should you not be using make image-with-arch ?

davidhadas commented 2 months ago

Somehow the CI tests use hack/build.sh -i (i.e. make image) even when building multiple arch images.

This results in multi arch reaching the build_caa_payload_image() function which can only handle a single image (as it is using --load).

stevenhorsman commented 2 months ago

The e2e_run_all workflow uses caa_build_and_push.yaml: https://github.com/confidential-containers/cloud-api-adaptor/blob/0db87d03d6887884d2c27a0b86dccc1a51bfa8f7/.github/workflows/e2e_run_all.yaml#L83 which runs make image whereas the publish image on push and release workflows use the multi-arch version I think (which I think for performance reasons builds all the images for a single arch and then makes a manifest for them at the end. I don't know why it is using different code, it might be possible to unify it.

davidhadas commented 2 months ago

/hold till we figure this out.