ref: refs/heads/master
commit 991e553dad608b0f1e493670baa570f340cd72de
Author: Duncan Mac-Vicar P
Date: Mon Jul 20 14:43:53 2009 +0200
code cleanup and testsuite update
- stop using IO.popen
- factor methods in smaller methods, otherwise test is hell
- make testsuite actually pass
If you are going to pass something you got from the web, parse it in the
controller layer, but a library class does not have any reason to get
time as strings, which makes default parameters a mess.
---
.../status/app/controllers/status_controller.rb | 24 +++-
plugins/status/lib/status.rb | 127 +++++++++++---------
plugins/status/test/unit/status_test.rb | 98 ++++++++++++----
3 files changed, 162 insertions(+), 87 deletions(-)
diff --git a/plugins/status/app/controllers/status_controller.rb b/plugins/status/app/controllers/status_controller.rb
index 33d26f6..56db2df 100644
--- a/plugins/status/app/controllers/status_controller.rb
+++ b/plugins/status/app/controllers/status_controller.rb
@@ -52,10 +52,15 @@ class StatusController < ApplicationController
f.close
@status = Status.new
- @status.collect_data(params[:start], params[:stop], params[:data])
- #logger.debug "SHOW: #{@status.inspect}"
-
- render :show
+ # use now if time is not valid
+ begin
+ stop = params[:stop].blank? ? Time.now : Time.at(params[:stop].to_i)
+ start = params[:start].blank? ? stop - 300 : Time.at(params[:start].to_i)
+ @status.collect_data(start, stop, params[:data])
+ render :show
+ rescue Exception => e
+ render :text => e.to_s, :status => 400 # bad request
+ end
end
end
@@ -72,9 +77,14 @@ class StatusController < ApplicationController
render ErrorResult.error(403, 1, "no permission") and return
else
@status = Status.new
- #logger.debug params.inspect
- @status.set_datapath()
- @status.collect_data(params[:start], params[:stop], params[:data])
+
+ begin
+ stop = params[:stop].blank? ? Time.now : Time.at(params[:stop].to_i)
+ start = params[:start].blank? ? stop - 300 : Time.at(params[:start].to_i)
+ @status.collect_data(start, stop, params[:data])
+ rescue Exception => e
+ render :text => e.to_s, :status => 400 # bad request
+ end
end
end
diff --git a/plugins/status/lib/status.rb b/plugins/status/lib/status.rb
index 42cb73d..abf6e3a 100644
--- a/plugins/status/lib/status.rb
+++ b/plugins/status/lib/status.rb
@@ -1,19 +1,22 @@
+#
+# Wrapper over collectd
+#
class Status
require 'scr'
require 'yaml'
attr_accessor :data,
- :datapath,
:health_status,
:metrics
-
+
def initialize()
@scr = Scr.instance
@health_status = nil
@data = Hash.new
+ # force initialization of datapath
+ datapath
start_collectd
- @datapath = set_datapath
@metrics = available_metrics
#find the correct plugin path for the config file
@@ -29,6 +32,20 @@ class Status
@limits = YAML.load(File.open(File.join(plugin_config_dir, "status_limits.yaml"))) if File.exists?(File.join(plugin_config_dir, "status_limits.yaml"))
end
+ # returns the data path
+ def datapath
+ if @datapath.blank?
+ # if no datapath is set, use the first directory in /var/lib/collectd
+ @datapath = Dir.glob("/var/lib/collectd/*").first
+ end
+ @datapath
+ end
+
+ # set path of stored rrd files, default: /var/lib/collectd/$host.$domain
+ def datapath=(path=nil)
+ @datapath = path.chomp("/")
+ end
+
def start_collectd
cmd = @scr.execute(["collectd"])
@timestamp = Time.now
@@ -39,43 +56,34 @@ class Status
@timestamp = nil
end
- # set path of stored rrd files, default: /var/lib/collectd/$host.$domain
- def set_datapath(path=nil)
- if path.blank? #FIXME: read currently used rrdb file from /etc/collectd.conf
- cmd = IO.popen("ls /var/lib/collectd/")
- path = cmd.read
- cmd.close
- path = path.split " " #FIXME
- @datapath = "/var/lib/collectd/#{path[0].strip}"
- else # set default path
- @datapath = path.chomp("/")
- end
- return @datapath
+ # returns available metric types, which are the directories in the
+ # data path.
+ # requires datapath to be configured
+ # ie: ['cpu', 'memory']
+ def metric_types
+ Dir.glob(File.join(datapath,"*")).reject{|x| not File.directory?(x) }.map{|x| File.basename(x) }
end
+ # returns the full path of metric databases for a metric type
+ # ie: /var/foo/cpu/file.rrd
+ def metric_files(metrictype)
+ Dir.glob(File.join(datapath, metrictype, "*.rrd"))
+ end
+
+ # creates a hash from metric type (cpu, etc) to
def available_metrics
- @metrics = Hash.new
- cmd = IO.popen("ls #{@datapath}")
- output = cmd.read
- cmd.close
- output.split(" ").each do |l|
- fp = IO.popen("ls #{@datapath}/#{l}")
- files = fp.read
- fp.close
+ metrics = Hash.new
+ # look in datapath, except for non directories, and get the last
+ # component of the path ie: interface, cpu, etc
+ metric_types.each do |metrictype|
rrds = Array.new
- files.split(" ").each do |d|
- rrds << d if d.include? ".rrd" #only .rrd files
- @metrics["#{l}"] = { :rrds => rrds}
+ metric_files(metrictype).each do |rrdfile|
+ rrds << rrdfile
+ #puts "#{rrdfile} at #{metrictype}"
end
+ metrics["#{metrictype}"] = { :rrds => rrds}
end
- return @metrics
- end
-
- def available_metric_files
- cmd = IO.popen("ls #{@datapath}..")
- lines = cmd.read.split "\n"
- cmd.close
- return lines
+ metrics
end
def determine_status
@@ -92,20 +100,20 @@ class Status
case data
when nil, "all", "All" # all metrics
@metrics.each_pair do |m, n|
- @metrics["#{m}"][:rrds].each do |rrdb|
- result["#{rrdb}".chomp(".rrd")] = fetch_metric("#{m}/#{rrdb}", start, stop)
+ @metrics[m][:rrds].each do |rrdb|
+ result[File.basename(rrdb).chomp('.rrd')] = fetch_metric(rrdb, start, stop)
end
- @data["#{m}"] = result
+ @data[m] = result
result = Hash.new
end
else # only metrics in data
data.each do |d|
@metrics.each_pair do |m, n|
if m.include?(d)
- @metrics["#{m}"][:rrds].each do |rrdb|
- result["#{rrdb}".chomp(".rrd")] = fetch_metric("#{m}/#{rrdb}", start, stop)
+ @metrics[m][:rrds].each do |rrdb|
+ result[File.basename(rrdb).chomp('.rrd')] = fetch_metric(rrdb, start, stop)
end
- @data["#{m}"] = result
+ @data[m] = result
result = Hash.new
end
end
@@ -115,23 +123,29 @@ class Status
return @data
end
+ # runs the rddtool on file with start time and end time
+ # default last 5 minutes from now
+ def run_rrdtool(file, start, stop)
+ stop = Time.now if stop.nil?
+ # fetch last 5 minutes
+ start = stop - 300 if start.nil?
+
+ cmd = IO.popen("rrdtool fetch #{file} AVERAGE --start #{start.strftime("%H:%M,%m/%d/%Y")} --end #{stop.strftime("%H:%M,%m/%d/%Y")}")
+ output = cmd.read
+ cmd.close
+ output
+ end
+
# creates one metric for defined period
+ # parameters are the file to read and the
+ # time interval
+ #
+ # If no time is given, last 5 minutes are used
def fetch_metric(rrdfile, start=nil, stop=nil)
result = Hash.new
- if start.blank?
- start = "--start #{Time.now.strftime("%H:%M,%m/%d/%Y")}"
- else
- start = "--start #{start}"
- end
- if stop.blank?
- stop = "--end #{Time.now.strftime("%H:%M,%m/%d/%Y")}"
- else
- stop = "--end #{stop}"
- end
- cmd = IO.popen("rrdtool fetch #{@datapath}/#{rrdfile} AVERAGE #{start} #{stop}")
- output = cmd.read
- cmd.close
- return "failed" if output.blank?
+ output = run_rrdtool(rrdfile, start, stop)
+
+ raise "Error running collectd rrdtool" if output =~ /ERROR/ or output.nil?
labels=""
output = output.gsub(",", ".") # translates eg. 1,234e+07 to 1.234e+07
@@ -163,8 +177,7 @@ class Status
end
#setting the limits
- result.each do |key, value|
-
+ result.each do |key, value|
path = rrdfile.chomp(".rrd")
path +="/" + key if key!="value" #do not take care about the value flag
path = path.tr('-','_')
@@ -176,7 +189,7 @@ class Status
return result
else
- return "failed"
+ raise "error reading data from rrdtool"
end
end
end
diff --git a/plugins/status/test/unit/status_test.rb b/plugins/status/test/unit/status_test.rb
index ed8cac4..830afb3 100644
--- a/plugins/status/test/unit/status_test.rb
+++ b/plugins/status/test/unit/status_test.rb
@@ -11,39 +11,40 @@ class StatusTest < ActiveSupport::TestCase
def test_set_datapath
Scr.instance.stubs(:execute).with(["collectd"]).returns(nil)
- status = Status.new()
- assert status.set_datapath("/var/lib/collectd/test/"), "/var/lib/collectd/test"
- status = Status.new()
- assert_equal status.set_datapath("/test/foo.bar"), "/test/foo.bar"
+ status = Status.new
+ assert (status.datapath = "/var/lib/collectd/test/"), "/var/lib/collectd/test"
+ status = Status.new
+ assert_equal (status.datapath = "/test/foo.bar"), "/test/foo.bar"
end
def test_set_datapath_default
Scr.instance.stubs(:execute).with(["collectd"]).returns(nil)
+
IO.stubs(:popen).with("hostname").returns(String) #FIXME: replace String with IO
IO.stubs(:popen).with("domainname").returns(String) # returns(IO.new(2, "r+")) dont work
IO.stubs(:popen).with("ls /var/lib/collectd/").returns(String) # because of missing EOF token
String.stubs(:read).with(nil).returns("test")
String.stubs(:close).with(nil).returns(nil)
- status = Status.new()
- assert_equal status.set_datapath(), "/var/lib/collectd/test"
+ # test that datapath is initialized correctly
+ Dir.stubs(:glob).returns(["/var/lib/collectd/test","/var/lib/collectd/test2"])
+ status = Status.new
+ assert_equal "/var/lib/collectd/test", status.datapath
end
def test_available_metrics
Scr.instance.stubs(:execute).with(["collectd"]).returns(nil)
+ status = Status.new
-# stubs(:set_datapath)
-
- IO.stubs(:popen).with("ls /var/lib/collectd").returns(String) #FIXME: replace String with IO
- IO.stubs(:popen).with("ls /var/lib/collectd/cpu").returns(String)
- IO.stubs(:popen).with("ls /var/lib/collectd/memory").returns(String)
- String.stubs(:read).with(nil).returns("cpu memory")
- String.stubs(:close).with(nil).returns(nil)
-
- status = Status.new()
+ # simulate environment
+ status.stubs(:metric_types).returns(['cpu', 'memory'])
+ status.stubs(:metric_files).with('cpu').returns(['/var/lib/collectd/cpu/cpuheat.rrd'])
+ status.stubs(:metric_files).with('memory').returns(['/var/lib/collectd/memory/memory.rrd'])
+
status.datapath = "/var/lib/collectd"
- assert_equal status.available_metrics, {"memory"=>{:rrds=>[]}, "cpu"=>{:rrds=>[]}}
-#{"memory"=>{:rrds=>["cpu", "memory"]}, "cpu"=>{:rrds=>["cpu", "memory"]}}
+ fake_metrics = {"memory"=>{:rrds=>["/var/lib/collectd/memory/memory.rrd"]},
+ "cpu"=>{:rrds=>["/var/lib/collectd/cpu/cpuheat.rrd"]}}
+ assert_equal fake_metrics, status.available_metrics
end
def test_collect_data
@@ -52,13 +53,64 @@ class StatusTest < ActiveSupport::TestCase
def test_fetch_data
Scr.instance.stubs(:execute).with(["collectd"]).returns(nil)
- IO.stubs(:popen).with("rrdtool fetch /test/memory-free.rrd AVERAGE --start #{Time.now.strftime("%H:%M,%m/%d/%Y")} --stop #{Time.now.strftime("%H:%M,%m/%d/%Y")}").returns(String)
- String.stubs(:read).with(nil).returns(" value\n\n1247156690: nan\n1247156700: nan\n1247156710: nan\n")
- String.stubs(:close).with(nil).returns(nil)
+ stop = Time.now
+ start = Time.now - 300
+
+ status = Status.new
+
+ rrd_output = < {"value" => \
- {"T_1247156690" => "nan", "T_1247156700" => "nan", "T_1247156710" => "nan"}}}
+
+ expected_response = {"tx"=>
+ {"T_1248093000"=>"1.5542857143e+01",
+ "T_1248092930"=>"2.8285714286e+00",
+ "T_1248092370"=>"2.1435714286e+01",
+ "T_1248092580"=>"4.3878571429e+01",
+ "T_1248092790"=>"4.0714285714e+00",
+ "T_1248093070"=>"2.7071428571e+00",
+ "T_1248092440"=>"4.0992857143e+01",
+ "T_1248092650"=>"9.7942857143e+01",
+ "T_1248092860"=>"2.4142857143e+00",
+ "T_1248092300"=>"4.0500000000e+00",
+ "T_1248092510"=>"3.4264285714e+01",
+ "T_1248092720"=>"2.9928571429e+00"},
+ "rx"=>
+ {"T_1248093000"=>"3.1314285714e+01",
+ "T_1248092930"=>"2.5092857143e+01",
+ "T_1248092370"=>"4.7314285714e+01",
+ "T_1248092580"=>"7.7485714286e+01",
+ "T_1248092790"=>"2.2585714286e+01",
+ "T_1248093070"=>"2.3064285714e+01",
+ "T_1248092440"=>"5.7578571429e+01",
+ "T_1248092650"=>"1.1698571429e+02",
+ "T_1248092860"=>"2.4292857143e+01",
+ "T_1248092300"=>"2.4628571429e+01",
+ "T_1248092510"=>"4.9271428571e+01",
+ "T_1248092720"=>"2.3042857143e+01"}}
+
+ assert_equal expected_response, status.fetch_metric("/test/memory-free.rrd", start, stop)
end
end
--
To unsubscribe, e-mail: yast-commit+unsubscribe@opensuse.org
For additional commands, e-mail: yast-commit+help@opensuse.org
