Skip to content

PC: Move cleanup to dedicated always_run test module#25182

Merged
pdostal merged 7 commits intoos-autoinst:masterfrom
pdostal:new_cleanup
Apr 8, 2026
Merged

PC: Move cleanup to dedicated always_run test module#25182
pdostal merged 7 commits intoos-autoinst:masterfrom
pdostal:new_cleanup

Conversation

@pdostal
Copy link
Copy Markdown
Member

@pdostal pdostal commented Apr 1, 2026

@pdostal pdostal marked this pull request as draft April 1, 2026 17:41
@os-autoinst os-autoinst deleted a comment from github-actions bot Apr 1, 2026
@pdostal pdostal force-pushed the new_cleanup branch 2 times, most recently from 05cf8dc to 787edc4 Compare April 1, 2026 18:24
@os-autoinst os-autoinst deleted a comment from github-actions bot Apr 2, 2026
@pdostal pdostal force-pushed the new_cleanup branch 2 times, most recently from aacca22 to 51b9617 Compare April 2, 2026 08:12
Copy link
Copy Markdown
Contributor

@grisu48 grisu48 left a comment

Choose a reason for hiding this comment

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

Minor nitpicks plus two comments:

  1. Perhaps we want to keep ssh_interactive_end.pm as a module which terminates the ssh tunnels but doesn't do the instance cleanup. Leaving resources dangling is technical debt that we can avoid here
  2. I suggest to rename publiccloud/cleanup.pm to publiccloud/destroy.pm as it follows the terraform nomenclature. I think this makes it cleared from the module name what is going to happen, but that's just a suggestion.

@os-autoinst os-autoinst deleted a comment from github-actions bot Apr 2, 2026
@os-autoinst os-autoinst deleted a comment from github-actions bot Apr 2, 2026
@os-autoinst os-autoinst deleted a comment from github-actions bot Apr 2, 2026
@os-autoinst os-autoinst deleted a comment from github-actions bot Apr 2, 2026
@os-autoinst os-autoinst deleted a comment from github-actions bot Apr 2, 2026
Copy link
Copy Markdown
Contributor

@alvarocarvajald alvarocarvajald left a comment

Choose a reason for hiding this comment

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

Changes generally LGTM, but I think some YAML schedules are missing here. For example sles4sap/publiccloud/qesap_terraform gets loaded from:

  • sles4sap_gnome_saptune.yaml
  • sles4sap_ibsm_embargo_check.yml
  • sles4sap_gnome_saptune_product.yaml
  • sles4sap_gnome_saptune_maintenance.yaml

@os-autoinst os-autoinst deleted a comment from github-actions bot Apr 7, 2026
@pdostal
Copy link
Copy Markdown
Member Author

pdostal commented Apr 7, 2026

Changes generally LGTM, but I think some YAML schedules are missing here. For example sles4sap/publiccloud/qesap_terraform gets loaded from:

I didn't find any. Can you please help me with that? See:

$ rg ssh_interactive_end schedule/sles4sap/
$ 

@pdostal pdostal marked this pull request as ready for review April 7, 2026 13:56
Copy link
Copy Markdown
Contributor

@mpagot mpagot left a comment

Choose a reason for hiding this comment

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

LGTM

@alvarocarvajald
Copy link
Copy Markdown
Contributor

Changes generally LGTM, but I think some YAML schedules are missing here. For example sles4sap/publiccloud/qesap_terraform gets loaded from:

I didn't find any. Can you please help me with that? See:

This VR covers it: https://pdostal-workbench.qe.prg2.suse.org/tests/341

Copy link
Copy Markdown
Contributor

@alvarocarvajald alvarocarvajald left a comment

Choose a reason for hiding this comment

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

LGTM

@pdostal pdostal merged commit fafe56d into os-autoinst:master Apr 8, 2026
13 checks passed
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.

5 participants