-
Notifications
You must be signed in to change notification settings - Fork 82
Cleanup image cache in update compass script #2744
Conversation
/retest |
@@ -9,4 +9,7 @@ set -o errexit | |||
CURRENT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" | |||
INSTALLATION_DIR=${CURRENT_DIR}/../../ | |||
|
|||
docker exec k3d-kyma-agent-0 sh -c "ctr images rm \$(ctr image list -q name~=eu.gcr.io/kyma-project/incubator)" |
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.
From the man page - ctr is an unsupported debug and administrative client for interacting with the containerd daemon. Because it is unsupported, the commands, options, and operations are not guaranteed to be backward compatible or stable from release to release of the containerd project.
Could be because of that in the future releases to encounter the same problem that this PR fix? the drawback of the problem is it's tricky to be found
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.
Wouldn't we have to explicitly bump the k3d version? If it updates and the command fails, then it's better to have the job fail than to give false positives/negatives. Also from what I understand is that they do not gurantee the backwards compatibility, but it's not very likely to happen. Plus in the logs we can see when and which images it has removed.
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.
Also considering this is a tool for managing the containerd images, I assume there will always be a way to remove an image, so worst case scenario, it would require to change the script for cleaning up the images.
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.
That's valid, my point was if the "breaking change" is not a breaking but rather the command not working as expected or misbehaves and we encounter the same issue which is hard to notice in first glance. If the command directly fail that okay because we'll find out something is wrong and as you mentioned most probably there will be another option to fix it/delete the image.
I just saw that the ctr
is unsupported and was wondering if will be an impact for us. But if you think there is no such probability or the impact is low and we can fast determine what's happening we can use it.
/test pre-main-compass-gke-benchmark |
1 similar comment
/test pre-main-compass-gke-benchmark |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DimitarPetrov, PetarTodorovv The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold cancel |
Description
Changes proposed in this pull request:
Pull Request status
chart/compass/values.yaml
is updated