Skip to content

Implement the new cleanup step#3904

Merged
psss merged 9 commits into
mainfrom
psss-cleanup-step
Jul 29, 2025
Merged

Implement the new cleanup step#3904
psss merged 9 commits into
mainfrom
psss-cleanup-step

Conversation

@psss
Copy link
Copy Markdown
Member

@psss psss commented Jul 21, 2025 •
edited
Loading

Introduce a separate cleanup step to handle guest cleanup and workdir pruning so that the finish step can be reserved for user finishing tasks only without having to always stop and remove guests.

Fix #1737.

Pull Request Checklist

  • implement the feature
  • extend the test coverage
  • update the specification
  • adjust plugin docstring
  • modify the json schema
  • mention the version
  • include a release note

@psss psss added step | cleanup Stuff related to the cleanup step breakage Change will change current behaviour labels Jul 21, 2025
@psss psss added this to the 1.54 milestone Jul 21, 2025
@psss psss added this to planning Jul 21, 2025
@github-project-automation github-project-automation Bot moved this to backlog in planning Jul 21, 2025
@psss psss moved this from backlog to implement in planning Jul 21, 2025
@psss psss mentioned this pull request Jul 23, 2025
Open
@psss psss force-pushed the psss-cleanup-step branch from 9026913 to 0c6a574 Compare July 23, 2025 21:30
@psss psss added the ci | full test Pull request is ready for the full test execution label Jul 23, 2025
@psss psss marked this pull request as ready for review July 23, 2025 21:32
graphite-app[bot]
graphite-app Bot reviewed Jul 23, 2025
View reviewed changes
Comment thread docs/releases.rst Outdated
@psss
Copy link
Copy Markdown
Member Author

psss commented Jul 23, 2025

@AthreyVinay, @vaibhavdaren, although this one is a bit longer, I think it might be worth if you review this pull request as it goes through the whole step implemtation and touches all relevant places. It could give you some good insight of how the step implementation, schema handling, command line options and docs generation works.

@psss psss moved this from implement to review in planning Jul 24, 2025
@psss psss force-pushed the psss-cleanup-step branch from 0c6a574 to ea81693 Compare July 24, 2025 09:35
martinhoyer
martinhoyer approved these changes Jul 24, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@martinhoyer martinhoyer left a comment

Choose a reason for hiding this comment

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

Awesome!

Took me some time to understand tbh, and am trying to come up with any feedback, but got nothing. Tested it briefly, with virtual provisioning and as a user, I very much appreciate, going forward, not accumulating 50 tmt VMs to clean manually from time to time :)

The "relationship" with tmt clean cli also seems logical.

fwiw, I like it very much, but am not 100% confident around these parts of code, so my review have a somewhat limited value.

@happz
Copy link
Copy Markdown
Contributor

happz commented Jul 24, 2025

Tested it briefly, with virtual provisioning and as a user, I very much appreciate, going forward, not accumulating 50 tmt VMs to clean manually from time to time :)

Shouldn't the current finish clean those as well? There are certainly no VMs piling up on my workstation.

happz
happz reviewed Jul 24, 2025
View reviewed changes
Comment thread tmt/steps/cleanup/__init__.py Outdated
happz
happz reviewed Jul 24, 2025
View reviewed changes
Comment thread tmt/steps/cleanup/__init__.py Outdated
@psss
Copy link
Copy Markdown
Member Author

psss commented Jul 24, 2025

Shouldn't the current finish clean those as well? There are certainly no VMs piling up on my workstation.

Yes, the current finish implementation definitely takes care of the guest cleanup. The functionality was only moved.

@psss
Copy link
Copy Markdown
Member Author

psss commented Jul 24, 2025

/tests/provision/container/toolbox seems to be failing repeatedly but the problem does not seem to be related.

@psss psss force-pushed the psss-cleanup-step branch from 4c3e09e to 33a4603 Compare July 24, 2025 13:36
@psss psss requested a review from happz July 24, 2025 13:50
therazix
therazix approved these changes Jul 25, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@therazix therazix left a comment

Choose a reason for hiding this comment

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

LGTM

@psss psss force-pushed the psss-cleanup-step branch from 33a4603 to 2df8867 Compare July 28, 2025 20:44
@psss psss moved this from review to merge in planning Jul 28, 2025
psss added 9 commits July 29, 2025 09:36
@psss
Implement the new cleanup step
fa4b2b6 
Introduce a separate `cleanup` step to handle guest cleanup and
workdir pruning so that the `finish` step can be reserved for user
finishing tasks only without having to always stop and remove
guests.

Fix #1737.
@psss
Remove the new guest and workdir options
a233d0b 
Will be covered later in a separate pull request.
See #3911.
@psss
Add docs, spec, schema, release note
5c5e146
@psss
Typing plus some other adjustments
e019068
@psss
Adjust test coverage
eb2a554
@psss
No actions during the cleanup step at all
cf51b39
@psss
Fix a typo
50674f5
@psss
A couple more test adjustments, copy & paste typos
70ce282
@psss
Rename existing prunning test, add basic coverage for guest cleanup
176ae73
@psss psss force-pushed the psss-cleanup-step branch from 2df8867 to 176ae73 Compare July 29, 2025 07:36
AthreyVinay
AthreyVinay approved these changes Jul 29, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@AthreyVinay AthreyVinay left a comment

Choose a reason for hiding this comment

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

Thanks @psss for looping me in, looks great, and yeah, definitely a solid way to get familiar with the different aspects.

@psss psss merged commit d8abc46 into main Jul 29, 2025
17 checks passed
@psss psss deleted the psss-cleanup-step branch July 29, 2025 09:51
@github-project-automation github-project-automation Bot moved this from merge to done in planning Jul 29, 2025
@psss psss self-assigned this Jul 29, 2025
psss added a commit that referenced this pull request Jul 30, 2025
@psss
Add the new cleanup step to tmt try as well
a12ec50 
The `tmt try` command needs to be updated as well to include the new
`cleanup` command. Without this change provisioned guests would not be
stopped and removed upon quit from the `tmt try` session.

Related: #3904
@psss psss mentioned this pull request Jul 30, 2025
Merged
2 tasks
psss added a commit that referenced this pull request Jul 30, 2025
@psss
Add the new cleanup step to tmt try as well (#3933)
8076f74 
The `tmt try` command needs to be updated as well to include the new
`cleanup` command. Without this change provisioned guests would not be
stopped and removed upon quit from the `tmt try` session.

Related: #3904
bajertom pushed a commit that referenced this pull request Aug 11, 2025
@psss @bajertom
Implement the new cleanup step (#3904)
ad1e49c 
Introduce a separate `cleanup` step to handle guest cleanup and workdir
pruning so that the `finish` step can be reserved for user finishing
tasks only without having to always stop and remove guests.

Fix #1737.
bajertom pushed a commit that referenced this pull request Aug 11, 2025
@psss @bajertom
Add the new cleanup step to tmt try as well (#3933)
3b3ef9a 
The `tmt try` command needs to be updated as well to include the new
`cleanup` command. Without this change provisioned guests would not be
stopped and removed upon quit from the `tmt try` session.

Related: #3904
@psss psss mentioned this pull request Oct 1, 2025
Open
@coiby coiby mentioned this pull request Nov 23, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@graphite-app graphite-app[bot] graphite-app[bot] left review comments

@happz happz happz approved these changes

@AthreyVinay AthreyVinay AthreyVinay approved these changes

@therazix therazix therazix approved these changes

@lukaszachy lukaszachy Awaiting requested review from lukaszachy

@vaibhavdaren vaibhavdaren Awaiting requested review from vaibhavdaren

+1 more reviewer

@martinhoyer martinhoyer martinhoyer approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@psss psss

Labels

breakage Change will change current behaviour ci | full test Pull request is ready for the full test execution step | cleanup Stuff related to the cleanup step

Projects

planning
Status: done

Milestone

1.54

Development

Successfully merging this pull request may close these issues.

Introduce a new step for cleanup tasks

5 participants

@psss @happz @AthreyVinay @martinhoyer @therazix