-
Notifications
You must be signed in to change notification settings - Fork 29
Conversation
salt/addons/dex/legacy.sls
Outdated
@@ -0,0 +1,24 @@ | |||
include: | |||
- .main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this legacy.sls
invoked from anywhere? I cannot find any reference...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah - I forgot to add it back into init.sls
.
I am still hoping to find a good way to not having to show the dependencies in the state files and the orchestration (orch.kubernetes
) as well.
salt/addons/dex/pre.sls
Outdated
# Unfortunately using the default include mechanism of salt, to force order | ||
# doesn't seem to work when running via orchestrate runner, so this is | ||
# split in different files that the orch runner calls with explicit | ||
# dependencies. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't enough with an include
and a require
in the state that requires it? I find this split in different files a bit confusing...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find the whole dex
refactoring very untidy and confusing. I would leave it as it was unless there is a very good reason for this change...
salt/addons/dex/main.sls
Outdated
explicit-pre-dependency: | ||
test.nop: | ||
- require: | ||
- sls: addons.dex.pre |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thiss is not very elegant.. do we really need this? We are already setting the order in the orchestration...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@inercia I also didn't like having to artificially splitting the states.
The problem I had with everything being in one file - the copying
of the 15-secret.yaml
did not happen in that case - I assumed that is because the macro-expansion have to run first, which already run other states.
However I just tried moving everything into one file again and this time it worked ...
ea7d58a
to
4e78948
Compare
@inercia Apparently the |
(Attempt to make the automatic synchronization work for custom execution modules - seems not to work)
Otherwise the kubectl_apply_dir_template macro will fail, as the file does not exist when it tries to run `salt.hashutil.digest` on it.
Before this release the format did not use the 'interfaces' key.
Otherwise the states would fail if no nodes are returned in the `tgt` expression.
Changes required to make our salt states run with
salt 2018.3
.