-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #7943: Merge distributePolicy from initial promises and system technique #874
Conversation
@@ -30,6 +30,12 @@ all: rudder-templates-cli.jar | |||
java -jar rudder-templates-cli.jar --outext .cf --outdir initial-promises/node-server/inventory/1.0 techniques/system/inventory/1.0/virtualMachines.st | |||
java -jar rudder-templates-cli.jar --outext .cf --outdir initial-promises/node-server/inventory/1.0 techniques/system/inventory/1.0/fetchFusionTools.st | |||
java -jar rudder-templates-cli.jar --outext .cf --outdir initial-promises/node-server/inventory/1.0 techniques/system/inventory/1.0/fusionAgent.st | |||
sed -i -e 's/.*TRACKINGKEY.*/ "TRACKINGKEY": "root-DP@@root-distributePolicy@@00",/' variables.json | |||
cp techniques/system/distributePolicy/1.0/rudder-ncf-conf.st initial-promises/node-server/distributePolicy/ncf/ncf.conf |
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'm not comfortable with this just being a simple cp, while the file is a .st file and is listed in the technique's metadata.xml file as a TML (not a file that is just copied).
I realise it doesn't have any StringTemplate syntax in today, but if someone adds some in, this cp will directly include it in initial promises, and probably break a lot of things.
I see two alternatives:
- We change the metadata.xml to list this file as a simple file to copy, not a TML (best, IMHO)
- We run this file through the jar to de-StringTemplate it for initial-promises, in case one day it contains StringTemplate stuff (unncessary, IMHO, but least requires less changes)
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 thought I would change the extension and the metadata.xml in the .cf cleaning PR.
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.
OK, so long as we don't forget
PR updated |
80ab0c7
to
6c23ad7
Compare
Aside from the missing file copy (relay syslog config), this looks good to me! 👍 |
Ping @peckpeck |
PR updated |
6c23ad7
to
54e80df
Compare
PR updated |
54e80df
to
d599ac9
Compare
@@ -44,4 +52,7 @@ clean: | |||
rm -f initial-promises/node-server/node-server/inventory/1.0/virtualMachines.st | |||
rm -f initial-promises/node-server/node-server/inventory/1.0/fetchFusionTools.cf | |||
rm -f initial-promises/node-server/node-server/inventory/1.0/fusionAgent.cf | |||
|
|||
rm -f initial-promises/node-server/distributePolicy/ncf/ncf.conf | |||
rm -f initial-promises/node-server/distributePolicy/rsyslog.conf/rudder-rsyslog-root.conf |
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.
Missing rudder-rsyslog-relay.conf
Aside from those 2 comments this looks good to me! |
d599ac9
to
4b4bfb6
Compare
PR updated |
https://www.rudder-project.org/redmine/issues/7943