Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Retry if RPM lock is temporarily unavailable #62204

Merged
merged 22 commits into from
Aug 22, 2022
Merged

Retry if RPM lock is temporarily unavailable #62204

merged 22 commits into from
Aug 22, 2022

Conversation

witekest
Copy link
Contributor

@witekest witekest commented Jun 22, 2022 •
edited
Loading

What does this PR do?

Wait and retry if Zypper fails acquiring RPM lock file.

What issues does this PR fix or reference?

Fixes: SUSE/spacewalk#18163

Previous Behavior

Reportedly highstate was failing in case when RPM lock file was used by some other process.

New Behavior

Wait and retry if Zypper fails acquiring RPM lock file.

Merge requirements satisfied?

Commits signed with GPG?

No

@witekest witekest requested a review from a team as a code owner June 22, 2022 12:54
@witekest witekest requested review from garethgreenaway and removed request for a team June 22, 2022 12:54
@welcome
Copy link

welcome bot commented Jun 22, 2022

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at [email protected]. We’re glad you’ve joined our community and look forward to doing awesome things with you!

@witekest witekest force-pushed the rpm_lock branch 2 times, most recently from d18caf4 to af7285b Compare June 22, 2022 14:39
meaksh
meaksh suggested changes Jun 22, 2022
View reviewed changes
Copy link
Contributor

@meaksh meaksh left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @witekest ! 👍

I did some comments spotting some things I think we should improve.

salt/modules/zypperpkg.py Outdated Show resolved Hide resolved
salt/modules/zypperpkg.py Outdated Show resolved Hide resolved
@witekest witekest requested a review from meaksh June 28, 2022 15:04
@witekest
Copy link
Contributor Author

I've split now the handling of Zypper and RPM locks as the latter does not store information about the locking process. I've tested it by manually acquiring exclusive lock over the RPM lock file. Works as expected and retries the installation until the lock gets released.

witekest and others added 5 commits July 11, 2022 14:47
@witekest
Retry if RPM lock is temporarily unavailable
cc18709
@witekest
Add changelog entry
813f41a
@witekest
Check if RPM file lock can get acquired
3a010d9
@witekest @meaksh
Add complete err object to data[]
003a1c2 
Co-authored-by: Pablo Suárez Hernández <[email protected]>
@witekest
Simple handling of RPM lock file
c17c8c9 
Differently as Zypper RPM does not write the process ID into the lock
file. Therefore RPM file lock has to be handled seperately.
meaksh
meaksh reviewed Jul 20, 2022
View reviewed changes
tests/unit/modules/test_zypperpkg.py Show resolved Hide resolved
tests/unit/modules/test_zypperpkg.py Outdated Show resolved Hide resolved
tests/unit/modules/test_zypperpkg.py Show resolved Hide resolved
tests/unit/modules/test_zypperpkg.py Show resolved Hide resolved
@meaksh
Copy link
Contributor

meaksh commented Jul 21, 2022

@witekest it seems these changes causes some failures in "windows" as fcntl is not available.

We should try to avoid this, for example by doing "try/except" on the import as it is done here: https://github.com/saltstack/salt/blob/master/salt/utils/files.py#L26-L32

@witekest
Copy link
Contributor Author

@witekest it seems these changes causes some failures in "windows" as fcntl is not available.

We should try to avoid this, for example by doing "try/except" on the import as it is done here: https://github.com/saltstack/salt/blob/master/salt/utils/files.py#L26-L32

Thanks. Good hint. Updated the code accordingly.

meaksh
meaksh previously approved these changes Aug 4, 2022
View reviewed changes
Copy link
Contributor

@meaksh meaksh left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks! 👍

@witekest
Copy link
Contributor Author

witekest commented Aug 8, 2022

The users have reportedly validated the code by testing it in their environment. Feedback was positive.

@nicholasmhughes
Copy link
Collaborator

re-run pr-ubuntu-2204-amd64-py3-m2crypto-pytest

witekest added a commit to witekest/salt that referenced this pull request Aug 16, 2022
@witekest
Retry if RPM lock is temporarily unavailable
8c048c9 
Backported from saltstack/salt#62204

Signed-off-by: Witek Bedyk <[email protected]>
witekest added a commit to witekest/salt that referenced this pull request Aug 16, 2022
@witekest
Retry if RPM lock is temporarily unavailable
6f56227 
Backported from saltstack/salt#62204

Signed-off-by: Witek Bedyk <[email protected]>
@witekest witekest mentioned this pull request Aug 16, 2022
Merged
@s0undt3ch s0undt3ch added the Sulfur v3006.0 release code name and version label Aug 16, 2022
@s0undt3ch s0undt3ch added this to the Sulphur v3006.0 milestone Aug 16, 2022
@nicholasmhughes
Copy link
Collaborator

@witekest it looks like pre-commit is failing isort and black checks. Please check the Contributing documentation for information on installing and using pre-commit. Thank you!

twangboy
twangboy previously approved these changes Aug 17, 2022
View reviewed changes
@Ch3LL Ch3LL merged commit 51f19f1 into saltstack:master Aug 22, 2022
@welcome
Copy link

welcome bot commented Aug 22, 2022

Congratulations on your first PR being merged! 🎉

@witekest
Copy link
Contributor Author

Thanks!

meaksh pushed a commit to openSUSE/salt that referenced this pull request Aug 29, 2022
@witekest
Retry if RPM lock is temporarily unavailable (#547)
cedde10 
* Retry if RPM lock is temporarily unavailable

Backported from saltstack/salt#62204

Signed-off-by: Witek Bedyk <[email protected]>

* Sync formating fixes from upstream

Signed-off-by: Witek Bedyk <[email protected]>
meaksh pushed a commit to openSUSE/salt that referenced this pull request Aug 29, 2022
@witekest @meaksh
Retry if RPM lock is temporarily unavailable (#547)
e0ad71c 
* Retry if RPM lock is temporarily unavailable

Backported from saltstack/salt#62204

Signed-off-by: Witek Bedyk <[email protected]>

* Sync formating fixes from upstream

Signed-off-by: Witek Bedyk <[email protected]>
meaksh pushed a commit to openSUSE/salt that referenced this pull request Aug 29, 2022
@witekest @meaksh
Retry if RPM lock is temporarily unavailable (#547)
ab056af 
* Retry if RPM lock is temporarily unavailable

Backported from saltstack/salt#62204

Signed-off-by: Witek Bedyk <[email protected]>

* Sync formating fixes from upstream

Signed-off-by: Witek Bedyk <[email protected]>
meaksh added a commit to openSUSE/salt that referenced this pull request Aug 29, 2022
@meaksh @witekest
Retry if RPM lock is temporarily unavailable (#547) (#551)
5459d07 
* Retry if RPM lock is temporarily unavailable

Backported from saltstack/salt#62204

Signed-off-by: Witek Bedyk <[email protected]>

* Sync formating fixes from upstream

Signed-off-by: Witek Bedyk <[email protected]>

Signed-off-by: Witek Bedyk <[email protected]>
Co-authored-by: Witek Bedyk <[email protected]>
meaksh added a commit to openSUSE/salt that referenced this pull request Aug 29, 2022
@meaksh @witekest
Retry if RPM lock is temporarily unavailable (#547) (#552)
5011c0e 
* Retry if RPM lock is temporarily unavailable

Backported from saltstack/salt#62204

Signed-off-by: Witek Bedyk <[email protected]>

* Sync formating fixes from upstream

Signed-off-by: Witek Bedyk <[email protected]>

Signed-off-by: Witek Bedyk <[email protected]>
Co-authored-by: Witek Bedyk <[email protected]>
meaksh pushed a commit to openSUSE/salt that referenced this pull request Dec 28, 2022
@witekest @meaksh
Retry if RPM lock is temporarily unavailable (#547)
0903c0f 
* Retry if RPM lock is temporarily unavailable

Backported from saltstack/salt#62204

Signed-off-by: Witek Bedyk <[email protected]>

* Sync formating fixes from upstream

Signed-off-by: Witek Bedyk <[email protected]>
meaksh pushed a commit to openSUSE/salt that referenced this pull request Dec 28, 2022
@witekest @meaksh
Retry if RPM lock is temporarily unavailable (#547)
ca499e2 
* Retry if RPM lock is temporarily unavailable

Backported from saltstack/salt#62204

Signed-off-by: Witek Bedyk <[email protected]>

* Sync formating fixes from upstream

Signed-off-by: Witek Bedyk <[email protected]>
meaksh pushed a commit to openSUSE/salt that referenced this pull request Dec 29, 2022
@witekest @meaksh
Retry if RPM lock is temporarily unavailable (#547)
0be294a 
* Retry if RPM lock is temporarily unavailable

Backported from saltstack/salt#62204

Signed-off-by: Witek Bedyk <[email protected]>

* Sync formating fixes from upstream

Signed-off-by: Witek Bedyk <[email protected]>
meaksh pushed a commit to openSUSE/salt that referenced this pull request Dec 29, 2022
@witekest @meaksh
Retry if RPM lock is temporarily unavailable (#547)
4a9ec33 
* Retry if RPM lock is temporarily unavailable

Backported from saltstack/salt#62204

Signed-off-by: Witek Bedyk <[email protected]>

* Sync formating fixes from upstream

Signed-off-by: Witek Bedyk <[email protected]>
meaksh pushed a commit to openSUSE/salt that referenced this pull request Dec 29, 2022
@witekest @meaksh
Retry if RPM lock is temporarily unavailable (#547)
86c4b8f 
* Retry if RPM lock is temporarily unavailable

Backported from saltstack/salt#62204

Signed-off-by: Witek Bedyk <[email protected]>

* Sync formating fixes from upstream

Signed-off-by: Witek Bedyk <[email protected]>
meaksh pushed a commit to openSUSE/salt that referenced this pull request Dec 29, 2022
@witekest @meaksh
Retry if RPM lock is temporarily unavailable (#547)
6d3f88c 
* Retry if RPM lock is temporarily unavailable

Backported from saltstack/salt#62204

Signed-off-by: Witek Bedyk <[email protected]>

* Sync formating fixes from upstream

Signed-off-by: Witek Bedyk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vzhestkov vzhestkov vzhestkov left review comments

@twangboy twangboy twangboy approved these changes

@Ch3LL Ch3LL Ch3LL approved these changes

@meaksh meaksh meaksh left review comments

@garethgreenaway garethgreenaway Awaiting requested review from garethgreenaway garethgreenaway was automatically assigned from saltstack/team-core

Assignees
No one assigned
Labels
Sulfur v3006.0 release code name and version
Projects
None yet
Milestone
Sulfur v3006.0
Development

Successfully merging this pull request may close these issues.
7 participants
@witekest @meaksh @nicholasmhughes @Ch3LL @twangboy @vzhestkov @s0undt3ch