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.

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

Race condition between robosignatory signing builds and Builds being added to the db #1111

Closed
bowlofeggs opened this issue Nov 15, 2016 · 0 comments · Fixed by #1117

Comments

@bowlofeggs
Copy link
Member

commented Nov 15, 2016

The signed handler often sees messages from robosignatory that it has signed a build, but then the Build isn't found in Bodhi's database. This is due to the Build not being committed before Bodhi tells Koji to add the build to the pre-signing tag. This causes updates to never make it to the signed state, which causes the masher to never move them to the updates-testing repository.

The fix itself will be easy, but we will need to expend a lot of effort in testing the fix since it is unlikely that there are tests for the specific function in question.

@bowlofeggs bowlofeggs added this to the Bodhi 2.3.2 milestone Nov 15, 2016

@bowlofeggs bowlofeggs self-assigned this Nov 15, 2016

bowlofeggs added a commit to bowlofeggs/bodhi that referenced this issue Nov 16, 2016
Commit db transaction after creating new Builds in new_update().
This commit alters new_update() so that the database transaction
get committed after creating new Builds in new_update(). This will
solve a race condition between robosignatory signing Builds before
they are committed to the database, which should ensure that the
signed handler will find the Builds once Robosignatory emits the
message that they have been signed.

In order for the existing tests to pass, two changes were needed in
the BaseTestCase: The tests need to start and end a transaction,
and the Session needs to be removed when the test is complete.

This commit also modifies an existing test to ensure that Builds
remain after the transaction is aborted.

fixes fedora-infra#1111
bowlofeggs added a commit that referenced this issue Nov 17, 2016
Merge pull request #1117 from bowlofeggs/1111
Add tests, drop unreachable code, and fix #1111
bowlofeggs added a commit that referenced this issue Nov 17, 2016
Commit db transaction after creating new Builds in new_update().
This commit alters new_update() so that the database transaction
get committed after creating new Builds in new_update(). This will
solve a race condition between robosignatory signing Builds before
they are committed to the database, which should ensure that the
signed handler will find the Builds once Robosignatory emits the
message that they have been signed.

In order for the existing tests to pass, two changes were needed in
the BaseTestCase: The tests need to start and end a transaction,
and the Session needs to be removed when the test is complete.

This commit also modifies an existing test to ensure that Builds
remain after the transaction is aborted.

fixes #1111
amolkahat added a commit to amolkahat/bodhi that referenced this issue Feb 2, 2017
Commit db transaction after creating new Builds in new_update().
This commit alters new_update() so that the database transaction
get committed after creating new Builds in new_update(). This will
solve a race condition between robosignatory signing Builds before
they are committed to the database, which should ensure that the
signed handler will find the Builds once Robosignatory emits the
message that they have been signed.

In order for the existing tests to pass, two changes were needed in
the BaseTestCase: The tests need to start and end a transaction,
and the Session needs to be removed when the test is complete.

This commit also modifies an existing test to ensure that Builds
remain after the transaction is aborted.

fixes fedora-infra#1111
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.