Add --fix to create-env to force a stemcell re-upload

cmd/create_env.go

@@ -24,5 +24,10 @@ func (c *CreateEnvCmd) Run(stage boshui.Stage, opts CreateEnvOpts) error {
 
 	depPreparer := c.envProvider(opts.Args.Manifest.Path, opts.StatePath, opts.VarFlags.AsVariables(), opts.OpsFlags.AsOp()) //nolint:staticcheck
 
-	return depPreparer.PrepareDeployment(stage, opts.Recreate, opts.RecreatePersistentDisks, opts.SkipDrain)
+	return depPreparer.PrepareDeployment(stage, DeploymentOptions{
+		Recreate:                opts.Recreate,
+		RecreatePersistentDisks: opts.RecreatePersistentDisks,
+		FixStemcell:             opts.Fix,
+		SkipDrain:               opts.SkipDrain,
+	})
 }

cmd/create_env_test.go

@@ -585,6 +585,8 @@ var _ = Describe("CreateEnvCmd", func() {
 			err := command.Run(fakeStage, defaultCreateEnvOpts)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(mockStemcellManager.UploadCallCount()).To(Equal(1))
+			_, _, gotFix := mockStemcellManager.UploadArgsForCall(0)
+			Expect(gotFix).To(BeFalse())
 		})
 
 		It("adds a new 'deploying' event logger stage", func() {
@@ -688,6 +690,25 @@ var _ = Describe("CreateEnvCmd", func() {
 				Expect(err).NotTo(HaveOccurred())
 				Expect(mockDeployer.DeployCallCount()).To(Equal(1))
 			})
+
+			It("deploys if `fix` flag is specified, even with no manifest or release changes", func() {
+				defaultCreateEnvOpts.Fix = true
+
+				err := command.Run(fakeStage, defaultCreateEnvOpts)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(mockDeployer.DeployCallCount()).To(Equal(1))
+			})
+
+			It("passes `fix` through to the stemcell upload", func() {
+				defaultCreateEnvOpts.Fix = true
+
+				err := command.Run(fakeStage, defaultCreateEnvOpts)
+				Expect(err).NotTo(HaveOccurred())
+
+				Expect(mockStemcellManager.UploadCallCount()).To(Equal(1))
+				_, _, gotFix := mockStemcellManager.UploadArgsForCall(0)
+				Expect(gotFix).To(BeTrue())
+			})
 		})
 
 		Context("when parsing the cpi deployment manifest fails", func() {

cmd/deployment_preparer.go

@@ -100,7 +100,20 @@ type DeploymentPreparer struct {
 	targetProvider                          biinstall.TargetProvider
 }
 
-func (c *DeploymentPreparer) PrepareDeployment(stage biui.Stage, recreate bool, recreatePersistentDisks bool, skipDrain bool) (err error) {
+// DeploymentOptions carries the per-run switches for PrepareDeployment.
+type DeploymentOptions struct {
+	Recreate                bool
+	RecreatePersistentDisks bool
+	FixStemcell             bool
+	SkipDrain               bool
+}
+
+func (c *DeploymentPreparer) PrepareDeployment(stage biui.Stage, opts DeploymentOptions) (err error) {
+	recreate := opts.Recreate
+	recreatePersistentDisks := opts.RecreatePersistentDisks
+	fix := opts.FixStemcell
+	skipDrain := opts.SkipDrain
+
 	c.ui.BeginLinef("Deployment state: '%s'\n", c.deploymentStateService.Path())
 
 	if !c.deploymentStateService.Exists() {
@@ -183,7 +196,7 @@ func (c *DeploymentPreparer) PrepareDeployment(stage biui.Stage, recreate bool,
 		return bosherr.WrapError(err, "Checking if deployment has changed")
 	}
 
-	if isDeployed && !recreate && !recreatePersistentDisks {
+	if isDeployed && !recreate && !recreatePersistentDisks && !fix {
 		c.ui.BeginLinef("No deployment, stemcell or release changes. Skipping deploy.\n")
 		return nil
 	}
@@ -204,6 +217,7 @@ func (c *DeploymentPreparer) PrepareDeployment(stage biui.Stage, recreate bool,
 				deploymentManifest,
 				manifestSHA,
 				skipDrain,
+				fix,
 				stage,
 				cloud,
 			)
@@ -234,12 +248,13 @@ func (c *DeploymentPreparer) deploy(
 	deploymentManifest bideplmanifest.Manifest,
 	manifestSHA string,
 	skipDrain bool,
+	fix bool,
 	stage biui.Stage,
 	cloud bicloud.Cloud,
 ) (err error) {
 	stemcellManager := c.stemcellManagerFactory.NewManager(cloud)
 
-	cloudStemcell, err := stemcellManager.Upload(extractedStemcell, stage)
+	cloudStemcell, err := stemcellManager.Upload(extractedStemcell, stage, fix)
 	if err != nil {
 		return err
 	}

cmd/opts/opts.go

@@ -197,6 +197,7 @@ type CreateEnvOpts struct {
 	StatePath               string `long:"state" value-name:"PATH" description:"State file path"`
 	Recreate                bool   `long:"recreate" description:"Recreate VM in deployment"`
 	RecreatePersistentDisks bool   `long:"recreate-persistent-disks" description:"Recreate persistent disks in the deployment"`
+	Fix                     bool   `long:"fix" description:"Forces re-upload of the stemcell; any existing stemcell image is orphaned and the VM is recreated"`
 	PackageDir              string `long:"package-dir" value-name:"DIR" description:"Package cache location override"`
 	cmd
 }

cmd/opts/opts_test.go

@@ -868,6 +868,12 @@ var _ = Describe("Opts", func() {
 				`long:"skip-drain" description:"Skip running drain and pre-stop scripts"`,
 			))
 		})
+
+		It("has --fix", func() {
+			Expect(getStructTagForName("Fix", opts)).To(Equal(
+				`long:"fix" description:"Forces re-upload of the stemcell; any existing stemcell image is orphaned and the VM is recreated"`,
+			))
+		})
 	})
 
 	Describe("CreateEnvArgs", func() {

config/stemcell_repo.go

@@ -13,6 +13,7 @@ type StemcellRepo interface // StemcellRepo persists stemcells metadata
 	FindCurrent() (StemcellRecord, bool, error)
 	ClearCurrent() error
 	Save(name, version, cid string, apiVersion int) (StemcellRecord, error)
+	SaveOrUpdate(name, version, cid string, apiVersion int) (StemcellRecord, error)
 	Find(name, version string) (StemcellRecord, bool, error)
 	All() ([]StemcellRecord, error)
 	Delete(StemcellRecord) error
@@ -68,6 +69,47 @@ func (r stemcellRepo) Save(name, version, cid string, apiVersion int) (StemcellR
 	return stemcellRecord, err
 }
 
+// SaveOrUpdate replaces any record with the same name and version instead of
+// rejecting it as a duplicate, repointing CurrentStemcellID at the replacement
+// in the same write. An empty CurrentStemcellID makes FindUnused treat every
+// stemcell as unused, which on AWS deregisters live AMIs (#731).
+func (r stemcellRepo) SaveOrUpdate(name, version, cid string, apiVersion int) (StemcellRecord, error) {
+	stemcellRecord := StemcellRecord{}
+
+	err := r.updateConfig(func(config *DeploymentState) error {
+		newRecord := StemcellRecord{
+			Name:       name,
+			Version:    version,
+			CID:        cid,
+			ApiVersion: apiVersion,
+		}
+
+		var err error
+		newRecord.ID, err = r.uuidGenerator.Generate()
+		if err != nil {
+			return bosherr.WrapError(err, "Generating stemcell id")
+		}
+
+		keptRecords := []StemcellRecord{}
+		for _, oldRecord := range config.Stemcells {
+			if oldRecord.Name == name && oldRecord.Version == version {
+				if config.CurrentStemcellID == oldRecord.ID {
+					config.CurrentStemcellID = newRecord.ID
+				}
+				continue
+			}
+			keptRecords = append(keptRecords, oldRecord)
+		}
+
+		config.Stemcells = append(keptRecords, newRecord)
+		stemcellRecord = newRecord
+
+		return nil
+	})
+
+	return stemcellRecord, err
+}
+
 func (r stemcellRepo) Find(name, version string) (StemcellRecord, bool, error) {
 	_, records, err := r.load()
 	if err != nil {

config/stemcell_repo_test.go

@@ -29,6 +29,77 @@ var _ = Describe("StemcellRepo", func() {
 		repo = NewStemcellRepo(deploymentStateService, fakeUUIDGenerator)
 	})
 
+	Describe("SaveOrUpdate", func() {
+		It("saves a new record when none matches", func() {
+			fakeUUIDGenerator.GeneratedUUID = "fake-uuid-1"
+			record, err := repo.SaveOrUpdate("fake-name", "fake-version", "fake-cid", apiVersion)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(record.CID).To(Equal("fake-cid"))
+
+			records, err := repo.All()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(records).To(HaveLen(1))
+		})
+
+		It("replaces an existing record with the same name and version", func() {
+			fakeUUIDGenerator.GeneratedUUID = "fake-uuid-1"
+			_, err := repo.SaveOrUpdate("fake-name", "fake-version", "old-cid", apiVersion)
+			Expect(err).ToNot(HaveOccurred())
+
+			fakeUUIDGenerator.GeneratedUUID = "fake-uuid-2"
+			_, err = repo.SaveOrUpdate("fake-name", "fake-version", "new-cid", apiVersion)
+			Expect(err).ToNot(HaveOccurred())
+
+			records, err := repo.All()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(records).To(HaveLen(1))
+			Expect(records[0].CID).To(Equal("new-cid"))
+		})
+
+		It("does not reject a duplicate name and version the way Save does", func() {
+			fakeUUIDGenerator.GeneratedUUID = "fake-uuid-1"
+			_, err := repo.Save("fake-name", "fake-version", "old-cid", apiVersion)
+			Expect(err).ToNot(HaveOccurred())
+
+			fakeUUIDGenerator.GeneratedUUID = "fake-uuid-2"
+			_, err = repo.SaveOrUpdate("fake-name", "fake-version", "new-cid", apiVersion)
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		It("repoints CurrentStemcellID at the replacement rather than leaving it empty", func() {
+			fakeUUIDGenerator.GeneratedUUID = "fake-uuid-1"
+			oldRecord, err := repo.SaveOrUpdate("fake-name", "fake-version", "old-cid", apiVersion)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(repo.UpdateCurrent(oldRecord.ID)).To(Succeed())
+
+			fakeUUIDGenerator.GeneratedUUID = "fake-uuid-2"
+			newRecord, err := repo.SaveOrUpdate("fake-name", "fake-version", "new-cid", apiVersion)
+			Expect(err).ToNot(HaveOccurred())
+
+			current, found, err := repo.FindCurrent()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(found).To(BeTrue(), "CurrentStemcellID must never be left empty")
+			Expect(current.ID).To(Equal(newRecord.ID))
+			Expect(current.CID).To(Equal("new-cid"))
+		})
+
+		It("leaves CurrentStemcellID alone when it points at an unrelated record", func() {
+			fakeUUIDGenerator.GeneratedUUID = "other-uuid"
+			otherRecord, err := repo.Save("other-name", "other-version", "other-cid", apiVersion)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(repo.UpdateCurrent(otherRecord.ID)).To(Succeed())
+
+			fakeUUIDGenerator.GeneratedUUID = "fake-uuid-1"
+			_, err = repo.SaveOrUpdate("fake-name", "fake-version", "new-cid", apiVersion)
+			Expect(err).ToNot(HaveOccurred())
+
+			current, found, err := repo.FindCurrent()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(found).To(BeTrue())
+			Expect(current.ID).To(Equal(otherRecord.ID))
+		})
+	})
+
 	Describe("Save", func() {
 		It("saves the stemcell record using the config service", func() {
 			_, err := repo.Save("fake-name", "fake-version", "fake-cid", apiVersion)