cycle detection in object dependencies broken? #80

New Issue

ungleich-gitea · 2021-11-20T13:23:37Z

ungleich-gitea commented

2021-11-20 13:23:37 +00:00

$ for f in manifest/bug type/__foo/manifest type/__bar/manifest; do echo; while read -r l; do echo "$f: $l"; done < "$f"; done

manifest/bug: #!/bin/sh -e
manifest/bug: export CDIST_ORDER_DEPENDENCY=1
manifest/bug: __foo
manifest/bug: __bar

type/__foo/manifest: #!/bin/sh -e
type/__foo/manifest: export CDIST_ORDER_DEPENDENCY=1
type/__foo/manifest: __file /tmp/foo

type/__bar/manifest: #!/bin/sh -e
type/__bar/manifest: export CDIST_ORDER_DEPENDENCY=1
type/__bar/manifest: __file /tmp/bar

$ cdist config -i manifest/bug localhost
INFO: [42527]: localhost: Starting configuration run
ERROR: [42527]: localhost: Cycle detected in object dependencies:
__file/tmp/foo -> __bar/ -> __foo/ -> __file/tmp/foo!

``` $ for f in manifest/bug type/__foo/manifest type/__bar/manifest; do echo; while read -r l; do echo "$f: $l"; done < "$f"; done manifest/bug: #!/bin/sh -e manifest/bug: export CDIST_ORDER_DEPENDENCY=1 manifest/bug: __foo manifest/bug: __bar type/__foo/manifest: #!/bin/sh -e type/__foo/manifest: export CDIST_ORDER_DEPENDENCY=1 type/__foo/manifest: __file /tmp/foo type/__bar/manifest: #!/bin/sh -e type/__bar/manifest: export CDIST_ORDER_DEPENDENCY=1 type/__bar/manifest: __file /tmp/bar $ cdist config -i manifest/bug localhost INFO: [42527]: localhost: Starting configuration run ERROR: [42527]: localhost: Cycle detected in object dependencies: __file/tmp/foo -> __bar/ -> __foo/ -> __file/tmp/foo! ```

poljakowski was assigned by ungleich-gitea

2021-11-20 13:23:37 +00:00

ungleich-gitea closed this issue

2021-11-20 13:23:37 +00:00

ungleich-gitea commented

2021-11-20 15:35:30 +00:00

closed

ungleich-gitea commented

2021-11-20 15:35:32 +00:00

Fixed in https://code.ungleich.ch/ungleich-public/cdist/merge_requests/815.

ungleich-gitea commented

2021-11-20 15:35:33 +00:00

mentioned in merge request !815

ungleich-gitea commented

2021-11-20 15:35:34 +00:00

@steven Yes, that is in the idea I am talking about.
I will experiment with it.

@steven Yes, that is in the idea I am talking about. I will experiment with it.

ungleich-gitea commented

2021-11-20 15:35:36 +00:00

CDIST_ORDER_DEPENDENCY should only be valid within the manifest where it is used.
That we have a global file tracking this is at least one of the problems.

What if we have a 'type_creation_order' (or whatever the file is called) per object, and also store it inside the object?

Guess that's similar to what @poljakowski talks about.

CDIST_ORDER_DEPENDENCY should only be valid within the manifest where it is used. That we have a global file tracking this is at least one of the problems. What if we have a 'type_creation_order' (or whatever the file is called) per object, and also store it inside the object? Guess that's similar to what @poljakowski talks about.

ungleich-gitea commented

2021-11-20 15:35:37 +00:00

CDIST_ORDER_DEPENDENCY is evil, it breaks things.

yes, but it is necessary evil. i, for one, support this evil env var.

> `CDIST_ORDER_DEPENDENCY` is evil, it breaks things. yes, but it is necessary evil. i, for one, support this evil env var.

ungleich-gitea commented

2021-11-20 15:35:38 +00:00

@steven @nico What if we try to change how CDIST_ORDER_DEPENDENCY works?
How about defining context in which this env var is valid, and a local list of type creation order start from beginning in each context?

E.g. we have init manifest:

export CDIST_ORDER_DEPENDENCY=1
a
b
c

type a:

export CDIST_ORDER_DEPENDENCY=1
a1
a2
a3

type b:

export CDIST_ORDER_DEPENDENCY=1
b1
b2
b3

type c:

export CDIST_ORDER_DEPENDENCY=1
c1
c2

First context is init manifest, so we have empty creation order, and then a, b, c.
Then, type a is second context, with empty creation order in the beginning, and then a1, a2, a3.
The same for types b and c.

In the end we have this dependency tree:

+-->a-------->a1
|   |        /\
|   |         |
|   +-------->a2
|   |         /\
|   |         |
|   +-------->a3
|
b----+------->b1
/|   |        /\
|    +------->b2
|    |        /\
|    +------->b3
c-------->c1
|         /\
|         |
+-------->c2

For this context, cdist would need additional state variable(s).
In the beginning order dep is switched off.
When env var is detected and state is off we turn it on.
When env var is not in env any more and state is on then we turn it off.
When type manifest finishes (if we can detect this) we turn it off.

With the above dependencies, a1, b1 and c1 could be executed first, in arbitrary order.

@steven @nico Is this behavior compatible with the cdist order dep intention?

Or should every type defined by c be processed before any type defined in b and c?

Just a thought, without any experiment, yet.

@steven @nico What if we try to change how `CDIST_ORDER_DEPENDENCY` works? How about defining context in which this env var is valid, and a local list of type creation order start from beginning in each context? E.g. we have init manifest: ``` export CDIST_ORDER_DEPENDENCY=1 a b c ``` type a: ``` export CDIST_ORDER_DEPENDENCY=1 a1 a2 a3 ``` type b: ``` export CDIST_ORDER_DEPENDENCY=1 b1 b2 b3 ``` type c: ``` export CDIST_ORDER_DEPENDENCY=1 c1 c2 ``` First context is init manifest, so we have empty creation order, and then `a, b, c`. Then, type `a` is second context, with empty creation order in the beginning, and then `a1, a2, a3`. The same for types `b` and `c`. In the end we have this dependency tree: ``` +-->a-------->a1 | | /\ | | | | +-------->a2 | | /\ | | | | +-------->a3 | b----+------->b1 /| | /\ | +------->b2 | | /\ | +------->b3 c-------->c1 | /\ | | +-------->c2 ``` For this context, cdist would need additional state variable(s). In the beginning order dep is switched off. When env var is detected and state is off we turn it on. When env var is not in env any more and state is on then we turn it off. When type manifest finishes (if we can detect this) we turn it off. With the above dependencies, a1, b1 and c1 could be executed first, in arbitrary order. @steven @nico Is this behavior compatible with the cdist order dep intention? Or should every type defined by `c` be processed before any type defined in `b` and `c`? Just a thought, without any experiment, yet.

ungleich-gitea commented

2021-11-20 15:35:40 +00:00

@ander CDIST_ORDER_DEPENDENCY instructs cdist to record last created type as a requirement for current processed type, from type creation order list.

For the first example above, this is what happens:

First init manifest is executed.

__foo created
type_creation_order = __foo
__foo finished

__bar created
type creation order = __foo, __bar
inject require __foo for __bar
__bar finished

After init manifest, type's manifests are processed.

__file/tmp/foo created
type creation order = __foo, __bar, __file/tmp/foo
inject __bar for __file/tmp/foo
autorequire __file/tmp/foo for __foo (type autorequires all types it uses)

but now you get cycle:
__foo -> __file/tmp/foo -> __bar -> __foo

CDIST_ORDER_DEPENDENCY is evil, it breaks things.

https://www.cdi.st/manual/latest/cdist-best-practice.html#perils-of-cdist-order-dependency

@ander CDIST_ORDER_DEPENDENCY instructs cdist to record last created type as a requirement for current processed type, from type creation order list. For the first example above, this is what happens: First init manifest is executed. __foo created type_creation_order = __foo __foo finished __bar created type creation order = __foo, __bar inject require __foo for __bar __bar finished After init manifest, type's manifests are processed. __file/tmp/foo created type creation order = __foo, __bar, __file/tmp/foo inject __bar for __file/tmp/foo autorequire __file/tmp/foo for __foo (type autorequires all types it uses) but now you get cycle: __foo -> __file/tmp/foo -> __bar -> __foo `CDIST_ORDER_DEPENDENCY` is evil, it breaks things. https://www.cdi.st/manual/latest/cdist-best-practice.html#perils-of-cdist-order-dependency

ungleich-gitea commented

2021-11-20 15:35:41 +00:00

assigned to @poljakowski

ungleich-gitea commented

2021-11-20 15:35:42 +00:00

if first type in initial manifest doesn't have CDIST_ORDER_DEPENDENCY, then it works:

$ for f in manifest/bug type/__foo/manifest type/__bar/manifest type/__baz/manifest; do echo; while read -r l; do echo "$f: $l"; done < "$f"; done

manifest/bug: #!/bin/sh -e
manifest/bug: export CDIST_ORDER_DEPENDENCY=1
manifest/bug: __baz
manifest/bug: __foo
manifest/bug: __bar

type/__foo/manifest: #!/bin/sh -e
type/__foo/manifest: export CDIST_ORDER_DEPENDENCY=1
type/__foo/manifest: __file /tmp/foo

type/__bar/manifest: #!/bin/sh -e
type/__bar/manifest: export CDIST_ORDER_DEPENDENCY=1
type/__bar/manifest: __file /tmp/bar

type/__baz/manifest: #!/bin/sh -e
type/__baz/manifest: __file /tmp/baz

if first type in initial manifest doesn't have CDIST_ORDER_DEPENDENCY, then it works: ``` $ for f in manifest/bug type/__foo/manifest type/__bar/manifest type/__baz/manifest; do echo; while read -r l; do echo "$f: $l"; done < "$f"; done manifest/bug: #!/bin/sh -e manifest/bug: export CDIST_ORDER_DEPENDENCY=1 manifest/bug: __baz manifest/bug: __foo manifest/bug: __bar type/__foo/manifest: #!/bin/sh -e type/__foo/manifest: export CDIST_ORDER_DEPENDENCY=1 type/__foo/manifest: __file /tmp/foo type/__bar/manifest: #!/bin/sh -e type/__bar/manifest: export CDIST_ORDER_DEPENDENCY=1 type/__bar/manifest: __file /tmp/bar type/__baz/manifest: #!/bin/sh -e type/__baz/manifest: __file /tmp/baz ```

ungleich-gitea commented

2021-11-20 15:35:44 +00:00

changed the description

Sign in to join this conversation.