Merge pull request #3221 from fotioslindiakos/Bug989706

Merged by openshift-bot

node/lib/openshift-origin-node/utils/cgroups/libcgroup.rb

@@ -25,9 +25,7 @@ module OpenShift
   module Runtime
     module Utils
       class Cgroups
-
         class Libcgroup
-
           @@DEFAULT_CGROUP_ROOT='/openshift'
           @@DEFAULT_CGROUP_SUBSYSTEMS='cpu,cpuacct,memory,net_cls,freezer'
 
@@ -141,6 +139,20 @@ def uid
             @uid_cache ||= Etc.getpwnam(@uuid).uid
           end
 
+          def self.usage
+            cmd = 'grep -H "" */{cpu.stat,cpuacct.usage,cpu.cfs_quota_us} 2> /dev/null'
+            (out, err, rc) = ::OpenShift::Runtime::Utils::oo_spawn(cmd, :chdir => cgroup_paths['cpu'], :expected_exitstatus => 0, :quiet => true)
+            parse_usage(out)
+          end
+
+          def self.parse_usage(info)
+            info.lines.to_a.inject(Hash.new{|h,k| h[k] = {}} ) do |h,line|
+              (uuid, key, val) = line.split(/\W/).values_at(0,-2,-1)
+              h[uuid][key.to_sym] = val.to_i
+              h
+            end
+          end
+
           # Public: Create a cgroup namespace for the gear
           def create(defaults={})
             newcfg = Hash["perm", {}, *(subsystems.map { |s| [s,{}] }.flatten)]
@@ -467,8 +479,6 @@ def overwrite_with_safe_swap(filename)
 
             r
           end
-
-
         end
       end
     end

node/lib/openshift-origin-node/utils/cgroups/throttler.rb

@@ -2,7 +2,6 @@
 
 require 'active_support/core_ext/numeric/time'
 require 'openshift-origin-node/utils/cgroups'
-require 'openshift-origin-node/utils/shell_exec'
 require 'syslog'
 require_relative 'monitored_gear'
 
@@ -11,10 +10,10 @@ module Runtime
     module Utils
       class Cgroups
         class Throttler
+
           attr_reader :wanted_keys, :uuids, :running_apps, :threshold, :interval
 
           @@conf_file = '/etc/openshift/resource_limits.conf'
-          @@cgroups_dir = Libcgroup.cgroup_paths["cpu"]
 
           def initialize
             # Make sure we create a MonitoredGear for the root OpenShift cgroup
@@ -50,20 +49,6 @@ def initialize
 
           # Loop through all lines from grep and contruct a hash
           # TODO: Should this be moved into libcgroup?
-          def parse_usage(info)
-            info.lines.to_a.inject(Hash.new{|h,k| h[k] = {}} ) do |h,line|
-              (uuid, key, val) = line.split(/\W/).values_at(0,-2,-1)
-              h[uuid][key.to_sym] = val.to_i
-              h
-            end
-          end
-
-          # TODO: Should this be moved into libcgroup?
-          def get_usage
-            cmd = 'grep -H "" */{cpu.stat,cpuacct.usage,cpu.cfs_quota_us} 2> /dev/null'
-            out = ::OpenShift::Runtime::Utils::oo_spawn(cmd, :chdir => @@cgroups_dir).first
-            out
-          end
 
           def start
             Thread.new do
@@ -75,8 +60,7 @@ def start
           end
 
           def tick
-            usage = get_usage
-            vals = parse_usage(usage)
+            vals = Libcgroup.usage
             vals = Hash[vals.map{|uuid,hash| [uuid,hash.select{|k,v| wanted_keys.include?k}]}]
 
             update(vals)

node/lib/openshift-origin-node/utils/logger/split_trace_logger.rb

@@ -36,18 +36,18 @@ module NodeLogger
       #
       class SplitTraceLogger
         PROFILES = {
-            :standard => {
-                file_config:   'PLATFORM_LOG_FILE',
+          :standard => {
+            file_config:   'PLATFORM_LOG_FILE',
             level_config:  'PLATFORM_LOG_LEVEL',
             default_file:  File.join(File::SEPARATOR, %w{var log openshift node platform.log}),
             default_level: Logger::DEBUG
-        },
-            :trace    => {
+          },
+          :trace    => {
             file_config:   'PLATFORM_TRACE_LOG_FILE',
             level_config:  'PLATFORM_TRACE_LOG_LEVEL',
             default_file:  File.join(File::SEPARATOR, %w{var log openshift node platform-trace.log}),
             default_level: Logger::INFO
-        }
+          }
         }
 
         def initialize(config, context)

node/lib/openshift-origin-node/utils/node_logger.rb

@@ -123,6 +123,10 @@ def self.logger
       def self.context
         @context ||= {}
       end
+
+      def self.set_logger(logger)
+        @logger = logger
+      end
     end
   end
 end

node/lib/openshift-origin-node/utils/shell_exec.rb

@@ -74,6 +74,7 @@ def to_s
       #                                provided +IO+ object.
       #   :err                       : If specified, STDERR from the child process will be redirected to the
       #                                provided +IO+ object.
+      #   :quiet                     : If specified, the output from the command will not be logged
       #
       # NOTE: If the +out+ or +err+ options are specified, the corresponding return value from +oo_spawn+
       # will be the incoming/provided +IO+ objects instead of the buffered +String+ output. It's the
@@ -123,7 +124,7 @@ def self.oo_spawn(command, options = {})
               write_stderr.close
 
               out, err, status = read_results(pid, read_stdout, read_stderr, options)
-              NodeLogger.logger.debug { "Shell command '#{command}' ran. rc=#{status.exitstatus} out=#{out}" }
+              NodeLogger.logger.debug { "Shell command '#{command}' ran. rc=#{status.exitstatus} out=#{options[:quiet] ? "[SILENCED]" : out}" }
 
               if (!options[:expected_exitstatus].nil?) && (status.exitstatus != options[:expected_exitstatus])
                 raise ::OpenShift::Runtime::Utils::ShellExecutionException.new(

node/test/unit/cgroup_test.rb

@@ -60,6 +60,22 @@ def setup
     @cls = OpenShift::Runtime::Utils::Cgroups
     @clsany = @cls.any_instance
     @cgroups = @cls.new(@uuid)
+
+    @mock_usage = {
+      "good" => {
+        usage: 1,
+        throttled_time: 2,
+        nr_periods: 3,
+        cfs_quota_us: 4
+      },
+      "bad" => {
+        usage: 5,
+        throttled_time: 6,
+        nr_periods: 7,
+        cfs_quota_us: 8
+      }
+    }
+    @mock_usage_str = fake_usage(@mock_usage)
   end
 
   def call_set_call(meth, templ)
@@ -232,4 +248,27 @@ def test_show_templates
     end
   end
 
+  def test_parse_usage
+    usage = OpenShift::Runtime::Utils::Cgroups::Libcgroup.parse_usage(@mock_usage_str)
+    assert_equal @mock_usage, usage
+  end
+
+  protected
+  def fake_usage(gears)
+    gears.inject("") do |a,(uuid,vals)|
+      v = vals.clone
+      v[:uuid] = uuid
+      str = usage_template % v
+      a << str
+    end.lines.map(&:strip).join("\n")
+  end
+
+  def usage_template
+    <<-STR
+      %<uuid>s/cpuacct.usage:%<usage>d
+      %<uuid>s/cpu.stat:throttled_time %<throttled_time>d
+      %<uuid>s/cpu.stat:nr_periods %<nr_periods>d
+      %<uuid>s/cpu.cfs_quota_us:%<cfs_quota_us>d
+    STR
+  end
 end

node/test/unit/throttler_test.rb

@@ -36,8 +36,6 @@ def setup
       }
     }
 
-    @mock_usage_str = fake_usage(@mock_usage)
-
     @mock_apps = {
       "A" => mock(@@mg.to_s),
       "B" => mock(@@mg.to_s),
@@ -62,11 +60,6 @@ def test_init
     assert_equal threshold, throttler.threshold
   end
 
-  def test_parse_usage
-    usage = @throttler.parse_usage(@mock_usage_str)
-    assert_equal @mock_usage, usage
-  end
-
   def test_update
     good = @mock_apps.values.first
     # Ensure our data is updated twice
@@ -307,22 +300,4 @@ def with_mock_apps(args)
 
     yield mock_apps, mock_util
   end
-
-  def fake_usage(gears)
-    gears.inject("") do |a,(uuid,vals)|
-      v = vals.clone
-      v[:uuid] = uuid
-      str = usage_template % v
-      a << str
-    end.lines.map(&:strip).join("\n")
-  end
-
-  def usage_template
-    <<-STR
-      %<uuid>s/cpuacct.usage:%<usage>d
-      %<uuid>s/cpu.stat:throttled_time %<throttled_time>d
-      %<uuid>s/cpu.stat:nr_periods %<nr_periods>d
-      %<uuid>s/cpu.cfs_quota_us:%<cfs_quota_us>d
-    STR
-  end
 end