podvm: Use local files for build and reference versions file

[tumberino]

This introduces new Make targets in the root Makefile to create the podvm image.

If doing all from scratch make docker-builder docker-binaries docker-image. Else if binaries and builder already exist for current versions.yaml file then make docker-image will be enough.

I haven't update the rhel or centos versions yet

[stevenhorsman]

Sorry for the basic question - is this supposed to replace the current direct docker approach? If so I think we need to update the podvm README.md doc to reflect this?

[tumberino]

Sorry for the basic question - is this supposed to replace the current direct docker approach? If so I think we need to update the podvm README.md doc to reflect this?

It is a helper rather than a complete replacement, but you're right the docs will need updating too.

[wainersm]

hi @jtumber-ibm , I hope to have some time today or tomorrow to review this. In meanwhile I was trying a different solution to allow caching kata-agent/AA to speed up the CI jobs, I am glad you sent this because I was having a really hard time to solve some cases :)

[wainersm]

@jtumber-ibm how does this PR is related with #1018 ? does this supersedes PR 1018?

[wainersm]

@jtumber-ibm still trying to understand your changes, I just glanced at the code. One question that I have is about f binaries and builder already exist for current versions.yaml file then make docker-image will be enough...

Apart from versions.yaml,it seems that build flags and build scripts themselves that would invalidate a cache. For example, suppose we decide to change ttrc to false in https://github.com/confidential-containers/cloud-api-adaptor/blob/main/podvm/Makefile.inc#L149 while keeping the same version of guest components.

[tumberino]

@jtumber-ibm how does this PR is related with #1018 ? does this supersedes PR 1018?

It does supersede #1018, I'll ask @yoheiueda if it is okay to close #1018

[tumberino]

@jtumber-ibm still trying to understand your changes, I just glanced at the code. One question that I have is about f binaries and builder already exist for current versions.yaml file then make docker-image will be enough...

Apart from versions.yaml,it seems that build flags and build scripts themselves that would invalidate a cache. For example, suppose we decide to change ttrc to false in https://github.com/confidential-containers/cloud-api-adaptor/blob/main/podvm/Makefile.inc#L149 while keeping the same version of guest components.

That is true, and a good point. I don't have a great answer for how to solve it though. Do you have a recommendation for how to track those changes / invalidate the cache when the build changes? My thought was to add a build-version entry into the versions.yaml but that isn't easy to track against the actual changes

[katexochen]

jtumber-ibm still trying to understand your changes, I just glanced at the code. One question that I have is about f binaries and builder already exist for current versions.yaml file then make docker-image will be enough...
Apart from versions.yaml,it seems that build flags and build scripts themselves that would invalidate a cache. For example, suppose we decide to change ttrc to false in https://github.com/confidential-containers/cloud-api-adaptor/blob/main/podvm/Makefile.inc#L149 while keeping the same version of guest components.

That is true, and a good point. I don't have a great answer for how to solve it though. Do you have a recommendation for how to track those changes / invalidate the cache when the build changes? My thought was to add a build-version entry into the versions.yaml but that isn't easy to track against the actual changes

I don't think we should build an entire build system from scratch. If we want such features, we will have to use a real build system like Nix or Bazel.

[wainersm]

jtumber-ibm still trying to understand your changes, I just glanced at the code. One question that I have is about f binaries and builder already exist for current versions.yaml file then make docker-image will be enough...
Apart from versions.yaml,it seems that build flags and build scripts themselves that would invalidate a cache. For example, suppose we decide to change ttrc to false in https://github.com/confidential-containers/cloud-api-adaptor/blob/main/podvm/Makefile.inc#L149 while keeping the same version of guest components.

That is true, and a good point. I don't have a great answer for how to solve it though. Do you have a recommendation for how to track those changes / invalidate the cache when the build changes? My thought was to add a build-version entry into the versions.yaml but that isn't easy to track against the actual changes

I don't think we should build an entire build system from scratch. If we want such features, we will have to use a real build system like Nix or Bazel.

Yes, sure, don't want to re-invent the wheels. I am not sure about Nix, but even with bazel we would still have problems because the main concern is the build inside containers.

okay, back to the problem... ignore it? i.e. assume in a rare situation we might need to change build parameters but not versions, then defer the solution?

If not ignore then I can think of either:

• Add a new property to versions to version the build configuration (e.g. build_version=N) that we can bump in case of we want to force the rebuild for whatever reason.
• Apart from versions.yaml, compute the build files (Makefiles, etc) in the VERSION_HASH. This is the approach used by kata containers, btw.

Both approach are error prone: one might forget to update the build_version. One might forget to include new build files on the computation of the VERSION_HASH.

BTW, @fidencio has recently added (kata-containers/kata-containers#8100) the infra to allow building kata-agent separated from the entire rootfs and, IIUC, the idea is to cache the kata-agent binary just like the other components. In other words, soon we might be able to consume kata-agent binaries... In Fabiano we trust, In fabiano we rest our hopes!

[wainersm]

@jtumber-ibm perhaps one last thing not relate with code properly saying. When we (@bpradipt and I) wrote the first implementation, the idea of pulling the sources from git tree was to enable reproducibility. Switching to copying the local sources to the image we lose reproducibility if I am not mistaken. As a developer I love the simplicity that this solution brings to my life (I can even feel the inner peace on my heart growing...) but I'm not sure how it impacts on peer pods' distributors. Is this (loosing reproducibility) something already discussed on the community? I missed many discussions recently so it might be case....

[katexochen]

When we (bpradipt and I) wrote the first implementation, the idea of pulling the sources from git tree was to enable reproducibility.

@wainersm IMO pulling code from GitHub has nothing to do with reproducibility. You can totally have reproducible builds from a local checkout. If you pull things from remote, you have to pin everything you pull in (by hash!). We never did that, and the way we pulled things, and still pull things, is not reproducible at all. Some examples from current main:

We don't even pin the base image by hash. The meaning of the tag changes multiple times a week:

cloud-api-adaptor/podvm/Dockerfile.podvm_builder

Line 9 in cf07e8a

FROM ubuntu:20.04

We pull CAA from main. If I build two times and you merge a PR in between, I get different results:

cloud-api-adaptor/podvm/Dockerfile.podvm_builder

Line 57 in cf07e8a

ARG CAA_SRC_REF="main"

Kata isn't pinned as well, as is any other of the many tools we are building:

cloud-api-adaptor/podvm/Dockerfile.podvm_builder

Line 60 in cf07e8a

ARG KATA_SRC_BRANCH="CCv0"

Currently, the only reproducible bit in our builds yq (and the stuff we already pinned in versions.yaml):

cloud-api-adaptor/podvm/Dockerfile.podvm_builder

Lines 14 to 15 in cf07e8a

	ARG YQ_VERSION="v4.35.1"
	ARG YQ_CHECKSUM="sha256:bd695a6513f1196aeda17b174a15e9c351843fb1cef5f9be0af170f2dd744f08"

[katexochen]

I am not sure about Nix, but even with bazel we would still have problems because the main concern is the build inside containers.

I don't get that point. You wouldn't use those tools within a container, they come with their own sandbox.

Maybe we continue the discussion in the issue. I was trying to bring up reproducible builds some time ago, but there wasn't much feedback #1035 (comment) (beside the excellent work of @jtumber-ibm to at least implement the versions.yaml thing)

[katexochen]

LGTM. Local build was successful, full build took 38 min.

[stevenhorsman]

I've not had a chance to look through all the code, but I've run it locally with ARCH=s390x make podvm-builder podvm-binaries podvm-image and it created an image at least. I hit one problem on the way and I think we might need to update the documentation of how to build the podvm based on this as an easier way to run it locally. I hope to review it more fully next week. Thanks and sorry for the delays in the review

[stevenhorsman]

Is there an minimum minor version we need as well? I tried this with 4.2 (as I'm lazy and it's the example in the doc: https://github.com/mikefarah/yq#wget) and got:

~/go/src/github.com/confidential-containers/cloud-api-adaptor# make docker-builder docker-binaries docker-image
Error: unknown command ".cloudimg.ubuntu.focal.amd64.url | select(. != null)" for "yq"
Run 'yq --help' for usage.

This went away when I used v4.35.1 like you reference above.

[katexochen]

One of those problems we could solve with a reproducible dev environment provided by Nix...

[tumberino]

mikefarah/yq#1158 (comment)

We could specify eval (e) in our commands instead to ensure we are compatible with all the 4.x minor versions

[surajssd]

What is blocking this PR?

[katexochen]

What is blocking this PR?

It needs another review (and a rebase).

[stevenhorsman]

What is blocking this PR?

I think JT needs to rebase once they are back from vacation and it needs another review, which I intend to do once it's rebased to test the images that are created, and run it on s390x and x86, rather than just built a single one to test out the process. I think some doc updates might be important as well as I mention in my comment last week.

[tumberino]

@surajssd @stevenhorsman rebased and doc update added

[stevenhorsman]

I recreated an s390x podvm based on the documentation and tried it out locally and it seems to pass the tests, so LGTM. Thanks for the updates!