Copilot commented on code in PR #4539:
URL: https://github.com/apache/solr/pull/4539#discussion_r3472118108
##########
solr/docker/build.gradle:
##########
@@ -187,68 +169,99 @@ task assemblePackaging(type: Sync) {
into packagingDir
}
-task dockerBuild() {
- group = 'Docker'
- description = 'Build Solr docker image'
+abstract class DockerBuildTask extends DefaultTask {
+ @Inject
+ abstract ExecOperations getExecOperations()
- // Ensure that the docker image is rebuilt on build-arg changes or changes
in the docker context
- inputs.properties([
- baseDockerImage: baseDockerImage
- ])
- var solrTgzConfiguration = isImageSlim() ? configurations.solrSlimTgz :
configurations.solrFullTgz
- inputs.files(solrTgzConfiguration)
- inputs.property("isSlimImage", isImageSlim())
- dependsOn(solrTgzConfiguration)
+ @Internal
+ abstract Property<Configuration> getSolrTgzConfiguration()
+
+ @Internal
+ abstract Property<Boolean> getIsSlimImage()
+
+ @Internal
+ abstract Property<String> getDockerfileDistSuffix()
- doLast {
- exec {
- standardInput = solrTgzConfiguration.singleFile.newDataInputStream()
+ @Internal
+ abstract Property<String> getImageFile()
+
+ @TaskAction
+ void buildDockerImage() {
+ var solrTgzConfig = solrTgzConfiguration.get()
+ def distSuffix = dockerfileDistSuffix.get()
+ def imgFile = imageFile.get()
+ def baseImage = inputs.properties.baseDockerImage
+ def projectVersion = project.version
+
+ getExecOperations().exec {
+ standardInput = solrTgzConfig.singleFile.newDataInputStream()
commandLine "docker", "build",
- "-f", "solr-${->
project.version}${dockerImageDistSuffix}/docker/Dockerfile",
- "--iidfile", imageIdFile,
- "--build-arg", "BASE_IMAGE=${-> inputs.properties.baseDockerImage}",
+ "-f", "solr-${projectVersion}${distSuffix}/docker/Dockerfile",
+ "--iidfile", imgFile,
+ "--build-arg", "BASE_IMAGE=${baseImage}",
"-"
}
- }
- // Print information on the image after it has been created
- doLast {
- def dockerImageId = file(imageIdFile).text
+ def dockerImageId = project.file(imgFile).text
project.logger.lifecycle("Solr Docker Image Created")
- project.logger.lifecycle("\tID: \t${-> dockerImageId}")
- project.logger.lifecycle("\tBase Image: \t${-> baseDockerImage}")
- project.logger.lifecycle("\tSolr Version: \t${-> project.version}")
- project.logger.lifecycle("\tSolr Distribution: \t${isImageSlim() ? "Slim"
: "Full"}")
+ project.logger.lifecycle("\tID: \t${dockerImageId}")
+ project.logger.lifecycle("\tBase Image: \t${baseImage}")
+ project.logger.lifecycle("\tSolr Version: \t${projectVersion}")
+ project.logger.lifecycle("\tSolr Distribution: \t${isSlimImage.get() ?
"Slim" : "Full"}")
}
-
- outputs.files(imageIdFile)
}
-task dockerTag(dependsOn: tasks.dockerBuild) {
+task dockerBuild(type: DockerBuildTask) {
group = 'Docker'
- description = 'Tag Solr docker image'
+ description = 'Build Solr docker image'
- def dockerImageIdFile = file(imageIdFile)
- // Ensure that the docker image is re-tagged if the image ID or desired tag
changes
+ // Ensure that the docker image is rebuilt on build-arg changes or changes
in the docker context
inputs.properties([
- dockerImageName: dockerImageName,
+ baseDockerImage: baseDockerImage
])
- inputs.file(dockerImageIdFile)
+ var tgzConfig = isImageSlim() ? configurations.solrSlimTgz :
configurations.solrFullTgz
+ solrTgzConfiguration.set(tgzConfig)
+ isSlimImage.set(isImageSlim())
+ dockerfileDistSuffix.set(dockerImageDistSuffix.toString())
+ imageFile.set(imageIdFile.toString())
+ inputs.files(tgzConfig)
+ inputs.property("isSlimImage", isImageSlim())
+ dependsOn(tgzConfig)
+
+ outputs.files(imageIdFile)
+}
+
+abstract class DockerTagTask extends DefaultTask {
+ @Inject
+ abstract ExecOperations getExecOperations()
- doLast {
+ @TaskAction
+ void tagDockerImage() {
+ def dockerImageIdFile = project.file(imageIdFile)
def dockerImageId = dockerImageIdFile.text
- exec {
+ getExecOperations().exec {
commandLine "docker", "tag", dockerImageId,
inputs.properties.dockerImageName
Review Comment:
DockerTagTask.tagDockerImage() references `imageIdFile` and
`dockerImageName` as script-local variables. Task classes defined in Gradle
scripts don’t have access to those locals at execution time, so this will fail
when the task runs. Use the declared task inputs instead (the image id file is
already an input, and the tag is already provided via inputs.properties).
##########
solr/docker/build.gradle:
##########
@@ -267,52 +280,56 @@ task testDocker(type: TestDockerImageTask, dependsOn:
tasks.dockerBuild) {
testCasesExclude.value(excludeCases)
}
-task dockerPush(dependsOn: tasks.dockerTag) {
- group = 'Docker'
- description = 'Push Solr docker image'
+abstract class DockerPushTask extends DefaultTask {
+ @Inject
+ abstract ExecOperations getExecOperations()
+
+ private String getBuildxBuilderInfo() {
+ def builderCheckOutput = new ByteArrayOutputStream()
+ def builderCheckResult = getExecOperations().exec {
+ ignoreExitValue = true
+ standardOutput = builderCheckOutput
+ errorOutput = new ByteArrayOutputStream()
+ commandLine "docker", "buildx", "inspect"
+ }
- // Ensure that the docker image is re-pushed if the image ID or tag changes
- inputs.properties([
- dockerImageName: dockerImageName,
- dockerImagePlatforms: dockerImagePlatforms
- ])
- inputs.file(imageIdFile)
+ if (builderCheckResult.exitValue != 0) {
+ throw new GradleException("Docker Buildx is not available or no builder
is configured.\n" +
+ "Please ensure Docker Buildx is installed and create a builder:\n" +
+ " docker buildx create --name solr-builder --use\n" +
+ "Or use an existing builder:\n" +
+ " docker buildx use <builder-name>")
+ }
- // We don't want to push a docker image unless the tests have passed
- mustRunAfter tasks.testDocker
+ return builderCheckOutput.toString()
+ }
- doLast {
+ @TaskAction
+ void pushDockerImage() {
def platformValue =
inputs.properties.dockerImagePlatforms.toString().trim()
validatePlatformFormat(platformValue)
if (isMultiPlatform()) {
Review Comment:
DockerPushTask.pushDockerImage() calls `validatePlatformFormat()` and
`isMultiPlatform()`, but those are script-local closures, not
methods/properties on the task class. This will throw at runtime. Inline the
validation and compute the multi-platform flag from the input string.
##########
solr/docker/build.gradle:
##########
@@ -501,112 +532,125 @@ if (''.equals(releaseGpgFingerprint)) {
}
}
+abstract class TestBuildDockerfileOfficialTask extends Copy {
+ @Inject
+ abstract ExecOperations getExecOperations()
+
+ @Input
+ abstract Property<String> getVariantName()
+
+ @Input
+ abstract Property<String> getLowerVariantName()
+
+ @Internal
+ abstract DirectoryProperty getMockHttpdHomeDir()
+
+ @TaskAction
+ @Override
+ protected void copy() {
+ super.copy()
+ def mockHttpdHome = mockHttpdHomeDir.get().asFile
+ def variant = getVariantName().get()
+ def lowerVariant = getLowerVariantName().get()
+ def mockServerIdFile =
project.file("$smokeTestOfficial/$lowerVariant/dockerfile-mock-artifact-server-cid.txt")
+
+ def mainException = null;
+
+ try {
+ logger.lifecycle('Running mock HTTPD server our testing...');
+ getExecOperations().exec {
+ commandLine 'docker', 'run',
+ '--cidfile', mockServerIdFile,
+ '--rm',
+ '-d',
+ '-v', "${mockHttpdHome.absoluteFile}:/data/${-> project.version}",
+ '-w', '/data',
+ '-p', '9876:9876',
+ 'python:3-alpine', 'python', '-m', 'http.server', '9876'
+ }
+ try {
+ def mockServerId = mockServerIdFile.text
+ def mockServerIpStdOut = new ByteArrayOutputStream()
+ getExecOperations().exec {
+ commandLine 'docker',
+ 'inspect',
+ "--format={{range
.NetworkSettings.Networks}}{{.IPAddress}}{{end}}",
+ mockServerId
+ standardOutput = mockServerIpStdOut
+ }
+ def mockServerIp = mockServerIpStdOut.toString().trim()
+
+ logger.lifecycle('Running docker build on Dockerfile.official...');
+ def downloadHost = "mock-downloads.apache.org"
+ if (!project(":solr:distribution").ext.withSignedArtifacts) {
+ downloadHost = "mock-downloads.test.org"
+ }
+ getExecOperations().exec {
+ standardInput =
+
project.file("${dockerfilesDirPath}/Dockerfile.official-${lowerVariant}")
+ .newDataInputStream()
+ commandLine 'docker', 'build',
+ '--add-host', "${downloadHost}:${mockServerIp}",
+ '--no-cache',
+ "--iidfile", imageIdFileOfficial(variant),
+ '--build-arg',
"SOLR_DOWNLOAD_SERVER=http://${downloadHost}:9876";,
+ '--tag', officialDockerImageName(variant),
+ '-'
+ }
+
+ def officialDockerImageId =
project.file(imageIdFileOfficial(variant)).text
+
+ project.logger.lifecycle("\"Official $variant\" Solr Docker Image
Tagged")
+ project.logger.lifecycle("\tID: \t$officialDockerImageId")
+ project.logger.lifecycle("\tTag:
\t${officialDockerImageName(variant)}")
Review Comment:
This task action still calls `imageIdFileOfficial(variant)` and
`officialDockerImageName(variant)` (script-local closures). After computing
`imageIdFile` and `officialDockerImageName` locally, use those instead for
`--iidfile`, tagging, and logging so the task runs correctly.
##########
solr/docker/build.gradle:
##########
@@ -325,15 +342,29 @@ task dockerPush(dependsOn: tasks.dockerTag) {
project.logger.lifecycle("Solr Docker Multi-Platform Image Pushed:
\t$dockerImageName")
project.logger.lifecycle("\tPlatforms: \t$platformValue")
} else {
- // Standard push (works for both standard builds and single-platform
buildx builds)
- exec {
+ getExecOperations().exec {
commandLine "docker", "push", dockerImageName
}
project.logger.lifecycle("Solr Docker Image Pushed: \t$dockerImageName")
}
}
}
+task dockerPush(type: DockerPushTask, dependsOn: tasks.dockerTag) {
+ group = 'Docker'
+ description = 'Push Solr docker image'
+
+ // Ensure that the docker image is re-pushed if the image ID or tag changes
+ inputs.properties([
+ dockerImageName: dockerImageName,
+ dockerImagePlatforms: dockerImagePlatforms
+ ])
Review Comment:
DockerPushTask is updated to read `baseDockerImage`,
`dockerImageDistSuffix`, and the tgz configuration name from inputs, but the
dockerPush task currently doesn’t provide them. Add these to
`inputs.properties` so the task action can resolve them consistently.
##########
solr/docker/build.gradle:
##########
@@ -501,112 +532,125 @@ if (''.equals(releaseGpgFingerprint)) {
}
}
+abstract class TestBuildDockerfileOfficialTask extends Copy {
+ @Inject
+ abstract ExecOperations getExecOperations()
+
+ @Input
+ abstract Property<String> getVariantName()
+
+ @Input
+ abstract Property<String> getLowerVariantName()
+
+ @Internal
+ abstract DirectoryProperty getMockHttpdHomeDir()
+
+ @TaskAction
+ @Override
+ protected void copy() {
+ super.copy()
+ def mockHttpdHome = mockHttpdHomeDir.get().asFile
+ def variant = getVariantName().get()
+ def lowerVariant = getLowerVariantName().get()
+ def mockServerIdFile =
project.file("$smokeTestOfficial/$lowerVariant/dockerfile-mock-artifact-server-cid.txt")
Review Comment:
TestBuildDockerfileOfficialTask.copy() references `smokeTestOfficial`
(script-local) which won’t be visible from inside the task class at execution
time. Compute the path from `project.layout.buildDirectory` and also precompute
`imageIdFile`/`officialDockerImageName` for later use in this task action.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
