From f9ef3189f2908633e9002b316b5d6990f5318722 Mon Sep 17 00:00:00 2001 From: Nayden Petrov Date: Tue, 3 Nov 2026 16:31:06 +0200 Subject: [PATCH 1/2] Fix stale DNS records when resurrector recreates a VM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When recreate_vm deletes a VM, LocalDnsRecord entries for that instance were not removed before the new VM is created. If recreation failed partway through, other VMs were left with a DNS record pointing to a dead IP indefinitely. Call delete_dns_for_instance immediately after delete_vm_from_cloud, consistent with how InstanceDeleter handles permanent instance deletion. DNS cleanup failure is treated as non-fatal since the VM is already gone and recreation must proceed regardless. This was an oversight from when LocalDnsManager was introduced — InstanceDeleter and the errand manager were updated at the time, but CloudcheckHelper was not. --- src/bosh-director/lib/bosh/director/cloudcheck_helper.rb | 7 +++++++ .../bosh/director/problem_handlers/unresponsive_agent.rb | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/bosh-director/lib/bosh/director/cloudcheck_helper.rb b/src/bosh-director/lib/bosh/director/cloudcheck_helper.rb index fbb4ea32979..05484c19857 100644 --- a/src/bosh-director/lib/bosh/director/cloudcheck_helper.rb +++ b/src/bosh-director/lib/bosh/director/cloudcheck_helper.rb @@ -54,6 +54,7 @@ def recreate_vm_without_wait(instance_model) def recreate_vm(instance_model, wait_for_running = true) @logger.debug("Recreating Vm: #{instance_model})") delete_vm_from_cloud(instance_model) + delete_dns_for_instance_safely(instance_model) deployment_model = instance_model.deployment factory = Bosh::Director::DeploymentPlan::PlannerFactory.create(@logger) @@ -167,6 +168,12 @@ def vm_creator(template_cache, dns_encoder, link_provider_intents) @vm_creator ||= VmCreator.new(@logger, template_cache, dns_encoder, agent_broadcaster, link_provider_intents) end + def delete_dns_for_instance_safely(instance_model) + LocalDnsManager.create(Config.root_domain, @logger).delete_dns_for_instance(instance_model) + rescue StandardError => e + @logger.warn("Failed to clean up DNS records for #{instance_model}, continuing: #{e.message}") + end + def validate_spec(spec) handler_error('Unable to look up VM apply spec') unless spec diff --git a/src/bosh-director/lib/bosh/director/problem_handlers/unresponsive_agent.rb b/src/bosh-director/lib/bosh/director/problem_handlers/unresponsive_agent.rb index d3303587195..fdc13569ea8 100644 --- a/src/bosh-director/lib/bosh/director/problem_handlers/unresponsive_agent.rb +++ b/src/bosh-director/lib/bosh/director/problem_handlers/unresponsive_agent.rb @@ -57,7 +57,7 @@ def instance_group resolution :delete_vm do plan { 'Delete VM' } - action { validate; delete_vm_from_cloud(@instance) } + action { validate; delete_vm_from_cloud(@instance); delete_dns_for_instance_safely(@instance) } end resolution :delete_vm_reference do From e6208c0fa7f7e4a99985369e8a9a641a5aaed62f Mon Sep 17 00:00:00 2001 From: Nayden Petrov Date: Wed, 4 Nov 2026 09:14:42 +0200 Subject: [PATCH 2/2] Add unit tests for DNS cleanup in cloudcheck_helper and unresponsive_agent --- .../bosh/director/cloudcheck_helper_spec.rb | 35 +++++++++++++++++++ .../unresponsive_agent_spec.rb | 25 +++++++++++++ 2 files changed, 60 insertions(+) diff --git a/src/bosh-director/spec/unit/bosh/director/cloudcheck_helper_spec.rb b/src/bosh-director/spec/unit/bosh/director/cloudcheck_helper_spec.rb index edbd57123f4..ec01aafd999 100644 --- a/src/bosh-director/spec/unit/bosh/director/cloudcheck_helper_spec.rb +++ b/src/bosh-director/spec/unit/bosh/director/cloudcheck_helper_spec.rb @@ -326,6 +326,41 @@ def expect_vm_gets_created end end end + + context 'DNS cleanup' do + let(:local_dns_manager) { instance_double(LocalDnsManager) } + + before do + fake_job_context + allow(Config).to receive(:root_domain).and_return('bosh') + allow(LocalDnsManager).to receive(:create).and_return(local_dns_manager) + allow(local_dns_manager).to receive(:delete_dns_for_instance) + allow(local_dns_manager).to receive(:update_dns_record_for_instance) + allow(vm_deleter).to receive(:delete_for_instance) + allow(vm_creator).to receive(:create_for_instance_plan) + allow(rendered_templates_persister).to receive(:persist) + allow(agent_client).to receive(:apply) + allow(agent_client).to receive(:run_script) + allow(agent_client).to receive(:start) + allow(DnsNameGenerator).to receive(:dns_record_name).and_return('some.dns.record') + allow(template_cache).to receive(:clean_cache!) + end + + it 'deletes DNS records before recreating the VM' do + calls = [] + allow(vm_deleter).to receive(:delete_for_instance) { calls << :delete_vm } + allow(local_dns_manager).to receive(:delete_dns_for_instance) { calls << :delete_dns } + allow(vm_creator).to receive(:create_for_instance_plan) { calls << :create_vm } + test_problem_handler.apply_resolution(:recreate_vm) + expect(calls).to eq(%i[delete_vm delete_dns create_vm]) + end + + it 'continues recreation when DNS cleanup raises' do + allow(local_dns_manager).to receive(:delete_dns_for_instance).and_raise(StandardError, 'dns error') + expect(vm_creator).to receive(:create_for_instance_plan) + test_problem_handler.apply_resolution(:recreate_vm) + end + end end end end diff --git a/src/bosh-director/spec/unit/bosh/director/problem_handlers/unresponsive_agent_spec.rb b/src/bosh-director/spec/unit/bosh/director/problem_handlers/unresponsive_agent_spec.rb index 76da589dfb2..859db0f28a6 100644 --- a/src/bosh-director/spec/unit/bosh/director/problem_handlers/unresponsive_agent_spec.rb +++ b/src/bosh-director/spec/unit/bosh/director/problem_handlers/unresponsive_agent_spec.rb @@ -332,6 +332,31 @@ def expect_vm_to_be_created Models::Vm.where(cid: 'vm-cid').count }.from(1).to(0) end + + context 'DNS cleanup' do + let(:local_dns_manager) { instance_double(LocalDnsManager) } + + before do + allow(agent).to receive(:ping).and_raise(RpcTimeout) + allow(cloud).to receive(:delete_vm).with('vm-cid') + allow(Config).to receive(:root_domain).and_return('bosh') + allow(LocalDnsManager).to receive(:create).and_return(local_dns_manager) + allow(local_dns_manager).to receive(:delete_dns_for_instance) + end + + it 'cleans up DNS records after deleting the VM' do + calls = [] + allow(cloud).to receive(:delete_vm) { calls << :delete_vm } + allow(local_dns_manager).to receive(:delete_dns_for_instance) { calls << :delete_dns } + handler.apply_resolution(:delete_vm) + expect(calls).to eq(%i[delete_vm delete_dns]) + end + + it 'continues when DNS cleanup raises' do + allow(local_dns_manager).to receive(:delete_dns_for_instance).and_raise(StandardError, 'dns error') + expect { handler.apply_resolution(:delete_vm) }.not_to raise_error + end + end end describe 'delete_vm_reference resolution' do