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

Strange behavior of build when AND condition is satisfied #2957

Open
ibu1224 opened this issue Nov 21, 2023 · 4 comments
Open

Strange behavior of build when AND condition is satisfied #2957

ibu1224 opened this issue Nov 21, 2023 · 4 comments

Comments

@ibu1224
Copy link
Contributor

ibu1224 commented Nov 21, 2023

What happened:
As depicted in the image below, when the AND condition is met and executed, the build is displayed inaccurately.

Parent event
スクリーンショット 2023-11-21 11 40 35

Child event
スクリーンショット 2023-11-21 11 40 44

I thought the event that TARGET runs was the event that restarted the D build. I think that the matter shown in the parent event is not the intended behavior, as the status appears to be failing on the UI.

Event tab
スクリーンショット 2023-11-21 14 25 22

This is not a UI rendering issue.
It is probably a problem with the EventId associated with the build creation.

Is this expected behavior? Do you think this is a bug?

What you expected to happen:
TARGET builds should be displayed on
child events, which can be confusing on the user interface as it is linked to failed events.

How to reproduce it:
pipeline: https://cd.screwdriver.cd/pipelines/12604/events/778589

  1. Create a TARGET pipeline by joining jobs A, B, C, and D.
  2. Set a delay for Job A before execution.
  3. Deliberately trigger failure for Job D. Then, restart Job D's build.
  4. Ensure successful completion of Job D before proceeding with Job A, and guarantee the success of Job A as well.
  5. draw the TARGET job to the parent event.
@y-oksaku
Copy link
Contributor

y-oksaku commented Dec 12, 2023

The most recently completed build within the JOIN list triggers the next jobs.
Therefore, the last completed build A triggers the TARGET job.
Subsequently, the TARGET build starts because all statuses of the latest build within the same event group are SUCCESS.
As the TARGET build is executed within the same event, the final state will appear as depicted in the image.

This behavior is not a bug but is based on the specification where the last completed build within the JOIN list triggers the next jobs.
The same kind of display occurs when using OR conditions. (ex. requires: [ failure1, failure2, ~success ])

While the UI display may indeed appear strange, to facilitate user understanding of This specification, I think it would be better to implement the following:

  • Display which build triggered the next build (Display parent build ids)
  • Display the snapshot of the build status used for trigger determination at the time of the JOIN

@jithine
Copy link
Member

jithine commented Jan 20, 2024

This is indeed a strange behavior, user expectation would have been the join job running in the current event and not in a past event.

I propose that we should still create a build in current event, which should inherit previous builds properties such as metadata & updated parentBuild status. Which can be used to determine if it should be executed

@y-oksaku
Copy link
Contributor

y-oksaku commented Jan 22, 2024

It seems good. However, we also need to consider the following:

  • Which event to prioritize when there are multiple restart builds (e.g. D1 and D2)
    • We could use the event creation time, build creation time, or build finish time, etc.
  • How to prevent multiple builds when triggering a child event from a parent event
    • Currently, we prevent multiple builds by using RedLock with a pair (pipeline id, event id). Thus, maybe we cannot prevent that when triggering other events.

I suggest that we clarify the desired behavior, including any other trigger issues, before making corrections.

@jithine
Copy link
Member

jithine commented Jan 25, 2024

@y-oksaku to your second point. What if lock based on pipeline id & group event id, more ops might end up getting processed sequentially, but that should be okay.

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

No branches or pull requests

3 participants