From 5f1e0ce0ffbdac3ce20c4d75e5ebebe205e21108 Mon Sep 17 00:00:00 2001 From: "Ruben S. Montero" Date: Fri, 21 Feb 2020 14:20:53 +0100 Subject: [PATCH] L #4089: Fix linting and code refactoring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit co-authored-by: Christian González --- install.sh | 3 +- share/linters/.rubocop.yml | 5 +- src/vmm_mad/remotes/firecracker/shutdown | 2 + src/vmm_mad/remotes/lib/command.rb | 68 +++++++++++++++++++ src/vmm_mad/remotes/lib/firecracker/client.rb | 6 +- .../remotes/lib/firecracker/command.rb | 45 +----------- .../remotes/lib/firecracker/microvm.rb | 52 +++----------- .../remotes/lib/firecracker/opennebula_vm.rb | 15 ++-- src/vmm_mad/remotes/lib/lxd/command.rb | 38 +---------- .../remotes/bridge/clean.d/firecracker | 2 + src/vnm_mad/remotes/bridge/pre.d/firecracker | 1 - src/vnm_mad/remotes/lib/nic.rb | 4 +- 12 files changed, 108 insertions(+), 133 deletions(-) create mode 100644 src/vmm_mad/remotes/lib/command.rb diff --git a/install.sh b/install.sh index 9657eaf1eb..0deb7fcec5 100755 --- a/install.sh +++ b/install.sh @@ -836,7 +836,8 @@ PM_EXEC_PACKET_SCRIPTS="src/pm_mad/remotes/packet/cancel \ # $REMOTES_LOCATION/vmm/lib #------------------------------------------------------------------------------- -VMM_EXEC_LIB_FILES="src/vmm_mad/remotes/lib/poll_common.rb" +VMM_EXEC_LIB_FILES="src/vmm_mad/remotes/lib/poll_common.rb \ + src/vmm_mad/remotes/lib/command.rb" #------------------------------------------------------------------------------- # VMM Lib vcenter files, used by the vCenter Driver to be installed in diff --git a/share/linters/.rubocop.yml b/share/linters/.rubocop.yml index 232519d50b..6ded4b01d6 100644 --- a/share/linters/.rubocop.yml +++ b/share/linters/.rubocop.yml @@ -730,8 +730,11 @@ Lint/ShadowingOuterLocalVariable: # Check for debugger info Lint/Debugger: - Enabled: true + Enabled: true +# Check file permissions +Lint/ScriptPermission: + Enabled: false ######### # METRICS diff --git a/src/vmm_mad/remotes/firecracker/shutdown b/src/vmm_mad/remotes/firecracker/shutdown index cbe81e5764..4e9cc1c794 100755 --- a/src/vmm_mad/remotes/firecracker/shutdown +++ b/src/vmm_mad/remotes/firecracker/shutdown @@ -27,7 +27,9 @@ vm_id = ARGV[2] xml = STDIN.read # TODO, custom socket path for client +# rubocop:disable Layout/LineLength client = FirecrackerClient.new("/srv/jailer/firecracker/one-#{vm_id}/root/api.socket") +# rubocop:enable Layout/LineLength microvm = MicroVM.new_from_xml(xml, client) # Stop VNC diff --git a/src/vmm_mad/remotes/lib/command.rb b/src/vmm_mad/remotes/lib/command.rb new file mode 100644 index 0000000000..c8406811ff --- /dev/null +++ b/src/vmm_mad/remotes/lib/command.rb @@ -0,0 +1,68 @@ +#!/usr/bin/ruby + +# -------------------------------------------------------------------------- # +# Copyright 2002-2019, OpenNebula Project, OpenNebula Systems # +# # +# Licensed under the Apache License, Version 2.0 (the "License"); you may # +# not use this file except in compliance with the License. You may obtain # +# a copy of the License at # +# # +# http://www.apache.org/licenses/LICENSE-2.0 # +# # +# Unless required by applicable law or agreed to in writing, software # +# distributed under the License is distributed on an "AS IS" BASIS, # +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # +# See the License for the specific language governing permissions and # +# limitations under the License. # +#--------------------------------------------------------------------------- # + +require 'open3' + +# This module can be used to execute commands. It wraps popen3 and provides +# locking capabilites using flock +module Command + + def self.execute(cmd, block) + stdout = '' + stderr = '' + + begin + fd = lock if block + + stdout, stderr, s = Open3.capture3(cmd) + ensure + unlock(fd) if block + end + + [s.exitstatus, stdout, stderr] + end + + def self.execute_once(cmd, lock) + execute(cmd, lock) unless running?(cmd.split[0]) + end + + def self.execute_rc_log(cmd, lock = false) + rc, _stdout, stderr = execute(cmd, lock) + + puts stderr unless rc.zero? + + rc.zero? + end + + # Return true if command is running + def self.running?(command) + !`ps --noheaders -C #{command}`.empty? + end + + def self.lock + lfd = File.open(LOCK_FILE, 'w') + lfd.flock(File::LOCK_EX) + + lfd + end + + def self.unlock(lfd) + lfd.close + end + +end diff --git a/src/vmm_mad/remotes/lib/firecracker/client.rb b/src/vmm_mad/remotes/lib/firecracker/client.rb index b015627664..75b2e4c519 100644 --- a/src/vmm_mad/remotes/lib/firecracker/client.rb +++ b/src/vmm_mad/remotes/lib/firecracker/client.rb @@ -21,8 +21,8 @@ require 'socket' require 'json' # -# Firecracker API Client. This class is used to interact with Firecracker. Wraps API calls -# through the REST API +# Firecracker API Client. This class is used to interact with Firecracker. +# Wraps API calls through the REST API # class FirecrackerClient @@ -35,11 +35,13 @@ class FirecrackerClient API_RETRY = 5 # Attempts, in case a response is failed to read from LXD def initialize(socket_path) + # rubocop:disable Style/RescueStandardError begin @socket = socket(socket_path) rescue raise "Failed to open socket: #{socket_path}" end + # rubocop:enable Style/RescueStandardError end # Performs HTTP::Get diff --git a/src/vmm_mad/remotes/lib/firecracker/command.rb b/src/vmm_mad/remotes/lib/firecracker/command.rb index bbd1fb398e..a936e5dc82 100644 --- a/src/vmm_mad/remotes/lib/firecracker/command.rb +++ b/src/vmm_mad/remotes/lib/firecracker/command.rb @@ -16,7 +16,7 @@ # limitations under the License. # #--------------------------------------------------------------------------- # -require 'open3' +require_relative '../lib/command' # This module can be used to execute commands. It wraps popen3 and provides # locking capabilites using flock @@ -24,47 +24,4 @@ module Command LOCK_FILE = '/tmp/onefirecracker-lock' - def self.execute(cmd, block) - stdout = '' - stderr = '' - - begin - fd = lock if block - - stdout, stderr, s = Open3.capture3(cmd) - ensure - unlock(fd) if block - end - - [s.exitstatus, stdout, stderr] - end - - def self.execute_once(cmd, lock) - execute(cmd, lock) unless running?(cmd.split[0]) - end - - def self.execute_rc_log(cmd, lock = false) - rc, _stdout, stderr = execute(cmd, lock) - - puts stderr unless rc.zero? - - rc.zero? - end - - # Return true if command is running - def self.running?(command) - !`ps --noheaders -C #{command}`.empty? - end - - def self.lock - lfd = File.open(LOCK_FILE, 'w') - lfd.flock(File::LOCK_EX) - - lfd - end - - def self.unlock(lfd) - lfd.close - end - end diff --git a/src/vmm_mad/remotes/lib/firecracker/microvm.rb b/src/vmm_mad/remotes/lib/firecracker/microvm.rb index 020f4a7a1e..d6ac598d68 100644 --- a/src/vmm_mad/remotes/lib/firecracker/microvm.rb +++ b/src/vmm_mad/remotes/lib/firecracker/microvm.rb @@ -20,11 +20,13 @@ $LOAD_PATH.unshift File.dirname(__FILE__) require 'client' require 'opennebula_vm' -require 'command' # TODO, use same class LXD # This class interacts with Firecracker class MicroVM + # rubocop:disable Naming/AccessorMethodName + # rubocop:disable Layout/LineLength + #--------------------------------------------------------------------------- # Class constructors & static methods #--------------------------------------------------------------------------- @@ -42,56 +44,21 @@ class MicroVM # Location for maping the context @map_location = "#{@one.sysds_path}/#{@one.vm_id}/map_context" - if !@one.nil? - @rootfs_dir = "/srv/jailer/firecracker/#{@one.vm_name}/root" - @context_path = "#{@rootfs_dir}/context" - end + return if @one.nil? + + @rootfs_dir = "/srv/jailer/firecracker/#{@one.vm_name}/root" + @context_path = "#{@rootfs_dir}/context" end class << self - # Returns specific container, by its name - # Params: - # +name+:: container name - def get(name, one_xml, client) - info = client.get("#{CONTAINERS}/#{name}")['metadata'] - - one = nil - one = OpenNebulaVM.new(one_xml) if one_xml - - Container.new(info, one, client) - end - - # Creates container from a OpenNebula VM xml description + # Creates microVM from a OpenNebula VM xml description def new_from_xml(one_xml, client) one = OpenNebulaVM.new(one_xml) MicroVM.new(one.to_fc, one, client) end - # Returns an array of container objects - def get_all(client) - containers = [] - - container_names = client.get(CONTAINERS)['metadata'] - container_names.each do |name| - name = name.split('/').last - containers.push(get(name, nil, client)) - end - - containers - end - - # Returns boolean indicating the container exists(true) or not (false) - def exist?(name, client) - client.get("#{CONTAINERS}/#{name}") - true - rescue LXDError => e - raise e if e.code != 404 - - false - end - end #--------------------------------------------------------------------------- @@ -302,4 +269,7 @@ class MicroVM rc end + # rubocop:enable Naming/AccessorMethodName + # rubocop:enable Layout/LineLength + end diff --git a/src/vmm_mad/remotes/lib/firecracker/opennebula_vm.rb b/src/vmm_mad/remotes/lib/firecracker/opennebula_vm.rb index 68099ba8f7..6bff9cde72 100644 --- a/src/vmm_mad/remotes/lib/firecracker/opennebula_vm.rb +++ b/src/vmm_mad/remotes/lib/firecracker/opennebula_vm.rb @@ -42,7 +42,7 @@ class FirecrackerConfiguration < Hash begin merge!(YAML.load_file("#{__dir__}/#{FIRECRACKERRC}")) - rescue => e + rescue StandardError => e OpenNebula.log_error e end end @@ -52,6 +52,9 @@ end # This class parses and wraps the information in the Driver action data class OpenNebulaVM + # rubocop:disable Naming/PredicateName + # rubocop:disable Naming/AccessorMethodName + attr_reader :xml, :vm_id, :vm_name, :sysds_path, :rootfs_id, :fcrc #--------------------------------------------------------------------------- @@ -148,7 +151,7 @@ class OpenNebulaVM def jailer_params(hash) hash['id'] = "one-#{vm_id}" - hash['node'] = get_numa_node # TODO, check how use NUMA nodes (@xml.element('//TEMPLATE//NUMA_NODE')) + hash['node'] = get_numa_node hash['exec-file'] = @exec_file hash['uid'] = @uid hash['gid'] = @gid @@ -156,7 +159,7 @@ class OpenNebulaVM end def firecracker_params(hash) - hash['--config-file'] = 'deployment.file' + hash['config-file'] = 'deployment.file' end #--------------------------------------------------------------------------- @@ -229,7 +232,8 @@ class OpenNebulaVM #--------------------------------------------------------------------------- def get_numa_node - rc, nodes, = Command.execute('ls /sys/devices/system/node | grep node', false) + rc, nodes, = Command.execute('ls /sys/devices/system/node | grep node', + false) return -1 unless rc.zero? @@ -264,6 +268,9 @@ class OpenNebulaVM data && data['TYPE'].casecmp('vnc').zero? end + # rubocop:enable Naming/PredicateName + # rubocop:enable Naming/AccessorMethodName + end # This class abstracts the access to XML elements. It provides basic methods diff --git a/src/vmm_mad/remotes/lib/lxd/command.rb b/src/vmm_mad/remotes/lib/lxd/command.rb index 9437144bfb..9e68eac5d3 100644 --- a/src/vmm_mad/remotes/lib/lxd/command.rb +++ b/src/vmm_mad/remotes/lib/lxd/command.rb @@ -16,7 +16,7 @@ # limitations under the License. # #--------------------------------------------------------------------------- # -require 'open3' +require_relative '../lib/command' # This module can be used to execute commands. It wraps popen3 and provides # locking capabilites using flock @@ -24,40 +24,4 @@ module Command LOCK_FILE = '/tmp/onelxd-lock' - def self.execute(cmd, block) - rc = -1 - stdout = '' - stderr = '' - - begin - fd = lock if block - - stdout, stderr, s = Open3.capture3(cmd) - ensure - unlock(fd) if block - end - - [s.exitstatus, stdout, stderr] - end - - def self.execute_once(cmd, lock) - execute(cmd, lock) unless running?(cmd.split[0]) - end - - # Return true if command is running - def self.running?(command) - !`ps --noheaders -C #{command}`.empty? - end - - def self.lock - lfd = File.open(LOCK_FILE,"w") - lfd.flock(File::LOCK_EX) - - lfd - end - - def self.unlock(lfd) - lfd.close - end - end diff --git a/src/vnm_mad/remotes/bridge/clean.d/firecracker b/src/vnm_mad/remotes/bridge/clean.d/firecracker index 134778d7bc..b592623e4d 100755 --- a/src/vnm_mad/remotes/bridge/clean.d/firecracker +++ b/src/vnm_mad/remotes/bridge/clean.d/firecracker @@ -67,6 +67,7 @@ if !rc.success? exit(-1) end +# rubocop:disable Lint/RescueException begin hm = VNMMAD::NoVLANDriver.from_base64(template64, xpath_nics, deploy_id) hm.deactivate @@ -75,3 +76,4 @@ rescue Exception => e OpenNebula.log_error(e.backtrace) exit 1 end +# rubocop:enable Lint/RescueException diff --git a/src/vnm_mad/remotes/bridge/pre.d/firecracker b/src/vnm_mad/remotes/bridge/pre.d/firecracker index 561a2625cf..60a10c7bf1 100755 --- a/src/vnm_mad/remotes/bridge/pre.d/firecracker +++ b/src/vnm_mad/remotes/bridge/pre.d/firecracker @@ -61,7 +61,6 @@ template.elements.each(xpath_nics) do |nic_element| _, _, rc = Open3.capture3(cmd) break unless rc.success? - end exit(-1) unless rc.success? diff --git a/src/vnm_mad/remotes/lib/nic.rb b/src/vnm_mad/remotes/lib/nic.rb index d43fae7a36..dc946265bd 100644 --- a/src/vnm_mad/remotes/lib/nic.rb +++ b/src/vnm_mad/remotes/lib/nic.rb @@ -166,8 +166,8 @@ module VNMMAD end - # A NIC using Firecracker. This class implements functions to get the physical - # interface that the NIC is using, based on the MAC address + # A NIC using Firecracker. This class implements functions to get the + # physical interface that the NIC is using, based on the MAC address class NicFirecracker < Hash VNMNetwork::HYPERVISORS['firecracker'] = self