Don't clear current stemcell on VM delete

[Ivaylogi98]

What

Removes the stemcellRepo.ClearCurrent() call from vm.Delete() (deployment/vm/vm.go), along with the now-unused stemcellRepo dependency it pulled onto the vm struct, NewVM/NewVMWithMetadata, NewManager, and NewManagerFactory.

Fixes #731.

Why

vm.Delete() unconditionally cleared current_stemcell_id (set it to "") whenever the old VM was torn down. Inside create-env's delete-then-recreate cycle, the intended sequence is:

1. vm.Delete() clears current_stemcell_id
2. new VM boots → cloudStemcell.PromoteAsCurrent() sets it to the new stemcell record
3. stemcellManager.DeleteUnused() reaps every record whose ID ≠ current

On the happy path, step 2 overwrites the clear from step 1, so it has no observable effect. But when the replacement VM never comes up (agent timeout, network issue, or a failure inside vmManager.Create before promote), step 2 is never reached and bosh-state.json is persisted with current_stemcell_id: "" while the stemcell record and its IaaS image remain.

On the next create-env run, FindUnused (stemcell/manager.go) treats every record as unused when the current pointer is empty (found == false), and DeleteUnused deregisters the still-in-use image (e.g. an AWS AMI). Every subsequent create_vm that references it then fails:

CPI 'create_vm' method responded with error:
CmdError{"type":"Bosh::Clouds::CloudError","message":"could not find AMI 'ami-xxxxxxxxxxxxxxxxx'","ok_to_retry":false}

This is more likely to surface on unattended pipelines that auto-retry a failed create-env.

The clear was always redundant

PromoteAsCurrent (stemcell/cloud_stemcell.go) calls repo.UpdateCurrent(id) unconditionally — it never reads the prior value — so the clear in vm.Delete() contributed nothing on the success path. It was introduced in bd573fe8 (Nov 2014) as defensive symmetry (clear on teardown, set on build), but even in that original code PromoteAsCurrent ran before DeleteUnused, so the clear only ever had an effect in the failure window, where it is purely destructive.

With it removed, a failed deploy leaves current_stemcell_id pointing at the stemcell the deployment is configured to use, so DeleteUnused leaves it alone. A genuinely superseded stemcell is still reaped — but only after a successful deploy where PromoteAsCurrent moves the pointer to a newer record.

Scope / not affected

• delete-env (deployment.Delete()) deletes stemcells through an explicit cloudStemcell.Delete() step, independent of vm.Delete() — unchanged.
• bosh delete-vm is a director API command (director.Deployment.DeleteVM) and never touches the local stemcell repo — unchanged.
• stemcell/manager.go and config/stemcell_repo.go (ClearCurrent is still used by cloudStemcell.Delete()) are left as-is. The VM.Delete() interface signature is unchanged.

Testing

• Dropped the two clears current stemcell in the stemcell repo cases in deployment/vm/vm_test.go (they asserted the removed behavior).
• go build ./... and go test ./deployment/vm/... ./stemcell/... ./config/... pass.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 03003354-173e-474c-82a5-2f3abff6cbb9

📥 Commits

Reviewing files that changed from the base of the PR and between f232dd0 and 620b91a.

📒 Files selected for processing (6)

• cmd/env_factory.go
• deployment/vm/manager.go
• deployment/vm/manager_factory.go
• deployment/vm/manager_test.go
• deployment/vm/vm.go
• deployment/vm/vm_test.go

💤 Files with no reviewable changes (5)

• deployment/vm/manager_test.go
• deployment/vm/vm.go
• deployment/vm/vm_test.go
• deployment/vm/manager.go
• deployment/vm/manager_factory.go

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

Walkthrough

The VM and manager constructors no longer accept or store stemcellRepo. Manager factory wiring and VM construction call sites were updated. vm.Delete no longer clears the current stemcell repository. Tests were updated to match the new constructors and deletion behavior.

Priority: ⬆️ High

Severity of issue fixed: High

Merge Risk: ⚪ Minimal · up to 620b9

The change preserves the current stemcell across failed VM recreation attempts and is ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly states the primary change: VM deletion no longer clears the current stemcell.
Description check	✅ Passed	The description directly explains the removed ClearCurrent call, its failure-mode impact, affected dependencies, scope, and testing.
Linked Issues check	✅ Passed	The change satisfies issue #731. vm.Delete() no longer calls stemcellRepo.ClearCurrent(), so a failed VM recreation does not erase current_stemcell_id. The VM and manager constructors no longer …
Out of Scope Changes check	✅ Passed	The changes stay within issue #731. They remove the destructive stemcell clearing call and its now-unused dependency from related constructors, factories, and tests. No unrelated behavior changes are …

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-wrongfully-deregistered-ami-images

---

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

_{Comment @coderabbitai help to get the list of available commands.}