On 05/18/2011 11:47 PM, Mo Morsi wrote:
Comments below
On 05/18/2011 04:26 PM, Mike Orazi wrote:
> ---
> Rakefile | 3 ++-
> bin/aeolus-cleanup | 5 +++--
> bin/aeolus-configure | 3 ++-
> bin/aeolus-node | 10 ++++++++++
> conf/default_cleanup | 6 ++++++
> conf/default_configure | 6 ++++++
> contrib/aeolus-configure.spec | 13 +++++++++++--
> 7 files changed, 40 insertions(+), 6 deletions(-)
> create mode 100644 bin/aeolus-node
> create mode 100644 conf/default_cleanup
> create mode 100644 conf/default_configure
>
> diff --git a/Rakefile b/Rakefile
> index 1104573..a6f7e2a 100644
> --- a/Rakefile
> +++ b/Rakefile
> @@ -19,7 +19,7 @@ RPM_SPEC = "contrib/aeolus-configure.spec"
> rpm_task =
> Rake::RpmTask.new(RPM_SPEC) do |rpm|
> rpm.need_tar = true
> - rpm.package_files.include("bin/*", "recipes/**/*")
> + rpm.package_files.include("bin/*", "recipes/**/*",
"conf/*")
> rpm.topdir = "#{RPMBUILD_DIR}"
> end
>
> @@ -53,3 +53,4 @@ begin
> rescue LoadError
> # need net-ssh and net-scp needed to run remote specs
> end
> +
There were a few whitespace errors, could you take care of before pushing.
Will do.
> diff --git a/bin/aeolus-cleanup b/bin/aeolus-cleanup
> index 87fb405..d064b32 100644
> --- a/bin/aeolus-cleanup
> +++ b/bin/aeolus-cleanup
> @@ -2,8 +2,9 @@
>
> export FACTER_AEOLUS_ENABLE_HTTPS=true
> export FACTER_AEOLUS_ENABLE_SECURITY=false
> -puppet /usr/share/aeolus-configure/aeolus_uninstall.pp \
> +puppet /usr/share/aeolus-configure/modules/aeolus_recipe/manifests/defaults.pp \
> --modulepath=/usr/share/aeolus-configure/modules/ \
> - --logdest=/var/log/aeolus-configure/aeolus-cleanup.log \
> + --external_nodes '/usr/sbin/aeolus-node cleanup' --node_terminus exec
\
Do we need this /usr/sbin/aeolus-node script? Can we just use the node
yaml directly w/ a --node_terminus set to 'yaml'?
We might be able to away without the script but I'm not sure. I'd
rather leave it as is for now and take it out in a future patch if it is
possible to remove it.
> + --logdest=/var/log/aeolus-configure/aeolus-configure.log \
> --logdest=console
>
> diff --git a/bin/aeolus-configure b/bin/aeolus-configure
> index 6d87cb1..a4fae56 100644
> --- a/bin/aeolus-configure
> +++ b/bin/aeolus-configure
> @@ -2,8 +2,9 @@
>
> export FACTER_AEOLUS_ENABLE_HTTPS=true
> export FACTER_AEOLUS_ENABLE_SECURITY=false
> -puppet /usr/share/aeolus-configure/aeolus_recipe.pp \
> +puppet /usr/share/aeolus-configure/modules/aeolus_recipe/manifests/defaults.pp \
> --modulepath=/usr/share/aeolus-configure/modules/ \
> + --external_nodes '/usr/sbin/aeolus-node configure' --node_terminus
exec \
> --logdest=/var/log/aeolus-configure/aeolus-configure.log \
> --logdest=console
>
> diff --git a/bin/aeolus-node b/bin/aeolus-node
> new file mode 100644
> index 0000000..2339d42
> --- /dev/null
> +++ b/bin/aeolus-node
> @@ -0,0 +1,10 @@
> +#! /bin/sh
> +
> +CAT=/bin/cat
> +NODE_DIR=/etc/aeolus-configure/nodes
> +
> +# look for node setting files, or revert to default
> +[ -f nodes/$2_$1 ]&& NODE=$2_$1 || NODE=default_$1
I see this was changed in last patch in the set, any way these changes
could be merged into one?
> +
> +$CAT $NODE_DIR/$NODE
> +
> diff --git a/conf/default_cleanup b/conf/default_cleanup
> new file mode 100644
> index 0000000..5ab666a
> --- /dev/null
> +++ b/conf/default_cleanup
> @@ -0,0 +1,6 @@
> +---
> +parameters:
> + #ssh_port: 22
> +classes:
> +- aeolus-conductor
> +
> diff --git a/conf/default_configure b/conf/default_configure
> new file mode 100644
> index 0000000..5ab666a
> --- /dev/null
> +++ b/conf/default_configure
> @@ -0,0 +1,6 @@
> +---
> +parameters:
> + #ssh_port: 22
> +classes:
> +- aeolus-conductor
> +
Same with these node files.
Other than these notes looks good and looks like it works as intended.
-Mo
_______________________________________________
aeolus-devel mailing list
aeolus-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/aeolus-devel
Thanks for the review.
I'll see about the possibility of squashing down into fewer commits when
I merge these changes in. I was mostly concerned with trying to
separate out the workflow a bit so the git mv commit wouldn't make it
harder to follow the other changes. Looking at the patch set a couple
of the changes should be fairly squash-able.
Mike