From 70aa33ffe511e034a6937b86a81d29bf07dbec1f Mon Sep 17 00:00:00 2001 From: Alex Monteiro Date: Tue, 3 Oct 2017 22:09:17 +0000 Subject: [PATCH 1/3] Fixing bug in set_storage_pool and set_snapshot_pool methods in Volumes --- CHANGELOG.md | 5 +++++ lib/oneview-sdk/resource/api200/volume.rb | 7 +++++-- .../resource/api500/c7000/volume.rb | 14 +++++++++---- lib/oneview-sdk/version.rb | 2 +- .../shared_examples/volume/api500/create.rb | 16 +++++++++++++-- .../shared_examples/volume/create.rb | 8 +++++++- spec/unit/resource/api200/volume_spec.rb | 8 +++++++- .../unit/resource/api500/c7000/volume_spec.rb | 20 +++++++++++++++---- 8 files changed, 65 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe0c592b2..c89503eb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## v5.1.1 + +#### Bug fixes & Enhancements +- [#279](https://github.com/HewlettPackard/oneview-sdk-ruby/issues/279) Bug when setting the Storage Pool and Snapshot Pool when trying to make an update + # v5.1.0 #### Bug fixes & Enhancements diff --git a/lib/oneview-sdk/resource/api200/volume.rb b/lib/oneview-sdk/resource/api200/volume.rb index 2b7adeaa0..074821a4d 100644 --- a/lib/oneview-sdk/resource/api200/volume.rb +++ b/lib/oneview-sdk/resource/api200/volume.rb @@ -68,8 +68,11 @@ def set_storage_system(storage_system) # @param [OneviewSDK::StoragePool] storage_pool Storage pool def set_storage_pool(storage_pool) assure_uri(storage_pool) - self['provisioningParameters'] ||= {} - self['provisioningParameters']['storagePoolUri'] = storage_pool['uri'] + if self['provisioningParameters'] + self['provisioningParameters']['storagePoolUri'] = storage_pool['uri'] + else + self['storagePoolUri'] = storage_pool['uri'] + end end # Adds the storage volume template to the volume diff --git a/lib/oneview-sdk/resource/api500/c7000/volume.rb b/lib/oneview-sdk/resource/api500/c7000/volume.rb index 4a69da5c1..46f0c22bd 100644 --- a/lib/oneview-sdk/resource/api500/c7000/volume.rb +++ b/lib/oneview-sdk/resource/api500/c7000/volume.rb @@ -62,16 +62,22 @@ def delete(flag = :all) # @param [OneviewSDK::StoragePool] storage_pool Storage pool. def set_storage_pool(storage_pool) assure_uri(storage_pool) - @data['properties'] ||= {} - @data['properties']['storagePool'] = storage_pool['uri'] + if @data['properties'] + @data['properties']['storagePool'] = storage_pool['uri'] + else + @data['storagePoolUri'] = storage_pool['uri'] + end end # Sets the snapshot pool to the volume # @param [OneviewSDK::StoragePool] storage_pool Storage Pool to use for snapshots. def set_snapshot_pool(storage_pool) assure_uri(storage_pool) - @data['properties'] ||= {} - @data['properties']['snapshotPool'] = storage_pool['uri'] + if @data['properties'] + @data['properties']['snapshotPool'] = storage_pool['uri'] + else + @data['snapshotPoolUri'] = storage_pool['uri'] + end end # Creates a new volume on the storage system from a snapshot of a volume. diff --git a/lib/oneview-sdk/version.rb b/lib/oneview-sdk/version.rb index f2bb17628..b39a4f5e7 100644 --- a/lib/oneview-sdk/version.rb +++ b/lib/oneview-sdk/version.rb @@ -11,5 +11,5 @@ # Gem version defined here module OneviewSDK - VERSION = '5.1.0'.freeze + VERSION = '5.1.1'.freeze end diff --git a/spec/integration/shared_examples/volume/api500/create.rb b/spec/integration/shared_examples/volume/api500/create.rb index 67dd7b08f..d48589a33 100644 --- a/spec/integration/shared_examples/volume/api500/create.rb +++ b/spec/integration/shared_examples/volume/api500/create.rb @@ -231,18 +231,30 @@ end describe '#set_storage_pool' do + it 'set_storage_pool in properties' do + item = described_class.new(current_client, name: VOLUME_NAME, properties: {}) + item.set_storage_pool(storage_pool) + expect(item['properties']['storagePool']).to eq(storage_pool['uri']) + end + it 'set_storage_pool' do item = described_class.new(current_client, name: VOLUME_NAME) item.set_storage_pool(storage_pool) - expect(item['properties']['storagePool']).to eq(storage_pool['uri']) + expect(item['storagePoolUri']).to eq(storage_pool['uri']) end end describe '#set_snapshot_pool' do + it 'set_snapshot_pool in properties' do + item = described_class.new(current_client, name: VOLUME_NAME, properties: {}) + item.set_snapshot_pool(storage_pool) + expect(item['properties']['snapshotPool']).to eq(storage_pool['uri']) + end + it 'set_snapshot_pool' do item = described_class.new(current_client, name: VOLUME_NAME) item.set_snapshot_pool(storage_pool) - expect(item['properties']['snapshotPool']).to eq(storage_pool['uri']) + expect(item['snapshotPoolUri']).to eq(storage_pool['uri']) end end diff --git a/spec/integration/shared_examples/volume/create.rb b/spec/integration/shared_examples/volume/create.rb index 6908d2850..d2350fbc1 100644 --- a/spec/integration/shared_examples/volume/create.rb +++ b/spec/integration/shared_examples/volume/create.rb @@ -112,10 +112,16 @@ end describe '#set_storage_pool' do + it 'set_storage_pool in provisioningParameters' do + item = described_class.new(current_client, name: VOLUME_NAME, provisioningParameters: {}) + item.set_storage_pool(storage_pool) + expect(item['provisioningParameters']['storagePoolUri']).to eq(storage_pool['uri']) + end + it 'set_storage_pool' do item = described_class.new(current_client, name: VOLUME_NAME) item.set_storage_pool(storage_pool) - expect(item['provisioningParameters']['storagePoolUri']).to eq(storage_pool['uri']) + expect(item['storagePoolUri']).to eq(storage_pool['uri']) end end diff --git a/spec/unit/resource/api200/volume_spec.rb b/spec/unit/resource/api200/volume_spec.rb index 694b49018..41ceacc8c 100644 --- a/spec/unit/resource/api200/volume_spec.rb +++ b/spec/unit/resource/api200/volume_spec.rb @@ -70,10 +70,16 @@ end describe '#set_storage_pool' do - it 'sets the storagePoolUri' do + it 'sets the storagePoolUri in provisioningParameters' do + @item['provisioningParameters'] = {} @item.set_storage_pool(OneviewSDK::StoragePool.new(@client_200, uri: '/rest/fake')) expect(@item['provisioningParameters']['storagePoolUri']).to eq('/rest/fake') end + + it 'sets the storagePoolUri' do + @item.set_storage_pool(OneviewSDK::StoragePool.new(@client_200, uri: '/rest/fake')) + expect(@item['storagePoolUri']).to eq('/rest/fake') + end end describe '#set_storage_volume_template' do diff --git a/spec/unit/resource/api500/c7000/volume_spec.rb b/spec/unit/resource/api500/c7000/volume_spec.rb index e5cd3bb7f..18eebb0e6 100644 --- a/spec/unit/resource/api500/c7000/volume_spec.rb +++ b/spec/unit/resource/api500/c7000/volume_spec.rb @@ -87,19 +87,31 @@ end describe '#set_storage_pool' do - it 'sets the storagePool attribute' do - item = described_class.new(@client_500) + it 'sets the storagePool attribute in properties' do + item = described_class.new(@client_500, properties: {}) item.set_storage_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake')) expect(item['properties']['storagePool']).to eq('/rest/fake') end + + it 'sets the storagePoolUri attribute' do + item = described_class.new(@client_500) + item.set_storage_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake')) + expect(item['storagePoolUri']).to eq('/rest/fake') + end end describe '#set_snapshot_pool' do - it 'sets the snapshotPool attribute' do - item = described_class.new(@client_500) + it 'sets the snapshotPool attribute in properties' do + item = described_class.new(@client_500, properties: {}) item.set_snapshot_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake')) expect(item['properties']['snapshotPool']).to eq('/rest/fake') end + + it 'sets the snapshotPoolUri attribute' do + item = described_class.new(@client_500) + item.set_snapshot_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake')) + expect(item['snapshotPoolUri']).to eq('/rest/fake') + end end describe '#create_from_snapshot' do From 69290c4b15d578e4c9f31b09705056fca40a9bd6 Mon Sep 17 00:00:00 2001 From: Alex Monteiro Date: Wed, 4 Oct 2017 19:41:37 +0000 Subject: [PATCH 2/3] Adjusting the set_storage_pool and snapshot_pool methods in volumes --- lib/oneview-sdk/resource/api200/volume.rb | 6 +--- .../resource/api500/c7000/volume.rb | 9 ++--- .../resource/api200/volume/update_spec.rb | 1 + .../api300/c7000/volume/update_spec.rb | 1 + .../api300/synergy/volume/update_spec.rb | 1 + .../api500/c7000/volume/update_spec.rb | 1 + .../api500/synergy/volume/update_spec.rb | 1 + .../volume/api200/update_snapshot_pool.rb | 32 +++++++++++++++++ .../volume/api500/update_snapshot_pool.rb | 34 +++++++++++++++++++ spec/unit/resource/api200/volume_spec.rb | 4 +-- .../unit/resource/api500/c7000/volume_spec.rb | 11 +++--- 11 files changed, 82 insertions(+), 19 deletions(-) create mode 100644 spec/integration/shared_examples/volume/api200/update_snapshot_pool.rb create mode 100644 spec/integration/shared_examples/volume/api500/update_snapshot_pool.rb diff --git a/lib/oneview-sdk/resource/api200/volume.rb b/lib/oneview-sdk/resource/api200/volume.rb index 074821a4d..2f3e266ef 100644 --- a/lib/oneview-sdk/resource/api200/volume.rb +++ b/lib/oneview-sdk/resource/api200/volume.rb @@ -68,11 +68,7 @@ def set_storage_system(storage_system) # @param [OneviewSDK::StoragePool] storage_pool Storage pool def set_storage_pool(storage_pool) assure_uri(storage_pool) - if self['provisioningParameters'] - self['provisioningParameters']['storagePoolUri'] = storage_pool['uri'] - else - self['storagePoolUri'] = storage_pool['uri'] - end + self['provisioningParameters']['storagePoolUri'] = storage_pool['uri'] if self['provisioningParameters'] end # Adds the storage volume template to the volume diff --git a/lib/oneview-sdk/resource/api500/c7000/volume.rb b/lib/oneview-sdk/resource/api500/c7000/volume.rb index 46f0c22bd..579327206 100644 --- a/lib/oneview-sdk/resource/api500/c7000/volume.rb +++ b/lib/oneview-sdk/resource/api500/c7000/volume.rb @@ -62,11 +62,7 @@ def delete(flag = :all) # @param [OneviewSDK::StoragePool] storage_pool Storage pool. def set_storage_pool(storage_pool) assure_uri(storage_pool) - if @data['properties'] - @data['properties']['storagePool'] = storage_pool['uri'] - else - @data['storagePoolUri'] = storage_pool['uri'] - end + @data['properties']['storagePool'] = storage_pool['uri'] if @data['properties'] end # Sets the snapshot pool to the volume @@ -76,7 +72,8 @@ def set_snapshot_pool(storage_pool) if @data['properties'] @data['properties']['snapshotPool'] = storage_pool['uri'] else - @data['snapshotPoolUri'] = storage_pool['uri'] + @data['deviceSpecificAttributes'] ||= {} + @data['deviceSpecificAttributes']['snapshotPoolUri'] = storage_pool['uri'] end end diff --git a/spec/integration/resource/api200/volume/update_spec.rb b/spec/integration/resource/api200/volume/update_spec.rb index 56a415f64..11719da15 100644 --- a/spec/integration/resource/api200/volume/update_spec.rb +++ b/spec/integration/resource/api200/volume/update_spec.rb @@ -3,4 +3,5 @@ RSpec.describe OneviewSDK::Volume, integration: true, type: UPDATE do let(:current_client) { $client } include_examples 'VolumeUpdateExample', 'integration context' + include_examples 'VolumeSnapshotPoolUpdateExample', 'integration context' end diff --git a/spec/integration/resource/api300/c7000/volume/update_spec.rb b/spec/integration/resource/api300/c7000/volume/update_spec.rb index a3d656c0d..2c4e9e80c 100644 --- a/spec/integration/resource/api300/c7000/volume/update_spec.rb +++ b/spec/integration/resource/api300/c7000/volume/update_spec.rb @@ -4,4 +4,5 @@ RSpec.describe klass, integration: true, type: UPDATE do let(:current_client) { $client_300 } include_examples 'VolumeUpdateExample', 'integration api300 context' + include_examples 'VolumeSnapshotPoolUpdateExample', 'integration api300 context' end diff --git a/spec/integration/resource/api300/synergy/volume/update_spec.rb b/spec/integration/resource/api300/synergy/volume/update_spec.rb index b74be37c2..e18064528 100644 --- a/spec/integration/resource/api300/synergy/volume/update_spec.rb +++ b/spec/integration/resource/api300/synergy/volume/update_spec.rb @@ -4,4 +4,5 @@ RSpec.describe klass, integration: true, type: UPDATE do let(:current_client) { $client_300_synergy } include_examples 'VolumeUpdateExample', 'integration api300 context' + include_examples 'VolumeSnapshotPoolUpdateExample', 'integration api300 context' end diff --git a/spec/integration/resource/api500/c7000/volume/update_spec.rb b/spec/integration/resource/api500/c7000/volume/update_spec.rb index 68ea3e5b0..14b020e63 100644 --- a/spec/integration/resource/api500/c7000/volume/update_spec.rb +++ b/spec/integration/resource/api500/c7000/volume/update_spec.rb @@ -4,4 +4,5 @@ RSpec.describe klass, integration: true, type: UPDATE do let(:current_client) { $client_500 } include_examples 'VolumeUpdateExample', 'integration api500 context' + include_examples 'VolumeSnapshotPoolUpdateExample API500', 'integration api500 context' end diff --git a/spec/integration/resource/api500/synergy/volume/update_spec.rb b/spec/integration/resource/api500/synergy/volume/update_spec.rb index ab0db0464..2916384ca 100644 --- a/spec/integration/resource/api500/synergy/volume/update_spec.rb +++ b/spec/integration/resource/api500/synergy/volume/update_spec.rb @@ -4,4 +4,5 @@ RSpec.describe klass, integration: true, type: UPDATE do let(:current_client) { $client_500_synergy } include_examples 'VolumeUpdateExample', 'integration api500 context' + include_examples 'VolumeSnapshotPoolUpdateExample API500', 'integration api500 context' end diff --git a/spec/integration/shared_examples/volume/api200/update_snapshot_pool.rb b/spec/integration/shared_examples/volume/api200/update_snapshot_pool.rb new file mode 100644 index 000000000..afce1ff03 --- /dev/null +++ b/spec/integration/shared_examples/volume/api200/update_snapshot_pool.rb @@ -0,0 +1,32 @@ +# (C) Copyright 2017 Hewlett Packard Enterprise Development LP +# +# 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. + +RSpec.shared_examples 'VolumeSnapshotPoolUpdateExample' do |context_name| + include_context context_name + + let(:item) { described_class.find_by(current_client, name: VOLUME2_NAME).first } + + describe '#update snapshot pool' do + it 'updating the snapshot pool' do + old_snapshot_pool = resource_class_of('StoragePool').find_by(current_client, uri: item['snapshotPoolUri']).first + new_snapshot_pool = resource_class_of('StoragePool').get_all(current_client).first + item.set_snapshot_pool(new_snapshot_pool) + item.update + item.retrieve! + expect(item['snapshotPoolUri']).to eq(new_snapshot_pool['uri']) + # Returning to original snapshot pool + item.set_snapshot_pool(old_snapshot_pool) + item.update + item.retrieve! + expect(item['snapshotPoolUri']).to eq(old_snapshot_pool['uri']) + end + end +end diff --git a/spec/integration/shared_examples/volume/api500/update_snapshot_pool.rb b/spec/integration/shared_examples/volume/api500/update_snapshot_pool.rb new file mode 100644 index 000000000..f41f328d4 --- /dev/null +++ b/spec/integration/shared_examples/volume/api500/update_snapshot_pool.rb @@ -0,0 +1,34 @@ +# (C) Copyright 2017 Hewlett Packard Enterprise Development LP +# +# 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. + +RSpec.shared_examples 'VolumeSnapshotPoolUpdateExample API500' do |context_name| + include_context context_name + + let(:item) { described_class.find_by(current_client, name: VOLUME2_NAME).first } + + describe '#update snapshot pool' do + it 'updating the snapshot pool' do + old_snapshot_pool = resource_class_of('StoragePool').find_by(current_client, uri: item['deviceSpecificAttributes']['snapshotPoolUri']).first + new_snapshot_pool = resource_class_of('StoragePool').find_by(current_client, isManaged: false).first + new_snapshot_pool.manage(true) + item.set_snapshot_pool(new_snapshot_pool) + item.update + item.retrieve! + expect(item['deviceSpecificAttributes']['snapshotPoolUri']).to eq(new_snapshot_pool['uri']) + # Returning to original snapshot pool + item.set_snapshot_pool(old_snapshot_pool) + item.update + item.retrieve! + expect(item['deviceSpecificAttributes']['snapshotPoolUri']).to eq(old_snapshot_pool['uri']) + new_snapshot_pool.manage(false) + end + end +end diff --git a/spec/unit/resource/api200/volume_spec.rb b/spec/unit/resource/api200/volume_spec.rb index 41ceacc8c..96273f56c 100644 --- a/spec/unit/resource/api200/volume_spec.rb +++ b/spec/unit/resource/api200/volume_spec.rb @@ -76,9 +76,9 @@ expect(@item['provisioningParameters']['storagePoolUri']).to eq('/rest/fake') end - it 'sets the storagePoolUri' do + it 'should not sets the storagePoolUri without provisioningParameters' do @item.set_storage_pool(OneviewSDK::StoragePool.new(@client_200, uri: '/rest/fake')) - expect(@item['storagePoolUri']).to eq('/rest/fake') + expect(@item['storagePoolUri']).to_not be end end diff --git a/spec/unit/resource/api500/c7000/volume_spec.rb b/spec/unit/resource/api500/c7000/volume_spec.rb index 18eebb0e6..538015026 100644 --- a/spec/unit/resource/api500/c7000/volume_spec.rb +++ b/spec/unit/resource/api500/c7000/volume_spec.rb @@ -93,10 +93,10 @@ expect(item['properties']['storagePool']).to eq('/rest/fake') end - it 'sets the storagePoolUri attribute' do + it 'should not sets the storagePoolUri without properties' do item = described_class.new(@client_500) item.set_storage_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake')) - expect(item['storagePoolUri']).to eq('/rest/fake') + expect(item['storagePoolUri']).to_not be end end @@ -108,9 +108,9 @@ end it 'sets the snapshotPoolUri attribute' do - item = described_class.new(@client_500) + item = described_class.new(@client_500, deviceSpecificAttributes: {}) item.set_snapshot_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake')) - expect(item['snapshotPoolUri']).to eq('/rest/fake') + expect(item['deviceSpecificAttributes']['snapshotPoolUri']).to eq('/rest/fake') end end @@ -322,11 +322,10 @@ end describe '#get_volume_template_uri' do - let(:instance_item) { described_class.new(@client_500) } + let(:instance_item) { described_class.new(@client_500, storagePoolUri: '/storage-pools/1') } before do storage_pool = OneviewSDK::API500::C7000::StoragePool.new(@client_500, storageSystemUri: '/storage-systems/1', uri: '/storage-pools/1') - instance_item.set_storage_pool(storage_pool) expect(storage_pool).to receive(:retrieve!).and_return(true) expect(OneviewSDK::API500::C7000::StoragePool).to receive(:new).and_return(storage_pool) storage_system = OneviewSDK::API500::C7000::StorageSystem.new(@client_500) From 70735753cfaf7411c008bbb737e9780283cba8cf Mon Sep 17 00:00:00 2001 From: Alex Monteiro Date: Thu, 5 Oct 2017 02:37:38 +0000 Subject: [PATCH 3/3] Fixing review comments in volumes --- lib/oneview-sdk/resource/api200/volume.rb | 13 ++++++- .../resource/api500/c7000/volume.rb | 19 +++++++--- .../volume/api200/update_snapshot_pool.rb | 24 ++++++------- .../shared_examples/volume/api500/create.rb | 14 +++----- .../volume/api500/update_snapshot_pool.rb | 21 ++++++----- .../shared_examples/volume/create.rb | 9 ++--- spec/unit/resource/api200/volume_spec.rb | 23 ++++++++---- .../unit/resource/api500/c7000/volume_spec.rb | 35 ++++++++++++------- 8 files changed, 93 insertions(+), 65 deletions(-) diff --git a/lib/oneview-sdk/resource/api200/volume.rb b/lib/oneview-sdk/resource/api200/volume.rb index 2f3e266ef..12806bf77 100644 --- a/lib/oneview-sdk/resource/api200/volume.rb +++ b/lib/oneview-sdk/resource/api200/volume.rb @@ -39,6 +39,14 @@ def create self end + # Update resource attributes + # @param [Hash] attributes attributes to be updated + # @return [OneviewSDK::Volume] self + def update(attributes = {}) + @data.delete('provisioningParameters') + super + end + # Deletes the resource from OneView or from Oneview and storage system # @param [Symbol] flag Delete storage system from Oneview only or in storage system as well # @return [true] if resource was deleted successfully @@ -59,16 +67,19 @@ def delete(flag = :all) # Sets the storage system to the volume # @param [OneviewSDK::StorageSystem] storage_system Storage System + # @note The storageSystemUri attribute should not be set in the updated. Once created, this attribute is read only. def set_storage_system(storage_system) assure_uri(storage_system) set('storageSystemUri', storage_system['uri']) end # Sets the storage pool to the volume + # @note The storagePoolUri attribute should not be set in the updated. Once created, this attribute is read only. # @param [OneviewSDK::StoragePool] storage_pool Storage pool def set_storage_pool(storage_pool) assure_uri(storage_pool) - self['provisioningParameters']['storagePoolUri'] = storage_pool['uri'] if self['provisioningParameters'] + self['provisioningParameters'] ||= {} + self['provisioningParameters']['storagePoolUri'] = storage_pool['uri'] end # Adds the storage volume template to the volume diff --git a/lib/oneview-sdk/resource/api500/c7000/volume.rb b/lib/oneview-sdk/resource/api500/c7000/volume.rb index 579327206..e9caaeec5 100644 --- a/lib/oneview-sdk/resource/api500/c7000/volume.rb +++ b/lib/oneview-sdk/resource/api500/c7000/volume.rb @@ -43,6 +43,14 @@ def create self end + # Update resource attributes + # @param [Hash] attributes attributes to be updated + # @return [OneviewSDK::Volume] self + def update(attributes = {}) + @data.delete('properties') + OneviewSDK::Resource.instance_method(:update).bind(self).call(attributes) + end + # Deletes the resource from OneView or from Oneview and storage system # @param [Symbol] flag Delete storage system from Oneview only or in storage system as well. # Flags: :all = removes the volume from oneview and storage system. :oneview = removes from oneview only. @@ -60,20 +68,23 @@ def delete(flag = :all) # Sets the storage pool to the volume # @param [OneviewSDK::StoragePool] storage_pool Storage pool. + # @note The storagePoolUri attribute should not be set in the updated. Once created, this attribute is read only. def set_storage_pool(storage_pool) assure_uri(storage_pool) - @data['properties']['storagePool'] = storage_pool['uri'] if @data['properties'] + @data['properties'] ||= {} + @data['properties']['storagePool'] = storage_pool['uri'] end # Sets the snapshot pool to the volume # @param [OneviewSDK::StoragePool] storage_pool Storage Pool to use for snapshots. def set_snapshot_pool(storage_pool) assure_uri(storage_pool) - if @data['properties'] - @data['properties']['snapshotPool'] = storage_pool['uri'] - else + if @data['uri'] @data['deviceSpecificAttributes'] ||= {} @data['deviceSpecificAttributes']['snapshotPoolUri'] = storage_pool['uri'] + else + @data['properties'] ||= {} + @data['properties']['snapshotPool'] = storage_pool['uri'] end end diff --git a/spec/integration/shared_examples/volume/api200/update_snapshot_pool.rb b/spec/integration/shared_examples/volume/api200/update_snapshot_pool.rb index afce1ff03..45bcf9e85 100644 --- a/spec/integration/shared_examples/volume/api200/update_snapshot_pool.rb +++ b/spec/integration/shared_examples/volume/api200/update_snapshot_pool.rb @@ -12,21 +12,21 @@ RSpec.shared_examples 'VolumeSnapshotPoolUpdateExample' do |context_name| include_context context_name - let(:item) { described_class.find_by(current_client, name: VOLUME2_NAME).first } - describe '#update snapshot pool' do it 'updating the snapshot pool' do - old_snapshot_pool = resource_class_of('StoragePool').find_by(current_client, uri: item['snapshotPoolUri']).first - new_snapshot_pool = resource_class_of('StoragePool').get_all(current_client).first - item.set_snapshot_pool(new_snapshot_pool) - item.update - item.retrieve! - expect(item['snapshotPoolUri']).to eq(new_snapshot_pool['uri']) + volume = described_class.find_by(current_client, name: VOLUME4_NAME).first + storage_pools = resource_class_of('StoragePool').find_by(current_client, storageSystemUri: volume['storageSystemUri']) + old_snapshot_pool = resource_class_of('StoragePool').find_by(current_client, uri: volume['snapshotPoolUri']).first + new_snapshot_pool = storage_pools.select { |pool| pool['uri'] != old_snapshot_pool['uri'] }.first + volume.set_snapshot_pool(new_snapshot_pool) + volume.update + volume.retrieve! + expect(volume['snapshotPoolUri']).to eq(new_snapshot_pool['uri']) # Returning to original snapshot pool - item.set_snapshot_pool(old_snapshot_pool) - item.update - item.retrieve! - expect(item['snapshotPoolUri']).to eq(old_snapshot_pool['uri']) + volume.set_snapshot_pool(old_snapshot_pool) + volume.update + volume.retrieve! + expect(volume['snapshotPoolUri']).to eq(old_snapshot_pool['uri']) end end end diff --git a/spec/integration/shared_examples/volume/api500/create.rb b/spec/integration/shared_examples/volume/api500/create.rb index d48589a33..dfb1faacc 100644 --- a/spec/integration/shared_examples/volume/api500/create.rb +++ b/spec/integration/shared_examples/volume/api500/create.rb @@ -231,30 +231,24 @@ end describe '#set_storage_pool' do - it 'set_storage_pool in properties' do - item = described_class.new(current_client, name: VOLUME_NAME, properties: {}) - item.set_storage_pool(storage_pool) - expect(item['properties']['storagePool']).to eq(storage_pool['uri']) - end - it 'set_storage_pool' do item = described_class.new(current_client, name: VOLUME_NAME) item.set_storage_pool(storage_pool) - expect(item['storagePoolUri']).to eq(storage_pool['uri']) + expect(item['properties']['storagePool']).to eq(storage_pool['uri']) end end describe '#set_snapshot_pool' do it 'set_snapshot_pool in properties' do - item = described_class.new(current_client, name: VOLUME_NAME, properties: {}) + item = described_class.new(current_client, name: VOLUME_NAME) item.set_snapshot_pool(storage_pool) expect(item['properties']['snapshotPool']).to eq(storage_pool['uri']) end it 'set_snapshot_pool' do - item = described_class.new(current_client, name: VOLUME_NAME) + item = described_class.get_all(current_client).first item.set_snapshot_pool(storage_pool) - expect(item['snapshotPoolUri']).to eq(storage_pool['uri']) + expect(item['deviceSpecificAttributes']['snapshotPoolUri']).to eq(storage_pool['uri']) end end diff --git a/spec/integration/shared_examples/volume/api500/update_snapshot_pool.rb b/spec/integration/shared_examples/volume/api500/update_snapshot_pool.rb index f41f328d4..238b5590b 100644 --- a/spec/integration/shared_examples/volume/api500/update_snapshot_pool.rb +++ b/spec/integration/shared_examples/volume/api500/update_snapshot_pool.rb @@ -12,22 +12,21 @@ RSpec.shared_examples 'VolumeSnapshotPoolUpdateExample API500' do |context_name| include_context context_name - let(:item) { described_class.find_by(current_client, name: VOLUME2_NAME).first } - describe '#update snapshot pool' do it 'updating the snapshot pool' do - old_snapshot_pool = resource_class_of('StoragePool').find_by(current_client, uri: item['deviceSpecificAttributes']['snapshotPoolUri']).first + volume = described_class.find_by(current_client, name: VOLUME5_NAME).first + old_snapshot_pool = resource_class_of('StoragePool').find_by(current_client, uri: volume['deviceSpecificAttributes']['snapshotPoolUri']).first new_snapshot_pool = resource_class_of('StoragePool').find_by(current_client, isManaged: false).first new_snapshot_pool.manage(true) - item.set_snapshot_pool(new_snapshot_pool) - item.update - item.retrieve! - expect(item['deviceSpecificAttributes']['snapshotPoolUri']).to eq(new_snapshot_pool['uri']) + volume.set_snapshot_pool(new_snapshot_pool) + volume.update + volume.retrieve! + expect(volume['deviceSpecificAttributes']['snapshotPoolUri']).to eq(new_snapshot_pool['uri']) # Returning to original snapshot pool - item.set_snapshot_pool(old_snapshot_pool) - item.update - item.retrieve! - expect(item['deviceSpecificAttributes']['snapshotPoolUri']).to eq(old_snapshot_pool['uri']) + volume.set_snapshot_pool(old_snapshot_pool) + volume.update + volume.retrieve! + expect(volume['deviceSpecificAttributes']['snapshotPoolUri']).to eq(old_snapshot_pool['uri']) new_snapshot_pool.manage(false) end end diff --git a/spec/integration/shared_examples/volume/create.rb b/spec/integration/shared_examples/volume/create.rb index d2350fbc1..f2d05eb86 100644 --- a/spec/integration/shared_examples/volume/create.rb +++ b/spec/integration/shared_examples/volume/create.rb @@ -71,6 +71,7 @@ item2 = described_class.new(current_client, options.merge(name: VOLUME4_NAME, snapshotUri: "#{item[:uri]}/snapshots/#{snap['uri']}")) item2.set_storage_system(storage_system) item2.set_storage_pool(storage_pool) + item2.set_snapshot_pool(storage_pool) expect { item2.create }.to_not raise_error expect(item2.retrieve!).to eq(true) expect(item2['name']).to eq(VOLUME4_NAME) @@ -112,16 +113,10 @@ end describe '#set_storage_pool' do - it 'set_storage_pool in provisioningParameters' do - item = described_class.new(current_client, name: VOLUME_NAME, provisioningParameters: {}) - item.set_storage_pool(storage_pool) - expect(item['provisioningParameters']['storagePoolUri']).to eq(storage_pool['uri']) - end - it 'set_storage_pool' do item = described_class.new(current_client, name: VOLUME_NAME) item.set_storage_pool(storage_pool) - expect(item['storagePoolUri']).to eq(storage_pool['uri']) + expect(item['provisioningParameters']['storagePoolUri']).to eq(storage_pool['uri']) end end diff --git a/spec/unit/resource/api200/volume_spec.rb b/spec/unit/resource/api200/volume_spec.rb index 96273f56c..65680c1bc 100644 --- a/spec/unit/resource/api200/volume_spec.rb +++ b/spec/unit/resource/api200/volume_spec.rb @@ -35,6 +35,21 @@ end end + describe '#update' do + it 'updating a volume' do + fake_response = FakeResponse.new + item = described_class.new(@client_200, uri: 'rest/fake', name: 'Volume') + item.set_storage_pool(OneviewSDK::StoragePool.new(@client_200, uri: '/rest/fake2')) + allow_any_instance_of(OneviewSDK::Client).to receive(:rest_put).and_return(fake_response) + allow_any_instance_of(OneviewSDK::Client).to receive(:response_handler) + .with(fake_response).and_return('name' => 'Volume2') + data = { 'uri' => item['uri'], 'name' => 'Volume2' } + expect(@client_200).to receive(:rest_put).with('rest/fake', { 'body' => data }, 200) + item.update(name: 'Volume2') + expect(item['name']).to eq('Volume2') + end + end + describe '#delete' do it 'raises an exception when is passed as invalid flag' do allow_any_instance_of(OneviewSDK::Client).to receive(:response_handler).and_return(true) @@ -70,16 +85,10 @@ end describe '#set_storage_pool' do - it 'sets the storagePoolUri in provisioningParameters' do - @item['provisioningParameters'] = {} + it 'sets the storagePoolUri' do @item.set_storage_pool(OneviewSDK::StoragePool.new(@client_200, uri: '/rest/fake')) expect(@item['provisioningParameters']['storagePoolUri']).to eq('/rest/fake') end - - it 'should not sets the storagePoolUri without provisioningParameters' do - @item.set_storage_pool(OneviewSDK::StoragePool.new(@client_200, uri: '/rest/fake')) - expect(@item['storagePoolUri']).to_not be - end end describe '#set_storage_volume_template' do diff --git a/spec/unit/resource/api500/c7000/volume_spec.rb b/spec/unit/resource/api500/c7000/volume_spec.rb index 538015026..6b26ee9ea 100644 --- a/spec/unit/resource/api500/c7000/volume_spec.rb +++ b/spec/unit/resource/api500/c7000/volume_spec.rb @@ -64,6 +64,20 @@ end end + describe '#update' do + it 'updating a volume' do + item = described_class.new(@client_500, uri: 'rest/fake', name: 'Volume') + item.set_storage_pool(OneviewSDK::API500::C7000::StoragePool.new(@client_500, uri: '/rest/fake2')) + allow_any_instance_of(OneviewSDK::Client).to receive(:rest_put).and_return(fake_response) + allow_any_instance_of(OneviewSDK::Client).to receive(:response_handler) + .with(fake_response).and_return('name' => 'Volume2') + data = { 'uri' => item['uri'], 'name' => 'Volume2' } + expect(@client_500).to receive(:rest_put).with('rest/fake', { 'body' => data }, 500) + item.update(name: 'Volume2') + expect(item['name']).to eq('Volume2') + end + end + describe '#delete' do it 'raises an exception when is passed as invalid flag' do allow_any_instance_of(OneviewSDK::Client).to receive(:response_handler).and_return(true) @@ -87,30 +101,24 @@ end describe '#set_storage_pool' do - it 'sets the storagePool attribute in properties' do - item = described_class.new(@client_500, properties: {}) - item.set_storage_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake')) - expect(item['properties']['storagePool']).to eq('/rest/fake') - end - - it 'should not sets the storagePoolUri without properties' do + it 'sets the storagePool attribute' do item = described_class.new(@client_500) item.set_storage_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake')) - expect(item['storagePoolUri']).to_not be + expect(item['properties']['storagePool']).to eq('/rest/fake') end end describe '#set_snapshot_pool' do it 'sets the snapshotPool attribute in properties' do - item = described_class.new(@client_500, properties: {}) + item = described_class.new(@client_500) item.set_snapshot_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake')) expect(item['properties']['snapshotPool']).to eq('/rest/fake') end it 'sets the snapshotPoolUri attribute' do - item = described_class.new(@client_500, deviceSpecificAttributes: {}) - item.set_snapshot_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake')) - expect(item['deviceSpecificAttributes']['snapshotPoolUri']).to eq('/rest/fake') + item = described_class.new(@client_500, uri: 'rest/fake') + item.set_snapshot_pool(OneviewSDK::StoragePool.new(@client_500, uri: '/rest/fake2')) + expect(item['deviceSpecificAttributes']['snapshotPoolUri']).to eq('/rest/fake2') end end @@ -322,10 +330,11 @@ end describe '#get_volume_template_uri' do - let(:instance_item) { described_class.new(@client_500, storagePoolUri: '/storage-pools/1') } + let(:instance_item) { described_class.new(@client_500) } before do storage_pool = OneviewSDK::API500::C7000::StoragePool.new(@client_500, storageSystemUri: '/storage-systems/1', uri: '/storage-pools/1') + instance_item.set_storage_pool(storage_pool) expect(storage_pool).to receive(:retrieve!).and_return(true) expect(OneviewSDK::API500::C7000::StoragePool).to receive(:new).and_return(storage_pool) storage_system = OneviewSDK::API500::C7000::StorageSystem.new(@client_500)