Skip to content

Remove unused SDK volumes#457

Merged
dmitry-lyfar merged 4 commits into
mainfrom
feature/sdk-volume-gc
Aug 20, 2025
Merged

Remove unused SDK volumes#457
dmitry-lyfar merged 4 commits into
mainfrom
feature/sdk-volume-gc

Conversation

@dmitry-lyfar
Copy link
Copy Markdown
Collaborator

@dmitry-lyfar dmitry-lyfar commented Aug 12, 2025

Description

Implement tracking and removing unused SDK volumes. The implementation uses a clean up handler for the tasks of install-sdk or unregister-sdk type. Once a change with such a task is complete, it may indicate that an SDK volume became unused due to:

  • workshop was removed
  • SDK volume was detached from a workshop as a result of a refresh change and is no longer used by any other workshop
  • workshop launch failed

If an SDK volume is found to be unused for longer than the cool down time (1 hr), it will be removed from the system. This is done to prevent workshopd to remove volumes that have a high chance to be immediately reused on a following refresh or launch.

Self-review quick check

  • Make decisions that cost a lot to reverse explicit in the PR description.
  • Avoid nested conditions.
  • Delete dead code and redundant comments.
  • Normalise symmetries by sticking to doing identical things identically.
// one way to handle errors
if err := f(); err != nil {
   ...
}

// one way to handle multiple returns
val, err := f()
if err != nil {
   ...
}
...
  • Check that coupled code elements, files, and directories are adjacent. For example, test data is stored as close as possible to a test.
  • Put variable declaration and initialisation together.
  • Divide large expressions into digestable and self-explanatory ones. Use multiple variables if required.
  • Put a blank line between two logically different chunks of code.
  • Follow the style guide for new error messages.

Docs

  • I have checked and added or updated relevant documentation.
  • I have checked and added or updated relevant release notes.
  • I have included the technical author in the review.

Or:

  • I confirm the PR has no implications for documentation.

@dmitry-lyfar dmitry-lyfar changed the base branch from feature/try-sdks to main August 12, 2025 06:02
@dmitry-lyfar dmitry-lyfar changed the title Feature/sdk volume gc Remove unused SDK volumes Aug 12, 2025
@dmitry-lyfar dmitry-lyfar force-pushed the feature/sdk-volume-gc branch from 4f8bccd to d1e9907 Compare August 13, 2025 00:05
@dmitry-lyfar dmitry-lyfar self-assigned this Aug 13, 2025
@dmitry-lyfar dmitry-lyfar force-pushed the feature/sdk-volume-gc branch 2 times, most recently from d3c62a1 to dbf6261 Compare August 14, 2025 05:56
@dmitry-lyfar dmitry-lyfar requested a review from Copilot August 14, 2025 06:00

This comment was marked as outdated.

@dmitry-lyfar dmitry-lyfar force-pushed the feature/sdk-volume-gc branch 2 times, most recently from 3a5bf03 to efd2882 Compare August 15, 2025 01:43
@dmitry-lyfar dmitry-lyfar requested a review from Copilot August 15, 2025 03:35

This comment was marked as outdated.

@dmitry-lyfar dmitry-lyfar force-pushed the feature/sdk-volume-gc branch 6 times, most recently from a0900e6 to ee5dce8 Compare August 18, 2025 23:35
@dmitry-lyfar dmitry-lyfar requested a review from Copilot August 18, 2025 23:36

This comment was marked as outdated.

@dmitry-lyfar dmitry-lyfar force-pushed the feature/sdk-volume-gc branch from ee5dce8 to a9c06dc Compare August 19, 2025 01:14
@dmitry-lyfar dmitry-lyfar requested a review from Copilot August 19, 2025 01:15

This comment was marked as outdated.

@dmitry-lyfar dmitry-lyfar marked this pull request as ready for review August 19, 2025 01:22
@dmitry-lyfar dmitry-lyfar force-pushed the feature/sdk-volume-gc branch 2 times, most recently from ffb0d11 to 4d2d1b2 Compare August 19, 2025 01:26
Comment thread internal/workshop/fakebackend/backend.go Outdated
Comment thread internal/workshop/lxd/tests/integration/workshop_test.go Outdated
Comment thread internal/workshop/lxd/tests/integration/workshop_test.go Outdated
Comment thread internal/workshop/lxd/tests/integration/workshop_test.go Outdated
Comment thread internal/workshop/fakebackend/backend.go Outdated
Comment thread internal/overlord/sdkstate/handlers_test.go
Comment thread internal/overlord/sdkstate/handlers_test.go Outdated
Comment thread internal/overlord/sdkstate/handlers.go Outdated
Comment thread internal/overlord/sdkstate/handlers.go Outdated
Comment thread internal/overlord/sdkstate/handlers.go
@dmitry-lyfar dmitry-lyfar force-pushed the feature/sdk-volume-gc branch from a22e45a to 803a764 Compare August 19, 2025 23:51
@dmitry-lyfar dmitry-lyfar requested a review from Copilot August 20, 2025 00:57
@dmitry-lyfar dmitry-lyfar force-pushed the feature/sdk-volume-gc branch from 803a764 to 11a81ee Compare August 20, 2025 00:58
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements tracking and removal of unused SDK volumes through a cooldown-based cleanup mechanism. The implementation uses cleanup handlers for install-sdk and unregister-sdk tasks to identify unused volumes after workshop removal, SDK detachment, or launch failures, with a 1-hour cooldown period before deletion.

  • Adds volume usage tracking by mapping volumes to workshops/projects
  • Implements cleanup handlers that delete unused SDK volumes after cooldown period
  • Refactors volume data structures to separate setup information from usage tracking

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
internal/workshop/workshop.go Removes unused WorkshopName function
internal/workshop/lxd/tests/integration/workshop_test.go Updates tests for new volume structure and adds volume usage tracking tests
internal/workshop/lxd/tests/helper/helper.go Renames parameter for clarity in MockSdkTarball
internal/workshop/lxd/lxd_backend_volume.go Implements volume usage tracking and parsing workshop/project info from URLs
internal/workshop/lxd/lxd_backend_project.go Uses relocated workshopName function
internal/workshop/lxd/lxd_backend.go Adds helper functions for workshop name parsing
internal/workshop/fakebackend/backend.go Updates fake backend to support new volume tracking structure
internal/workshop/backend.go Splits VolumeInfo into VolumeSetup and adds workshop tracking
internal/overlord/workshopstate/request.go Updates for new volume structure
internal/overlord/workshopstate/handlers.go Updates for new volume structure
internal/overlord/sdkstate/manager.go Adds cleanup handlers for SDK volume management
internal/overlord/sdkstate/handlers_test.go Adds comprehensive tests for volume cleanup behavior
internal/overlord/sdkstate/handlers.go Implements the core cleanup logic with cooldown tracking
internal/overlord/ifacestate/ifacemgr_test.go Updates test for new volume structure
internal/overlord/hookstate/handlers_test.go Updates tests for new volume structure
internal/daemon/api_workshops_test.go Adds volume cleanup verification to integration tests
internal/daemon/api_workshops.go Sets error status on failed workshop operations
.github/workflows/cover.yaml Removes unused branchname parameter

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread internal/workshop/lxd/lxd_backend_volume.go
Comment thread internal/overlord/sdkstate/handlers.go
Comment thread internal/overlord/sdkstate/handlers.go
@dmitry-lyfar dmitry-lyfar merged commit f66aee0 into main Aug 20, 2025
7 checks passed
@dmitry-lyfar dmitry-lyfar deleted the feature/sdk-volume-gc branch August 20, 2025 01:01
@github-actions
Copy link
Copy Markdown

TICS Quality Gate

✔️ Passed

workshop

All conditions passed

See the results in the TICS Viewer

The following files have been checked for this project
  • internal/daemon/api_workshops.go
  • internal/overlord/sdkstate/handlers.go
  • internal/overlord/sdkstate/manager.go
  • internal/overlord/workshopstate/handlers.go
  • internal/overlord/workshopstate/request.go
  • internal/workshop/backend.go
  • internal/workshop/fakebackend/backend.go
  • internal/workshop/lxd/lxd_backend_project.go
  • internal/workshop/lxd/lxd_backend_volume.go
  • internal/workshop/lxd/lxd_backend.go
  • internal/workshop/workshop.go

Automatic-tests / code-coverage / TICS GitHub Action

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.

3 participants