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

CSS-4747 Remove the jimm snap before attempting to install it. #998

Merged
merged 3 commits into from
Jul 13, 2023

Conversation

alesstimec
Copy link
Collaborator

Description

The machine charm needs to remove the jimm snap (if it is installed) before installing the snap from the jimm-snap resource.

Engineering checklist

Check only items that apply

  • Documentation updated
  • Covered by unit tests
  • Covered by integration tests

Test instructions

Notes for code reviewers

Copy link
Contributor

@ale8k ale8k left a comment

Choose a reason for hiding this comment

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

Presuming you've tested it and it works lgtm.

Copy link
Contributor

@kian99 kian99 left a comment

Choose a reason for hiding this comment

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

lgtm with a thought

@@ -275,6 +275,9 @@ def _install_snap(self):
if not path:
self.unit.status = BlockedStatus("waiting for jimm-snap resource")
return
# remove the jimm snap if it is already installed.
self._snap("remove", "jimm")
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we not do a snap refresh instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

jimm snap is not installed from snapstore.. but instead from a resource.. i don't see a way to say "refresh from this path" using snaps

@alesstimec alesstimec merged commit 69f4cd3 into canonical:main Jul 13, 2023
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.

4 participants