Skip to content
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

delete image if it exists #101

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,13 @@ public class DockerSteps {
*
* @param image fully qualified image name in <code>name:tag</code> representation
*/
@Given("the docker image {word} does not exist on the device")
@Given("deleted the docker image {word} if exists on the device")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be - "delete the docker image {word} if it exists on the device"

public void checkDockerImageIsMissing(String image) {
Predicate<Set<String>> predicate = Set::isEmpty;
checkDockerImagePresence(image, predicate.negate(),
"The image " + image + " is already on the device. Please remove the image and try again.");
if (isDockerImagePresence(image, predicate.negate())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To check if the image is present or not, predicate should not be negated here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry.. why? There is not logic change in here. Instead of error out the code. I just return boolean result?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Earlier it was going to throw error of the image exists, but now you are returning true if it exists.

removeDockerImage(image);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the original ticket: Is there any risk to customer data? What if the customer has a Docker image installed with the same name?

}

}

@Then("I can check that the docker image {word} exists on the device")
Expand All @@ -65,18 +67,30 @@ public void removeDockerImage(String image) {
LOGGER.debug("Removed docker image {}: {}", image, result);
}

private void checkDockerImagePresence(String image, Predicate<Set<String>> validity, String message) {
private boolean isDockerImagePresence(String image, Predicate<Set<String>> validity) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name of this method needs to change. I suggest naming it "checkPresence" since it is taking in a predicate.

// This could be improved by using the API on a local host.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the extra line if not needed

Set<String> parts = new HashSet<>(Arrays.stream(image.split(":")).collect(Collectors.toSet()));
String[] result = platform.commands().executeToString(CommandInput.builder()
.line("docker").addArgs("images")
.build())
.split("\\r?\\n");

Arrays.stream(result)
.map(String::trim)
.flatMap(line -> Arrays.stream(line.split("\\s+")))
.forEach(parts::remove);

if (!validity.test(parts)) {
return false;
}

return true;
}

private void checkDockerImagePresence(String image, Predicate<Set<String>> validity, String message) {

if (!isDockerImagePresence(image, validity)) {
throw new IllegalStateException(message);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Feature: Greengrass V2 Docker Component

@Docker @IDT
Scenario: I can deploy Docker containers as Greengrass Components
Given the docker image amazon/amazon-ec2-metadata-mock:v1.9.0 does not exist on the device
Given deleted the docker image amazon/amazon-ec2-metadata-mock:v1.9.0 if exists on the device
And I create a Greengrass deployment with components
| DockerHubAmazonContainer | classpath:/greengrass/component/recipes/DockerHubAmazonContainer.yaml |
When I deploy the Greengrass deployment configuration
Expand Down