dweiss commented on a change in pull request #2197:
URL: https://github.com/apache/lucene-solr/pull/2197#discussion_r557282449



##########
File path: solr/docker/build.gradle
##########
@@ -18,106 +18,203 @@
 import com.google.common.base.Preconditions
 import com.google.common.base.Strings
 
-apply plugin: 'base'
-apply plugin: 'com.palantir.docker'
-
-subprojects {
-  apply plugin: 'base'
-  apply plugin: 'com.palantir.docker'
-}
-
 description = 'Solr Docker image'
 
-def dockerPackage = project(':solr:docker:package')
-
-dependencies {
-  docker dockerPackage
-}
+apply plugin: 'base'
 
+// Solr Docker inputs
 def dockerImageRepo = propertyOrEnvOrDefault("solr.docker.imageRepo", 
"SOLR_DOCKER_IMAGE_REPO", "apache/solr")
 def dockerImageTag = propertyOrEnvOrDefault("solr.docker.imageTag", 
"SOLR_DOCKER_IMAGE_TAG", "${version}")
 def dockerImageName = propertyOrEnvOrDefault("solr.docker.imageName", 
"SOLR_DOCKER_IMAGE_NAME", "${dockerImageRepo}:${dockerImageTag}")
 def baseDockerImage = propertyOrEnvOrDefault("solr.docker.baseImage", 
"SOLR_DOCKER_BASE_IMAGE", 'openjdk:11-jre-slim')
 def githubUrlOrMirror = propertyOrEnvOrDefault("solr.docker.githubUrl", 
"SOLR_DOCKER_GITHUB_URL", 'github.com')
 
-docker {
-  name = dockerImageName
-  files file('include')
-  buildArgs(['BASE_IMAGE' : baseDockerImage, 'SOLR_PACKAGE_IMAGE' : 
'apache/solr-build:local-package', 'SOLR_VERSION': "${version}", 'GITHUB_URL': 
githubUrlOrMirror])
+// Build directory locations
+def dockerBuildDistribution = "$buildDir/distributions"
+def imageIdFile = "$buildDir/image-id"
+
+configurations {
+  packaging {
+    canBeResolved = true
+  }
+  dockerImage {
+    canBeResolved = true
+  }
 }
 
-tasks.docker {
-  // In order to create the solr docker image, the solr package image must be 
created first.
-  dependsOn(dockerPackage.tasks.docker)
+dependencies {
+  packaging project(path: ":solr:packaging", configuration: 'archives')
+
+  dockerImage files(imageIdFile) {
+    builtBy 'dockerBuild'
+  }
+}
+
+task dockerTar(type: Tar) {
+  group = 'Docker'
+  description = 'Package docker context to prepare for docker build'
+
+  dependsOn configurations.packaging
+  into('scripts') {
+    from file('scripts')
+    fileMode 755
+  }
+  into('releases') {
+    from configurations.packaging
+    include '*.tgz'
+  }
+  from file('Dockerfile')
+  destinationDirectory = file(dockerBuildDistribution)
+  extension 'tgz'
+  compression = Compression.GZIP
+}
+
+task dockerBuild(dependsOn: tasks.dockerTar) {
+  group = 'Docker'
+  description = 'Build Solr docker image'
+
+  // Ensure that the docker image is rebuilt on build-arg changes or changes 
in the docker context
+  inputs.properties([
+          baseDockerImage: baseDockerImage,
+          githubUrlOrMirror: githubUrlOrMirror,
+          version: version
+  ])
+  inputs.dir(dockerBuildDistribution)
+
+  // Docker build must run after the docker context has been created & tarred
+  mustRunAfter tasks.dockerTar

Review comment:
       It looks like a dependency to me, not a precondition - the behavior you 
described (dockerTag rebuilding the image in the second command) makes sense to 
me. If there is more than one option controlling something, you should just 
specify them (second command) and not rely on any previous build commands.
   
   This is my gradle side viewpoint. I don't know that much about docker to be 
able to tell whether this makes sense from how these deployment processes are 
organized.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to