Native disk update

src/bosh_azure_cpi/Gemfile

@@ -12,7 +12,7 @@ gem 'jwt',              '~> 2.2', '>= 2.2.2'
 gem 'deep_merge',       '~> 1.2', '>= 1.2.1'
 gem 'net-smtp'
 # fast_jsonparser has a GCC requirement that cannot be guaranteed by all VMs the CPI might run on.
-install_if (-> { (`uname`.include?('Linux') && !(`lsb_release -sr`.include?('16.04') if system('which lsb_release > /dev/null 2>&1'))) || `gcc -dumpversion`.to_i > 6 }) do
+install_if(-> { (`uname`.include?('Linux') && !(`lsb_release -sr`.include?('16.04') if system('which lsb_release > /dev/null 2>&1'))) || `gcc -dumpversion`.to_i > 6 }) do
   gem 'fast_jsonparser', '~> 0.6.0'
 end
 

src/bosh_azure_cpi/lib/cloud/azure/cloud.rb

@@ -479,6 +479,44 @@ def resize_disk(disk_cid, new_size)
       end
     end
 
+    def update_disk(disk_cid, new_size, cloud_properties)
+      @logger.info("update_disk(#{disk_cid}, #{new_size}, #{cloud_properties})")
+      raise Bosh::Clouds::NotSupported, 'Native disk update only supported for managed disks' unless @use_managed_disks
+
+      @telemetry_manager.monitor('initialize') do
+        _init_azure
+      end
+
+      with_thread_name("update_disk(#{disk_cid}, #{new_size}, #{cloud_properties})") do
+        @telemetry_manager.monitor('update_disk', id: disk_cid) do
+          disk_id = DiskId.parse(disk_cid, _azure_config.resource_group_name)
+          disk_name = disk_id.disk_name
+          disk = @disk_manager2.get_data_disk(disk_id)
+          raise Bosh::Clouds::NotSupported, "Disk '#{disk_name}' not found" if disk.nil?
+
+          caching = cloud_properties['caching']
+          raise Bosh::Clouds::NotSupported, 'Disk caching cannot be modified' if caching && (caching != disk[:caching])
+
+          new_size_in_gib = mib_to_gib(new_size)
+          old_size_in_gib = disk[:disk_size]
+          raise Bosh::Clouds::NotSupported, 'Disk size cannot be decreased' if old_size_in_gib > new_size_in_gib
+
+          new_size_in_gib = nil if new_size_in_gib == old_size_in_gib
+          account_type = cloud_properties['storage_account_type']
+          iops = cloud_properties['iops']
+          mbps = cloud_properties['mbps']
+
+          if new_size_in_gib.nil? && account_type.nil? && iops.nil? && mbps.nil?
+            @logger.info("Skip native disk update of '#{disk_name}' because no change is needed")
+            return
+          end
+
+          @disk_manager2.update_disk(disk_id, new_size_in_gib, account_type, iops, mbps)
+          @logger.info("Finished update of disk '#{disk_name}'")
+        end
+      end
+    end
+
     def mib_to_gib(size)
       (size / 1024.0).ceil
     end

src/bosh_azure_cpi/lib/cloud/azure/disk/disk_manager2.rb

@@ -45,6 +45,24 @@ def create_disk(disk_id, location, size, storage_account_type, zone = nil, iops
       @azure_client.create_empty_managed_disk(resource_group_name, disk_params)
     end
 
+    def update_disk(disk_id, size = nil, storage_account_type = nil, iops = nil, mbps = nil)
+      @logger.info("update_disk(#{disk_id}, #{size}, #{storage_account_type}, #{iops}, #{mbps})")
+      disk_params = {}
+      disk_params[:disk_size] = size if size
+      disk_params[:account_type] = storage_account_type if storage_account_type
+      disk_params[:iops] = iops if iops
+      disk_params[:mbps] = mbps if mbps
+
+      resource_group_name = disk_id.resource_group_name
+      unless disk_params.any?
+        @logger.info("No need to update disk '#{disk_id.disk_name}' in resource group '#{resource_group_name}'")
+        return
+      end
+
+      @logger.info("Start to update disk '#{disk_id.disk_name}' in resource group '#{resource_group_name}' with new parameters '#{disk_params}'")
+      @azure_client.update_managed_disk(resource_group_name, disk_id.disk_name, disk_params)
+    end
+
     def create_disk_from_blob(disk_id, blob_uri, location, storage_account_type, storage_account_id, zone = nil)
       @logger.info("create_disk_from_blob(#{disk_id}, #{blob_uri}, #{location}, #{storage_account_type}, #{storage_account_id}, #{zone})")
       resource_group_name = disk_id.resource_group_name

src/bosh_azure_cpi/lib/cloud/azure/restapi/azure_client.rb

@@ -924,6 +924,47 @@ def create_empty_managed_disk(resource_group_name, params)
       http_put(url, disk)
     end
 
+    # Update a managed disk based on the supplied configuration.
+    #
+    # ==== Attributes
+    #
+    # @param [String] resource_group_name  - Name of resource group.
+    # @param [String] name                 - Name of the disk that should be updated.
+    # @param [Hash]   params               - Parameters for creating the empty managed disk.
+    #
+    # ==== params
+    #
+    # Accepted key/value pairs are:
+    # * +:location+                     - String. The new location of the disk.
+    # * +:tags+                         - Hash. The new tags of the disk.
+    # * +:account_type+                 - String. Specifies the new account type for the disk.
+    #                                     Allowed values: Standard_LRS, StandardSSD_LRS, Premium_LRS, PremiumV2_LRS,
+    #                                     UltraSSD_LRS.
+    # * +:disk_size+                    - Integer. Specifies the new size in GB of the disk.
+    # * +:zone+                         - String. The new zone of the disk.
+    #                                     Allowed values: 1, 2, 3
+    # * +:iops+                         - Integer. Specifies the new IOPS allowed for this disk.
+    # * +:mbps+                         - Integer. Specifies the new MBps allowed for this disk.
+    #
+    # @See https://learn.microsoft.com/en-us/rest/api/compute/disks/update?view=rest-compute-2023-04-02&tabs=HTTP
+    def update_managed_disk(resource_group_name, name, params)
+      raise AzureNotFoundError, "Disk parameter 'name' must not be empty" if name.empty?
+
+      request_body = { 'properties' => {} }
+      request_body['location'] = params[:location] if params[:location]
+      request_body['tags'] = params[:tags] if params[:tags]
+      request_body['sku'] = { 'name' => params[:account_type] } if params[:account_type]
+      request_body['properties']['diskSizeGB'] = params[:disk_size] if params[:disk_size]
+      request_body['properties']['diskIOPSReadWrite'] = params[:iops] if params[:iops]
+      request_body['properties']['diskMBpsReadWrite'] = params[:mbps] if params[:mbps]
+      request_body.delete('properties') if request_body['properties'].empty?
+
+      if request_body.any?
+        url = rest_api_url(REST_API_PROVIDER_COMPUTE, REST_API_DISKS, resource_group_name: resource_group_name, name: name)
+        http_patch(url, request_body)
+      end
+    end
+
     def resize_managed_disk(resource_group_name, params)
       url = rest_api_url(REST_API_PROVIDER_COMPUTE, REST_API_DISKS, resource_group_name: resource_group_name, name: params[:name])
       disk = {
@@ -2030,6 +2071,7 @@ def parse_vm_size(result)
     # @return [Hash, nil]
     def parse_managed_disk(result)
       managed_disk = nil
+
       unless result.nil?
         managed_disk = {}
         managed_disk[:id]        = result['id']
@@ -2040,6 +2082,8 @@ def parse_managed_disk(result)
         managed_disk[:sku_tier]  = result['sku']['tier']
         managed_disk[:zone]      = result['zones'][0] unless result['zones'].nil?
         properties = result['properties']
+        managed_disk[:iops]      = properties['diskIOPSReadWrite']
+        managed_disk[:mbps]      = properties['diskMBpsReadWrite']
         managed_disk[:provisioning_state] = properties['provisioningState']
         managed_disk[:disk_size]          = properties['diskSizeGB']
       end
@@ -2723,7 +2767,9 @@ def get_http_common_headers(response)
     def remove_resources_from_vm(vm)
       vm.delete('resources')
       if vm['identity'] && vm['identity']['type'] == 'UserAssigned'
-        vm['identity']['userAssignedIdentities'] = vm['identity']['userAssignedIdentities'].keys.inject({}) { |result, k| result[k] = {}; result }
+        vm['identity']['userAssignedIdentities'] = vm['identity']['userAssignedIdentities'].keys.each_with_object({}) do |k, result|
+          result[k] = {}
+        end
       end
       vm
     end

src/bosh_azure_cpi/spec/integration/managed_disks_spec.rb

@@ -9,7 +9,7 @@
     described_class.new(cloud_options_managed, Bosh::AzureCloud::Cloud::CURRENT_API_VERSION)
   end
 
-  let(:size) { 10_000 }
+  let(:old_size) { 10_000 }
   let(:new_size) { 20_000 }
 
   before { @disk_id_pool = [] }
@@ -23,21 +23,97 @@
 
   context 'Resize a disk to a higher size' do
     it 'should work' do
-      @logger.info("Create new managed disk with #{size} MiB")
-      disk_cid = cpi_managed.create_disk(size, {})
+      @logger.info("Create new managed disk with #{old_size} MiB")
+      disk_cid = cpi_managed.create_disk(old_size, {})
       expect(disk_cid).not_to be_nil
       @disk_id_pool.push(disk_cid)
 
       @logger.info("Resize managed disk '#{disk_cid}' to #{new_size} MiB")
       cpi_managed.resize_disk(disk_cid, new_size)
 
-      disk_id_obj = Bosh::AzureCloud::DiskId.parse(disk_cid, @azure_config.resource_group_name)
-      disk = get_azure_client.get_managed_disk_by_name(@default_resource_group_name, disk_id_obj.disk_name)
+      disk = get_disk(disk_cid)
       expect(disk[:disk_size]).to eq(cpi_managed.mib_to_gib(new_size))
 
       @logger.info("Delete managed disk '#{disk_cid}'")
       cpi_managed.delete_disk(disk_cid)
       @disk_id_pool.delete(disk_cid)
     end
   end
+
+  describe '#update_disk' do
+    context 'When disk properties change on a storage type that does support changing it' do
+      let(:old_storage_account_type) { 'Premium_LRS' }
+      let(:new_storage_account_type) { 'PremiumV2_LRS' }
+      let(:new_iops) { 3500 } # The default value of iops is 3000 for PremiumV2_LRS
+      let(:new_mbps) { 125 } # The default value of mbps is 120 for PremiumV2_LRS
+
+      it 'should update the disk properties accordingly' do
+        @logger.info("Create a managed disk with #{old_size} MiB")
+        disk_cid = cpi_managed.create_disk(old_size, { 'storage_account_type' => old_storage_account_type })
+        @disk_id_pool.push(disk_cid) if disk_cid
+
+        @logger.info("Check the disk properties of the created disk '#{disk_cid}'")
+        disk = get_disk(disk_cid)
+        expect(disk[:disk_size]).not_to eq(cpi_managed.mib_to_gib(new_size))
+        expect(disk[:iops]).not_to eq(new_iops)
+        expect(disk[:mbps]).not_to eq(new_mbps)
+        expect(disk[:sku_name]).to eq(old_storage_account_type)
+
+        @logger.info("Update managed disk '#{disk_cid}' to #{new_size} MiB, iops: #{new_iops}, mbps: #{new_mbps}, storage_account_type: #{new_storage_account_type}")
+        cloud_properties = { 'iops' => new_iops, 'mbps' => new_mbps, 'storage_account_type' => new_storage_account_type }
+        cpi_managed.update_disk(disk_cid, new_size, cloud_properties)
+
+        @logger.info("Check the disk properties of the updated disk '#{disk_cid}'")
+        disk = get_disk(disk_cid)
+        expect(disk[:disk_size]).to eq(cpi_managed.mib_to_gib(new_size))
+        expect(disk[:iops]).to eq(new_iops)
+        expect(disk[:mbps]).to eq(new_mbps)
+        expect(disk[:sku_name]).to eq(new_storage_account_type)
+
+        @logger.info("Delete managed disk '#{disk_cid}'")
+        cpi_managed.delete_disk(disk_cid)
+        @disk_id_pool.delete(disk_cid)
+      end
+    end
+
+    context 'When disk properties change on a storage type that does not support changing it' do
+      let(:old_storage_account_type) { 'Standard_LRS' }
+      let(:new_storage_account_type) { 'Premium_LRS' }
+      let(:new_iops) { 1000 }
+      let(:new_mbps) { 120 }
+
+      it 'raises no error and sets the disk type specific default' do
+        @logger.info("Create a managed disk with #{old_size} MiB")
+        disk_cid = cpi_managed.create_disk(old_size, { 'storage_account_type' => old_storage_account_type })
+        @disk_id_pool.push(disk_cid) if disk_cid
+
+        @logger.info("Check the disk properties of the managed disk '#{disk_cid}'")
+        disk = get_disk(disk_cid)
+        expect(disk[:sku_name]).to eq(old_storage_account_type)
+        expect(disk[:iops]).to eq(500) # iops equals the default value of iops for Standard_LRS
+        expect(disk[:mbps]).to eq(60) # mbps equals the default value of mbps for Standard_LRS
+
+        @logger.info("Try to update managed disk '#{disk_cid}' to iops: #{new_iops}, mbps: #{new_mbps}")
+        cloud_properties = { 'iops' => new_iops, 'mbps' => new_mbps, 'storage_account_type' => new_storage_account_type }
+        expect do
+          cpi_managed.update_disk(disk_cid, old_size, cloud_properties)
+        end.not_to raise_error
+
+        @logger.info("Check the disk properties of the updated disk '#{disk_cid}'")
+        disk = get_disk(disk_cid)
+        expect(disk[:sku_name]).to eq(new_storage_account_type)
+        expect(disk[:iops]).to eq(120) # iops equals the default value of iops for Premium_LRS
+        expect(disk[:mbps]).to eq(25) # mbps equals the default value of mbps for Premium_LRS
+
+        @logger.info("Delete managed disk '#{disk_cid}'")
+        cpi_managed.delete_disk(disk_cid)
+        @disk_id_pool.delete(disk_cid)
+      end
+    end
+  end
+
+  def get_disk(disk_cid)
+    disk_id_obj = Bosh::AzureCloud::DiskId.parse(disk_cid, @azure_config.resource_group_name)
+    get_azure_client.get_managed_disk_by_name(@default_resource_group_name, disk_id_obj.disk_name)
+  end
 end

src/bosh_azure_cpi/spec/unit/azure_client/azure_client_spec.rb

@@ -176,6 +176,69 @@
     end
   end
 
+  describe '#update_managed_disk' do
+    let(:resource_name) { 'xtreme-disk' }
+    let(:resource_group_name) { 'xtreme-resource-group-name' }
+    let(:resource_url) { 'some-resource-url' }
+
+    before do
+      allow(azure_client).to receive(:rest_api_url)
+        .with(Bosh::AzureCloud::AzureClient::REST_API_PROVIDER_COMPUTE,
+              Bosh::AzureCloud::AzureClient::REST_API_DISKS,
+              { resource_group_name: resource_group_name, name: resource_name }).and_return(resource_url)
+      allow(azure_client).to receive(:http_patch).with(resource_url, anything)
+    end
+
+    it 'should update the disk tags' do
+      tags = { 'foo' => 'bar' }
+      expect(azure_client).to receive(:http_patch).with(resource_url, { 'tags' => tags })
+
+      azure_client.update_managed_disk(resource_group_name, resource_name, { tags: tags })
+    end
+
+    it 'should update the location' do
+      location = 'eastus'
+      expect(azure_client).to receive(:http_patch).with(resource_url, { 'location' => location })
+
+      azure_client.update_managed_disk(resource_group_name, resource_name, { location: location })
+    end
+
+    it 'should update the account type' do
+      account_type = 'Standard_LRS'
+      expect(azure_client).to receive(:http_patch).with(resource_url, { 'sku' => { 'name' => account_type } })
+
+      azure_client.update_managed_disk(resource_group_name, resource_name, { account_type: account_type })
+    end
+
+    it 'should update the disks size iops and mbps' do
+      size = 4
+      iops = 3100
+      mbps = 150
+      expected_properties = {
+        'properties' => {
+          'diskSizeGB' => size,
+          'diskIOPSReadWrite' => iops,
+          'diskMBpsReadWrite' => mbps
+        }
+      }
+      expect(azure_client).to receive(:http_patch).with(resource_url, expected_properties)
+
+      azure_client.update_managed_disk(resource_group_name, resource_name, { disk_size: size, iops: iops, mbps: mbps })
+    end
+
+    it 'should not send a request if no params to update' do
+      expect(azure_client).not_to receive(:http_patch)
+
+      azure_client.update_managed_disk(resource_group_name, resource_name, {})
+    end
+
+    it 'should fail if no disk name is passed' do
+      expect do
+        azure_client.update_managed_disk(resource_group_name, '', { 'anything' => 'anything' })
+      end.to raise_error(/Disk parameter 'name' must not be empty/)
+    end
+  end
+
   describe '#_parse_name_from_id' do
     context 'when id is empty' do
       it 'should raise an error' do

src/bosh_azure_cpi/spec/unit/azure_client/managed_disks_spec.rb

@@ -414,7 +414,9 @@
         zones: ['fake-zone'],
         properties: {
           provisioningState: 'd',
-          diskSizeGB: 'e'
+          diskSizeGB: 'e',
+          diskMBpsReadWrite: 11,
+          diskIOPSReadWrite: 22
         }
       }
     end
@@ -430,7 +432,9 @@
         sku_tier: 'g',
         zone: 'fake-zone',
         provisioning_state: 'd',
-        disk_size: 'e'
+        disk_size: 'e',
+        mbps: 11,
+        iops: 22
       }
     end
 

src/bosh_azure_cpi/spec/unit/blob_manager_spec.rb

@@ -605,7 +605,7 @@
       metadata = {}
 
       expect(blob_service).to receive(:create_blob_snapshot)
-        .with(container_name, blob_name, { metadata: metadata,request_id: request_id })
+        .with(container_name, blob_name, { metadata: metadata, request_id: request_id })
         .and_return(snapshot_time)
 
       expect(