depends_on ignored if server is restarted

[alexef]

Component

server

Describe the bug

assume a multi pipeline set up, with:

• pipeline a
• pipeline b, depends_on: [pipeline a]

while a is running and b is pending, restart the drone server

what is expected: b stays in pending until a completes
what happens: b immediately starts running, ignoring a which is also still running

System Info

{
  "source": "https://github.com/woodpecker-ci/woodpecker",
  "version": "v0.14.2"
}

I know this is an older version, I will now try to reproduce the issue on the latest release.

Additional context

maybe related to: #200

Validations

• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Checked that the bug isn't fixed in the next version already [https://woodpecker-ci.org/faq#which-version-of-woodpecker-should-i-use]
• Check that this is a concrete bug. For Q&A join our Discord Chat Server or the Matrix room.

[alexef]

Update: we (cc: @lukaszgyg) were able to reproduce the issue (same steps) also on latest stable:

{
  "source": "https://github.com/woodpecker-ci/woodpecker",
  "version": "0.15.6"
}

[alexef]

By checking the code (it's been a while since I touched it), it seems like we have two representation of a Task, with a slight difference:

• queue.Task (in memory, has Dependencies and DepStatus)
• model.Task (in datastore, has only Dependencies)

the task.ShouldRun method will check DepStatus. However, when we load tasks in memory (at start up) we don't fill the DepStatus property. When this is empty, the ShouldRun returns true.

LATER EDIT: the only place where we seem to write DepStatus is in the finished implementation. I believe this logic should also be added to persistent.go when we load a cold server.

@anbraten does this make sense?

[anbraten]

@alexef Good catch. Yes it definitely seems to be missing DepStatus.

[6543]

I'm not happy how queue is implemented atm at all ...: #1518 , ...

I think that could be done in one catch :)

[alexef]

Agree a rewrite is granted, but I would like to see this bug fixed (as it negatively impacts our organisation). Will prepare a pr :)

[alexef]

changes are in master.

now trying to also backport to v0.15 #1625