diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index b2de823..88162ae 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -628,7 +628,9 @@ module Puppet
being evaluated. This allows you to interactively see exactly
what is being done."],
:summarize => [false,
- "Whether to print a transaction summary."
+ "Whether to print a transaction summary."],
+ :changes => [false,
+ "Whether to print the transaction changes."
]
)
diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb
index 42e92f4..4cd3ba1 100644
--- a/lib/puppet/resource/catalog.rb
+++ b/lib/puppet/resource/catalog.rb
@@ -151,7 +151,7 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
yield transaction if block_given?
- transaction.send_report if host_config and (Puppet[:report] or Puppet[:summarize])
+ transaction.send_report if host_config and (Puppet[:report] or Puppet[:summarize] or Puppet[:changes])
return transaction
ensure
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index f09ca80..06c3c73 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -512,6 +512,13 @@ class Transaction
report.graph()
end
+ report.changes = report.sumup(@events)
+ if Puppet[:changes]
+ unless report.changes.empty?
+ puts report.displaychanges
+ end
+ end
+
if Puppet[:summarize]
puts report.summary
end
diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb
index e5b8650..77b8aec 100644
--- a/lib/puppet/transaction/report.rb
+++ b/lib/puppet/transaction/report.rb
@@ -10,7 +10,7 @@ class Puppet::Transaction::Report
indirects :report, :terminus_class => :processor
- attr_accessor :logs, :metrics, :time, :host
+ attr_accessor :logs, :metrics, :time, :host, :changes
# This is necessary since Marshall doesn't know how to
# dump hash with default proc (see below @records)
@@ -58,6 +58,29 @@ class Puppet::Transaction::Report
@records[metric] << object
end
+ # Provide a summary of the changes
+ def displaychanges
+ ret = ""
+
+ # display the different changes for each type
+ changes.keys.sort.each { |t|
+ ret += "%s:\n" % t
+ changes[t].keys.sort.each { |e|
+ # Count the number of occurences for each value in the changes[t][e]
+ count = changes[t][e].inject(Hash.new(0)) {|h,i| h[i] += 1; h }
+ ret += count.to_a.collect { |c, count|
+ if count > 1
+ " %15s %s (%s)" % [c + ":", e, count]
+ else
+ " %15s %s" % [c + ":", e]
+ end
+ }.join("\n")
+ ret += "\n"
+ }
+ }
+ return ret
+ end
+
# Provide a summary of this report.
def summary
ret = ""
@@ -83,5 +106,19 @@ class Puppet::Transaction::Report
end
return ret
end
+
+ # Copy events in a structure that can be return with the report
+ def sumup(events)
+ report= {}
+ events.each { |e|
+ type = e.source.class.name.to_s.capitalize
+ title = e.source.title.to_s
+ change = e.name.to_s
+ report[type] = {} if !report[type].is_a?(Hash)
+ report[type][title] = [] if !report[type][title].is_a?(Array)
+ report[type][title].push(change)
+ }
+ report
+ end
end
--
1.6.2.2
Stéphan Gorget wrote:
> Signed-off-by: Stéphan Gorget <gor...@ocre.cea.fr>
> ---
> lib/puppet/defaults.rb | 4 ++-
> lib/puppet/resource/catalog.rb | 2 +-
> lib/puppet/transaction.rb | 7 ++++++
> lib/puppet/transaction/report.rb | 39 +++++++++++++++++++++++++++++++++++++-
> 4 files changed, 49 insertions(+), 3 deletions(-)
Tests?
Regards
James Turnbull
- --
Author of:
* Pro Linux Systems Administration
(http://tinyurl.com/linuxadmin)
* Pulling Strings with Puppet
(http://tinyurl.com/pupbook)
* Pro Nagios 2.0
(http://tinyurl.com/pronagios)
* Hardening Linux
(http://tinyurl.com/hardeninglinux)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFKVurK9hTGvAxC30ARAswdAKCdj0VaDmo7c+Oj4OkczQOXT+QCrACfbZG6
GkPfO+FGziIde8hHB/QSg+A=
=cgIF
-----END PGP SIGNATURE-----
>> - What the output of 'puppetd --changes' will look like?
>
>Those Event instances, in some kind of serialized form. Probably
>defaulting to yaml but tunable via the default_serialization_format or
>whatever that parameter is.
Puppetd was designed to be used as a daemon and does it quite well.
But it lacks a good interface for day to day use, interactively, by admins, in CLI mode, not in daemon.
Having a easily human readable way to known what Puppet has done is important.
So Puppet must have a day to display and sum up the event log without dumping a serialization format.
We need something like:
Package:
installed: httpd, httpd-devel, ntp
removed: iptables6
Service:
started: httpd
File:
...
Please, no dump :)
You would have been displeased if 'ls -l' had returned its output in YAML ;)
Thank
Aurelien
Yeah, that's a good point. We've been talking about ways to make make
the basic interaction a bit better, but you're right that providing a
non-dump format is a good idea. How do you think we should handle
switching between the machine-readable and human-readable formats?
--
Love is a snowmobile racing across the tundra and then suddenly it
flips over, pinning you underneath. At night, the ice weasels come.
--Matt Groening
Can't we do both? Have a --changes flag that does both and a
--changes-serial and --changes-text that outputs each variation? Add a
--serialisation-format option etc.
Regards
James Turnbull
++
It's going to be a structured format internally anyway.
I can see wanting both invocations regularly. One for end users, one
for automated data scraping and reporting.
>
> Regards
>
> James Turnbull
>
> --
> Author of:
> * Pro Linux Systems Administration
> (http://tinyurl.com/linuxadmin)
> * Pulling Strings with Puppet
> (http://tinyurl.com/pupbook)
> * Pro Nagios 2.0
> (http://tinyurl.com/pronagios)
> * Hardening Linux
> (http://tinyurl.com/hardeninglinux)
>
>
--
Nigel Kersten
nig...@google.com
System Administrator
Google, Inc.
>
> On Tue, Jul 14, 2009 at 3:02 PM, James Turnbull<ja...@lovedthanlost.net
> > wrote:
>> Luke Kanies wrote:
>>> Yeah, that's a good point. We've been talking about ways to make
>>> make
>>> the basic interaction a bit better, but you're right that
>>> providing a
>>> non-dump format is a good idea. How do you think we should handle
>>> switching between the machine-readable and human-readable formats?
>>>
>>
>> Can't we do both? Have a --changes flag that does both and a
>> --changes-serial and --changes-text that outputs each variation?
>> Add a
>> --serialisation-format option etc.
>
> ++
>
> It's going to be a structured format internally anyway.
>
> I can see wanting both invocations regularly. One for end users, one
> for automated data scraping and reporting.
Right, the question is, what's a good interface? I don't particularly
like supporting three settings for this one thing...
--
Real freedom lies in wildness, not in civilization.
-- Charles Lindbergh
+1
>
> By the way:
> --no-daemonize is really boring to enter, please add a short option :)
> --changes should also a short-cut also :)
There should probably be proper defaults when using --test (which
already contains --no-daemonize).
Regards, DavidS
Hello
I'm interest by helping fixing this but I'm not sure I will have time to dig into Puppet code.
Moreover, it seems puppet arguments are a bit tricky because lot of arguments are not handled in the main script, but just added to Puppet[:xxx] and test at any place in source code, their difficult to track, and mainly to modify (I do not want to be the guy you will change all the 'if Puppet[:trace]' with 'if current loglevel is greater or equal to trace' :p)
Regarding to short opts, they seems to work, it just seems than in 0.24.8, in puppetd, -D flag is never tested.
What do you think of:
1 - fix -D
2 - use Puppet::Utils::Log.level for all trace output
3 - Add --loglevel|-L option (possible value are err,warn,notice,info,debug,trace,evaltrace)
4 - (difficult part) fix the :trace :evaltrace use in puppet code... :/
Other things should wait for Event code rewritting.
Aurelien
-------- Message d'origine--------
De: puppe...@googlegroups.com de la part de Luke Kanies
Date: jeu. 16/07/2009 03:00
À: puppe...@googlegroups.com
Objet : Re: RE : RE : RE : [Puppet-dev] Re: [PATCH/puppet 1/1] Feature #2400 : add display changes and add them in YAML to the report
Actually looking at the code I don't see a short option for no-daemonize
in the new Application class.
> Luke Kanies wrote:
>>>
>>> By the way:
>>> --no-daemonize is really boring to enter, please add a short
>>> option :)
>>
>> -D theoretically works, but I just tried and it didn't actually
>> work. :/
>
> Actually looking at the code I don't see a short option for no-
> daemonize
> in the new Application class.
It's in defaults.rb:
:daemonize => { :default => true,
:desc => "Send the process into the background. This is
the default.",
:short => "D"
},
The applications don't define this setting, they just test for it in
Puppet[:daemonize], which should correctly be set by our getopt parser
interpreting -D.
--
I think that all good, right thinking people in this country are sick
and tired of being told that all good, right thinking people in this
country are fed up with being told that all good, right thinking people
in this country are fed up with being sick and tired. I'm certainly
not, and I'm sick and tired of being told that I am.
-- Monty Python
But the getopt parser in application.rb only seems to handle long options.
# used to declare code that handle an option
def option(*options, &block)
long = options.find { |opt| opt =~ /^--/
}.gsub(/^--(?:\[no-\])?([^ =]+).*$/, '\1' ).gsub('-','_')
fname = "handle_#{long}"
if (block_given?)
meta_def(symbolize(fname), &block)
else
meta_def(symbolize(fname)) do |value|
self.options["#{long}".to_sym] = value
end
end
@opt_parser.on(*options) do |value|
self.send(symbolize(fname), value)
end
end
Unless I am misreading this?
Ah, I see. That's what needs to be fixed, then.
--
A government that robs Peter to pay Paul can always depend on the
support of Paul. -- George Bernard Shaw
I don't think this part of the code is at fault (I didn't test anything
yet).
I think it is when we convert the Pupppet::Settings defaults to
OptionParser.
I'll fix this ASAP.
Thanks,
--
Brice Figureau
My Blog: http://www.masterzen.fr/
That's what I've read from Aurelien's mails too. But I think exception
traces should always be printed if they signal no known error (in which
case a proper error message is more helpful than the stack trace).
Implementing that should be as easy as setting Puppet[:trace] always to
true and (not|only) removing the ifs.
Regards, DavidS
.Regarding --trace I was just saying that, from my point of view, trace is some debugging information, more precise than just debug, and so, must be an upper level of logging verbosity.
loglevel could have values starting from:
0-nothing
1-err
2-warn (dislays err and warn messages)
3-notice (err + warn + notice)
4-info (above + info)
5-debug (above + debug)
6-trace (above + trace)
(and maybe more...)
Something classic in logging system like
http://www.ruby-doc.org/core/classes/Logger.html
.About -D, my point was to have a short options for --no-daemonize, if -D means daemonize, this is not really usefull because that the standard behaviour of puppetd (what I would expect from something whose named ends with a letter 'd').
So 'puppetd' == 'puppetd -D' :)
If you use puppetd as a daemon, short options is not so important because you do not starts daemon by hand every days, start scripts will do it for you.
But, if you use puppetd as a CLI command (what I do every times), short options is really usefull. And working with puppetd as a CLI tools, means, at least: "puppetd -o --no-daemonize"
That's just a simple letter alias for --no-daemonize :)
Hoping it is clearer now :)
Aurelien
-------- Message d'origine--------
De: puppe...@googlegroups.com de la part de Luke Kanies
Date: ven. 17/07/2009 00:18
À: puppe...@googlegroups.com
Objet : Re: RE : RE : RE : RE : [Puppet-dev] Re: [PATCH/puppet 1/1] Feature #2400 : add display changes and add them in YAML to the report
On 16/07/09 3:00, Luke Kanies wrote:
> On Jul 15, 2009, at 7:29 AM, <Aurelien....@CEA.FR> wrote:
>> By the way:
>> --no-daemonize is really boring to enter, please add a short option :)
>
> -D theoretically works, but I just tried and it didn't actually work. :/
I just tried on 0.25 and traced all the way and -D works fine. But used
with --test on puppetd, then --test is winning because it is processed
afterward. I bet it's what you tested (ie puppetd --test -D), or is it
that you wanted it to mean "--no-daemonize" ?
So I looked to 0.24.8, and it has exactly the same behavior, so I did
the right thing when I applied the application stuff :-)
Now the question is: should we change this behaviour and have --test
processed at the moment it appears on the command line (so it can be
overriden by other parameters)?
[snipped last part of discussion]
>
> Hi,
>
> On 16/07/09 3:00, Luke Kanies wrote:
>> On Jul 15, 2009, at 7:29 AM, <Aurelien....@CEA.FR> wrote:
>>> By the way:
>>> --no-daemonize is really boring to enter, please add a short
>>> option :)
>>
>> -D theoretically works, but I just tried and it didn't actually
>> work. :/
>
> I just tried on 0.25 and traced all the way and -D works fine. But
> used
> with --test on puppetd, then --test is winning because it is processed
> afterward. I bet it's what you tested (ie puppetd --test -D), or is it
> that you wanted it to mean "--no-daemonize" ?
Yeah, I realized that -D == --daemonize, which is not actually what we
want. Duh. My bad.
>
> So I looked to 0.24.8, and it has exactly the same behavior, so I did
> the right thing when I applied the application stuff :-)
>
> Now the question is: should we change this behaviour and have --test
> processed at the moment it appears on the command line (so it can be
> overriden by other parameters)?
I think the main thing was not having a short form of --no-daemonize,
and I don't think the current system will provide an easy way to add a
short alias for disabling booleans.
--
Criminal: A person with predatory instincts who has not sufficient
capital to form a corporation. --Howard Scott
Yes, that won't be easy, mainly because I don't know how to represent a
no-short option (ie +D instead of -D, that feel, hu, awkward).
I'm wondering what's the posix way of handling this, if there's one...
I assigned you #2420, feel free to reject or bounce it back to me if you
want me to try to fix it.
Also, note that -D is the only short option we have for the whole
defaults settings (I mean outside of apps). Getting rid of it will,
somehow _generalize_ the options :-)
--no-D? :)
>
> I'm wondering what's the posix way of handling this, if there's one...
> I assigned you #2420, feel free to reject or bounce it back to me if
> you
> want me to try to fix it.
Will do.
>
> Also, note that -D is the only short option we have for the whole
> defaults settings (I mean outside of apps). Getting rid of it will,
> somehow _generalize_ the options :-)
Weird. We really just need to use them a lot more, rather than
getting rid of the one we have.
--
The whole secret of life is to be interested in one thing profoundly
and in a thousand things well. -- Horace Walpole
I do not think there is a POSIX way for that.
At least there is a GNU way.
In fact, the behaviour --foo and --no-foo <=> --foo=no it's not
something we can call "standard", this is more puppetish.
So up to us to define a Puppet way to attribute short options for them.
One proposition could be
Assign a letter, lowercase means 'yes', uppercase means 'no'
By example:
--daemonize|-d
--no-daemonize|-D
(but this example conflicts with --debug|-d)
>> Also, note that -D is the only short option we have for the whole
>> defaults settings (I mean outside of apps). Getting rid of it will,
>> somehow _generalize_ the options :-)
> Weird. We really just need to use them a lot more, rather than
> getting rid of the one we have.
If this is the only option using this trick, as it behaves like the
default option, I quite sure near nobody use it.
So, it is possible to redefined how this is handled.
Another possibility, is to define special args, in 'puppetd' and
'puppet' scripts, on a case by case basis.
--
Aurelien Degremont
CEA
>
> Luke Kanies a écrit :
>>> I'm wondering what's the posix way of handling this, if there's
>>> one...
>
> I do not think there is a POSIX way for that.
> At least there is a GNU way.
>
> In fact, the behaviour --foo and --no-foo <=> --foo=no it's not
> something we can call "standard", this is more puppetish.
I've seen this somewhere before, I'm pretty sure, but I don't know
where.
>
> So up to us to define a Puppet way to attribute short options for
> them.
> One proposition could be
>
> Assign a letter, lowercase means 'yes', uppercase means 'no'
> By example:
>
> --daemonize|-d
> --no-daemonize|-D
>
> (but this example conflicts with --debug|-d)
Yeah. And even then, we need a way to specify short inverse options,
which our system doesn't currently provide.
>
>>> Also, note that -D is the only short option we have for the whole
>>> defaults settings (I mean outside of apps). Getting rid of it will,
>>> somehow _generalize_ the options :-)
>> Weird. We really just need to use them a lot more, rather than
>> getting rid of the one we have.
>
> If this is the only option using this trick, as it behaves like the
> default option, I quite sure near nobody use it.
> So, it is possible to redefined how this is handled.
True. How would you like to change it?
>
> Another possibility, is to define special args, in 'puppetd' and
> 'puppet' scripts, on a case by case basis.
Yeah. That's probably the best option in the short term.
--
It is absurd to divide people into good and bad. People are either
charming or tedious. -- Oscar Wilde