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

Add force tag when deleting images in version chooser #2459

Merged

Conversation

JoaoMario109
Copy link
Contributor

Closes #2431

@Williangalvani
Copy link
Member

Williangalvani commented Mar 14, 2024

will this actually alllow the api to delete the currently running image? would this kill the container?
can you try it at http://blueos.local:8081/v1.0/ui/#/default/main.delete_version ?

@patrickelectric patrickelectric added the merge-after-stable Should be merged only after next stable release label Mar 15, 2024
@JoaoMario109
Copy link
Contributor Author

JoaoMario109 commented Mar 15, 2024

will this actually alllow the api to delete the currently running image? would this kill the container? can you try it at http://blueos.local:8081/v1.0/ui/#/default/main.delete_version ?

I added the force option because it enables the removal of the image even in cases where a container was stopped but not terminated. Within our existing model, the BlueOS frontend initiates a delete operation only for images that are not currently running. Therefore, introducing the force option will not alter the existing behavior. However, it will provide a safeguard by ensuring that images can still be removed under certain conditions. For instance, if a container is manually stopped on the Raspberry Pi but not terminated, and a subsequent attempt is made to delete the image, without the force option, the image would remain as a dangling image, preventing its deletion.

@Williangalvani
Copy link
Member

will this actually alllow the api to delete the currently running image? would this kill the container? can you try it at http://blueos.local:8081/v1.0/ui/#/default/main.delete_version ?

I added the force option because it enables the removal of the image even in cases where a container was stopped but not terminated. Within our existing model, the BlueOS frontend initiates a delete operation only for images that are not currently running. Therefore, introducing the force option will not alter the existing behavior. However, it will provide a safeguard by ensuring that images can still be removed under certain conditions. For instance, if a container is manually stopped on the Raspberry Pi but not terminated, and a subsequent attempt is made to delete the image, without the force option, the image would remain as a dangling image, preventing its deletion.

I understand your point, my concern is that someone could use the API to delete the image from the currently running container, in that case I'm not sure what would happen.

BUT, after seeing the code, I see we already have a safeguard for that, so we're good =]

@patrickelectric patrickelectric merged commit b834032 into bluerobotics:master Mar 25, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-after-stable Should be merged only after next stable release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Add force remove when deleting local images
4 participants