openstack/cookbook-openstack-object-storage
Code Issues Proposed changes

Cleanup minor rubocop offenses

Browse Source 
Cleaned up all the minor rubocop issues, the ones left relate to
complex logic and what I think is a bug in rubocop for nested
vs compact modules/class definitions.

Change-Id: I5beb8597322d1ca50f9246a3136711e32a5b3123
This commit is contained in:
Mark Vanderwiel
2015-05-29 11:57:29 -05:00
parent 4f778233cf
commit 2950687911
23 changed files with 70 additions and 148 deletions
Download Patch File Download Diff File Expand all files Collapse all files

81
.rubocop_todo.yml
View File

@@ -1,95 +1,24 @@
# This configuration was generated by `rubocop --auto-gen-config`
# on 2015-02-27 14:02:16 -0600 using RuboCop version 0.29.1.
# on 2015-05-29 11:53:01 -0500 using RuboCop version 0.29.1.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.
# Offense count: 1
# Cop supports --auto-correct.
Lint/DeprecatedClassMethods:
Enabled: false
# Offense count: 7
# Cop supports --auto-correct.
Lint/UnusedBlockArgument:
Enabled: false
# Offense count: 5
# Offense count: 4
Metrics/AbcSize:
Max: 254
Max: 169
# Offense count: 2
Metrics/PerceivedComplexity:
Max: 19
Max: 15
# Offense count: 5
# Offense count: 8
# Configuration parameters: EnforcedStyle, SupportedStyles.
Style/ClassAndModuleChildren:
Enabled: false
# Offense count: 5
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles.
Style/EmptyLinesAroundBlockBody:
Enabled: false
# Offense count: 6
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles.
Style/FirstParameterIndentation:
Enabled: false
# Offense count: 1
# Cop supports --auto-correct.
Style/IndentArray:
Enabled: false
# Offense count: 2
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles.
Style/IndentHash:
Enabled: false
# Offense count: 1
# Cop supports --auto-correct.
Style/LineEndConcatenation:
Enabled: false
# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles.
Style/MultilineOperationIndentation:
Enabled: false
# Offense count: 4
# Configuration parameters: EnforcedStyle, MinBodyLength, SupportedStyles.
Style/Next:
Enabled: false
# Offense count: 11
# Cop supports --auto-correct.
# Configuration parameters: PreferredDelimiters.
Style/PercentLiteralDelimiters:
Enabled: false
# Offense count: 1
# Cop supports --auto-correct.
Style/SelfAssignment:
Enabled: false
# Offense count: 17
# Cop supports --auto-correct.
Style/SingleSpaceBeforeFirstArg:
Enabled: false
# Offense count: 1
# Cop supports --auto-correct.
Style/SpaceBeforeComma:
Enabled: false
# Offense count: 8
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles.
Style/StringLiteralsInInterpolation:
Enabled: false

10
attributes/default.rb
View File

@@ -275,10 +275,10 @@ default['openstack']['sysctl']['net.ipv4.tcp_syncookies'] = 0
# Each rule gets evaluated with "candidate" set to the device name
# without the leading "/dev/") and info set to the node hash value.
default['openstack']['object-storage']['disk_test_filter'] = [
'candidate =~ /(sd|hd|xvd|vd)(?!a$)[a-z]+/',
"File.exist?('/dev/' + candidate)",
"not system('/sbin/parted /dev/' + candidate + ' -s print | grep linux-swap')",
"not info.has_key?('removable') or info['removable'] == 0.to_s"]
'candidate =~ /(sd|hd|xvd|vd)(?!a$)[a-z]+/',
"File.exist?('/dev/' + candidate)",
"not system('/sbin/parted /dev/' + candidate + ' -s print | grep linux-swap')",
"not info.has_key?('removable') or info['removable'] == 0.to_s"]
#-------------------
# template overrides
@@ -477,7 +477,7 @@ case platform_family
when 'rhel'
default['openstack']['object-storage']['platform'] = {
'disk_format' => 'ext4',
'proxy_packages' => %w{openstack-swift-proxy sudo cronie python-memcached},
'proxy_packages' => %w(openstack-swift-proxy sudo cronie python-memcached),
'object_packages' => ['openstack-swift-object', 'sudo', 'cronie'],
'container_packages' => ['openstack-swift-container', 'sudo', 'cronie'],
'account_packages' => ['openstack-swift-account', 'sudo', 'cronie'],

5
libraries/drive_utils.rb
View File

@@ -27,11 +27,12 @@ module DriveUtils
def locate_disks(enum_expression, filter_expressions)
candidate_disks = eval(enum_expression)
candidate_expression = 'candidate_disks.select{|candidate,info| (' +
filter_expressions.map { |x| "(#{x})" }.join(' and ') + ')}'
filter_expressions.map { |x| "(#{x})" }.join(' and ') +
')}'
# TODO(mancdaz): fix this properly so the above works in the first place
candidate_expression.gsub!(/\[\'removable\'\] = 0/, "['removable'].to_i == 0")
drives = Hash[eval(candidate_expression)]
Chef::Log.info("Using candidate drives: #{drives.keys.join(", ")}")
Chef::Log.info("Using candidate drives: #{drives.keys.join(', ')}")
drives.keys
end
end

30
metadata.rb
View File

@@ -1,20 +1,20 @@
name 'openstack-object-storage'
maintainer 'openstack-chef'
maintainer_email 'opscode-chef-openstack@googlegroups.com'
license 'Apache 2.0'
description 'Installs and configures Openstack Swift'
long_description IO.read(File.join(File.dirname(__FILE__), 'README.md'))
version '11.0.0'
name 'openstack-object-storage'
maintainer 'openstack-chef'
maintainer_email 'opscode-chef-openstack@googlegroups.com'
license 'Apache 2.0'
description 'Installs and configures Openstack Swift'
long_description IO.read(File.join(File.dirname(__FILE__), 'README.md'))
version '11.0.0'
recipe 'openstack-object-storage::account-server', 'Installs the swift account server'
recipe 'openstack-object-storage::client', 'Install the swift client'
recipe 'openstack-object-storage::container-server', 'Installs the swift container server'
recipe 'openstack-object-storage::management-server', 'Installs the swift management server'
recipe 'openstack-object-storage::object-server', 'Installs the swift object server'
recipe 'openstack-object-storage::proxy-server', 'Installs the swift proxy server'
recipe 'openstack-object-storage::setup', 'Does initial setup of a swift cluster'
recipe 'openstack-object-storage::account-server', 'Installs the swift account server'
recipe 'openstack-object-storage::client', 'Install the swift client'
recipe 'openstack-object-storage::container-server', 'Installs the swift container server'
recipe 'openstack-object-storage::management-server', 'Installs the swift management server'
recipe 'openstack-object-storage::object-server', 'Installs the swift object server'
recipe 'openstack-object-storage::proxy-server', 'Installs the swift proxy server'
recipe 'openstack-object-storage::setup', 'Does initial setup of a swift cluster'
%w{ centos ubuntu redhat }.each do |os|
%w(centos ubuntu redhat).each do |os|
supports os
end

4
providers/disk.rb
View File

@@ -25,7 +25,7 @@ def load_current_resource
dev_name = @new_resource.name
@current = Chef::Resource::OpenstackObjectStorageDisk.new(dev_name)
parted_partition_parse dev_name
parted_partition_parse dev_name
parts = @current.part
unless @current.blocks
@@ -168,7 +168,7 @@ action :ensure_exists do
if req_size == :remaining
req_size = disk_blocks - current_block
else
req_size = req_size * 1024
req_size *= 1024
end
cur_size = cur[idx][:size]

8
providers/mounts.rb
View File

@@ -35,7 +35,7 @@ action :ensure_exists do
# walk through the devices, gathering information
proposed_devices.each do |device|
next unless ::File.exists?("/dev/#{device}")
next unless ::File.exist?("/dev/#{device}")
info = {}
info['device'] = device
@@ -43,7 +43,7 @@ action :ensure_exists do
info['format'] = @new_resource.format
info['uuid'] = Mixlib::ShellOut.new("blkid /dev/#{device} -s UUID -o value").run_command.stdout.strip
info['mountpoint'] = info['uuid'].split('-').join('')
info['mounted'] = Mixlib::ShellOut.new("mount | grep '#{path}/#{info["mountpoint"]}\'").run_command.status
info['mounted'] = Mixlib::ShellOut.new("mount | grep '#{path}/#{info['mountpoint']}\'").run_command.status
info['size'] = Mixlib::ShellOut.new("sfdisk -s /dev/#{device}").run_command.stdout.to_i / 1024
next if info['uuid'] == ''
@@ -152,8 +152,8 @@ action :ensure_exists do
new_resource.updated_by_last_action(true)
end
dev_info.reject { |k, v| v['mounted'] }.keys.each do |uuid|
dev_info[uuid]['mounted'] = Mixlib::ShellOut.new("mount | grep '#{path}/#{dev_info[uuid]["mountpoint"]}\'").run_command.status
dev_info.reject { |_k, v| v['mounted'] }.keys.each do |uuid|
dev_info[uuid]['mounted'] = Mixlib::ShellOut.new("mount | grep '#{path}/#{dev_info[uuid]['mountpoint']}\'").run_command.status
end
if @new_resource.publish_attributes && dev_info != {}

12
providers/ring_script.rb
View File

@@ -56,8 +56,8 @@ def generate_script # rubocop:disable Metrics/AbcSize
ring_data[:in_use][which] ||= {}
if ring_data[:parsed][which][:hosts]
ring_data[:parsed][which][:hosts].each do |ip, dev|
dev.each do |dev_id, devhash|
ring_data[:parsed][which][:hosts].each do |_ip, dev|
dev.each do |_dev_id, devhash|
ring_data[:in_use][which].store(devhash[:device], devhash[:id])
end
end
@@ -118,10 +118,10 @@ def generate_script # rubocop:disable Metrics/AbcSize
['account', 'container', 'object'].each do |which|
# remove available disks that are already in the ring
new_disks[which] = disk_data[:available][which].reject { |k, v| ring_data[:in_use][which].key?(k) }
new_disks[which] = disk_data[:available][which].reject { |k, _v| ring_data[:in_use][which].key?(k) }
# find all in-ring disks that are not in the cluster
missing_disks[which] = ring_data[:in_use][which].reject { |k, v| disk_data[:available][which].key?(k) }
missing_disks[which] = ring_data[:in_use][which].reject { |k, _v| disk_data[:available][which].key?(k) }
Chef::Log.info("#{which} Ring - Missing:\n#{PP.pp(missing_disks[which], '')}")
Chef::Log.info("#{which} Ring - New:\n#{PP.pp(new_disks[which], '')}")
@@ -159,9 +159,9 @@ def generate_script # rubocop:disable Metrics/AbcSize
# remove the disks -- sort to ensure consistent order
missing_disks[which].keys.sort.each do |mountpoint|
diskinfo = ring_data[:parsed][which][:hosts].select { |k, v| v.key?(mountpoint) }.map { |_, v| v[mountpoint] }[0]
diskinfo = ring_data[:parsed][which][:hosts].select { |_k, v| v.key?(mountpoint) }.map { |_, v| v[mountpoint] }[0]
Chef::Log.info("#{which} Missing diskinfo:\n#{PP.pp(diskinfo, '')}")
description = Hash[diskinfo.select { |k, v| [:zone, :ip, :device].include?(k) }].map { |k, v| "#{k}: #{v}" }.join(', ')
description = Hash[diskinfo.select { |k, _v| [:zone, :ip, :device].include?(k) }].map { |k, v| "#{k}: #{v}" }.join(', ')
s << "# #{description}\n"
s << "swift-ring-builder #{ring_path}/#{which}.builder remove d#{missing_disks[which][mountpoint]}\n"
must_rebalance = true

2
recipes/account-server.rb
View File

@@ -36,7 +36,7 @@ platform_options['account_packages'].each.each do |pkg|
end
svc_names = {}
%w{swift-account swift-account-auditor swift-account-reaper swift-account-replicator}.each do |svc|
%w(swift-account swift-account-auditor swift-account-reaper swift-account-replicator).each do |svc|
svc_names[svc] = svc_name(svc)
end

2
recipes/container-server.rb
View File

@@ -36,7 +36,7 @@ platform_options['container_packages'].each do |pkg|
end
svc_names = {}
%w{swift-container swift-container-auditor swift-container-replicator swift-container-updater}.each do |svc|
%w(swift-container swift-container-auditor swift-container-replicator swift-container-updater).each do |svc|
svc_names[svc] = svc_name(svc)
end

2
recipes/disks.rb
View File

@@ -47,7 +47,7 @@ Chef::Log.info("Located disks: #{disks}")
disks.each do |disk|
openstack_object_storage_disk "/dev/#{disk}" do
part [{ type: platform_options['disk_format'] , size: :remaining }]
part [{ type: platform_options['disk_format'], size: :remaining }]
action :ensure_exists
end
end

2
recipes/management-server.rb
View File

@@ -38,7 +38,7 @@ if node['openstack']['object-storage']['authmode'] == 'swauth'
when 'git'
git "#{Chef::Config[:file_cache_path]}/swauth" do
repository node['openstack']['object-storage']['swauth_repository']
revision node['openstack']['object-storage']['swauth_version']
revision node['openstack']['object-storage']['swauth_version']
action :sync
end

3
recipes/object-server.rb
View File

@@ -36,7 +36,7 @@ platform_options['object_packages'].each do |pkg|
end
svc_names = {}
%w{swift-object swift-object-replicator swift-object-auditor swift-object-updater}.each do |svc|
%w(swift-object swift-object-replicator swift-object-auditor swift-object-updater).each do |svc|
svc_names[svc] = svc_name(svc)
end
@@ -46,7 +46,6 @@ svc_names.values.each do |svc|
action [:enable, :start]
only_if '[ -e /etc/swift/object-server.conf ] && [ -e /etc/swift/object.ring.gz ]'
end
end
memcache_servers = memcached_servers.join ','

2
recipes/proxy-server.rb
View File

@@ -63,7 +63,7 @@ when 'swauth'
when 'git'
git "#{Chef::Config[:file_cache_path]}/swauth" do
repository node['openstack']['object-storage']['swauth_repository']
revision node['openstack']['object-storage']['swauth_version']
revision node['openstack']['object-storage']['swauth_version']
action :sync
end

13
recipes/ring-repo.rb
View File

@@ -43,13 +43,13 @@ end
execute 'create empty git repo' do
cwd '/tmp'
umask 022
command "mkdir $$; cd $$; git init; echo \"backups\" \> .gitignore; #{git_config_email} ; #{git_config_name} ; git add .gitignore; git commit -m 'initial commit' --author='chef <chef@openstack>'; git push file:///#{platform_options["git_dir"]}/rings master"
command "mkdir $$; cd $$; git init; echo \"backups\" \> .gitignore; #{git_config_email} ; #{git_config_name} ; git add .gitignore; git commit -m 'initial commit' --author='chef <chef@openstack>'; git push file:///#{platform_options['git_dir']}/rings master"
user node['openstack']['object-storage']['user']
action :nothing
end
directory 'git-directory' do
path "#{platform_options["git_dir"]}/rings"
path "#{platform_options['git_dir']}/rings"
owner node['openstack']['object-storage']['user']
group node['openstack']['object-storage']['group']
mode 00755
@@ -58,11 +58,11 @@ directory 'git-directory' do
end
execute 'initialize git repo' do
cwd "#{platform_options["git_dir"]}/rings"
cwd "#{platform_options['git_dir']}/rings"
umask 022
user node['openstack']['object-storage']['user']
command 'git init --bare && touch git-daemon-export-ok'
creates "#{platform_options["git_dir"]}/rings/config"
creates "#{platform_options['git_dir']}/rings/config"
action :run
notifies :run, 'execute[create empty git repo]', :immediately
end
@@ -99,18 +99,17 @@ end
execute 'checkout-rings' do # ~FC040
cwd '/etc/swift/ring-workspace'
command "git clone file://#{platform_options["git_dir"]}/rings"
command "git clone file://#{platform_options['git_dir']}/rings"
user node['openstack']['object-storage']['user']
creates '/etc/swift/ring-workspace/rings'
end
['account', 'container', 'object'].each do |ring_type|
part_power = ring_options['part_power']
min_part_hours = ring_options['min_part_hours']
replicas = ring_options['replicas']
Chef::Log.info("Building initial ring #{ring_type} using part_power=#{part_power}, " +
Chef::Log.info("Building initial ring #{ring_type} using part_power=#{part_power}, "\
"min_part_hours=#{min_part_hours}, replicas=#{replicas}")
execute "add #{ring_type}.builder" do
cwd '/etc/swift/ring-workspace/rings'

2
spec/account_spec.rb
View File

@@ -29,7 +29,7 @@ describe 'openstack-object-storage::account-server' do
end
it 'starts swift account services on boot' do
%w{swift-account swift-account-auditor swift-account-reaper swift-account-replicator}.each do |svc|
%w(swift-account swift-account-auditor swift-account-reaper swift-account-replicator).each do |svc|
expect(chef_run).to enable_service(svc)
end
end

2
spec/common_spec.rb
View File

@@ -149,7 +149,7 @@ describe 'openstack-object-storage::common' do
describe 'template contents' do
it 'uses the builder_ip variable' do
node.set['openstack']['object-storage']['git_builder_ip'] = 'git_builder_ip_value'
expect(chef_run).to render_file(file.name).with_content(%r(git clone git://git_builder_ip_value/rings /etc/swift/rings))
expect(chef_run).to render_file(file.name).with_content(%r{git clone git://git_builder_ip_value/rings /etc/swift/rings})
end
it 'uses the service_prefix variable' do

2
spec/container_spec.rb
View File

@@ -25,7 +25,7 @@ describe 'openstack-object-storage::container-server' do
it 'starts swift container services on boot' do
node.set['openstack']['object-storage']['container-server']['allowed_sync_hosts'] = %w(host1 host2)
%w{swift-container swift-container-auditor swift-container-replicator swift-container-updater swift-container-sync}.each do |svc|
%w(swift-container swift-container-auditor swift-container-replicator swift-container-updater swift-container-sync).each do |svc|
expect(chef_run).to enable_service(svc)
end
end

2
spec/disks_spec.rb
View File

@@ -27,7 +27,5 @@ describe 'openstack-object-storage::disks' do
it 'upgrades parted package' do
expect(chef_run).to upgrade_package('parted')
end
end
end

12
spec/identity_registration_spec.rb
View File

@@ -25,7 +25,7 @@ describe 'openstack-object-storage::identity_registration' do
context 'registers object storage endpoint' do
it 'with default values' do
expect(chef_run).to create_endpoint_openstack_identity_register(
'Register Object Storage Endpoint'
'Register Object Storage Endpoint'
).with(
auth_uri: 'http://127.0.0.1:35357/v2.0',
bootstrap_token: 'bootstrap-token',
@@ -48,7 +48,7 @@ describe 'openstack-object-storage::identity_registration' do
node.set['openstack']['endpoints']['admin']['object-storage-api']['uri'] = admin_url
expect(chef_run).to create_endpoint_openstack_identity_register(
'Register Object Storage Endpoint'
'Register Object Storage Endpoint'
).with(
auth_uri: 'http://127.0.0.1:35357/v2.0',
bootstrap_token: 'bootstrap-token',
@@ -71,7 +71,7 @@ describe 'openstack-object-storage::identity_registration' do
node.set['openstack']['endpoints']['internal']['object-storage-api']['uri'] = internal_url
expect(chef_run).to create_endpoint_openstack_identity_register(
'Register Object Storage Endpoint'
'Register Object Storage Endpoint'
).with(
auth_uri: 'http://127.0.0.1:35357/v2.0',
bootstrap_token: 'bootstrap-token',
@@ -94,7 +94,7 @@ describe 'openstack-object-storage::identity_registration' do
node.set['openstack']['endpoints']['public']['object-storage-api']['uri'] = public_url
expect(chef_run).to create_endpoint_openstack_identity_register(
'Register Object Storage Endpoint'
'Register Object Storage Endpoint'
).with(
auth_uri: 'http://127.0.0.1:35357/v2.0',
bootstrap_token: 'bootstrap-token',
@@ -116,7 +116,7 @@ describe 'openstack-object-storage::identity_registration' do
node.set['openstack']['endpoints']['internal']['object-storage-api']['uri'] = internal_url
node.set['openstack']['endpoints']['public']['object-storage-api']['uri'] = public_url
expect(chef_run).to create_endpoint_openstack_identity_register(
'Register Object Storage Endpoint'
'Register Object Storage Endpoint'
).with(
auth_uri: 'http://127.0.0.1:35357/v2.0',
bootstrap_token: 'bootstrap-token',
@@ -132,7 +132,7 @@ describe 'openstack-object-storage::identity_registration' do
it 'with custom region override' do
node.set['openstack']['object-storage']['region'] = 'swiftRegion'
expect(chef_run).to create_endpoint_openstack_identity_register(
'Register Object Storage Endpoint'
'Register Object Storage Endpoint'
).with(
auth_uri: 'http://127.0.0.1:35357/v2.0',
bootstrap_token: 'bootstrap-token',

2
spec/object_spec.rb
View File

@@ -25,7 +25,7 @@ describe 'openstack-object-storage::object-server' do
end
it 'starts swift object services on boot' do
%w{swift-object swift-object-replicator swift-object-auditor swift-object-updater}.each do |svc|
%w(swift-object swift-object-replicator swift-object-auditor swift-object-updater).each do |svc|
expect(chef_run).to enable_service(svc)
end
end

4
spec/ring-repo_spec.rb
View File

@@ -16,9 +16,7 @@ describe 'openstack-object-storage::ring-repo' do
end
it 'should not start xinetd services on boot' do
%w{xinetd}.each do |svc|
expect(chef_run).not_to enable_service(svc)
end
expect(chef_run).not_to enable_service('xinetd')
end
# FIXME(galstrom21): This spec file should just check that the LWRP

4
spec/rsync_spec.rb
View File

@@ -16,9 +16,7 @@ describe 'openstack-object-storage::rsync' do
end
it 'starts rsync service on boot' do
%w{rsync}.each do |svc|
expect(chef_run).to enable_service(svc)
end
expect(chef_run).to enable_service('rsync')
end
describe '/etc/rsyncd.conf' do

12
spec/spec_helper.rb
View File

@@ -7,14 +7,14 @@ ChefSpec::Coverage.start! { add_filter 'openstack-compute' }
LOG_LEVEL = :fatal
REDHAT_OPTS = {
platform: 'redhat',
version: '7.1',
log_level: LOG_LEVEL
platform: 'redhat',
version: '7.1',
log_level: LOG_LEVEL
}
UBUNTU_OPTS = {
platform: 'ubuntu',
version: '14.04',
log_level: LOG_LEVEL
platform: 'ubuntu',
version: '14.04',
log_level: LOG_LEVEL
}
SUSE_OPTS = {
platform: 'suse',