Repository navigation
fix(charm): remove unmanaged tmate containers before restarting the workload - #59
Merged
Merged
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The cleanup behavior is correctly implemented and tested; the remaining diagnostic-message ambiguity is non-blocking.
Review effort: Balanced
Findings: 1
What changed in this PR
Removes stray tmate containers before workload restarts to prevent port conflicts during upgrades and recovery.
Changes:
- Detects and removes containers using any tag or digest from the tmate image repository.
- Adds unit and integration coverage for cleanup and failure paths.
- Updates architecture, upgrade, and changelog documentation.
| File | Description |
|---|---|
src/tmate.py |
Implements repository-wide tmate container cleanup. |
tests/unit/test_tmate.py |
Tests matching, removal, ordering, and failures. |
tests/integration/test_upgrade.py |
Verifies unmanaged-container cleanup during refresh. |
docs/reference/charm-architecture.md |
Documents restart cleanup behavior. |
docs/how-to/upgrade.md |
Updates upgrade expectations. |
docs/changelog.md |
Records the user-facing fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Unused entries in .trivyignoreImage: ghcr.io/canonical/tmate-ssh-server:a0edf8f27d99ca10ec2b3cafa43ffe32feaea597-_1.1_amd64 The following CVEs are in |
cbartz
marked this pull request as ready for review
October 7, 2026 13:38
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Applicable spec: ISD-6336 (follow-up to #58)
Overview
Before (re)starting the workload, remove every container running a
ghcr.io/canonical/tmate-ssh-serverimage, not only the one named in the service unit. Unrelated containers are left alone.Rationale
Upgrading a 3-unit deployment to the #58 revision left two units in error: a tmate 0.1.1 container started outside the charm (Docker-generated name, up for 4 months) held port 10022, so the new 1.1 container could not bind and
ensure_daemon_runningtimed out. The charm only removed the container named in its unit, which no longer existed. Manually removing the stray container fixed both units immediately.Juju Events Changes
None.
install,update-statusandupgrade-charmall go throughensure_daemon_running, so all of them now clear stray tmate containers before restarting.Module Changes
tmate: newIMAGE_REPOSITORYconstant;_pull_image_and_remove_containerslists containers and force-removes those whose image belongs to that repository (any tag or digest), after pulling the image. Unit tests cover unmanaged, digest-pinned and unrelated containers, and listing/removal failures;test_upgradenow starts an unnamed 0.1.1 container on the baseline unit before refreshing. Docs and changelog updated.Library Changes
None.
Checklist
urgent,trivial,complex,documentation)AI-generated message.