From d726f9420c88f2e4053cb7393fb551c1a01514cf Mon Sep 17 00:00:00 2001 From: Ben Cressey Date: Fri, 8 Nov 2019 20:06:02 +0000 Subject: [PATCH 1/4] build: ignore top-level git directory Signed-off-by: Ben Cressey --- .dockerignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.dockerignore b/.dockerignore index 94b40f652da..4dcf5c4e9dd 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,3 +1,4 @@ +/.git /build/*.img /build/*.lz4 /build/*.tar From f1166e7567ea805ed4ebab0aabb6c2725737375e Mon Sep 17 00:00:00 2001 From: Ben Cressey Date: Fri, 8 Nov 2019 20:11:21 +0000 Subject: [PATCH 2/4] build: use docker instead of buildkit Newer versions of Docker have BuildKit integrated, and we already depend on Docker to provide some of our build tools. BuildKit's ability to copy files directly out of a build image is convenient, but we can work around that using Docker functionality, and eliminate the need to run another daemon for builds. Signed-off-by: Ben Cressey --- Dockerfile | 4 +- Makefile.toml | 3 +- tools/buildsys/src/builder.rs | 194 +++++++++++++++------------- tools/buildsys/src/builder/error.rs | 11 +- tools/buildsys/src/lib.rs | 3 +- 5 files changed, 117 insertions(+), 98 deletions(-) diff --git a/Dockerfile b/Dockerfile index 693595b0d50..8a700138e33 100644 --- a/Dockerfile +++ b/Dockerfile @@ -60,7 +60,7 @@ RUN --mount=source=.cargo,target=/home/builder/.cargo \ 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 +96,4 @@ RUN --mount=target=/host \ && echo ${NOCACHE} FROM scratch AS image -COPY --from=imgbuild /local/output/* / +COPY --from=imgbuild /local/output/* /output/ diff --git a/Makefile.toml b/Makefile.toml index dcbe84c9933..3849b80105a 100644 --- a/Makefile.toml +++ b/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" diff --git a/tools/buildsys/src/builder.rs b/tools/buildsys/src/builder.rs index f9c0ab90ffd..6d043e9318c 100644 --- a/tools/buildsys/src/builder.rs +++ b/tools/buildsys/src/builder.rs @@ -1,7 +1,7 @@ /*! -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; @@ -9,30 +9,32 @@ 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 { - 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::(), - ); - - // 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::(); + 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 { cmd("docker", args) .stderr_to_stdout() .run() .context(error::CommandExecution) } -/// Prepare the arguments for docker -fn docker_args() -> Result { - // 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 { - // 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(input: S) -> Vec +where + S: AsRef, +{ + // 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, diff --git a/tools/buildsys/src/builder/error.rs b/tools/buildsys/src/builder/error.rs index 809ca62deeb..4a99fe1d185 100644 --- a/tools/buildsys/src/builder/error.rs +++ b/tools/buildsys/src/builder/error.rs @@ -1,4 +1,5 @@ use snafu::Snafu; +use std::path::PathBuf; #[derive(Debug, Snafu)] #[snafu(visibility = "pub(super)")] @@ -6,11 +7,11 @@ 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 { diff --git a/tools/buildsys/src/lib.rs b/tools/buildsys/src/lib.rs index 7b3a2c0ba45..2734b6ca90a 100644 --- a/tools/buildsys/src/lib.rs +++ b/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. From 2628a27c0f11c0410bf1889284b5fb2278ccd0fa Mon Sep 17 00:00:00 2001 From: Ben Cressey Date: Wed, 13 Nov 2019 20:38:20 +0000 Subject: [PATCH 3/4] docs: update for BuildKit to Docker transition Signed-off-by: Ben Cressey --- INSTALL.md | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/INSTALL.md b/INSTALL.md index c4685bcc689..2b27df8dcbd 100644 --- a/INSTALL.md +++ b/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 From 15f4d490692dfba2725bcdc40a3fb979f3f57187 Mon Sep 17 00:00:00 2001 From: Ben Cressey Date: Wed, 13 Nov 2019 23:22:36 +0000 Subject: [PATCH 4/4] build: adjust cache volume logic Although the cache volume was previously created and was functional, the subsequent build stage could not see the expected marker file under either Docker 18.09 and 19.03. With this change, the marker file is now visible, which verifies that we are using the cache volume that matches the build stage. This is a follow-up to the fix in f5c863c8. Signed-off-by: Ben Cressey --- Dockerfile | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 8a700138e33..8a22ebec1f3 100644 --- a/Dockerfile +++ b/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,7 +63,7 @@ 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