From 620b91a2eca36934d130ac8749d97378317dc02a Mon Sep 17 00:00:00 2001 From: I761617 Date: Thu, 10 Sep 2026 16:14:06 +0300 Subject: [PATCH 1/3] Don't clear current stemcell on VM delete for delete-env path --- cmd/env_factory.go | 2 +- deployment/vm/manager.go | 5 ----- deployment/vm/manager_factory.go | 4 ---- deployment/vm/manager_test.go | 2 -- deployment/vm/vm.go | 10 ---------- deployment/vm/vm_test.go | 17 ----------------- 6 files changed, 1 insertion(+), 39 deletions(-) diff --git a/cmd/env_factory.go b/cmd/env_factory.go index 5b143e38d..29d4f1a1e 100644 --- a/cmd/env_factory.go +++ b/cmd/env_factory.go @@ -138,7 +138,7 @@ func NewEnvFactory( f.stemcellManagerFactory = bistemcell.NewManagerFactory(stemcellRepo) f.vmManagerFactory = bivm.NewManagerFactory( - vmRepo, stemcellRepo, diskDeployer, deps.UUIDGen, deps.FS, deps.Logger) + vmRepo, diskDeployer, deps.UUIDGen, deps.FS, deps.Logger) deploymentRepo := biconfig.NewDeploymentRepo(f.deploymentStateService) releaseRepo := biconfig.NewReleaseRepo(f.deploymentStateService, deps.UUIDGen) diff --git a/deployment/vm/manager.go b/deployment/vm/manager.go index 88c1d7263..e49ab1fa2 100644 --- a/deployment/vm/manager.go +++ b/deployment/vm/manager.go @@ -28,7 +28,6 @@ type Manager interface { type manager struct { vmRepo biconfig.VMRepo - stemcellRepo biconfig.StemcellRepo diskDeployer DiskDeployer agentClient biagentclient.AgentClient cloud bicloud.Cloud @@ -41,7 +40,6 @@ type manager struct { func NewManager( vmRepo biconfig.VMRepo, - stemcellRepo biconfig.StemcellRepo, diskDeployer DiskDeployer, agentClient biagentclient.AgentClient, cloud bicloud.Cloud, @@ -54,7 +52,6 @@ func NewManager( cloud: cloud, agentClient: agentClient, vmRepo: vmRepo, - stemcellRepo: stemcellRepo, diskDeployer: diskDeployer, uuidGenerator: uuidGenerator, fs: fs, @@ -77,7 +74,6 @@ func (m *manager) FindCurrent() (VM, bool, error) { vm := NewVM( vmCID, m.vmRepo, - m.stemcellRepo, m.diskDeployer, m.agentClient, m.cloud, @@ -149,7 +145,6 @@ func (m *manager) Create(stemcell bistemcell.CloudStemcell, deploymentManifest b vm := NewVMWithMetadata( cid, m.vmRepo, - m.stemcellRepo, m.diskDeployer, m.agentClient, m.cloud, diff --git a/deployment/vm/manager_factory.go b/deployment/vm/manager_factory.go index 432a23ba3..788379cd8 100644 --- a/deployment/vm/manager_factory.go +++ b/deployment/vm/manager_factory.go @@ -19,7 +19,6 @@ type ManagerFactory interface { type managerFactory struct { vmRepo biconfig.VMRepo - stemcellRepo biconfig.StemcellRepo diskDeployer DiskDeployer uuidGenerator boshuuid.Generator fs boshsys.FileSystem @@ -28,7 +27,6 @@ type managerFactory struct { func NewManagerFactory( vmRepo biconfig.VMRepo, - stemcellRepo biconfig.StemcellRepo, diskDeployer DiskDeployer, uuidGenerator boshuuid.Generator, fs boshsys.FileSystem, @@ -36,7 +34,6 @@ func NewManagerFactory( ) ManagerFactory { return &managerFactory{ vmRepo: vmRepo, - stemcellRepo: stemcellRepo, diskDeployer: diskDeployer, uuidGenerator: uuidGenerator, fs: fs, @@ -47,7 +44,6 @@ func NewManagerFactory( func (f *managerFactory) NewManager(cloud bicloud.Cloud, agentClient biagentclient.AgentClient) Manager { return NewManager( f.vmRepo, - f.stemcellRepo, f.diskDeployer, agentClient, cloud, diff --git a/deployment/vm/manager_test.go b/deployment/vm/manager_test.go index 3f37e5c63..5dee55b5b 100644 --- a/deployment/vm/manager_test.go +++ b/deployment/vm/manager_test.go @@ -60,7 +60,6 @@ var _ = Describe("Manager", func() { manager = NewManager( fakeVMRepo, - stemcellRepo, fakeDiskDeployer, fakeAgentClient, fakeCloud, @@ -130,7 +129,6 @@ var _ = Describe("Manager", func() { expectedVM := NewVMWithMetadata( "fake-vm-cid", fakeVMRepo, - stemcellRepo, fakeDiskDeployer, fakeAgentClient, fakeCloud, diff --git a/deployment/vm/vm.go b/deployment/vm/vm.go index bdd3f0ba2..281e388c6 100644 --- a/deployment/vm/vm.go +++ b/deployment/vm/vm.go @@ -53,7 +53,6 @@ type VM interface { type vm struct { cid string vmRepo biconfig.VMRepo - stemcellRepo biconfig.StemcellRepo diskDeployer DiskDeployer agentClient biagentclient.AgentClient cloud bicloud.Cloud @@ -67,7 +66,6 @@ type vm struct { func NewVM( cid string, vmRepo biconfig.VMRepo, - stemcellRepo biconfig.StemcellRepo, diskDeployer DiskDeployer, agentClient biagentclient.AgentClient, cloud bicloud.Cloud, @@ -78,7 +76,6 @@ func NewVM( return &vm{ cid: cid, vmRepo: vmRepo, - stemcellRepo: stemcellRepo, diskDeployer: diskDeployer, agentClient: agentClient, cloud: cloud, @@ -92,7 +89,6 @@ func NewVM( func NewVMWithMetadata( cid string, vmRepo biconfig.VMRepo, - stemcellRepo biconfig.StemcellRepo, diskDeployer DiskDeployer, agentClient biagentclient.AgentClient, cloud bicloud.Cloud, @@ -104,7 +100,6 @@ func NewVMWithMetadata( return &vm{ cid: cid, vmRepo: vmRepo, - stemcellRepo: stemcellRepo, diskDeployer: diskDeployer, agentClient: agentClient, cloud: cloud, @@ -302,11 +297,6 @@ func (vm *vm) Delete() error { return bosherr.WrapError(err, "Deleting vm from vm repo") } - err = vm.stemcellRepo.ClearCurrent() - if err != nil { - return bosherr.WrapError(err, "Clearing current stemcell from stemcell repo") - } - // returns bicloud.Error only if it is a VMNotFoundError return deleteErr } diff --git a/deployment/vm/vm_test.go b/deployment/vm/vm_test.go index 0bccad59c..22e42cadb 100644 --- a/deployment/vm/vm_test.go +++ b/deployment/vm/vm_test.go @@ -29,7 +29,6 @@ var _ = Describe("VM", func() { var ( vm VM fakeVMRepo *configfakes.FakeVMRepo - fakeStemcellRepo *configfakes.FakeStemcellRepo fakeDiskDeployer *vmfakes.FakeDiskDeployer fakeAgentClient *fakebiagentclient.FakeAgentClient fakeCloud *cloudfakes.FakeCloud @@ -61,12 +60,10 @@ var _ = Describe("VM", func() { fs = fakesys.NewFakeFileSystem() fakeCloud = &cloudfakes.FakeCloud{} fakeVMRepo = &configfakes.FakeVMRepo{} - fakeStemcellRepo = &configfakes.FakeStemcellRepo{} fakeDiskDeployer = &vmfakes.FakeDiskDeployer{} vm = NewVM( "fake-vm-cid", fakeVMRepo, - fakeStemcellRepo, fakeDiskDeployer, fakeAgentClient, fakeCloud, @@ -285,7 +282,6 @@ var _ = Describe("VM", func() { vm = NewVMWithMetadata( "fake-vm-cid", fakeVMRepo, - fakeStemcellRepo, fakeDiskDeployer, fakeAgentClient, fakeCloud, @@ -588,12 +584,6 @@ var _ = Describe("VM", func() { Expect(fakeVMRepo.ClearCurrentCallCount()).To(Equal(1)) }) - It("clears current stemcell in the stemcell repo", func() { - err := vm.Delete() - Expect(err).ToNot(HaveOccurred()) - Expect(fakeVMRepo.ClearCurrentCallCount()).To(Equal(1)) - }) - Context("when deleting vm in the cloud fails", func() { BeforeEach(func() { fakeCloud.DeleteVMReturns(errors.New("fake-delete-vm-error")) @@ -629,13 +619,6 @@ var _ = Describe("VM", func() { Expect(err).To(Equal(deleteErr)) Expect(fakeVMRepo.ClearCurrentCallCount()).To(Equal(1)) }) - - It("clears current stemcell in the stemcell repo", func() { - err := vm.Delete() - Expect(err).To(HaveOccurred()) - Expect(err).To(Equal(deleteErr)) - Expect(fakeVMRepo.ClearCurrentCallCount()).To(Equal(1)) - }) }) }) From 7f106524800467267195111fe99af9fcdf90b3e2 Mon Sep 17 00:00:00 2001 From: I761617 Date: Mon, 14 Sep 2026 15:11:18 +0300 Subject: [PATCH 2/3] Fix three missed NewManagerFactory call sites and add regression test --- deployment/deployment_test.go | 2 +- deployment/manager_test.go | 2 +- deployment/vm/vm_test.go | 21 +++++++++++++++++++++ integration/create_env_test.go | 2 +- 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/deployment/deployment_test.go b/deployment/deployment_test.go index 2ee900eba..7937b6db4 100644 --- a/deployment/deployment_test.go +++ b/deployment/deployment_test.go @@ -170,7 +170,7 @@ var _ = Describe("Deployment", func() { diskManagerFactory := bidisk.NewManagerFactory(diskRepo, logger) diskDeployer := bivm.NewDiskDeployer(diskManagerFactory, diskRepo, logger, false) - vmManagerFactory := bivm.NewManagerFactory(vmRepo, stemcellRepo, diskDeployer, fakeUUIDGenerator, fs, logger) + vmManagerFactory := bivm.NewManagerFactory(vmRepo, diskDeployer, fakeUUIDGenerator, fs, logger) sshTunnelFactory := bisshtunnel.NewFactory(logger) mockStateBuilderFactory = &statefakes.FakeBuilderFactory{} diff --git a/deployment/manager_test.go b/deployment/manager_test.go index 44fc35c3d..b81e30f36 100644 --- a/deployment/manager_test.go +++ b/deployment/manager_test.go @@ -177,7 +177,7 @@ var _ = Describe("Manager", func() { diskManagerFactory := bidisk.NewManagerFactory(diskRepo, logger) diskDeployer := bivm.NewDiskDeployer(diskManagerFactory, diskRepo, logger, false) - vmManagerFactory := bivm.NewManagerFactory(vmRepo, stemcellRepo, diskDeployer, fakeUUIDGenerator, fs, logger) + vmManagerFactory := bivm.NewManagerFactory(vmRepo, diskDeployer, fakeUUIDGenerator, fs, logger) sshTunnelFactory := bisshtunnel.NewFactory(logger) mockStateBuilderFactory = &statefakes.FakeBuilderFactory{} diff --git a/deployment/vm/vm_test.go b/deployment/vm/vm_test.go index 22e42cadb..826aa83bb 100644 --- a/deployment/vm/vm_test.go +++ b/deployment/vm/vm_test.go @@ -10,6 +10,7 @@ import ( "github.com/cloudfoundry/bosh-utils/logger/loggerfakes" biproperty "github.com/cloudfoundry/bosh-utils/property" fakesys "github.com/cloudfoundry/bosh-utils/system/fakes" + fakeuuid "github.com/cloudfoundry/bosh-utils/uuid/fakes" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -584,6 +585,26 @@ var _ = Describe("VM", func() { Expect(fakeVMRepo.ClearCurrentCallCount()).To(Equal(1)) }) + It("does not clear the current stemcell pointer (regression: issue #731)", func() { + stemcellFS := fakesys.NewFakeFileSystem() + uuidGen := &fakeuuid.FakeGenerator{} + deploymentStateService := biconfig.NewFileSystemDeploymentStateService(stemcellFS, uuidGen, logger, "/fake/state.json") + stemcellRepo := biconfig.NewStemcellRepo(deploymentStateService, uuidGen) + + record, err := stemcellRepo.Save("fake-stemcell-name", "fake-stemcell-version", "fake-stemcell-cid", 1) + Expect(err).ToNot(HaveOccurred()) + err = stemcellRepo.UpdateCurrent(record.ID) + Expect(err).ToNot(HaveOccurred()) + + err = vm.Delete() + Expect(err).ToNot(HaveOccurred()) + + currentRecord, found, err := stemcellRepo.FindCurrent() + Expect(err).ToNot(HaveOccurred()) + Expect(found).To(BeTrue(), "vm.Delete() must not clear current_stemcell_id") + Expect(currentRecord.CID).To(Equal("fake-stemcell-cid")) + }) + Context("when deleting vm in the cloud fails", func() { BeforeEach(func() { fakeCloud.DeleteVMReturns(errors.New("fake-delete-vm-error")) diff --git a/integration/create_env_test.go b/integration/create_env_test.go index a68f31483..54fe5d969 100644 --- a/integration/create_env_test.go +++ b/integration/create_env_test.go @@ -414,7 +414,7 @@ cloud_provider: stemcellManagerFactory = bistemcell.NewManagerFactory(stemcellRepo) diskManagerFactory = bidisk.NewManagerFactory(diskRepo, logger) diskDeployer = bivm.NewDiskDeployer(diskManagerFactory, diskRepo, logger, false) - vmManagerFactory = bivm.NewManagerFactory(vmRepo, stemcellRepo, diskDeployer, fakeAgentIDGenerator, fs, logger) + vmManagerFactory = bivm.NewManagerFactory(vmRepo, diskDeployer, fakeAgentIDGenerator, fs, logger) deployer := bidepl.NewDeployer( vmManagerFactory, instanceManagerFactory, From a3f57da7082b3158c2914b22023f7169bfcf60dd Mon Sep 17 00:00:00 2001 From: I761617 Date: Mon, 14 Sep 2026 15:55:44 +0300 Subject: [PATCH 3/3] Fix regression test: share DeploymentStateService between vmRepo and stemcellRepo --- deployment/vm/vm_test.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/deployment/vm/vm_test.go b/deployment/vm/vm_test.go index 826aa83bb..90650a28a 100644 --- a/deployment/vm/vm_test.go +++ b/deployment/vm/vm_test.go @@ -586,17 +586,27 @@ var _ = Describe("VM", func() { }) It("does not clear the current stemcell pointer (regression: issue #731)", func() { - stemcellFS := fakesys.NewFakeFileSystem() uuidGen := &fakeuuid.FakeGenerator{} - deploymentStateService := biconfig.NewFileSystemDeploymentStateService(stemcellFS, uuidGen, logger, "/fake/state.json") - stemcellRepo := biconfig.NewStemcellRepo(deploymentStateService, uuidGen) + sharedStateService := biconfig.NewFileSystemDeploymentStateService(fs, uuidGen, logger, "/fake/state.json") + realVMRepo := biconfig.NewVMRepo(sharedStateService) + stemcellRepo := biconfig.NewStemcellRepo(sharedStateService, uuidGen) record, err := stemcellRepo.Save("fake-stemcell-name", "fake-stemcell-version", "fake-stemcell-cid", 1) Expect(err).ToNot(HaveOccurred()) err = stemcellRepo.UpdateCurrent(record.ID) Expect(err).ToNot(HaveOccurred()) - err = vm.Delete() + realVM := NewVM( + "fake-vm-cid", + realVMRepo, + fakeDiskDeployer, + fakeAgentClient, + fakeCloud, + timeService, + fs, + logger, + ) + err = realVM.Delete() Expect(err).ToNot(HaveOccurred()) currentRecord, found, err := stemcellRepo.FindCurrent()