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

Don't overwrite final state of Copr build by an in-progress state #2252

Closed
lbarcziova opened this issue Nov 14, 2023 · 2 comments · Fixed by #2358
Closed

Don't overwrite final state of Copr build by an in-progress state #2252

lbarcziova opened this issue Nov 14, 2023 · 2 comments · Fixed by #2358
Assignees
Labels
area/copr Related to the integration with copr.fedorainfracloud.org/ gain/low This doesn't bring that much value to users. impact/low This issue impacts only a few users. kind/bug Something isn't working.

Comments

@lbarcziova
Copy link
Member

Example logs:

2023-11-13T14:47:52.864+00:00	Status reporter will report for GithubProject(namespace="rear", repo="rear"), commit=4434da63686f0615eb6936eab7c8b96a7c80b979, pr=3073
2023-11-13T14:47:52.864+00:00	Setting Github status check 'success' for check 'rpm-build:centos-stream-9-x86_64': RPMs were built successfully.
...

2023-11-13T14:47:53.363+00:00	Status reporter will report for GithubProject(namespace="rear", repo="rear"), commit=4434da63686f0615eb6936eab7c8b96a7c80b979, pr=3073
2023-11-13T14:47:53.363+00:00	Setting Github status check 'in_progress' for check 'rpm-build:centos-stream-9-x86_64': RPM build is in progress...
@lbarcziova lbarcziova added kind/bug Something isn't working. area/copr Related to the integration with copr.fedorainfracloud.org/ complexity/easy-fix impact/low This issue impacts only a few users. gain/low This doesn't bring that much value to users. labels Nov 14, 2023
@lachmanfrantisek
Copy link
Member

Let's add a check before processing in-progress ones and do it on our (=~database) level, not on GitHub.

@lachmanfrantisek lachmanfrantisek moved this from new to backlog in Packit Kanban Board Nov 16, 2023
@lachmanfrantisek lachmanfrantisek moved this from backlog to ready-to-refine in Packit Kanban Board Nov 16, 2023
@lbarcziova lbarcziova moved this from ready-to-refine to backlog in Packit Kanban Board Jan 22, 2024
@lbarcziova lbarcziova moved this from backlog to priority-backlog in Packit Kanban Board Jan 31, 2024
@majamassarini majamassarini moved this from priority-backlog to refined in Packit Kanban Board Feb 1, 2024
@mfocko
Copy link
Member

mfocko commented Feb 1, 2024

notes from the refinement discussion

Based on the description of the issue and some of the “breadcrumbs” I've noticed in the Sentry logs, the following cause is anticipated:

  1. Packit triggers the Copr build (hounds1 get released)
  2. SRPM build has finished, Copr sends an event about the SRPM being finished
  3. Copr event about successful SRPM build gets put into the queue
  4. Babysit catches the RPM that has already finished in the meantime
  5. Once we process the SRPM build, we override the PASS/FAIL from the babysit

Ideal outcome of this card would be making sure that we don't allow transitions from finished state (success, failure, error) to the pending or in progress state.

We also discussed the issues with testing this. Thinking about it, I would suggest simulating receival of the Copr event while mocking the database model with already finished state (as described above) and expect no change in the result.

Footnotes

  1. babysit task 😉

@lbarcziova lbarcziova moved this from refined to priority-backlog in Packit Kanban Board Feb 8, 2024
@mfocko mfocko moved this from priority-backlog to refined in Packit Kanban Board Feb 22, 2024
@lbarcziova lbarcziova self-assigned this Mar 1, 2024
@lbarcziova lbarcziova moved this from refined to in-progress in Packit Kanban Board Mar 1, 2024
lbarcziova added a commit to lbarcziova/packit-service that referenced this issue Mar 1, 2024
In case the end was already processed, only set the start time
and do not overwrite the end status and do not report this to user.

Fixes packit#2252
@lbarcziova lbarcziova moved this from in-progress to in-review in Packit Kanban Board Mar 1, 2024
softwarefactory-project-zuul bot added a commit that referenced this issue Mar 4, 2024
Don't overwrite final state of Copr build

In case the end was already processed, only set the start time and do not overwrite the end status and do not report this to user.
Fixes #2252
RELEASE NOTES BEGIN
We have fixed the bug about Packit overwriting the final state of the build in case the build start is being processed later than the end of the build.
RELEASE NOTES END

Reviewed-by: Matej Focko
@github-project-automation github-project-automation bot moved this from in-review to done in Packit Kanban Board Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/copr Related to the integration with copr.fedorainfracloud.org/ gain/low This doesn't bring that much value to users. impact/low This issue impacts only a few users. kind/bug Something isn't working.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants