-
Notifications
You must be signed in to change notification settings - Fork 362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support listing multiple images to remove #1206
Changes from all commits
de68c82
bba702a
c029597
aa9ede1
8bfdf04
2d53e77
de98052
e9d923e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,5 +23,11 @@ jobs: | |
run: ./gradlew test | ||
- name: Integration tests | ||
run: ./gradlew integrationTest | ||
- name: Upload test reports | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's really no need to upload test results, as you can view them with the help of a build scan. The build scan link is available at the end of each workflow run, on success and on failure. I'd say remove this change. |
||
if: ${{ failure() }} | ||
uses: actions/upload-artifact@v3 | ||
with: | ||
name: test-reports | ||
path: build/reports/tests | ||
- name: Assemble artifacts | ||
run: ./gradlew assemble javadoc asciidoctorAllGuides |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,13 +21,13 @@ class DockerRemoveImageFunctionalTest extends AbstractGroovyDslFunctionalTest { | |
dependsOn dockerfile | ||
inputDir = file("build/docker") | ||
} | ||
|
||
task removeImage(type: DockerRemoveImage) { | ||
dependsOn buildImage | ||
force = true | ||
targetImageId buildImage.getImageId() | ||
} | ||
|
||
task removeImageAndCheckRemoval(type: DockerListImages) { | ||
dependsOn removeImage | ||
showAll = true | ||
|
@@ -42,6 +42,75 @@ class DockerRemoveImageFunctionalTest extends AbstractGroovyDslFunctionalTest { | |
!result.output.contains("repository") | ||
} | ||
|
||
def "can remove multiple images"() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll also want to change all documentation and test cases to use the new method instead. |
||
// Given a few Official Images that are small... :-) | ||
String firstImage = "alpine:3" | ||
String secondImage = "busybox:1.36" | ||
String thirdImage = "bash:5.2.21" | ||
|
||
// And a build script that uses them... | ||
buildFile << """ | ||
import com.bmuschko.gradle.docker.tasks.image.Dockerfile | ||
import com.bmuschko.gradle.docker.tasks.image.DockerBuildImage | ||
import com.bmuschko.gradle.docker.tasks.image.DockerListImages | ||
import com.bmuschko.gradle.docker.tasks.image.DockerRemoveImage | ||
|
||
task firstDockerfile(type: Dockerfile) { | ||
from '$firstImage' | ||
label(['maintainer': '[email protected]']) | ||
} | ||
task buildFirstImage(type: DockerBuildImage) { | ||
dependsOn firstDockerfile | ||
inputDir = file("build/docker") | ||
} | ||
|
||
task secondDockerfile(type: Dockerfile) { | ||
from '$secondImage' | ||
label(['maintainer': '[email protected]']) | ||
} | ||
task buildSecondImage(type: DockerBuildImage) { | ||
dependsOn secondDockerfile | ||
inputDir = file("build/docker") | ||
} | ||
|
||
task thirdDockerfile(type: Dockerfile) { | ||
from '$thirdImage' | ||
label(['maintainer': '[email protected]']) | ||
} | ||
task buildThirdImage(type: DockerBuildImage) { | ||
dependsOn thirdDockerfile | ||
inputDir = file("build/docker") | ||
} | ||
|
||
task removeImages(type: DockerRemoveImage) { | ||
dependsOn buildFirstImage | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We won't really need the |
||
dependsOn buildSecondImage | ||
dependsOn buildThirdImage | ||
|
||
force = true | ||
|
||
images( | ||
buildFirstImage.getImageId(), | ||
buildSecondImage.getImageId(), | ||
buildThirdImage.getImageId() | ||
) | ||
} | ||
|
||
task removeImageAndCheckRemoval(type: DockerListImages) { | ||
dependsOn removeImages | ||
showAll = true | ||
dangling = true | ||
} | ||
""" | ||
|
||
when: | ||
BuildResult result = build('removeImageAndCheckRemoval') | ||
|
||
then: | ||
!result.output.contains("repository") | ||
} | ||
|
||
|
||
def "can remove image tagged in multiple repositories"() { | ||
buildFile << """ | ||
import com.bmuschko.gradle.docker.tasks.image.Dockerfile | ||
|
@@ -59,21 +128,21 @@ class DockerRemoveImageFunctionalTest extends AbstractGroovyDslFunctionalTest { | |
dependsOn dockerfile | ||
inputDir = file("build/docker") | ||
} | ||
|
||
task tagImage(type: DockerTagImage) { | ||
dependsOn buildImage | ||
repository = "repository" | ||
tag = "tag2" | ||
targetImageId buildImage.getImageId() | ||
} | ||
|
||
task tagImageSecondTime(type: DockerTagImage) { | ||
dependsOn tagImage | ||
repository = "repository" | ||
tag = "tag2" | ||
targetImageId buildImage.getImageId() | ||
} | ||
|
||
task removeImage(type: DockerRemoveImage) { | ||
dependsOn tagImageSecondTime | ||
force = true | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,29 +16,151 @@ | |
package com.bmuschko.gradle.docker.tasks.image; | ||
|
||
import com.github.dockerjava.api.command.RemoveImageCmd; | ||
import org.gradle.api.provider.ListProperty; | ||
import org.gradle.api.provider.Property; | ||
import org.gradle.api.provider.Provider; | ||
import org.gradle.api.tasks.Input; | ||
import org.gradle.api.tasks.Optional; | ||
|
||
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.concurrent.Callable; | ||
|
||
public class DockerRemoveImage extends DockerExistingImage { | ||
|
||
/** | ||
* Configures `--force` option when removing the images. | ||
*/ | ||
@Input | ||
@Optional | ||
public final Property<Boolean> getForce() { | ||
return force; | ||
} | ||
|
||
/** | ||
* Configures `--no-prune` option when removing the images. | ||
*/ | ||
@Input | ||
@Optional | ||
public final Property<Boolean> getNoPrune() { | ||
return noPrune; | ||
} | ||
|
||
private final Property<Boolean> force = getProject().getObjects().property(Boolean.class); | ||
private final Property<Boolean> noPrune = getProject().getObjects().property(Boolean.class); | ||
|
||
private final ListProperty<String> images = getProject().getObjects().listProperty(String.class); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have a look at |
||
|
||
/** | ||
* Sets the IDs or names of the images to be removed. | ||
* | ||
* @param images the names or IDs of the images. | ||
* @see #images(ListProperty) | ||
*/ | ||
public void images(List<String> images) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You will just need a method that returns a reference to the
|
||
this.images.set(images); | ||
} | ||
|
||
/** | ||
* Sets the IDs or names of the images to be removed. | ||
* | ||
* @param images the names or IDs of the images. | ||
* @see #images(ListProperty) | ||
*/ | ||
public void images(String... images) { | ||
this.images.set(Arrays.asList(images)); | ||
} | ||
|
||
/** | ||
* Sets the IDs or names of the images to be removed. | ||
* | ||
* @param images Image ID or name as {@link Callable} | ||
* @see #images(String...) | ||
* @see #images(ListProperty) | ||
*/ | ||
public void images(Callable<List<String>> images) { | ||
// TODO: How to do this, and is it necessary? | ||
// images(providers.provider(images)); | ||
} | ||
|
||
/** | ||
* Sets the IDs or names of the images to be removed. | ||
* | ||
* @param images Image ID or name as {@link Provider} | ||
* @see #images(String...) | ||
*/ | ||
public void images(ListProperty<String> images) { | ||
this.images.set(images); | ||
} | ||
|
||
// Overriding methods to provide a warning message. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove all of those methods and extend from |
||
/** | ||
* Sets the target image ID or name. | ||
* | ||
* @param imageId Image ID or name | ||
* @see #targetImageId(Callable) | ||
* @see #targetImageId(Provider) | ||
*/ | ||
@Override | ||
public void targetImageId(String imageId) { | ||
logWarning(); | ||
super.targetImageId(imageId); | ||
} | ||
|
||
/** | ||
* Sets the target image ID or name. | ||
* | ||
* @param imageId Image ID or name as Callable | ||
* @see #targetImageId(String) | ||
* @see #targetImageId(Provider) | ||
*/ | ||
@Override | ||
public void targetImageId(Callable<String> imageId) { | ||
logWarning(); | ||
super.targetImageId(imageId); | ||
} | ||
|
||
/** | ||
* Sets the target image ID or name. | ||
* | ||
* @param imageId Image ID or name as Provider | ||
* @see #targetImageId(String) | ||
* @see #targetImageId(Callable) | ||
*/ | ||
@Override | ||
public void targetImageId(Provider<String> imageId) { | ||
logWarning(); | ||
super.targetImageId(imageId); | ||
} | ||
|
||
private void logWarning() { | ||
getLogger().warn("Use property 'images' instead of 'targetImageId' when listing images to remove"); | ||
} | ||
|
||
@Override | ||
public void runRemoteCommand() { | ||
getLogger().quiet("Removing image with ID \'" + getImageId().get() + "\'."); | ||
RemoveImageCmd removeImageCmd = getDockerClient().removeImageCmd(getImageId().get()); | ||
java.util.Optional<String> imageId = java.util.Optional.ofNullable(this.getImageId().getOrNull()); | ||
List<String> imagesIds = this.images.get(); | ||
|
||
if (Boolean.TRUE.equals(force.getOrNull())) { | ||
removeImageCmd.withForce(force.get()); | ||
if (imageId.isPresent() && !imagesIds.isEmpty()) { | ||
throw new IllegalStateException("Project sets both properties 'images' and 'targetImageId', but only one is allowed. Please use 'images'."); | ||
} | ||
|
||
removeImageCmd.exec(); | ||
imageId.ifPresent(this::removeImage); | ||
imagesIds.forEach(this::removeImage); | ||
} | ||
|
||
private void removeImage(String imageId) { | ||
getLogger().quiet("Removing image with ID '{}'.", imageId); | ||
try (RemoveImageCmd removeImageCmd = getDockerClient().removeImageCmd(imageId)) { | ||
if (Boolean.TRUE.equals(force.getOrNull())) { | ||
removeImageCmd.withForce(force.get()); | ||
} | ||
|
||
if (Boolean.TRUE.equals(noPrune.getOrNull())) { | ||
removeImageCmd.withNoPrune(noPrune.get()); | ||
} | ||
removeImageCmd.exec(); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's really no need to upload test results, as you can view them with the help of a build scan. The build scan link is available at the end of each workflow run, on success and on failure. I'd say remove this change.