feat: add improved docker image layering (#32)

aaron-steinfeld · web-flow · commit 077d77502f08 · 2022-08-18T08:11:20.000-07:00
* feat: add improved docker image layering

* fix: mark script as input
diff --git a/hypertrace-gradle-docker-java-application-plugin/CHANGELOG.md b/hypertrace-gradle-docker-java-application-plugin/CHANGELOG.md
@@ -1,42 +1,61 @@
 # Changelog
 
-
 ## [0.2.4]
+
 - Default image has changed from gcr.io/distroless/java:11 to gcr.io/distroless/java:11-debug
 
 ## [0.3.0]
+
 - Added support for producing multiple variants of an application with different base images
 - Added a default variant, `slim` based off `gcr.io/distroless/java:11`
 
 ## [0.3.2]
+
 - Removed default `slim` variant
 
 ## [0.3.3]
+
 - Support Java 8+
 
 ## [0.4.0]
+
 - Change default image to `hypertrace/java:11`
 
 ## [0.5.0]
+
 - Exposed for configuration:
     - `maintainer`
     - `port`
     - `adminPort`
     - `serviceName`
     - `healthCheck`
     - `envVars`
+
 ## [0.7.0]
+
 - Remove variants
 - Remove internal usage of `com.bmuschko.docker-java-application`, cleaning up ghost tasks
 - Use gradle application start script to run application in docker, allowing reuse and accounting
   for env vars like JAVA_OPTS
 
 ## [0.7.1]
+
 - Simplify start script to use provided parameters as given
 
 ## [0.8.0]
+
 - Add COMMIT_SHA to the generated dockerfile as a build arg defaulting to unknown
 - Use COMMIT_SHA build arg to set a label `commit_sha` and environment variable `COMMIT_SHA`
 
 ## [0.8.2]
-- Add support for exposing multiple docker image ports via `ports` config
+
+- Add support for exposing multiple docker image ports via `ports` config
+
+## [0.9.6]
+
+- Improve docker layering by splitting libraries into 3 layers. From the top down:
+    - Local libraries being built as part of the target gradle build
+    - Organization libraries that satisfy the `orgLibrarySpec` property on the `javaApplication`
+      spec as documented in the README, but do not satisfy the local library criteria.
+    - External libraries, presumed to be any library that does not meet the criteria of either of
+      the above layers.
diff --git a/hypertrace-gradle-docker-java-application-plugin/README.md b/hypertrace-gradle-docker-java-application-plugin/README.md
@@ -1,11 +1,14 @@
-
 # Hypertrace Docker Java Application Plugin
+
 ###### org.hypertrace.docker-java-application-plugin
 
 ### Purpose
-This plugin is a highly opinionated plugin that configures the target project to build docker images of the target project.
-It configures the default image from `org.hypertrace.docker-plugin` to use the docker file produced by
-`com.bmuschko.docker-java-application`. It allows configuring several values, detailed below. 
+
+This plugin is a highly opinionated plugin that configures the target project to build docker images
+of the target project.
+It configures the default image from `org.hypertrace.docker-plugin` to use the docker file produced
+by
+`com.bmuschko.docker-java-application`. It allows configuring several values, detailed below.
 Additionally, the main class is taken from the application plugin - the
 default behavior of `com.bmuschko.docker-java-application` for identifying main classes is not used.
 An application can define multiple variants which allow specifying different base images to use.
@@ -19,12 +22,14 @@ An application can define multiple variants which allow specifying different bas
 - `port` (_DEPRECATED_ - replaced by `ports`)
     - Integer
     - No default
-    - Exposed in dockerfile if set. If used in conjunction with `ports`, the combined list will be exposed.
+    - Exposed in dockerfile if set. If used in conjunction with `ports`, the combined list will be
+      exposed.
 - `ports`
-  - Integer List
-  - No default
-  - Exposed in dockerfile if set. If used in conjunction with deprecated `port`, the combined list will be exposed.
-  - This does not set a default for the admin port
+    - Integer List
+    - No default
+    - Exposed in dockerfile if set. If used in conjunction with deprecated `port`, the combined list
+      will be exposed.
+    - This does not set a default for the admin port
 - `adminPort`
     - Integer
     - No default except for deprecated case described below
@@ -36,13 +41,19 @@ An application can define multiple variants which allow specifying different bas
     - Added to envVars as `SERVICE_NAME`
 - `healthCheck`
     - String
-    - If `adminPort` is set, defaults to `-interval=2s --start-period=15s --timeout=2s CMD wget  -qO http://127.0.0.1:${adminPort}/health &> /dev/null || exit`
+    - If `adminPort` is set, defaults
+      to `-interval=2s --start-period=15s --timeout=2s CMD wget -qO http://127.0.0.1:${adminPort}/health &> /dev/null || exit`
     - If `adminPort` is unset, `healthCheck` has no default
 - `envVars`
     - Map<String, String>
     - Defaults to `{"SERVICE_NAME": "${serviceName}"}`
     - Can be appended to with `put`, or overwritten with `set`
-
+- `orgLibrarySpec`
+    - `Spec<ResolvedArtifact>`
+    - Defaults to any artifact with a group starting with `org.hypertrace`
+    - This is used in layering the image to separate org libraries into a layer above external
+      libraries, under the assumption these will change (and thus invalidate the layer) more
+      frequently.
 
 For further configuration, use `org.hypertrace.docker-plugin` directly and create a dockerfile.
 
@@ -68,6 +79,7 @@ hypertraceDocker {
 ```
 
 Producing a dockerfile:
+
 ```Dockerfile
 FROM hypertrace/java:11
 LABEL maintainer="Hypertrace 'https://www.hypertrace.org/'"
@@ -82,6 +94,7 @@ ENV SERVICE_NAME=example-app
 ```
 
 Overriding or providing more variables:
+
 ```kotlin
 hypertraceDocker {
   defaultImage {
@@ -96,6 +109,7 @@ hypertraceDocker {
 ```
 
 Produces a diff of:
+
 ```diff
 -FROM hypertrace/java:11
 +FROM hypertrace/java:14
diff --git a/hypertrace-gradle-docker-java-application-plugin/src/main/java/org/hypertrace/gradle/docker/HypertraceDockerJavaApplication.java b/hypertrace-gradle-docker-java-application-plugin/src/main/java/org/hypertrace/gradle/docker/HypertraceDockerJavaApplication.java
@@ -2,10 +2,12 @@
 
 import java.util.Collections;
 import javax.inject.Inject;
+import org.gradle.api.artifacts.ResolvedArtifact;
 import org.gradle.api.model.ObjectFactory;
 import org.gradle.api.provider.ListProperty;
 import org.gradle.api.provider.MapProperty;
 import org.gradle.api.provider.Property;
+import org.gradle.api.specs.Spec;
 
 public class HypertraceDockerJavaApplication {
 
@@ -21,6 +23,7 @@ public class HypertraceDockerJavaApplication {
   public final ListProperty<Integer> ports;
   public final Property<String> healthCheck;
   public final MapProperty<String, String> envVars;
+  public Spec<ResolvedArtifact> orgLibrarySpec;
 
   @Inject
   public HypertraceDockerJavaApplication(
@@ -42,5 +45,13 @@ public HypertraceDockerJavaApplication(
     this.healthCheck = objectFactory.property(String.class)
                                     .convention(this.adminPort.map(adminPort -> String.format(
                                         "HEALTHCHECK --interval=2s --start-period=15s --timeout=2s CMD wget -qO- http://127.0.0.1:%d/health &> /dev/null || exit 1", adminPort)));
+    this.orgLibrarySpec = this::isHypertraceLibrary;
+  }
+
+  private boolean isHypertraceLibrary(ResolvedArtifact artifact) {
+    return artifact.getModuleVersion()
+                   .getId()
+                   .getGroup()
+                   .startsWith("org.hypertrace");
   }
 }
diff --git a/hypertrace-gradle-docker-java-application-plugin/src/main/java/org/hypertrace/gradle/docker/HypertraceDockerJavaApplicationPlugin.java b/hypertrace-gradle-docker-java-application-plugin/src/main/java/org/hypertrace/gradle/docker/HypertraceDockerJavaApplicationPlugin.java
@@ -12,19 +12,22 @@
 import java.io.File;
 import java.net.URL;
 import java.nio.file.Path;
-import java.time.Instant;
-import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
 import org.gradle.api.Plugin;
 import org.gradle.api.Project;
+import org.gradle.api.artifacts.Configuration;
+import org.gradle.api.artifacts.ResolvedArtifact;
 import org.gradle.api.file.Directory;
-import org.gradle.api.file.FileCollection;
 import org.gradle.api.plugins.ApplicationPlugin;
 import org.gradle.api.plugins.JavaApplication;
 import org.gradle.api.plugins.JavaPluginConvention;
 import org.gradle.api.provider.Provider;
 import org.gradle.api.resources.TextResource;
+import org.gradle.api.specs.Spec;
 import org.gradle.api.tasks.SourceSet;
 import org.gradle.api.tasks.SourceSetOutput;
 import org.gradle.api.tasks.Sync;
@@ -37,9 +40,9 @@ public class HypertraceDockerJavaApplicationPlugin implements Plugin<Project> {
   public static final String DOCKERFILE_TASK_NAME = "generateJavaApplicationDockerfile";
   public static final String DOCKER_START_SCRIPT_TASK_NAME = "generateJavaApplicationDockerStartScript";
   public static final String SYNC_BUILD_CONTEXT_TASK_NAME = "syncJavaApplicationDockerContext";
-  private static final String DOCKER_BUILD_CONTEXT_EXTERNAL_LIBS_DIR = "externalLibs";
-
   private static final String DOCKER_BUILD_CONTEXT_LOCAL_LIBS_DIR = "localLibs";
+  private static final String DOCKER_BUILD_CONTEXT_ORG_LIBS_DIR = "orgLibs";
+  private static final String DOCKER_BUILD_CONTEXT_EXTERNAL_LIBS_DIR = "externalLibs";
   private static final String DOCKER_BUILD_CONTEXT_CLASSES_DIR = "classes";
   private static final String DOCKER_BUILD_CONTEXT_RESOURCES_DIR = "resources";
   private static final String DOCKER_BUILD_CONTEXT_SCRIPTS_DIR = "scripts";
@@ -119,6 +122,9 @@ void createDockerfileTask(Project project, HypertraceDockerJavaApplication javaA
              dockerfile.copyFile(provideIfDirectoryExists(dockerfile.getDestDir()
                                                                     .map(dir -> dir.dir(DOCKER_BUILD_CONTEXT_EXTERNAL_LIBS_DIR)))
                  .map(unused -> new CopyFile(DOCKER_BUILD_CONTEXT_EXTERNAL_LIBS_DIR, DOCKER_BUILD_CONTEXT_EXTERNAL_LIBS_DIR)));
+             dockerfile.copyFile(provideIfDirectoryExists(dockerfile.getDestDir()
+                                                                    .map(dir -> dir.dir(DOCKER_BUILD_CONTEXT_ORG_LIBS_DIR)))
+                 .map(unused -> new CopyFile(DOCKER_BUILD_CONTEXT_ORG_LIBS_DIR, DOCKER_BUILD_CONTEXT_ORG_LIBS_DIR)));
              dockerfile.copyFile(provideIfDirectoryExists(dockerfile.getDestDir()
                                                                     .map(dir -> dir.dir(DOCKER_BUILD_CONTEXT_LOCAL_LIBS_DIR)))
                  .map(unused -> new CopyFile(DOCKER_BUILD_CONTEXT_LOCAL_LIBS_DIR, DOCKER_BUILD_CONTEXT_LOCAL_LIBS_DIR)));
@@ -148,6 +154,7 @@ void createDockerfileTask(Project project, HypertraceDockerJavaApplication javaA
   private void createDockerStartScriptTask(Project project) {
     project.getTasks()
            .register(DOCKER_START_SCRIPT_TASK_NAME, CreateStartScripts.class, startScript -> {
+             startScript.getInputs().file(this.getStartScriptTemplate(project));
              ((TemplateBasedScriptGenerator) startScript.getUnixStartScriptGenerator()).setTemplate(this.getStartScriptTemplate(project));
              startScript.setGroup(DockerPlugin.TASK_GROUP);
              startScript.setDescription("Creates a startup script for use by the docker container");
@@ -178,22 +185,32 @@ private void createContextSyncTask(Project project) {
                            .map(Dockerfile::getDestDir)
                            .get());
              sync.with(project.copySpec(spec -> {
-               // We split deps into two dirs depending on last modified time, with the assumption that any local jars will be recently built
-               Instant time = Instant.now()
-                                     .minus(30, ChronoUnit.MINUTES);
-               spec.into(DOCKER_BUILD_CONTEXT_LOCAL_LIBS_DIR, childSpec -> childSpec.from(getRuntimeClasspath(project)
-                   .filter(file -> Instant.ofEpochMilli(file.lastModified())
-                                          .isAfter(time))));
-               spec.into(DOCKER_BUILD_CONTEXT_EXTERNAL_LIBS_DIR, childSpec -> childSpec.from(getRuntimeClasspath(project)
-                   .filter(file -> Instant.ofEpochMilli(file.lastModified())
-                                          .isBefore(time))));
+               spec.into(DOCKER_BUILD_CONTEXT_LOCAL_LIBS_DIR, childSpec -> childSpec.from(this.buildFilteredLibraryProvider(project, this.buildLocalArtifactPredicate(project))));
+               spec.into(DOCKER_BUILD_CONTEXT_ORG_LIBS_DIR, childSpec -> childSpec.from(this.buildFilteredLibraryProvider(project, this.buildLocalArtifactPredicate(project)
+                                                                                                                                       .negate()
+                                                                                                                                       .and(this.getOrgLibPredicate(project)))));
+               spec.into(DOCKER_BUILD_CONTEXT_EXTERNAL_LIBS_DIR, childSpec -> childSpec.from(this.buildFilteredLibraryProvider(project, this.buildLocalArtifactPredicate(project)
+                                                                                                                                            .negate()
+                                                                                                                                            .and(this.getOrgLibPredicate(project)
+                                                                                                                                                     .negate()))));
                spec.into(DOCKER_BUILD_CONTEXT_CLASSES_DIR, childSpec -> childSpec.from(mainSourceSetOutput(project).getClassesDirs()));
                spec.into(DOCKER_BUILD_CONTEXT_RESOURCES_DIR, childSpec -> childSpec.from(mainSourceSetOutput(project).getResourcesDir()));
              }));
            });
   }
 
-  private FileCollection getRuntimeClasspath(Project project) {
+  private Predicate<ResolvedArtifact> buildLocalArtifactPredicate(Project project) {
+    // Either matching version or missing version indicating a composite project
+    return artifact -> {
+      String artifactVersion = artifact.getModuleVersion()
+                                       .getId()
+                                       .getVersion();
+      return artifactVersion.equals(project.getVersion()
+                                           .toString()) || artifactVersion.equals("unspecified");
+    };
+  }
+
+  private Configuration getRuntimeClasspath(Project project) {
     return project.getConfigurations()
                   .getByName(RUNTIME_CLASSPATH_CONFIGURATION_NAME);
   }
@@ -218,4 +235,18 @@ private TextResource getStartScriptTemplate(Project project) {
                   .getText()
                   .fromUri(requireNonNull(resourceUrl));
   }
+
+  private Provider<Collection<File>> buildFilteredLibraryProvider(Project project, Predicate<ResolvedArtifact> filterPredicate) {
+    return project.provider(() -> this.getRuntimeClasspath(project)
+                                      .getResolvedConfiguration()
+                                      .getResolvedArtifacts()
+                                      .stream()
+                                      .filter(filterPredicate)
+                                      .map(ResolvedArtifact::getFile)
+                                      .collect(Collectors.toSet()));
+  }
+
+  private Predicate<ResolvedArtifact> getOrgLibPredicate(Project project) {
+    return this.getHypertraceDockerApplicationExtension(project).orgLibrarySpec::isSatisfiedBy;
+  }
 }
diff --git a/hypertrace-gradle-docker-java-application-plugin/src/main/resources/application-start-script.template.sh b/hypertrace-gradle-docker-java-application-plugin/src/main/resources/application-start-script.template.sh
@@ -25,4 +25,4 @@ set -e
 
 DEFAULT_JVM_OPTS=${defaultJvmOpts}
 
-exec java \$DEFAULT_JVM_OPTS \$JAVA_OPTS -classpath '/app/resources:/app/classes:/app/localLibs/*:/app/externalLibs/*' ${mainClassName} \$@
+exec java \$DEFAULT_JVM_OPTS \$JAVA_OPTS -classpath '/app/resources:/app/classes:/app/localLibs/*:/app/orgLibs/*:/app/externalLibs/*' ${mainClassName} \$@

Original file line number	Diff line number	Diff line change
`@@ -25,4 +25,4 @@ set -e`
`25`	`25`
`26`	`26`	`DEFAULT_JVM_OPTS=${defaultJvmOpts}`
`27`	`27`
`28`		`-exec java \$DEFAULT_JVM_OPTS \$JAVA_OPTS -classpath '/app/resources:/app/classes:/app/localLibs/:/app/externalLibs/' ${mainClassName} \$@`
	`28`	`+exec java \$DEFAULT_JVM_OPTS \$JAVA_OPTS -classpath '/app/resources:/app/classes:/app/localLibs/:/app/orgLibs/:/app/externalLibs/*' ${mainClassName} \$@`