diff --git a/src/bosh-director/lib/bosh/director.rb b/src/bosh-director/lib/bosh/director.rb index 9a1cbdb8681..5b1e44bc1ca 100644 --- a/src/bosh-director/lib/bosh/director.rb +++ b/src/bosh-director/lib/bosh/director.rb @@ -237,6 +237,8 @@ module Director require 'bosh/director/jobs/helpers' require 'bosh/director/jobs/db_job' require 'bosh/director/jobs/orphan_disk' +require 'bosh/director/jobs/dynamic_disks/create_dynamic_disk' +require 'bosh/director/jobs/dynamic_disks/attach_dynamic_disk' require 'bosh/director/jobs/dynamic_disks/provide_dynamic_disk' require 'bosh/director/jobs/dynamic_disks/detach_dynamic_disk' require 'bosh/director/jobs/dynamic_disks/delete_dynamic_disk' diff --git a/src/bosh-director/lib/bosh/director/api/controllers/dynamic_disks_controller.rb b/src/bosh-director/lib/bosh/director/api/controllers/dynamic_disks_controller.rb index 2f9288af9ec..1c8c621fecc 100644 --- a/src/bosh-director/lib/bosh/director/api/controllers/dynamic_disks_controller.rb +++ b/src/bosh-director/lib/bosh/director/api/controllers/dynamic_disks_controller.rb @@ -5,6 +5,60 @@ module Api::Controllers class DynamicDisksController < BaseController include ValidationHelper + get '/', scope: :list_dynamic_disks do + disks = Models::DynamicDisk.eager(:deployment, vm: :instance).all.map do |disk| + { + name: disk.name, + disk_cid: disk.disk_cid, + deployment: disk.deployment&.name, + instance: disk.vm&.instance&.name, + availability_zone: disk.availability_zone, + size: disk.size, + disk_pool_name: disk.disk_pool_name, + cpi: disk.cpi, + metadata: disk.metadata, + } + end + json_encode(disks) + end + + post '/', scope: :create_dynamic_disks, consumes: :json do + request_hash = JSON.parse(request.body.read) + + deployment_name = safe_property(request_hash, 'deployment_name', class: String, min_length: 1) + az = safe_property(request_hash, 'az', class: String, min_length: 1) + disk_name = safe_property(request_hash, 'disk_name', class: String, min_length: 1) + disk_pool_name = safe_property(request_hash, 'disk_pool_name', class: String, min_length: 1) + disk_size = safe_property(request_hash, 'disk_size', class: Integer, min: 1) + metadata = safe_property(request_hash, 'metadata', class: Hash, optional: true) + + task = JobQueue.new.enqueue( + current_user, + Jobs::DynamicDisks::CreateDynamicDisk, + 'create dynamic disk', + [deployment_name, az, disk_name, disk_pool_name, disk_size, metadata], + ) + + redirect "/tasks/#{task.id}" + end + + post '/:disk_name/attach', scope: :attach_dynamic_disks, consumes: :json do + disk_name = safe_property(params, 'disk_name', class: String, min_length: 1) + + request_hash = JSON.parse(request.body.read) + instance_id = safe_property(request_hash, 'instance_id', class: String, min_length: 1) + metadata = safe_property(request_hash, 'metadata', class: Hash, optional: true) + + task = JobQueue.new.enqueue( + current_user, + Jobs::DynamicDisks::AttachDynamicDisk, + 'attach dynamic disk', + [disk_name, instance_id, metadata], + ) + + redirect "/tasks/#{task.id}" + end + post '/provide', scope: :provide_dynamic_disks, consumes: :json do request_hash = JSON.parse(request.body.read) diff --git a/src/bosh-director/lib/bosh/director/jobs/dynamic_disks/attach_dynamic_disk.rb b/src/bosh-director/lib/bosh/director/jobs/dynamic_disks/attach_dynamic_disk.rb new file mode 100644 index 00000000000..7795ca27252 --- /dev/null +++ b/src/bosh-director/lib/bosh/director/jobs/dynamic_disks/attach_dynamic_disk.rb @@ -0,0 +1,63 @@ +module Bosh::Director + module Jobs::DynamicDisks + class AttachDynamicDisk < Jobs::BaseJob + include Jobs::Helpers::DynamicDiskHelpers + include LockHelper + + @queue = :dynamic_disks + + def self.job_type + :attach_dynamic_disk + end + + def initialize(disk_name, instance_id, metadata = nil) + @disk_name = disk_name + @instance_id = instance_id + @metadata = metadata + end + + def perform + disk_model = Models::DynamicDisk.find(name: @disk_name) + raise "disk `#{@disk_name}` not found" if disk_model.nil? + + instance = Models::Instance.find(uuid: @instance_id) + raise "instance `#{@instance_id}` not found" if instance.nil? + + vm = instance.active_vm + raise "no active vm found for instance `#{@instance_id}`" if vm.nil? + + already_attached_same_vm = false + unless disk_model.vm.nil? + if disk_model.vm.id == vm.id + already_attached_same_vm = true + else + raise "disk `#{@disk_name}` is already attached to a different vm `#{disk_model.vm.cid}`" + end + end + + disk_az = disk_model.availability_zone + vm_az = vm.instance.availability_zone + unless disk_az.nil? || vm_az.nil? || disk_az == vm_az + raise "disk `#{@disk_name}` is in AZ `#{disk_az}` but instance `#{@instance_id}` is in AZ `#{vm_az}`" + end + + cloud = Bosh::Director::CloudFactory.create.get(disk_model.cpi, vm.stemcell_api_version) + + unless already_attached_same_vm + disk_hint = with_vm_lock(vm.cid, timeout: VM_LOCK_TIMEOUT) { cloud.attach_disk(vm.cid, disk_model.disk_cid) } + disk_model.update(vm_id: vm.id, availability_zone: vm.instance.availability_zone, disk_hint: disk_hint) + end + + if !@metadata.nil? && disk_model.metadata != @metadata + MetadataUpdater.build.update_dynamic_disk_metadata(cloud, disk_model, @metadata) + disk_model.update(metadata: @metadata) + end + + agent_client = AgentClient.with_agent_id(vm.agent_id, instance.name) + agent_client.add_dynamic_disk(disk_model.disk_cid, disk_model.disk_hint) + + "attached disk `#{@disk_name}` to vm `#{vm.cid}` in deployment `#{instance.deployment.name}`" + end + end + end +end diff --git a/src/bosh-director/lib/bosh/director/jobs/dynamic_disks/create_dynamic_disk.rb b/src/bosh-director/lib/bosh/director/jobs/dynamic_disks/create_dynamic_disk.rb new file mode 100644 index 00000000000..7de0a2ba140 --- /dev/null +++ b/src/bosh-director/lib/bosh/director/jobs/dynamic_disks/create_dynamic_disk.rb @@ -0,0 +1,77 @@ +module Bosh::Director + module Jobs::DynamicDisks + class CreateDynamicDisk < Jobs::BaseJob + include Jobs::Helpers::DynamicDiskHelpers + + @queue = :dynamic_disks + + def self.job_type + :create_dynamic_disk + end + + def initialize(deployment_name, az, disk_name, disk_pool_name, disk_size, metadata) + @deployment_name = deployment_name + @az = az + @disk_name = disk_name + @disk_pool_name = disk_pool_name + @disk_size = disk_size + @metadata = metadata + end + + def perform + disk_model = Models::DynamicDisk.find(name: @disk_name) + raise "disk `#{@disk_name}` already exists" unless disk_model.nil? + + deployment = Models::Deployment.find(name: @deployment_name) + raise "deployment `#{@deployment_name}` not found" if deployment.nil? + + # Find an active VM in the requested AZ to use as the hint for create_disk. + # The IaaS uses the VM's location to determine which AZ/datastore to place the disk in. + vm = find_active_vm_in_az(deployment, @az) + raise "no active VM found in deployment `#{@deployment_name}` in AZ `#{@az}`" if vm.nil? + + cloud = Bosh::Director::CloudFactory.create.get(vm.cpi) + + cloud_properties = find_disk_cloud_properties(deployment, @disk_pool_name).clone + cloud_properties['name'] = @disk_name + + disk_cid = cloud.create_disk(@disk_size, cloud_properties, vm.cid) + begin + disk_model = Models::DynamicDisk.create( + name: @disk_name, + disk_cid: disk_cid, + deployment_id: deployment.id, + size: @disk_size, + disk_pool_name: @disk_pool_name, + cpi: vm.cpi, + availability_zone: @az, + ) + rescue + cloud.delete_disk(disk_cid) + raise + end + + if !@metadata.nil? && disk_model.metadata != @metadata + MetadataUpdater.build.update_dynamic_disk_metadata(cloud, disk_model, @metadata) + disk_model.update(metadata: @metadata) + end + + disk_info = { disk_cid: disk_model.disk_cid } + + task_result.write(JSON.generate(disk_info)) + task_result.write("\n") + + "created disk `#{@disk_name}` in deployment `#{deployment.name}` in AZ `#{@az}`" + end + + private + + def find_active_vm_in_az(deployment, az) + deployment.instances + .select { |i| i.availability_zone == az } + .flat_map { |i| i.vms.select(&:active) } + .first + end + end + end +end diff --git a/src/bosh-director/lib/bosh/director/jobs/helpers/dynamic_disk_helpers.rb b/src/bosh-director/lib/bosh/director/jobs/helpers/dynamic_disk_helpers.rb index 2e46f7736e1..eeedb4ef49e 100644 --- a/src/bosh-director/lib/bosh/director/jobs/helpers/dynamic_disk_helpers.rb +++ b/src/bosh-director/lib/bosh/director/jobs/helpers/dynamic_disk_helpers.rb @@ -3,8 +3,9 @@ module Jobs::Helpers module DynamicDiskHelpers VM_LOCK_TIMEOUT = 60 - def find_disk_cloud_properties(instance, disk_pool_name) - teams = instance.deployment.teams + def find_disk_cloud_properties(deployment_or_instance, disk_pool_name) + deployment = deployment_or_instance.is_a?(Models::Deployment) ? deployment_or_instance : deployment_or_instance.deployment + teams = deployment.teams configs = Models::Config.latest_set_for_teams('cloud', *teams) raise 'No cloud configs provided' if configs.empty? @@ -16,4 +17,4 @@ def find_disk_cloud_properties(instance, disk_pool_name) end end end -end \ No newline at end of file +end diff --git a/src/bosh-director/spec/support/test_identity_provider.rb b/src/bosh-director/spec/support/test_identity_provider.rb index b4681e7c6e8..4d467fc8174 100644 --- a/src/bosh-director/spec/support/test_identity_provider.rb +++ b/src/bosh-director/spec/support/test_identity_provider.rb @@ -44,6 +44,7 @@ def user_scopes 'dynamic-disks-attacher' => ['bosh.dynamic-disks.attach'], 'dynamic-disks-detacher' => ['bosh.dynamic-disks.detach'], 'dynamic-disks-deleter' => ['bosh.dynamic-disks.delete'], + 'dynamic-disks-lister' => ['bosh.dynamic-disks.list'], 'outsider' => ['uaa.admin'], } end diff --git a/src/bosh-director/spec/unit/bosh/director/api/controllers/dynamic_disks_controller_spec.rb b/src/bosh-director/spec/unit/bosh/director/api/controllers/dynamic_disks_controller_spec.rb index a33f017519d..f308d4eff74 100644 --- a/src/bosh-director/spec/unit/bosh/director/api/controllers/dynamic_disks_controller_spec.rb +++ b/src/bosh-director/spec/unit/bosh/director/api/controllers/dynamic_disks_controller_spec.rb @@ -16,6 +16,8 @@ module Api before { App.new(config) } let(:instance_id) { 'fake-instance-id' } + let(:deployment_name) { 'fake-deployment' } + let(:az) { 'z1' } let(:disk_pool_name) { 'fake_disk_pool_name' } let(:disk_name) { 'fake_disk_name' } let(:disk_size) { 1000 } @@ -213,6 +215,327 @@ module Api end end + describe 'GET', '/' do + context 'when user is reader' do + before { basic_authorize('reader', 'reader') } + + it 'forbids access' do + expect(get('/').status).to eq(401) + end + end + + context 'when user has bosh.dynamic-disks.list scope' do + before { basic_authorize('dynamic-disks-lister', 'dynamic-disks-lister') } + + context 'when there are no dynamic disks' do + it 'returns an empty list' do + get '/' + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)).to eq([]) + end + end + + context 'when dynamic disks exist' do + let!(:deployment) { FactoryBot.create(:models_deployment, name: 'my-deployment') } + let!(:disk1) do + FactoryBot.create(:models_dynamic_disk, + name: 'disk-a', + disk_cid: 'cid-a', + deployment: deployment, + size: 1024, + disk_pool_name: 'large', + metadata: { 'env' => 'prod' }, + ) + end + let!(:disk2) do + FactoryBot.create(:models_dynamic_disk, + name: 'disk-b', + disk_cid: 'cid-b', + deployment: deployment, + size: 2048, + disk_pool_name: 'small', + ) + end + + it 'returns all dynamic disks including disk_pool_name and metadata' do + get '/' + expect(last_response.status).to eq(200) + body = JSON.parse(last_response.body) + expect(body.size).to eq(2) + expect(body).to include(hash_including( + 'name' => 'disk-a', + 'disk_cid' => 'cid-a', + 'deployment' => 'my-deployment', + 'size' => 1024, + 'disk_pool_name' => 'large', + 'metadata' => { 'env' => 'prod' }, + )) + expect(body).to include(hash_including( + 'name' => 'disk-b', + 'disk_cid' => 'cid-b', + 'deployment' => 'my-deployment', + 'size' => 2048, + 'disk_pool_name' => 'small', + )) + end + end + end + + context 'when user has admin permissions' do + before { authorize 'admin', 'admin' } + + it 'returns a list of dynamic disks' do + get '/' + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)).to eq([]) + end + end + end + + describe 'POST', '/' do + let(:content) do + JSON.generate({ + 'deployment_name' => deployment_name, + 'az' => az, + 'disk_pool_name' => disk_pool_name, + 'disk_name' => disk_name, + 'disk_size' => disk_size, + 'metadata' => metadata, + }) + end + + context 'when user is reader' do + before { basic_authorize('reader', 'reader') } + + it 'forbids access' do + expect(post('/', content, { 'CONTENT_TYPE' => 'application/json' }).status).to eq(401) + end + end + + context 'when user has bosh.dynamic-disks.create scope' do + before { basic_authorize('dynamic-disks-creator', 'dynamic-disks-creator') } + + it 'enqueues a CreateDynamicDisk task' do + expect_any_instance_of(Bosh::Director::JobQueue).to receive(:enqueue).with( + 'dynamic-disks-creator', + Jobs::DynamicDisks::CreateDynamicDisk, + 'create dynamic disk', + [deployment_name, az, disk_name, disk_pool_name, disk_size, metadata], + ).and_call_original + + post '/', content, { 'CONTENT_TYPE' => 'application/json' } + + expect_redirect_to_queued_task(last_response) + end + end + + context 'when user has only bosh.dynamic-disks.attach scope (not create)' do + before { basic_authorize('dynamic-disks-attacher', 'dynamic-disks-attacher') } + + it 'forbids access' do + expect(post('/', content, { 'CONTENT_TYPE' => 'application/json' }).status).to eq(401) + end + end + + context 'when user has admin permissions' do + before { authorize 'admin', 'admin' } + + it 'enqueues a CreateDynamicDisk task' do + expect_any_instance_of(Bosh::Director::JobQueue).to receive(:enqueue).with( + 'admin', + Jobs::DynamicDisks::CreateDynamicDisk, + 'create dynamic disk', + [deployment_name, az, disk_name, disk_pool_name, disk_size, metadata], + ).and_call_original + + post '/', content, { 'CONTENT_TYPE' => 'application/json' } + + expect_redirect_to_queued_task(last_response) + end + + context 'when deployment_name is missing' do + let(:deployment_name) { nil } + + it 'raises an error' do + post '/', content, { 'CONTENT_TYPE' => 'application/json' } + expect(last_response.status).to eq(400) + expect(JSON.parse(last_response.body)['description']).to include('deployment_name') + end + end + + context 'when deployment_name is empty' do + let(:deployment_name) { '' } + + it 'raises an error' do + post '/', content, { 'CONTENT_TYPE' => 'application/json' } + expect(last_response.status).to eq(400) + expect(JSON.parse(last_response.body)['description']).to include('deployment_name') + end + end + + context 'when az is missing' do + let(:az) { nil } + + it 'raises an error' do + post '/', content, { 'CONTENT_TYPE' => 'application/json' } + expect(last_response.status).to eq(400) + expect(JSON.parse(last_response.body)['description']).to include('az') + end + end + + context 'when az is empty' do + let(:az) { '' } + + it 'raises an error' do + post '/', content, { 'CONTENT_TYPE' => 'application/json' } + expect(last_response.status).to eq(400) + expect(JSON.parse(last_response.body)['description']).to include('az') + end + end + + context 'when disk_name is nil' do + let(:disk_name) { nil } + + it 'raises an error' do + post '/', content, { 'CONTENT_TYPE' => 'application/json' } + expect(last_response.status).to eq(400) + expect(JSON.parse(last_response.body)['description']).to include('disk_name') + end + end + + context 'when disk_name is empty' do + let(:disk_name) { '' } + + it 'raises an error' do + post '/', content, { 'CONTENT_TYPE' => 'application/json' } + expect(last_response.status).to eq(400) + expect(JSON.parse(last_response.body)['description']).to include('disk_name') + end + end + + context 'when disk_pool_name is nil' do + let(:disk_pool_name) { nil } + + it 'raises an error' do + post '/', content, { 'CONTENT_TYPE' => 'application/json' } + expect(last_response.status).to eq(400) + expect(JSON.parse(last_response.body)['description']).to include('disk_pool_name') + end + end + + context 'when disk_pool_name is empty' do + let(:disk_pool_name) { '' } + + it 'raises an error' do + post '/', content, { 'CONTENT_TYPE' => 'application/json' } + expect(last_response.status).to eq(400) + expect(JSON.parse(last_response.body)['description']).to include('disk_pool_name') + end + end + + context 'when disk_size is nil' do + let(:disk_size) { nil } + + it 'raises an error' do + post '/', content, { 'CONTENT_TYPE' => 'application/json' } + expect(last_response.status).to eq(400) + expect(JSON.parse(last_response.body)['description']).to include('disk_size') + end + end + + context 'when disk_size is 0' do + let(:disk_size) { 0 } + + it 'raises an error' do + post '/', content, { 'CONTENT_TYPE' => 'application/json' } + expect(last_response.status).to eq(400) + expect(JSON.parse(last_response.body)['description']).to include('disk_size') + end + end + end + end + + describe 'POST', '/:disk_name/attach' do + let(:content) { JSON.generate({ 'instance_id' => instance_id, 'metadata' => metadata }) } + + context 'when user is reader' do + before { basic_authorize('reader', 'reader') } + + it 'forbids access' do + expect(post('/disk_name/attach', content, { 'CONTENT_TYPE' => 'application/json' }).status).to eq(401) + end + end + + context 'when user has bosh.dynamic-disks.attach scope' do + before { basic_authorize('dynamic-disks-attacher', 'dynamic-disks-attacher') } + + it 'enqueues an AttachDynamicDisk task' do + expect_any_instance_of(Bosh::Director::JobQueue).to receive(:enqueue).with( + 'dynamic-disks-attacher', + Jobs::DynamicDisks::AttachDynamicDisk, + 'attach dynamic disk', + ['disk_name', instance_id, metadata], + ).and_call_original + + post '/disk_name/attach', content, { 'CONTENT_TYPE' => 'application/json' } + + expect_redirect_to_queued_task(last_response) + end + end + + context 'when user has only bosh.dynamic-disks.create scope (not attach)' do + before { basic_authorize('dynamic-disks-creator', 'dynamic-disks-creator') } + + it 'forbids access' do + expect(post('/disk_name/attach', content, { 'CONTENT_TYPE' => 'application/json' }).status).to eq(401) + end + end + + context 'when user has admin permissions' do + before { authorize 'admin', 'admin' } + + it 'enqueues an AttachDynamicDisk task' do + expect_any_instance_of(Bosh::Director::JobQueue).to receive(:enqueue).with( + 'admin', + Jobs::DynamicDisks::AttachDynamicDisk, + 'attach dynamic disk', + ['disk_name', instance_id, metadata], + ).and_call_original + + post '/disk_name/attach', content, { 'CONTENT_TYPE' => 'application/json' } + + expect_redirect_to_queued_task(last_response) + end + + context 'when metadata is omitted from the body' do + let(:content) { JSON.generate({ 'instance_id' => instance_id }) } + + it 'enqueues an AttachDynamicDisk task with nil metadata' do + expect_any_instance_of(Bosh::Director::JobQueue).to receive(:enqueue).with( + 'admin', + Jobs::DynamicDisks::AttachDynamicDisk, + 'attach dynamic disk', + ['disk_name', instance_id, nil], + ).and_call_original + + post '/disk_name/attach', content, { 'CONTENT_TYPE' => 'application/json' } + + expect_redirect_to_queued_task(last_response) + end + end + + context 'when instance_id is missing from the body' do + let(:content) { JSON.generate({}) } + + it 'raises an error' do + post '/disk_name/attach', content, { 'CONTENT_TYPE' => 'application/json' } + expect(last_response.status).to eq(400) + expect(JSON.parse(last_response.body)['description']).to include("instance_id") + end + end + end + end + describe 'DELETE', '/:disk_name' do context 'when user is reader' do before { basic_authorize('reader', 'reader') } diff --git a/src/bosh-director/spec/unit/bosh/director/jobs/dynamic_disks/attach_dynamic_disk_spec.rb b/src/bosh-director/spec/unit/bosh/director/jobs/dynamic_disks/attach_dynamic_disk_spec.rb new file mode 100644 index 00000000000..e3d921f2731 --- /dev/null +++ b/src/bosh-director/spec/unit/bosh/director/jobs/dynamic_disks/attach_dynamic_disk_spec.rb @@ -0,0 +1,237 @@ +require 'spec_helper' + +module Bosh::Director + describe Jobs::DynamicDisks::AttachDynamicDisk do + let(:disk_name) { 'fake-disk-name' } + let(:disk_cid) { 'fake-disk-cid' } + let(:disk_hint) { { 'id' => 'fake-disk-id' } } + let(:metadata) { nil } + + let(:agent_client) { instance_double(AgentClient) } + let(:cloud) { instance_double(Bosh::Clouds::ExternalCpi) } + + let(:instance) { FactoryBot.create(:models_instance) } + let!(:vm) { FactoryBot.create(:models_vm, instance: instance, active: true) } + + let(:attach_dynamic_disk_job) { Jobs::DynamicDisks::AttachDynamicDisk.new(disk_name, instance.uuid, metadata) } + let(:cloud_factory) { instance_double(CloudFactory, get: cloud) } + + before do + allow(Config).to receive(:name).and_return('fake-director-name') + allow(Config).to receive(:cloud_options).and_return('provider' => { 'path' => '/path/to/default/cpi' }) + allow(Config).to receive(:preferred_cpi_api_version).and_return(2) + allow(CloudFactory).to receive(:create).and_return(cloud_factory) + allow(AgentClient).to receive(:with_agent_id).with(vm.agent_id, instance.name).and_return(agent_client) + allow(cloud).to receive(:respond_to?).with(:set_disk_metadata).and_return(false) + end + + describe 'DJ job class expectations' do + let(:job_type) { :attach_dynamic_disk } + let(:queue) { :dynamic_disks } + it_behaves_like 'a DelayedJob job' + end + + describe '#perform' do + context 'when disk exists and is not attached' do + let!(:disk) do + FactoryBot.create( + :models_dynamic_disk, + name: disk_name, + disk_cid: disk_cid, + deployment: instance.deployment, + ) + end + + it 'attaches the disk to the VM and notifies the agent' do + expect(attach_dynamic_disk_job).to receive(:with_vm_lock).with(vm.cid, timeout: Jobs::Helpers::DynamicDiskHelpers::VM_LOCK_TIMEOUT).and_yield + expect(cloud).to receive(:attach_disk).with(vm.cid, disk_cid).and_return(disk_hint) + expect(agent_client).to receive(:add_dynamic_disk).with(disk_cid, disk_hint) + + result = attach_dynamic_disk_job.perform + + expect(result).to eq("attached disk `#{disk_name}` to vm `#{vm.cid}` in deployment `#{instance.deployment.name}`") + + disk.reload + expect(disk.vm).to eq(vm) + expect(disk.availability_zone).to eq(vm.instance.availability_zone) + end + + context 'when metadata is provided and CPI supports set_disk_metadata' do + let(:metadata) { { 'fake-key' => 'fake-value' } } + + it 'sets disk metadata after attaching' do + expect(attach_dynamic_disk_job).to receive(:with_vm_lock).with(vm.cid, timeout: Jobs::Helpers::DynamicDiskHelpers::VM_LOCK_TIMEOUT).and_yield + expect(cloud).to receive(:attach_disk).with(vm.cid, disk_cid).and_return(disk_hint) + expect(cloud).to receive(:respond_to?).with(:set_disk_metadata).and_return(true) + expect(cloud).to receive(:set_disk_metadata).with( + disk_cid, + hash_including('fake-key' => 'fake-value'), + ) + expect(agent_client).to receive(:add_dynamic_disk).with(disk_cid, disk_hint) + + attach_dynamic_disk_job.perform + + disk.reload + expect(disk.metadata).to eq(metadata) + end + end + + context 'when metadata is nil' do + let(:metadata) { nil } + + it 'does not call set_disk_metadata' do + expect(attach_dynamic_disk_job).to receive(:with_vm_lock).with(vm.cid, timeout: Jobs::Helpers::DynamicDiskHelpers::VM_LOCK_TIMEOUT).and_yield + expect(cloud).to receive(:attach_disk).with(vm.cid, disk_cid).and_return(disk_hint) + expect(cloud).not_to receive(:set_disk_metadata) + expect(agent_client).to receive(:add_dynamic_disk).with(disk_cid, disk_hint) + + attach_dynamic_disk_job.perform + end + end + end + + context 'when disk is already attached to the same VM' do + let!(:disk) do + FactoryBot.create( + :models_dynamic_disk, + name: disk_name, + disk_cid: disk_cid, + deployment: instance.deployment, + vm: vm, + disk_hint: disk_hint, + ) + end + + it 'skips CPI attach but still notifies the agent (idempotent retry)' do + expect(cloud).not_to receive(:attach_disk) + expect(agent_client).to receive(:add_dynamic_disk).with(disk_cid, disk_hint) + + result = attach_dynamic_disk_job.perform + + expect(result).to eq("attached disk `#{disk_name}` to vm `#{vm.cid}` in deployment `#{instance.deployment.name}`") + end + end + + context 'when disk is already attached to a different VM' do + let(:other_instance) { FactoryBot.create(:models_instance) } + let!(:other_vm) { FactoryBot.create(:models_vm, instance: other_instance, active: true) } + let!(:disk) do + FactoryBot.create( + :models_dynamic_disk, + name: disk_name, + disk_cid: disk_cid, + deployment: instance.deployment, + vm: other_vm, + ) + end + + it 'raises an error' do + expect { attach_dynamic_disk_job.perform }.to raise_error( + "disk `#{disk_name}` is already attached to a different vm `#{other_vm.cid}`", + ) + end + end + + context 'AZ mismatch between disk and target VM' do + let!(:disk) do + FactoryBot.create( + :models_dynamic_disk, + name: disk_name, + disk_cid: disk_cid, + deployment: instance.deployment, + availability_zone: 'z1', + ) + end + + before do + allow(instance).to receive(:availability_zone).and_return('z2') + instance.update(availability_zone: 'z2') + end + + it 'raises an error when disk AZ does not match VM AZ' do + expect { attach_dynamic_disk_job.perform }.to raise_error( + /disk `#{disk_name}` is in AZ `z1` but instance `#{instance.uuid}` is in AZ `z2`/, + ) + end + end + + context 'AZ match between disk and target VM' do + let!(:disk) do + FactoryBot.create( + :models_dynamic_disk, + name: disk_name, + disk_cid: disk_cid, + deployment: instance.deployment, + availability_zone: instance.availability_zone, + ) + end + + it 'attaches successfully when AZs match' do + expect(attach_dynamic_disk_job).to receive(:with_vm_lock).with(vm.cid, timeout: Jobs::Helpers::DynamicDiskHelpers::VM_LOCK_TIMEOUT).and_yield + expect(cloud).to receive(:attach_disk).with(vm.cid, disk_cid).and_return(disk_hint) + expect(agent_client).to receive(:add_dynamic_disk).with(disk_cid, disk_hint) + + expect { attach_dynamic_disk_job.perform }.not_to raise_error + end + end + + context 'when disk has no AZ set (created before AZ tracking)' do + let!(:disk) do + FactoryBot.create( + :models_dynamic_disk, + name: disk_name, + disk_cid: disk_cid, + deployment: instance.deployment, + availability_zone: nil, + ) + end + + it 'skips AZ check and attaches successfully' do + expect(attach_dynamic_disk_job).to receive(:with_vm_lock).with(vm.cid, timeout: Jobs::Helpers::DynamicDiskHelpers::VM_LOCK_TIMEOUT).and_yield + expect(cloud).to receive(:attach_disk).with(vm.cid, disk_cid).and_return(disk_hint) + expect(agent_client).to receive(:add_dynamic_disk).with(disk_cid, disk_hint) + + expect { attach_dynamic_disk_job.perform }.not_to raise_error + end + end + + context 'when disk does not exist' do + it 'raises an error' do + expect { attach_dynamic_disk_job.perform }.to raise_error("disk `#{disk_name}` not found") + end + end + + context 'when instance cannot be found' do + let!(:disk) do + FactoryBot.create( + :models_dynamic_disk, + name: disk_name, + disk_cid: disk_cid, + deployment: instance.deployment, + ) + end + let(:attach_dynamic_disk_job) { Jobs::DynamicDisks::AttachDynamicDisk.new(disk_name, 'unknown-instance-id') } + + it 'raises an error' do + expect { attach_dynamic_disk_job.perform }.to raise_error("instance `unknown-instance-id` not found") + end + end + + context 'when there is no active VM for the instance' do + let(:vm) { FactoryBot.create(:models_vm, instance: instance, active: false) } + let!(:disk) do + FactoryBot.create( + :models_dynamic_disk, + name: disk_name, + disk_cid: disk_cid, + deployment: instance.deployment, + ) + end + + it 'raises an error' do + expect { attach_dynamic_disk_job.perform }.to raise_error("no active vm found for instance `#{instance.uuid}`") + end + end + end + end +end diff --git a/src/bosh-director/spec/unit/bosh/director/jobs/dynamic_disks/create_dynamic_disk_spec.rb b/src/bosh-director/spec/unit/bosh/director/jobs/dynamic_disks/create_dynamic_disk_spec.rb new file mode 100644 index 00000000000..3bb5c38b0cf --- /dev/null +++ b/src/bosh-director/spec/unit/bosh/director/jobs/dynamic_disks/create_dynamic_disk_spec.rb @@ -0,0 +1,177 @@ +require 'spec_helper' + +module Bosh::Director + describe Jobs::DynamicDisks::CreateDynamicDisk do + let(:disk_name) { 'fake-disk-name' } + let(:disk_cid) { 'fake-disk-cid' } + let(:disk_pool_name) { 'fake-disk-pool-name' } + let(:disk_cloud_properties) { { 'fake-disk-cloud-property-key' => 'fake-disk-cloud-property-value' } } + let(:disk_size) { 1000 } + let(:metadata) { { 'fake-key' => 'fake-value' } } + let(:az) { 'z1' } + + let(:cloud) { instance_double(Bosh::Clouds::ExternalCpi) } + let(:deployment) { FactoryBot.create(:models_deployment) } + let(:instance) { FactoryBot.create(:models_instance, deployment: deployment, availability_zone: az) } + let!(:vm) { FactoryBot.create(:models_vm, instance: instance, active: true) } + + let(:task_result) { TaskDBWriter.new(:result_output, task.id) } + let(:task) { FactoryBot.create(:models_task, id: 42, username: 'user') } + + def parsed_task_result + JSON.parse(Models::Task.first(id: 42).result_output) + end + + let(:create_dynamic_disk_job) do + Jobs::DynamicDisks::CreateDynamicDisk.new(deployment.name, az, disk_name, disk_pool_name, disk_size, metadata) + end + + let!(:cloud_config) do + FactoryBot.create(:models_config_cloud, content: YAML.dump( + SharedSupport::DeploymentManifestHelper.simple_cloud_config.merge( + 'disk_types' => [ + { 'name' => disk_pool_name, + 'disk_size' => 1024, + 'cloud_properties' => disk_cloud_properties }, + ], + ), + )) + end + let(:cloud_factory) { instance_double(CloudFactory, get: cloud) } + + before do + allow(Config).to receive(:name).and_return('fake-director-name') + allow(Config).to receive(:cloud_options).and_return('provider' => { 'path' => '/path/to/default/cpi' }) + allow(Config).to receive(:preferred_cpi_api_version).and_return(2) + allow(Config).to receive(:result).and_return(task_result) + allow(CloudFactory).to receive(:create).and_return(cloud_factory) + allow(cloud).to receive(:respond_to?).with(:set_disk_metadata).and_return(false) + allow(create_dynamic_disk_job).to receive(:task_id).and_return(task.id) + end + + describe 'DJ job class expectations' do + let(:job_type) { :create_dynamic_disk } + let(:queue) { :dynamic_disks } + it_behaves_like 'a DelayedJob job' + end + + describe '#perform' do + context 'when disk does not exist' do + it 'creates the disk using a VM from the requested AZ as hint and stores the AZ' do + expected_cloud_properties = disk_cloud_properties.merge('name' => disk_name) + expect(cloud).to receive(:create_disk).with(disk_size, expected_cloud_properties, vm.cid).and_return(disk_cid) + + result = create_dynamic_disk_job.perform + + expect(result).to eq("created disk `#{disk_name}` in deployment `#{deployment.name}` in AZ `#{az}`") + expect(parsed_task_result).to eq({ 'disk_cid' => disk_cid }) + + model = Models::DynamicDisk.where(disk_cid: disk_cid).first + expect(model).not_to be_nil + expect(model.name).to eq(disk_name) + expect(model.size).to eq(disk_size) + expect(model.deployment_id).to eq(deployment.id) + expect(model.disk_pool_name).to eq(disk_pool_name) + expect(model.cpi).to eq(vm.cpi) + expect(model.availability_zone).to eq(az) + expect(model.vm).to be_nil + end + + context 'when DB write fails after IaaS disk creation' do + it 'deletes the IaaS disk and re-raises' do + expect(cloud).to receive(:create_disk).and_return(disk_cid) + allow(Models::DynamicDisk).to receive(:create).and_raise(Sequel::Error, 'DB failure') + expect(cloud).to receive(:delete_disk).with(disk_cid) + + expect { create_dynamic_disk_job.perform }.to raise_error(Sequel::Error, 'DB failure') + end + end + end + + context 'when disk already exists in the database' do + let!(:disk) do + FactoryBot.create( + :models_dynamic_disk, + name: disk_name, + disk_cid: disk_cid, + deployment: deployment, + disk_pool_name: disk_pool_name, + availability_zone: az, + ) + end + + it 'raises an error without calling create_disk' do + expect(cloud).not_to receive(:create_disk) + + expect { create_dynamic_disk_job.perform }.to raise_error("disk `#{disk_name}` already exists") + end + end + + context 'when metadata is provided and CPI supports set_disk_metadata' do + it 'sets disk metadata after creation' do + expected_cloud_properties = disk_cloud_properties.merge('name' => disk_name) + expect(cloud).to receive(:create_disk).with(disk_size, expected_cloud_properties, vm.cid).and_return(disk_cid) + expect(cloud).to receive(:respond_to?).with(:set_disk_metadata).and_return(true) + expect(cloud).to receive(:set_disk_metadata).with( + disk_cid, + hash_including('fake-key' => 'fake-value'), + ) + + create_dynamic_disk_job.perform + end + end + + context 'when disk_pool_name cannot be found in cloud config' do + let!(:cloud_config) do + FactoryBot.create(:models_config_cloud, content: YAML.dump( + SharedSupport::DeploymentManifestHelper.simple_cloud_config.merge( + 'disk_types' => [ + { 'name' => 'different-disk-type', + 'disk_size' => 1024, + 'cloud_properties' => {} }, + ], + ), + )) + end + + it 'raises an error before calling create_disk' do + expect(cloud).not_to receive(:create_disk) + expect { create_dynamic_disk_job.perform }.to raise_error(/Could not find disk pool by name/) + end + end + + context 'when deployment cannot be found' do + let(:create_dynamic_disk_job) do + Jobs::DynamicDisks::CreateDynamicDisk.new('unknown-deployment', az, disk_name, disk_pool_name, disk_size, metadata) + end + + it 'raises an error' do + expect { create_dynamic_disk_job.perform }.to raise_error('deployment `unknown-deployment` not found') + end + end + + context 'when no active VM exists in the requested AZ' do + let!(:vm) { FactoryBot.create(:models_vm, instance: instance, active: false) } + + it 'raises an error' do + expect { create_dynamic_disk_job.perform }.to raise_error( + "no active VM found in deployment `#{deployment.name}` in AZ `#{az}`", + ) + end + end + + context 'when VM exists in a different AZ' do + let(:other_az) { 'z2' } + let(:create_dynamic_disk_job) do + Jobs::DynamicDisks::CreateDynamicDisk.new(deployment.name, other_az, disk_name, disk_pool_name, disk_size, metadata) + end + + it 'raises an error when no VM found in requested AZ' do + expect { create_dynamic_disk_job.perform }.to raise_error( + "no active VM found in deployment `#{deployment.name}` in AZ `#{other_az}`", + ) + end + end + end + end +end