From 31e1b16507ebbdb8508136b68460de9a6f90aa84 Mon Sep 17 00:00:00 2001 From: test Date: Fri, 28 Oct 2022 14:44:09 +0200 Subject: [PATCH] [UPDATE] robocop output --- .../lib/cloud/azure/disk/disk_manager2.rb | 16 +- .../lib/cloud/azure/models/vm_cloud_props.rb | 6 +- .../lib/cloud/azure/restapi/azure_client.rb | 12 +- .../lib/cloud/azure/vms/vm_manager_disk.rb | 4 +- .../spec/unit/cloud/resize_disk_spec.rb | 4 + .../spec/unit/disk_manager2_spec.rb | 185 +++++++++--------- .../spec/unit/models/vm_cloud_props_spec.rb | 26 +-- 7 files changed, 127 insertions(+), 126 deletions(-) diff --git a/src/bosh_azure_cpi/lib/cloud/azure/disk/disk_manager2.rb b/src/bosh_azure_cpi/lib/cloud/azure/disk/disk_manager2.rb index 4aaeacb94..fd6b43387 100644 --- a/src/bosh_azure_cpi/lib/cloud/azure/disk/disk_manager2.rb +++ b/src/bosh_azure_cpi/lib/cloud/azure/disk/disk_manager2.rb @@ -157,10 +157,10 @@ def generate_ephemeral_disk_name(vm_name) def os_disk_placement(placement) case placement - when "resource-disk" - "ResourceDisk" - when "cache-disk" - "CacheDisk" + when 'resource-disk' + 'ResourceDisk' + when 'cache-disk' + 'CacheDisk' end end @@ -179,11 +179,9 @@ def os_disk(vm_name, stemcell_info, size, caching, use_root_disk_as_ephemeral) def ephemeral_os_disk(vm_name, stemcell_info, root_disk_size, ephemeral_disk_size, use_root_disk_as_ephemeral, placement) disk_size = get_os_disk_size(root_disk_size, stemcell_info, use_root_disk_as_ephemeral) - unless ephemeral_disk_size.nil? - if use_root_disk_as_ephemeral - disk_size = get_os_disk_size(stemcell_info.image_size, stemcell_info, use_root_disk_as_ephemeral) - disk_size += ephemeral_disk_size / 1024 - end + unless ephemeral_disk_size.nil? && !use_root_disk_as_ephemeral + disk_size = get_os_disk_size(stemcell_info.image_size, stemcell_info, use_root_disk_as_ephemeral) + disk_size += ephemeral_disk_size / 1024 end disk_placement = os_disk_placement(placement) diff --git a/src/bosh_azure_cpi/lib/cloud/azure/models/vm_cloud_props.rb b/src/bosh_azure_cpi/lib/cloud/azure/models/vm_cloud_props.rb index ad035fe92..ed80c28be 100644 --- a/src/bosh_azure_cpi/lib/cloud/azure/models/vm_cloud_props.rb +++ b/src/bosh_azure_cpi/lib/cloud/azure/models/vm_cloud_props.rb @@ -47,8 +47,8 @@ def initialize(vm_properties, global_azure_config) _default_root_disk_placement(root_disk_hash['placement']) ) - if @root_disk.placement != "remote" && ! @root_disk.type.nil? - cloud_error("Only one 'type' and 'placement' is allowed to be configured for the root_disk when 'placement' is not set to persistent") unless global_azure_config.use_managed_disks + if @root_disk.placement != 'remote' && !@root_disk.type.nil? && !global_azure_config.use_managed_disks + cloud_error("Only one 'type' and 'placement' is allowed to be configured for the root_disk when 'placement' is not set to persistent") end ephemeral_disk_hash = vm_properties.fetch('ephemeral_disk', {}) @@ -100,7 +100,7 @@ def initialize(vm_properties, global_azure_config) # persistent (default) - persistent os disk def _default_root_disk_placement(root_disk_placement) if root_disk_placement.nil? - "remote" + 'remote' else valid_placements = %w[resource-disk cache-disk remote] cloud_error("root_disk 'placement' must be one of 'resource-disk','cache-disk','remote'") unless valid_placements.include?(root_disk_placement) diff --git a/src/bosh_azure_cpi/lib/cloud/azure/restapi/azure_client.rb b/src/bosh_azure_cpi/lib/cloud/azure/restapi/azure_client.rb index c9b384fe5..63892c14f 100644 --- a/src/bosh_azure_cpi/lib/cloud/azure/restapi/azure_client.rb +++ b/src/bosh_azure_cpi/lib/cloud/azure/restapi/azure_client.rb @@ -361,13 +361,13 @@ def create_virtual_machine(resource_group_name, vm_params, network_interfaces, a unless vm_params[:ephemeral_os_disk].nil? os_disk = { - "diffDiskSettings"=> { - "option" => "Local" , - "placement" => vm_params[:ephemeral_os_disk][:disk_placement] #"ResourceDisk -> local ssd CacheDisk = cache" + 'diffDiskSettings' => { + 'option' => 'Local', + 'placement' => vm_params[:ephemeral_os_disk][:disk_placement] }, - "caching" => 'ReadOnly', - "createOption" => "FromImage", - 'name' => vm_params[:ephemeral_os_disk][:disk_name], + 'caching' => 'ReadOnly', + 'createOption' => 'FromImage', + 'name' => vm_params[:ephemeral_os_disk][:disk_name] } os_disk['diskSizeGB'] = vm_params[:ephemeral_os_disk][:disk_size] unless vm_params[:ephemeral_os_disk][:disk_size].nil? end diff --git a/src/bosh_azure_cpi/lib/cloud/azure/vms/vm_manager_disk.rb b/src/bosh_azure_cpi/lib/cloud/azure/vms/vm_manager_disk.rb index 80c8cab28..58bed3c32 100644 --- a/src/bosh_azure_cpi/lib/cloud/azure/vms/vm_manager_disk.rb +++ b/src/bosh_azure_cpi/lib/cloud/azure/vms/vm_manager_disk.rb @@ -9,10 +9,10 @@ def _build_disks(instance_id, stemcell_info, vm_props) ephemeral_disk = @disk_manager2.ephemeral_disk(instance_id.vm_name, vm_props.instance_type, vm_props.ephemeral_disk.size, vm_props.ephemeral_disk.type, vm_props.ephemeral_disk.use_root_disk) # check if disk has the default behavior - if vm_props.root_disk.placement == "remote" + if vm_props.root_disk.placement == 'remote' os_disk = @disk_manager2.os_disk(instance_id.vm_name, stemcell_info, vm_props.root_disk.size, vm_props.caching, vm_props.ephemeral_disk.use_root_disk) else - ephemeral_os_disk = @disk_manager2.ephemeral_os_disk(instance_id.vm_name,stemcell_info,vm_props.root_disk.size,vm_props.ephemeral_disk.size,vm_props.ephemeral_disk.use_root_disk, vm_props.root_disk.placement) + ephemeral_os_disk = @disk_manager2.ephemeral_os_disk(instance_id.vm_name, stemcell_info, vm_props.root_disk.size, vm_props.ephemeral_disk.size, vm_props.ephemeral_disk.use_root_disk, vm_props.root_disk.placement) end else storage_account_name = instance_id.storage_account_name diff --git a/src/bosh_azure_cpi/spec/unit/cloud/resize_disk_spec.rb b/src/bosh_azure_cpi/spec/unit/cloud/resize_disk_spec.rb index 5f293260a..2c8439f24 100644 --- a/src/bosh_azure_cpi/spec/unit/cloud/resize_disk_spec.rb +++ b/src/bosh_azure_cpi/spec/unit/cloud/resize_disk_spec.rb @@ -36,6 +36,7 @@ context 'when trying to resize to a larger disk size multiple of 1024' do let(:new_size) { 1_024_000 } + it 'should work' do expect(disk_manager2).to receive(:resize_disk).with(disk_id_object, 1000) expect(Bosh::Clouds::Config.logger).to receive(:info).with("Start resize of disk 'fake-disk-name' from 512 GiB to 1000 GiB") @@ -47,6 +48,7 @@ context 'when trying to resize to a larger disk size not multiple of 1024' do let(:new_size) { 1_024_100 } + it 'should work' do expect(disk_manager2).to receive(:resize_disk).with(disk_id_object, 1001) expect(Bosh::Clouds::Config.logger).to receive(:info).with("Start resize of disk 'fake-disk-name' from 512 GiB to 1001 GiB") @@ -58,6 +60,7 @@ context 'when trying to resize to the same disk size' do let(:new_size) { 512 * 1024 } + it 'should work' do expect(Bosh::Clouds::Config.logger).to receive(:info).with("Skip resize of disk 'fake-disk-name' because the new size of 512 GiB matches the actual disk size") expect do @@ -68,6 +71,7 @@ context 'when trying to resize to a smaller disk size' do let(:new_size) { 256_000 } + it 'should raise a NotSupported error' do expect do managed_cloud.resize_disk(disk_cid, new_size) diff --git a/src/bosh_azure_cpi/spec/unit/disk_manager2_spec.rb b/src/bosh_azure_cpi/spec/unit/disk_manager2_spec.rb index 11d1e7eb9..3c8488470 100644 --- a/src/bosh_azure_cpi/spec/unit/disk_manager2_spec.rb +++ b/src/bosh_azure_cpi/spec/unit/disk_manager2_spec.rb @@ -328,8 +328,8 @@ .and_return(false) end - # use_root_disk - context 'when use_root_disk is true and placement is != remote' do + # use_root_disk + context 'when use_root_disk is true and placement is != remote' do let(:vm_props) do props_factory.parse_vm_props( 'instance_type' => 'STANDARD_A1', @@ -346,7 +346,7 @@ it 'should return the sum size of the root and ephemeral_disk' do expect( - disk_manager2.ephemeral_os_disk(vm_name, stemcell_info, vm_props.root_disk.size, vm_props.ephemeral_disk.size, vm_props.ephemeral_disk.use_root_disk,vm_props.root_disk.placement) + disk_manager2.ephemeral_os_disk(vm_name, stemcell_info, vm_props.root_disk.size, vm_props.ephemeral_disk.size, vm_props.ephemeral_disk.use_root_disk, vm_props.root_disk.placement) ).to eq( disk_name: disk_name, disk_size: 5, @@ -356,106 +356,105 @@ end context 'when use_root_disk is false and placement is default' do - let(:vm_props) do - props_factory.parse_vm_props( - 'instance_type' => 'STANDARD_A1', - 'ephemeral_disk' => { - 'size' => 2048, - 'use_root_disk' => false - }, - 'root_disk' => { - 'size' => 3069, - } - ) - end - - it 'should return the size of the root disk' do - expect( - disk_manager2.ephemeral_os_disk(vm_name, stemcell_info, vm_props.root_disk.size, vm_props.ephemeral_disk.size, vm_props.ephemeral_disk.use_root_disk,vm_props.root_disk.placement) - ).to eq( - disk_name: disk_name, - disk_size: 3, - disk_placement: nil - ) - end - end + let(:vm_props) do + props_factory.parse_vm_props( + 'instance_type' => 'STANDARD_A1', + 'ephemeral_disk' => { + 'size' => 2048, + 'use_root_disk' => false + }, + 'root_disk' => { + 'size' => 3069 + } + ) + end - context 'when use_root_disk is false and placement is resource-disk' do - let(:vm_props) do - props_factory.parse_vm_props( - 'instance_type' => 'STANDARD_A1', - 'ephemeral_disk' => { - 'size' => 2048, - 'use_root_disk' => false - }, - 'root_disk' => { - 'size' => 3069, - 'placement' => 'resource-disk' - } - ) + it 'should return the size of the root disk' do + expect( + disk_manager2.ephemeral_os_disk(vm_name, stemcell_info, vm_props.root_disk.size, vm_props.ephemeral_disk.size, vm_props.ephemeral_disk.use_root_disk, vm_props.root_disk.placement) + ).to eq( + disk_name: disk_name, + disk_size: 3, + disk_placement: nil + ) + end end - it 'should return the size of the root disk' do - expect( - disk_manager2.ephemeral_os_disk(vm_name, stemcell_info, vm_props.root_disk.size, vm_props.ephemeral_disk.size, vm_props.ephemeral_disk.use_root_disk,vm_props.root_disk.placement) - ).to eq( - disk_name: disk_name, - disk_size: 3, - disk_placement: 'ResourceDisk' - ) - end - end + context 'when use_root_disk is false and placement is resource-disk' do + let(:vm_props) do + props_factory.parse_vm_props( + 'instance_type' => 'STANDARD_A1', + 'ephemeral_disk' => { + 'size' => 2048, + 'use_root_disk' => false + }, + 'root_disk' => { + 'size' => 3069, + 'placement' => 'resource-disk' + } + ) + end - context 'when use_root_disk is true and placement is default' do - let(:vm_props) do - props_factory.parse_vm_props( - 'instance_type' => 'STANDARD_A1', - 'ephemeral_disk' => { - 'size' => 2048, - 'use_root_disk' => true - }, - 'root_disk' => { - 'size' => 3069 - } - ) + it 'should return the size of the root disk' do + expect( + disk_manager2.ephemeral_os_disk(vm_name, stemcell_info, vm_props.root_disk.size, vm_props.ephemeral_disk.size, vm_props.ephemeral_disk.use_root_disk, vm_props.root_disk.placement) + ).to eq( + disk_name: disk_name, + disk_size: 3, + disk_placement: 'ResourceDisk' + ) + end end - it 'should return nil for placement' do - expect( - disk_manager2.ephemeral_os_disk(vm_name, stemcell_info, vm_props.root_disk.size, vm_props.ephemeral_disk.size, vm_props.ephemeral_disk.use_root_disk,vm_props.root_disk.placement) - ).to eq( - disk_name: disk_name, - disk_size: 5, - disk_placement: nil #not possible at workflow - ) - end - end + context 'when use_root_disk is true and placement is default' do + let(:vm_props) do + props_factory.parse_vm_props( + 'instance_type' => 'STANDARD_A1', + 'ephemeral_disk' => { + 'size' => 2048, + 'use_root_disk' => true + }, + 'root_disk' => { + 'size' => 3069 + } + ) + end - context 'when use_root_disk is true and placement is cache-disk' do - let(:vm_props) do - props_factory.parse_vm_props( - 'instance_type' => 'STANDARD_A1', - 'ephemeral_disk' => { - 'size' => 2048, - 'use_root_disk' => true - }, - 'root_disk' => { - 'placement' => 'cache-disk' - } - ) + it 'should return nil for placement' do + expect( + disk_manager2.ephemeral_os_disk(vm_name, stemcell_info, vm_props.root_disk.size, vm_props.ephemeral_disk.size, vm_props.ephemeral_disk.use_root_disk, vm_props.root_disk.placement) + ).to eq( + disk_name: disk_name, + disk_size: 5, + disk_placement: nil + ) + end end - it 'should return the size of the root disk' do - expect( - disk_manager2.ephemeral_os_disk(vm_name, stemcell_info, vm_props.root_disk.size, vm_props.ephemeral_disk.size, vm_props.ephemeral_disk.use_root_disk,vm_props.root_disk.placement) - ).to eq( - disk_name: disk_name, - disk_size: 5, - disk_placement: 'CacheDisk' - ) - end - end + context 'when use_root_disk is true and placement is cache-disk' do + let(:vm_props) do + props_factory.parse_vm_props( + 'instance_type' => 'STANDARD_A1', + 'ephemeral_disk' => { + 'size' => 2048, + 'use_root_disk' => true + }, + 'root_disk' => { + 'placement' => 'cache-disk' + } + ) + end + it 'should return the size of the root disk' do + expect( + disk_manager2.ephemeral_os_disk(vm_name, stemcell_info, vm_props.root_disk.size, vm_props.ephemeral_disk.size, vm_props.ephemeral_disk.use_root_disk, vm_props.root_disk.placement) + ).to eq( + disk_name: disk_name, + disk_size: 5, + disk_placement: 'CacheDisk' + ) + end + end end describe '#os_disk' do diff --git a/src/bosh_azure_cpi/spec/unit/models/vm_cloud_props_spec.rb b/src/bosh_azure_cpi/spec/unit/models/vm_cloud_props_spec.rb index 8fee36e20..087f7aaf8 100644 --- a/src/bosh_azure_cpi/spec/unit/models/vm_cloud_props_spec.rb +++ b/src/bosh_azure_cpi/spec/unit/models/vm_cloud_props_spec.rb @@ -420,13 +420,13 @@ context 'when root_disk is specified' do context 'with type and placement' do let(:vm_cloud_properties) do - { - 'instance_type' => 'Standard_D1', - 'root_disk' => { - 'type' => 'Premium_ZRS', - 'placement' => 'resource-disk' - } + { + 'instance_type' => 'Standard_D1', + 'root_disk' => { + 'type' => 'Premium_ZRS', + 'placement' => 'resource-disk' } + } end it 'should raise an error' do @@ -438,12 +438,12 @@ context 'with type wrong placement' do let(:vm_cloud_properties) do - { - 'instance_type' => 'Standard_D1', - 'root_disk' => { - 'placement' => 'local-persistent' - } + { + 'instance_type' => 'Standard_D1', + 'root_disk' => { + 'placement' => 'local-persistent' } + } end it 'should raise an error' do @@ -461,7 +461,7 @@ 'root_disk' => { 'placement' => 'cache-disk' } - },azure_config_managed + }, azure_config_managed ) end @@ -469,7 +469,7 @@ root_disk = vm_cloud_props.root_disk expect(root_disk.size).to be_nil expect(root_disk.type).to be_nil - expect(root_disk.placement).to eq("cache-disk") + expect(root_disk.placement).to eq('cache-disk') end end end