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

Consider not trying to uninstall helm releases that weren't successfully installed #27

Open
craigwalton-dsit opened this issue Dec 19, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@craigwalton-dsit
Copy link
Collaborator

E.g. inspect asks us to install 100 releases, we queue them up and before they're finished, user ctrl+c's and inspect asks us to uninstall them all, then we show a bunch of errors. Can we check if the helm releases are in our "tracked" list before attempting uninstall?

@craigwalton-dsit craigwalton-dsit added the enhancement New feature or request label Dec 19, 2024
@craigwalton-dsit
Copy link
Collaborator Author

craigwalton-dsit commented Jan 9, 2025

Currently, we have

    async def install(self, release: Release) -> None:
        """
        Installs a release and tracks it for eventual cleanup.

        Args:
          release (Release): The release to install and track.
        """
        # Track the release regardless of the install result.
        self._installed_releases.append(release)
        await release.install()

We add the release to self._installed_releases() before we've actually successfully installed it because:

  1. The install might fail e.g. Helm timeout in which case, an exception will be raised, but the release won't be uninstalled by Helm (unless we passed --atomic flag to helm install)
  2. If the install gets cancelled by Inspect (e.g. user hits ctrl+c or maybe even if there's an eval failure) then an asyncio.CancelledError is raised. We don't really know at what stage the cancellation was at though - was it whilst waiting for the semaphore, or did we actually get as far as starting helm install?

In case 1, it is correct to add it to self._installed_releases.
In case 2, if the cancellation happened before calling helm install we could avoid adding it to the list. But determining that is not trivial.

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

No branches or pull requests

1 participant