Merge pull request #506 from amazonlinux/docker-build

use Docker instead of BuildKit

.dockerignore

@@ -1,3 +1,4 @@
+/.git
 /build/*.img
 /build/*.lz4
 /build/*.tar

Dockerfile

@@ -15,10 +15,18 @@ RUN dnf -y groupinstall "C Development Tools and Libraries" \
 FROM origin AS util
 RUN dnf -y install createrepo_c e2fsprogs gdisk grub2-tools kpartx lz4 veritysetup dosfstools mtools
 
+# The experimental cache mount type doesn't expand arguments, so our choices are limited.
+# We can either reuse the same cache for all builds, which triggers overlayfs errors if
+# the builds run in parallel, or we can use a new cache for each build, which defeats the
+# purpose. We work around the limitation by materializing a per-build stage that can be
+# used as the source of the cache.
 FROM scratch AS cache
 ARG PACKAGE
 ARG ARCH
-COPY .dockerignore .${PACKAGE}.${ARCH}
+# We can't create directories via RUN in a scratch container, so take an existing one.
+COPY --chown=1000:1000 --from=base /tmp /cache
+# Ensure the ARG variables are used in the layer to prevent reuse by other builds.
+COPY --chown=1000:1000 .dockerignore /cache/.${PACKAGE}.${ARCH}
 
 FROM base AS rpmbuild
 ARG PACKAGE
@@ -55,12 +63,12 @@ RUN --mount=target=/host \
 
 USER builder
 RUN --mount=source=.cargo,target=/home/builder/.cargo \
-    --mount=type=cache,target=/home/builder/.cache,uid=1000,from=cache \
+    --mount=type=cache,target=/home/builder/.cache,from=cache,source=/cache \
     --mount=source=workspaces,target=/home/builder/rpmbuild/BUILD/workspaces \
     rpmbuild -ba --clean rpmbuild/SPECS/${PACKAGE}.spec
 
 FROM scratch AS rpm
-COPY --from=rpmbuild /home/builder/rpmbuild/RPMS/*/*.rpm /
+COPY --from=rpmbuild /home/builder/rpmbuild/RPMS/*/*.rpm /output/
 
 FROM util AS imgbuild
 ARG PACKAGES
@@ -96,4 +104,4 @@ RUN --mount=target=/host \
     && echo ${NOCACHE}
 
 FROM scratch AS image
-COPY --from=imgbuild /local/output/* /
+COPY --from=imgbuild /local/output/* /output/

INSTALL.md

@@ -21,25 +21,15 @@ To get these, run:
 cargo install cargo-make cargo-deny
 ```
 
-#### BuildKit
+#### Docker
 
-Thar uses [BuildKit](https://github.com/moby/buildkit) to orchestrate package and image builds.
-In turn, BuildKit uses [Docker](https://docs.docker.com/install/#supported-platforms) to run individual builds.
+Thar uses [Docker](https://docs.docker.com/install/#supported-platforms) to orchestrate package and image builds.
 
-You'll need to have Docker installed and running, but you don't need to install BuildKit.
-To start BuildKit as a Docker container, run:
+We recommend Docker 19.03 or later.
+Builds rely on Docker's integrated BuildKit support, which has received many fixes and improvements in newer versions.
 
-```
-docker run -t --rm \
-   --privileged \
-   --network=host \
-   --volume /var/run/docker.sock:/var/run/docker.sock:ro \
-   moby/buildkit:v0.6.2 \
-   --addr tcp://127.0.0.1:1234 \
-   --oci-worker true
-```
-
-You can run that in the background, or just interrupt the process after BuildKit says it's running - the important part will keep running in the background.
+You'll need to have Docker installed and running, with your user account added to the `docker` group.
+Docker's [post-installation steps for Linux](https://docs.docker.com/install/linux/linux-postinstall/) will walk you through that.
 
 ### Build process
 

Makefile.toml

@@ -6,13 +6,12 @@ BUILDSYS_ARCH = { script = ["uname -m"] }
 BUILDSYS_ROOT_DIR = "${CARGO_MAKE_WORKING_DIRECTORY}"
 BUILDSYS_OUTPUT_DIR = "${BUILDSYS_ROOT_DIR}/build"
 BUILDSYS_SOURCES_DIR = "${BUILDSYS_ROOT_DIR}/workspaces"
-BUILDSYS_BUILDKIT_CLIENT = "moby/buildkit:v0.6.2"
-BUILDSYS_BUILDKIT_SERVER = "tcp://127.0.0.1:1234"
 BUILDSYS_TIMESTAMP = { script = ["date +%s"] }
 BUILDSYS_VERSION = { script = ["git describe --tag --dirty || date +%Y%m%d"] }
 CARGO_HOME = "${BUILDSYS_ROOT_DIR}/.cargo"
 CARGO_MAKE_CARGO_ARGS = "--jobs 8 --offline --locked"
 GO_MOD_CACHE = "${BUILDSYS_ROOT_DIR}/.gomodcache"
+DOCKER_BUILDKIT = "1"
 
 [env.development]
 IMAGE = "aws-k8s"

tools/buildsys/src/builder.rs

@@ -1,38 +1,40 @@
 /*!
-This module handles the calls to the BuildKit server needed to execute package
-and image builds. The actual build steps and the expected parameters are defined
-in the repository's top-level Dockerfile.
+This module handles the calls to Docker needed to execute package and image
+builds. The actual build steps and the expected parameters are defined in
+the repository's top-level Dockerfile.
 
 */
 pub(crate) mod error;
 use error::Result;
 
 use duct::cmd;
 use rand::Rng;
+use sha2::{Digest, Sha512};
 use snafu::ResultExt;
 use std::env;
 use std::process::Output;
-use users::get_effective_uid;
 
 pub(crate) struct PackageBuilder;
 
 impl PackageBuilder {
-    /// Call `buildctl` to produce RPMs for the specified package.
+    /// Build RPMs for the specified package.
     pub(crate) fn build(package: &str) -> Result<(Self)> {
         let arch = getenv("BUILDSYS_ARCH")?;
-        let opts = format!(
-            "--opt target=rpm \
-             --opt build-arg:PACKAGE={package} \
-             --opt build-arg:ARCH={arch}",
+
+        let target = "rpm";
+        let build_args = format!(
+            "--build-arg PACKAGE={package} \
+             --build-arg ARCH={arch}",
             package = package,
             arch = arch,
         );
+        let tag = format!(
+            "buildsys-pkg-{package}-{arch}",
+            package = package,
+            arch = arch
+        );
 
-        let result = buildctl(&opts)?;
-        if !result.status.success() {
-            let output = String::from_utf8_lossy(&result.stdout);
-            return error::PackageBuild { package, output }.fail();
-        }
+        build(&target, &build_args, &tag)?;
 
         Ok(Self)
     }
@@ -41,103 +43,121 @@ impl PackageBuilder {
 pub(crate) struct ImageBuilder;
 
 impl ImageBuilder {
-    /// Call `buildctl` to create an image with the specified packages installed.
+    /// Build an image with the specified packages installed.
     pub(crate) fn build(packages: &[String]) -> Result<(Self)> {
         // We want PACKAGES to be a value that contains spaces, since that's
         // easier to work with in the shell than other forms of structured data.
         let packages = packages.join("|");
-
         let arch = getenv("BUILDSYS_ARCH")?;
-        let opts = format!(
-            "--opt target=image \
-             --opt build-arg:PACKAGES={packages} \
-             --opt build-arg:FLAVOR={name} \
-             --opt build-arg:ARCH={arch}",
-            packages = packages,
-            arch = arch,
-            name = getenv("IMAGE")?,
-        );
+        let name = getenv("IMAGE")?;
 
         // Always rebuild images since they are located in a different workspace,
         // and don't directly track changes in the underlying packages.
         getenv("BUILDSYS_TIMESTAMP")?;
 
-        let result = buildctl(&opts)?;
-        if !result.status.success() {
-            let output = String::from_utf8_lossy(&result.stdout);
-            return error::ImageBuild { packages, output }.fail();
-        }
+        let target = "image";
+        let build_args = format!(
+            "--build-arg PACKAGES={packages} \
+             --build-arg ARCH={arch} \
+             --build-arg FLAVOR={name}",
+            packages = packages,
+            arch = arch,
+            name = name,
+        );
+        let tag = format!("buildsys-img-{name}-{arch}", name = name, arch = arch);
+
+        build(&target, &build_args, &tag)?;
 
         Ok(Self)
     }
 }
 
-/// Invoke `buildctl` by way of `docker` with the arguments for a specific
-/// package or image build.
-fn buildctl(opts: &str) -> Result<Output> {
-    let docker_args = docker_args()?;
-    let buildctl_args = buildctl_args()?;
+/// Invoke a series of `docker` commands to drive a package or image build.
+fn build(target: &str, build_args: &str, tag: &str) -> Result<()> {
+    // Our Dockerfile is in the top-level directory.
+    let root = getenv("BUILDSYS_ROOT_DIR")?;
+    std::env::set_current_dir(&root).context(error::DirectoryChange { path: &root })?;
+
+    // Compute a per-checkout prefix for the tag to avoid collisions.
+    let mut d = Sha512::new();
+    d.input(&root);
+    let digest = hex::encode(d.result());
+    let suffix = &digest[..12];
+    let tag = format!("{}-{}", tag, suffix);
 
     // Avoid using a cached layer from a previous build.
-    let nocache = format!(
-        "--opt build-arg:NOCACHE={}",
-        rand::thread_rng().gen::<u32>(),
-    );
-
-    // Build the giant chain of args. Treat "|" as a placeholder that indicates
-    // where the argument should contain spaces after we split on whitespace.
-    let args = docker_args
-        .split_whitespace()
-        .chain(buildctl_args.split_whitespace())
-        .chain(opts.split_whitespace())
-        .chain(nocache.split_whitespace())
-        .map(|s| s.replace("|", " "));
+    let nocache = rand::thread_rng().gen::<u32>();
+    let nocache_args = format!("--build-arg NOCACHE={}", nocache);
+
+    // Accept additional overrides for Docker arguments. This is only for
+    // overriding network settings, and can be dropped when we no longer need
+    // network access during the build.
+    let docker_run_args = getenv("BUILDSYS_DOCKER_RUN_ARGS").unwrap_or_else(|_| "".to_string());
+
+    let build = args(format!(
+        "build . \
+         --target {target} \
+         {docker_run_args} \
+         {build_args} \
+         {nocache_args} \
+         --tag {tag}",
+        target = target,
+        docker_run_args = docker_run_args,
+        build_args = build_args,
+        nocache_args = nocache_args,
+        tag = tag,
+    ));
+
+    let output = getenv("BUILDSYS_OUTPUT_DIR")?;
+    let create = args(format!("create --name {tag} {tag} true", tag = tag));
+    let cp = args(format!("cp {}:/output/. {}", tag, output));
+    let rm = args(format!("rm --force {}", tag));
+    let rmi = args(format!("rmi --force {}", tag));
+
+    // Clean up the stopped container if it exists.
+    let _ = docker(&rm);
+
+    // Clean up the previous image if it exists.
+    let _ = docker(&rmi);
+
+    // Build the image, which builds the artifacts we want.
+    docker(&build)?;
+
+    // Create a stopped container so we can copy artifacts out.
+    docker(&create)?;
+
+    // Copy artifacts into our output directory.
+    docker(&cp)?;
+
+    // Clean up our stopped container after copying artifacts out.
+    docker(&rm)?;
+
+    // Clean up our image now that we're done.
+    docker(&rmi)?;
+
+    Ok(())
+}
 
-    // Run the giant docker invocation
+/// Run `docker` with the specified arguments.
+fn docker(args: &[String]) -> Result<Output> {
     cmd("docker", args)
         .stderr_to_stdout()
         .run()
         .context(error::CommandExecution)
 }
 
-/// Prepare the arguments for docker
-fn docker_args() -> Result<String> {
-    // Gather the user context.
-    let uid = get_effective_uid();
-
-    // Gather the environment context.
-    let root_dir = getenv("BUILDSYS_ROOT_DIR")?;
-    let buildkit_client = getenv("BUILDSYS_BUILDKIT_CLIENT")?;
-    let user_args = getenv("BUILDSYS_DOCKER_RUN_ARGS").unwrap_or_else(|_| "".to_string());
-
-    let docker_args = format!(
-        "run --init --rm --network host --user {uid}:{uid} \
-         --volume {root_dir}:{root_dir} --workdir {root_dir} \
-         {user_args} \
-         --entrypoint /usr/bin/buildctl {buildkit_client}",
-        uid = uid,
-        root_dir = root_dir,
-        user_args = user_args,
-        buildkit_client = buildkit_client
-    );
-
-    Ok(docker_args)
-}
-
-fn buildctl_args() -> Result<String> {
-    // Gather the environment context.
-    let output_dir = getenv("BUILDSYS_OUTPUT_DIR")?;
-    let buildkit_server = getenv("BUILDSYS_BUILDKIT_SERVER")?;
-
-    let buildctl_args = format!(
-        "--addr {buildkit_server} build --progress=plain \
-         --frontend=dockerfile.v0 --local context=. --local dockerfile=. \
-         --output type=local,dest={output_dir}",
-        buildkit_server = buildkit_server,
-        output_dir = output_dir
-    );
-
-    Ok(buildctl_args)
+/// Convert an argument string into a collection of positional arguments.
+fn args<S>(input: S) -> Vec<String>
+where
+    S: AsRef<str>,
+{
+    // Treat "|" as a placeholder that indicates where the argument should
+    // contain spaces after we split on whitespace.
+    input
+        .as_ref()
+        .split_whitespace()
+        .map(|s| s.replace("|", " "))
+        .collect()
 }
 
 /// Retrieve a BUILDSYS_* variable that we expect to be set in the environment,

tools/buildsys/src/builder/error.rs

@@ -1,16 +1,17 @@
 use snafu::Snafu;
+use std::path::PathBuf;
 
 #[derive(Debug, Snafu)]
 #[snafu(visibility = "pub(super)")]
 pub enum Error {
     #[snafu(display("Failed to execute command: {}", source))]
     CommandExecution { source: std::io::Error },
 
-    #[snafu(display("Failed to build package '{}':\n{}", package, output,))]
-    PackageBuild { package: String, output: String },
-
-    #[snafu(display("Failed to build image with '{}':\n{}", packages, output,))]
-    ImageBuild { packages: String, output: String },
+    #[snafu(display("Failed to change directory to '{}': {}", path.display(), source))]
+    DirectoryChange {
+        path: PathBuf,
+        source: std::io::Error,
+    },
 
     #[snafu(display("Missing environment variable '{}'", var))]
     Environment {

tools/buildsys/src/lib.rs

@@ -1,6 +1,5 @@
 /*!
-This library initiates an rpm or image build by running the BuildKit CLI inside
-a Docker container.
+This library carries out an rpm or image build using Docker.
 
 It is meant to be called by a Cargo build script. To keep those scripts simple,
 all of the configuration is taken from the environment.