Merge pull request #280 from HewlettPackard/bugfix/set_storages_volumes

Fix 'set_storage_pool' and 'set_snapshot_pool' methods in Volumes

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

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,12 +67,14 @@ 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)

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,6 +68,7 @@ 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'] ||= {}
@@ -70,8 +79,13 @@ def set_storage_pool(storage_pool)
         # @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['uri']
+            @data['deviceSpecificAttributes'] ||= {}
+            @data['deviceSpecificAttributes']['snapshotPoolUri'] = storage_pool['uri']
+          else
+            @data['properties'] ||= {}
+            @data['properties']['snapshotPool'] = storage_pool['uri']
+          end
         end
 
         # Creates a new volume on the storage system from a snapshot of a volume.

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

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

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

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

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

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

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
+
+  describe '#update snapshot pool' do
+    it 'updating the snapshot pool' do
+      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
+      volume.set_snapshot_pool(old_snapshot_pool)
+      volume.update
+      volume.retrieve!
+      expect(volume['snapshotPoolUri']).to eq(old_snapshot_pool['uri'])
+    end
+  end
+end

spec/integration/shared_examples/volume/api500/create.rb

@@ -239,11 +239,17 @@
   end
 
   describe '#set_snapshot_pool' do
-    it 'set_snapshot_pool' do
+    it 'set_snapshot_pool in properties' 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'])
     end
+
+    it 'set_snapshot_pool' do
+      item = described_class.get_all(current_client).first
+      item.set_snapshot_pool(storage_pool)
+      expect(item['deviceSpecificAttributes']['snapshotPoolUri']).to eq(storage_pool['uri'])
+    end
   end
 
   describe '#retrieve!' do

spec/integration/shared_examples/volume/api500/update_snapshot_pool.rb

@@ -0,0 +1,33 @@
+# (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
+
+  describe '#update snapshot pool' do
+    it 'updating the snapshot pool' do
+      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)
+      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
+      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
+end

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)

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)

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)
@@ -95,11 +109,17 @@
   end
 
   describe '#set_snapshot_pool' do
-    it 'sets the snapshotPool attribute' do
+    it 'sets the snapshotPool attribute in properties' do
       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, 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
 
   describe '#create_from_snapshot' do