Architecture #7114
closed
Make the bundlesequence be defined in a dedicated bundle
Added by François ARMAND over 9 years ago. Updated almost 9 years ago.
Description
Today, the bundlesequence is generated into promise.cf, what can be brittle and does not allow to add promisee for each bundle of the sequence.
We would like to genereate the bundlesequence in a dedicated file, with a list of "method / usebundle", and just call that bundle in promise.cf.
Updated by François ARMAND over 9 years ago
Some implementation question:
- do we put that bundle in its own file ? (I think not, in promise.cf)
- what name for the bundle ?
- no modification in initial-promises.cf... That won"t make them converge better.
- need to remove the old BUNDLELIST variable in the technique metadata, but keep it in the code to allows migration.
Updated by Jonathan CLARKE over 9 years ago
Hi,
Here are my opinions on these:
François ARMAND wrote:
Some implementation question:
- do we put that bundle in its own file ? (I think not, in promise.cf)
Yes, I think we should put that bundle in it's own file. "promises.cf" is awkward because it is the only entry point for cf-agent, so basically the less there is in it, the easier it is to make code modular and use parts of it. In particular, if one day we wanted to allow Rudder to be used as "just a part" of a bigger CFEngine policy, it would be very easy to do if you just need to add this new file/bundlename to your existing inputs/bundlesequence.
- what name for the bundle ?
I propose "rudder_directives". This name implies that it should only contain user-defined Rudder directives. Any system calls we make would preferably be kept separate (either left directly in the bundlesequence, or via another intermediary bundle like "rudder_system_directives".
- no modification in initial-promises.cf... That won"t make them converge better.
Why not? We should definitely keep them as much in sync as possible. However, there aren't any user-defined directives in initial-promises (by definition) so it probably would just be similar CFEngine-level changes, and no rudder_directives bundle call.
- need to remove the old BUNDLELIST variable in the technique metadata, but keep it in the code to allows migration.
Yep.
Updated by François ARMAND over 9 years ago
Jonathan CLARKE wrote:
Hi,
Here are my opinions on these:
François ARMAND wrote:
Some implementation question:
- do we put that bundle in its own file ? (I think not, in promise.cf)
Yes, I think we should put that bundle in it's own file. "promises.cf" is awkward because it is the only entry point for cf-agent, so basically the less there is in it, the easier it is to make code modular and use parts of it. In particular, if one day we wanted to allow Rudder to be used as "just a part" of a bigger CFEngine policy, it would be very easy to do if you just need to add this new file/bundlename to your existing inputs/bundlesequence.
Well, so what happen to inputs ? I understand that they can be only declared in "body control sequence" (so much for modularity). And it seems that it makes little gain to output the bundlesequence if the list of dependencies is kept on promise.st (we just made more difficult to debug problem that were apparent before, like a missing dependency).
Of course, if we can make several bundle sequence part, each semantic one in its own file, and only keep something minimalist in promise.cf, I'm all in. Someone versed in CFengine can help here ?
- what name for the bundle ?
I propose "rudder_directives". This name implies that it should only contain user-defined Rudder directives. Any system calls we make would preferably be kept separate (either left directly in the bundlesequence, or via another intermediary bundle like "rudder_system_directives".
OK with that name.
- no modification in initial-promises.cf... That won"t make them converge better.
Why not? We should definitely keep them as much in sync as possible. However, there aren't any user-defined directives in initial-promises (by definition) so it probably would just be similar CFEngine-level changes, and no rudder_directives bundle call.
Well, the refactoring of all the different promise.* to make them converge is far beyond my cfengine skills, and/or may imply the need to have some templating engine in the build process. It seems to far outreached the scope of that little modification.
Again, I'm all in for that goal. It's an heresy to have "almost duplicate" code, a nether ending source of bug, and an horror to maintain, with a lot of expert knowledge (like in "expert system": a rule, with 10 000 exception, that only a system with a big database, or the history of all reason why some choice should be made, can manage).
But it seemed to be a big, hard, long task. OK, it was about the whole deduplication of promises/initial-promises. Perhaps a subpart, for promises.* only, can be made. I can't do it (in a reasonable time), thought.
Updated by François ARMAND over 9 years ago
So, for the inputs, we could call list defined on other bundle, like it is done for ncf.
So we could have a "rudder_directives.inputs" for that, and keep things together on "rudder_directive" file. And make the same for ncf.
That still don't make the promise.cf of initial promises and the one of technique the same, but they should be much more alike.
Updated by François ARMAND over 9 years ago
- Status changed from In progress to Pending technical review
- Assignee changed from François ARMAND to Nicolas CHARLES
- Pull Request set to https://github.com/Normation/rudder-techniques/pull/736
Updated by Jonathan CLARKE about 9 years ago
- Status changed from Pending technical review to Discussion
- Assignee changed from Nicolas CHARLES to François ARMAND
Damn, I didn't see you'd answered. For some reason Redmine didn't send me a notification email. I've watched this issue now to make sure that I do get emails.
François ARMAND wrote:
Jonathan CLARKE wrote:
François ARMAND wrote:
Some implementation question:
- do we put that bundle in its own file ? (I think not, in promise.cf)
Yes, I think we should put that bundle in it's own file. "promises.cf" is awkward because it is the only entry point for cf-agent, so basically the less there is in it, the easier it is to make code modular and use parts of it. In particular, if one day we wanted to allow Rudder to be used as "just a part" of a bigger CFEngine policy, it would be very easy to do if you just need to add this new file/bundlename to your existing inputs/bundlesequence.
Well, so what happen to inputs ? I understand that they can be only declared in "body control sequence" (so much for modularity). And it seems that it makes little gain to output the bundlesequence if the list of dependencies is kept on promise.st (we just made more difficult to debug problem that were apparent before, like a missing dependency).
Of course, if we can make several bundle sequence part, each semantic one in its own file, and only keep something minimalist in promise.cf, I'm all in. Someone versed in CFengine can help here ?
Since CFEngine 3.6, included files can include other files thanks to "body file control". See https://docs.cfengine.com/latest/reference-components-file_control_promises.html for details.
Simply put, you could have this at the top of rudder_directives.st:
body file control {
inputs => { &RUDDER_DIRECTIVES_INPUTS& };
}
(I'm a little unsure about the exact StringTemplate syntax, but you get the gist)
This would make it a whole lot more modular, and means the above scenario would just involve adding rudder_directives.cf to inputs in promises.cf and rudder_directives to bundlesequence. Easy.
It does mean you would have to generate two separate list of file names - one for dependencies introduced by directives, and the old one we already have for promises.cf. Or maybe 3, so that we eventually have a rudder_system_directives.cf that contains it's own inputs list, that is generated, and so that promises.cf becomes static, one day...
- no modification in initial-promises.cf... That won"t make them converge better.
Why not? We should definitely keep them as much in sync as possible. However, there aren't any user-defined directives in initial-promises (by definition) so it probably would just be similar CFEngine-level changes, and no rudder_directives bundle call.
Well, the refactoring of all the different promise.* to make them converge is far beyond my cfengine skills, and/or may imply the need to have some templating engine in the build process. It seems to far outreached the scope of that little modification.
Again, I'm all in for that goal. It's an heresy to have "almost duplicate" code, a nether ending source of bug, and an horror to maintain, with a lot of expert knowledge (like in "expert system": a rule, with 10 000 exception, that only a system with a big database, or the history of all reason why some choice should be made, can manage).
But it seemed to be a big, hard, long task. OK, it was about the whole deduplication of promises/initial-promises. Perhaps a subpart, for promises.* only, can be made. I can't do it (in a reasonable time), thought.
OK, it's fine if you can't do it, but it doesn't mean it shouldn't happen. However, having looked at your pull request, I don't see what changes would need syncing with initial-promises, since the only difference is a call to rudder_directives, which obviously wouldn't happen in initial promises.
Updated by François ARMAND about 9 years ago
- Status changed from Discussion to Pending technical review
- Assignee changed from François ARMAND to Nicolas CHARLES
Updated by François ARMAND about 9 years ago
- Status changed from Pending technical review to Pending release
Applied in changeset rudder-techniques|2cac872a1ebf1719ab8788515b33ffbc70aa2b11.
Updated by Nicolas CHARLES about 9 years ago
Applied in changeset rudder-techniques|9d7266903ad270f8a795820a50c48de1054155c5.
Updated by Vincent MEMBRÉ almost 9 years ago
- Status changed from Pending release to Released
This bug has been fixed in Rudder 3.2.0~beta1 which was released today.