Skip to content

fix: #453 remove profile image #873

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

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Conversation

timcadman
Copy link
Contributor

@timcadman timcadman commented Jul 8, 2025

Closes #453

What's changed:

  • Stopping a profile stops and removes the container
  • Deleting a profile stops and removes the container, and removes the image from disk
  • Starting a profile checks with the image associated with that profile has been updated, and if it has removes the previous image from disk.

Implementation

It turned out to be quite tricky to reliably get the docker imageID based on the tags, so instead I've added a new slot to the ProfileConfig class which stores this information the first time the profile is started. It is then updated every time the profile is restarted in case it has changed, and this value is then used to delete the image from disk.

How to test:

  • Create an account in docker hub if you don't already have one
  • In terminal, log in to docker using docker login
  • Create a folder to hold test docker images and navigate to that folder

Run the following:

touch build-and-push.sh
chmod +x build-and-push.sh

Edit file eg with

vim build-and-push.sh

Now add this script, which will can be used to create a very minimal docker image for testing (note you need to change 'timmyjc' to your docker usename)

#!/bin/bash

VERSION="$1"

if [ -z "$VERSION" ]; then
  echo "Usage: $0 <version>"
  exit 1
fi

IMAGE_NAME="timmyjc/mytest"

# Create Dockerfile
cat <<EOF > Dockerfile
FROM alpine
CMD ["sh", "-c", "while true; do echo Hello from v$VERSION; sleep 30; done"]
EOF

# Build image with version tag
docker build -t $IMAGE_NAME:v$VERSION .

# Tag it as latest
docker tag $IMAGE_NAME:v$VERSION $IMAGE_NAME:latest

# Push both tags
docker push $IMAGE_NAME:v$VERSION
docker push $IMAGE_NAME:latest

Save and exit

:wq

Now create a new image using:

./build-and-push.sh 0.0.1

Check imageID, and remove local image so it is easier to test

 docker images | grep timmyjc
 docker rmi -f [imageID of created image]

Test that deleting the profile still works if the image has never been pulled

  • Now start Armadillo in local host, login, and go to the profiles page of the UI
  • Add a new profile in the UI with image [yourusername]/mytest:latest
  • In terminal , run docker images | grep [yourusername] - the image shouldn't show because the profile has been started
  • Delete the profile in the UI
  • Check that logs say: INFO o.m.armadillo.profile.DockerService - No image ID provided; skipping image removal

Test that the image will be deleted when the profile has been pulled and profile is running

  • Add a new profile in the UI with image [yourusername]/mytest:latest
  • Start profile, wait for it to start
  • Run docker images | grep [yourusername] - you should now see the image listed
  • Click the delete button in the UI
  • Check that you see these lines in the log:
    o.m.armadillo.profile.DockerService - Removed image tag '[yourusername]/mytest:latest'
    o.m.armadillo.profile.DockerService - Removed image ID 'sha256:xxx' from local Docker cache
  • Run docker images | grep [yourusername] - check there are no images still stored

Test that the image will be deleted when the profile has been pulled and profile is stopped

  • Add a new profile in the UI with image [yourusername]/mytest:latest
  • Start profile, wait for it to start
  • Stop profile
  • Run docker images | grep [yourusername] - you should now see the image listed
  • Delete profile
  • Check that these lines are displayed in the log:
    o.m.armadillo.profile.DockerService - Removed image tag '[yourusername]/mytest:latest'
    o.m.armadillo.profile.DockerService - Removed image ID 'sha256:xxx from local Docker cache`

Test that the image will be deleted when the profile is updated with a new image

  • Add a new profile in the UI with image [yourusername]/mytest:latest
  • Start profile, wait for it to start
  • Stop profile
  • In terminal, build and push a new docker image with an updated version:
./build-and-push.sh 0.0.2
  • Run docker images | grep [yourusername]
  • In the UI, restart the profile
  • Check in the log that you see the messages:
    INFO o.m.armadillo.profile.DockerService - Image ID for profile 'test' changed from 'sha256:xxx' to 'sha256:xxx' INFO o.m.armadillo.profile.DockerService - Removed image ID 'sha256:xxx`
  • Run docker images | grep [yourusername] - you should still see 'latest', but check that the imageID has changed from before and after the restart

@timcadman timcadman self-assigned this Jul 10, 2025
Comment on lines 160 to 164
LOG.info(
"Image ID for profile '{}' changed: {} -> {}",
profileName,
previousImageId,
currentImageId);

Check notice

Code scanning / SonarCloud

Logging should not be vulnerable to injection attacks

<!--SONAR_ISSUE_KEY:AZf0E7dVbcp79pO4O49i-->Change this code to not log user-controlled data. <p>See more on <a href="https://sonarcloud.io/project/issues?id=org.molgenis%3Aarmadillo-service&issues=AZf0E7dVbcp79pO4O49i&open=AZf0E7dVbcp79pO4O49i">SonarQube Cloud</a></p>
currentImageId);
removeImageIfUnused(previousImageId);
} else {
LOG.info("Image ID for profile '{}' unchanged: {}", profileName, currentImageId);

Check notice

Code scanning / SonarCloud

Logging should not be vulnerable to injection attacks

<!--SONAR_ISSUE_KEY:AZf0E7dVbcp79pO4O49j-->Change this code to not log user-controlled data. <p>See more on <a href="https://sonarcloud.io/project/issues?id=org.molgenis%3Aarmadillo-service&issues=AZf0E7dVbcp79pO4O49j&open=AZf0E7dVbcp79pO4O49j">SonarQube Cloud</a></p>
Copy link

@timcadman timcadman requested a review from marikaris July 14, 2025 10:17
@marikaris marikaris changed the title Chore/remove profile image fix: #453 remove profile image Jul 15, 2025
Comment on lines +155 to +175

String previousImageId = profileConfig.getLastImageId();
String currentImageId = dockerClient.inspectContainerCmd(containerName).exec().getImageId();

profileService.updateLastImageId(profileName, currentImageId);

String safeProfileName = StringEscapeUtils.escapeJava(profileName);
String safePrevImageId = StringEscapeUtils.escapeJava(previousImageId);
String safeCurrImageId = StringEscapeUtils.escapeJava(currentImageId);

if (previousImageId != null && !previousImageId.equals(currentImageId)) {
LOG.info(
"Image ID for profile '{}' changed from '{}' to '{}'",
safeProfileName,
safePrevImageId,
safeCurrImageId);
removeImageIfUnused(previousImageId);
} else {
LOG.info(
"Image ID for profile '{}' unchanged (still '{}')", safeProfileName, safeCurrImageId);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd move this to a separate function

@marikaris
Copy link
Collaborator

marikaris commented Jul 17, 2025

Hey! I've done a simple functional test to start with (not the extensive tests you described, I had some trouble login in to dockerhub via terminal).

  • I wanted to cleanup two of my existing profiles in armadillo, gypsum and neutron
  • I first checked in Docker desktop if the images were there, they were
  • I deleted both profiles via the UI
  • I checked if the images were gone, they were not
  • The logs show:
    2025-07-17 13:53:20.874 [http-nio-8080-exec-1|39CF9297] INFO o.m.armadillo.profile.DockerService - Couldn't remove container 'gypsum' because it doesn't exist 2025-07-17 13:53:20.874 [http-nio-8080-exec-1|39CF9297] INFO o.m.armadillo.profile.DockerService - No image ID provided; skipping image removal 2025-07-17 13:53:20.883 [http-nio-8080-exec-1|39CF9297] INFO o.m.armadillo.audit.AuditLogger - AuditEvent [timestamp=2025-07-17T11:53:20.883457Z, [email protected], type=DELETE_PROFILE, data={profile=gypsum, sessionId=13E9DAADBC5E4825DAC3398139CF9297, roles=[ROLE_SU, ROLE_DATASHIELD_RESEARCHER]}] 2025-07-17 13:53:20.918 [http-nio-8080-exec-7|39CF9297] INFO o.m.armadillo.audit.AuditLogger - AuditEvent [timestamp=2025-07-17T11:53:20.918583Z, [email protected], type=LIST_PROFILES, data={roles=[ROLE_SU, ROLE_DATASHIELD_RESEARCHER], sessionId=13E9DAADBC5E4825DAC3398139CF9297}] 2025-07-17 13:54:27.504 [http-nio-8080-exec-8|39CF9297] INFO o.m.armadillo.profile.DockerService - Failed to stop profile 'stoppingContainer has not profileName: neutron' because it doesn't exist 2025-07-17 13:54:27.508 [http-nio-8080-exec-8|39CF9297] INFO o.m.armadillo.profile.DockerService - Couldn't remove container 'neutron' because it doesn't exist 2025-07-17 13:54:27.508 [http-nio-8080-exec-8|39CF9297] INFO o.m.armadillo.profile.DockerService - No image ID provided; skipping image removal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Armadillo doesnt clean up unused profile volumes
2 participants