IronCore864
commented
3 months ago Not in the scope of this PR (or the issue https://github.com/canonical/pebble/issues/231 we are trying to fix), but I'd like to point out the painful experience of using requires/before/after when implementing and testing this feature.
This PR is my second one on this feature because, in the first draft, I made the mistake of assuming that "when handling task B, if B depends on task A, A should have already been processed, since the input is sorted already, so, simply put B in the lane where A already is." I ignored the possibility that you can configure A to require B but A before B, for example, the test case 3 mentioned above:
services:
a:
override: replace
command: sh -c "sleep 999"
startup: enabled
requires:
- b
before:
- b
b:
override: replace
command: sh -c "sleep 999"
startup: enabledThe logic in my first draft won't work, because the sorted order is (A, B), so when handling A, B's lane isn't known yet; and when handling B, since B doesn't require anything, you don't know which lane to put it in. To solve this problem, I created the second solution in this PR, where if A's dependency isn't known yet, create a lane, and put all dependencies in the same lane even before the task is created.
However, after giving this test case more thought, I concluded that the test case makes no sense, because "A requires B, but A must be started before B" is illogical:
- If "A requires B", it means "if B doesn't start, A won't". Or to put it another way, "B must start before A".
- If "A must be started before B", it means "B requires A", not the other way around.
So, saying that "A requires B" and "A must start before B" is illogical, the two parts counter each other.
I tried but failed to think of any use case where "A requires B", but "A must be started before B". Some analogies:
- Let's say we want to create a CDN (say, with AWS CloudFront, as service A), and the origin server is hosted in an AWS S3 bucket (service B). This whole "CDN" product can only work when both CloudFront and S3 are present, but you must create S3 before creating CloudFront because CloudFront needs to know the S3 URL to load content from. So, CloudFront depends on S3 (A requires B), and you must create S3 (B) before creating CloudFront (A).
- A web app comprises a backend (service A) and a database (B), and A requires B because the backend reads/writes to the DB. But you must start A before B? Starting A when B isn't started doesn't make sense because the whole product isn't ready for service anyway.
- Yet another analogy: I want to create an EC2 instance (A) but it requires an IAM role (B). The IAM role must be created first because when creating the EC2 instance, I need to assign the role to the instance. So, the EC2 requires the IAM role (A requires B) and the IAM role must be created before the EC2 (A must start AFTER, not BEFORE, B).
Using three keywords requires/before/after to handle dependency is an overkill, which brings more problems than solves.
I think we can drop "before/after", and rename "requires" to something like "depend-on" (depend-on means requires + after, in my book).
I prefer renaming "requires" to "depends-on" because "requires" in many cases isn't right:
- In the CDN analogy, the CloudFront doesn't "require" the S3 origin server. CloudFront can be created with an empty or invalid origin server. It's the CDN (the whole product that requires S3).
- In the web app analogy, the backend also doesn't really "require" the DB; without the DB, when accessing the backend service, it would throw some error saying data can't be loaded/persisted. It's the whole web app product that requires the DB.
If we drop "before/after" and leave only "depends-on", the layer in test case 3 can be fixed to:
services:
a:
override: replace
command: /bin/sh -c "echo test1; sleep 10"
startup: enabled
b:
override: replace
command: /bin/sh -c "echo test2; sleep 10"
depends-on:
- aAnd this is more than enough to solve any dependency requirements. Proof: Terraform does the same thing, even the name "depends-on" is borrowed from Terraform.
If we can do this, after sorting, the order is (A, B) and when handling B, A's lane is already decided. In this case, the feature in this PR can be much simpler:
# pseudo
for task in tasks:
if task.depends_on:
task.lane = task.depends_on.lane
task.wait_for(task.depends_on)
benhoyt
When starting services, put services that depend on each other (before/after/requires) in the same lane. If there is no dependency, put it in a stand-alone lane. Tasks only wait for tasks defined in the same lane.
Fixes https://github.com/canonical/pebble/issues/231.
Logic
The logic is as follows:
Tests
I mainly ran three types of test. The first is the one mentioned in https://github.com/canonical/pebble/issues/231:
With this feature, service C won't be in "hold" status, but rather, started. Logs:
As a second test, if C depends on B, C is still in "hold" because they are in the same lane:
Logs:
As a third test, I tested the "requires + before" combination:
Logs: