[neddp]

What is this change about?

When CloudcheckHelper#recreate_vm deletes a VM (via delete_vm_from_cloud), the LocalDnsRecord entries for that instance are not removed before the new VM is created. If recreation fails partway through, other VMs are left with a DNS record pointing to a dead IP indefinitely - the background DNS converger only re-sends the current blob, it does not fix stale records.

This change introduces a delete_dns_for_instance_safely private helper that removes the instance's DNS records and broadcasts the updated blob to all remaining VMs immediately after the old VM is deleted. It is called from both recreate_vm and the delete_vm resolution in UnresponsiveAgent, which had the same gap.

DNS cleanup failure is treated as non-fatal: at this point the VM is already gone and the operation must proceed regardless. A stale record in that case will be corrected by the next successful deploy or resurrection.

Please provide contextual information.

No prior issue or discussion. Discovered while investigating how sync_dns behaves during the resurrector lifecycle.

How should this change be described in bosh release notes?

Fixed: DNS records for a VM being resurrected are now removed immediately when the old VM is deleted, preventing other VMs from resolving a stale IP if recreation fails.

Does this PR introduce a breaking change?

No. During VM recreation there is now a short window where the instance is unresolvable (NXDOMAIN) rather than resolving to a dead IP.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

VM recreation and unresponsive-agent resolution now attempt to delete an instance’s DNS records after deleting its VM. The shared helper logs a warning and continues if DNS cleanup raises a StandardError.

Suggested reviewers: aramprice

Merge Risk: Minimal · up to e6208

DNS cleanup follows VM deletion and remains non-fatal on errors. No material merge risk was established.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: removing stale DNS records when the resurrector recreates a VM.
Description check		The description explains the problem, implementation, failure behavior, release-note wording, and breaking-change impact. It does not include a test execution list, tags, or the AI Review Feedback sec…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description explains the problem, implementation, failure behavior, release-note wording, and breaking-change impact. It does not include a test execution list, tags, or the AI Review Feedback section, but it is mostly complete and directly related to the changes.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 4 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 6d5cf652-d8af-463d-916b-c0afc99246f5

Commits

Reviewing files that changed from the base of the PR and between f9ef318 and e6208c0.

Files selected for processing: 2

• src/bosh-director/spec/unit/bosh/director/cloudcheck_helper_spec.rb
• src/bosh-director/spec/unit/bosh/director/problem_handlers/unresponsive_agent_spec.rb

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

[Copilot]

🟡 Changes recommended

Lifecycle specs leave the stale-DNS regression unprotected because they do not verify cleanup, broadcast, or non-fatal cleanup errors during VM recreation and standalone deletion.

1 open issue

• Add lifecycle coverage for DNS cleanup and non-fatal errors · New

What changed in this PR

This PR updates the Director’s VM recovery paths to remove stale DNS records after deleting a VM, preventing dead-IP lookups if recreation fails.

Changes:

• Remove instance DNS records and broadcast the updated blob before recreating a VM.
• Apply the same cleanup to the unresponsive-agent delete_vm resolution.
• Log DNS cleanup failures without aborting the operation.

File	Description
src/​bosh-director/​lib/​bosh/​director/​problem_handlers/​unresponsive_agent.rb	Adds DNS cleanup after the delete_vm resolution deletes the VM.
src/​bosh-director/​lib/​bosh/​director/​cloudcheck_helper.rb	Adds a non-fatal DNS cleanup helper and invokes it during VM recreation.

_{Review effort: Balanced}

[Copilot]

🟢 Approval recommended

No active review issues are recorded for the changes that clean up stale instance DNS during VM resurrection.

1 open issue

• Add lifecycle coverage for DNS cleanup and non-fatal errors

_{Review effort: Balanced}

[Copilot]

🟢 Approval recommended

No active review issues remain for the DNS cleanup change covering VM resurrection and deletion.

No open issues 🎉

1 resolved since last review

• Add lifecycle coverage for DNS cleanup and non-fatal errors

_{Review effort: Balanced}

[a-hassanin]

Makes sense! Thank you