Skip to content

Conversation

klmp200
Copy link

@klmp200 klmp200 commented Oct 7, 2025

When SR cleanup fails on linstor and glusterfs tests, a manual cleanup is needed.
Since tests removes software tools needed to interact with those technologies, we have to install them again to cleanup and it's annoying.

This PR ensures that those tools are still installed on the host machine when this scenario happens

@klmp200 klmp200 force-pushed the abartuc/sr-teardown branch 2 times, most recently from 3287421 to b10d6d5 Compare October 7, 2025 09:51
@stormi
Copy link
Member

stormi commented Oct 7, 2025

A PR title is important. It's our first contact with the change. Here, it looks like the title currently says the contrary of what the PR actually does :)

@klmp200 klmp200 changed the title Don't install sr tools on failed teardown Don't uninstall sr tools on failed teardown Oct 7, 2025
@klmp200
Copy link
Author

klmp200 commented Oct 7, 2025

A PR title is important. It's our first contact with the change. Here, it looks like the title currently says the contrary of what the PR actually does :)

Indeed, it does, I made a typo writing it ^^'
It was even contradicting the commit messages which is accurate with it's content :)

That's fixed

I'll add a proper description once I'm done with the content of the PR, it's not ready yet :)

@klmp200 klmp200 force-pushed the abartuc/sr-teardown branch from b10d6d5 to 46cd8ab Compare October 8, 2025 11:38
@klmp200 klmp200 requested review from Nambrok and glehmann October 8, 2025 12:14
@klmp200 klmp200 marked this pull request as ready for review October 8, 2025 12:14
Copy link
Member

@glehmann glehmann left a comment

Choose a reason for hiding this comment

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

LGTM

I had the same problem with xfsprogs yesterday. Could you also do that for the XFS SRs ?

@klmp200 klmp200 requested a review from glehmann October 10, 2025 12:31
@klmp200 klmp200 force-pushed the abartuc/sr-teardown branch from 51d6dd5 to db8d29c Compare October 14, 2025 08:57
@klmp200 klmp200 requested a review from stormi October 14, 2025 09:05
Copy link
Member

@glehmann glehmann left a comment

Choose a reason for hiding this comment

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

LGTM.
For the next PR, you should separate the type annotations and the actual code changes in multiple commits, to ease the review.

Leave glusterfs tools in place for manual cleanup after a failed SR cleanup
Also leave glusterfs volumes in place to avoid broken SR

Signed-off-by: Antoine Bartuccio <[email protected]>
Leave linstor tools in place for manual cleanup after a failed SR cleanup

Signed-off-by: Antoine Bartuccio <[email protected]>
Leave xfs tools in place for manual cleanup after a failed SR cleanup

Signed-off-by: Antoine Bartuccio <[email protected]>
@klmp200 klmp200 force-pushed the abartuc/sr-teardown branch from db8d29c to 5631935 Compare October 15, 2025 14:20
@klmp200
Copy link
Author

klmp200 commented Oct 15, 2025

After further testing and full documenting of the manual cleanup, I've discovered other steps that had to be skipped for glusterfs teardown.
Turns out that the underlying volume on each machine isn't managed by the driver and requires some additional manual steps.
Not skipping teardown steps creates a broken SR that can't be removed. This has been fixed :)

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.

4 participants