Skip to content

Commit

Permalink
Merge pull request cloudfoundry#664 from cloudfoundry/create-env-pass…
Browse files Browse the repository at this point in the history
…ed-disk-cids

Pass existing disk CIDs to CPI in create-env command
  • Loading branch information
rkoster authored Oct 3, 2024
2 parents 93e77a9 + 4aa96a7 commit 1a367b9
Show file tree
Hide file tree
Showing 16 changed files with 148 additions and 79 deletions.
11 changes: 6 additions & 5 deletions cloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type Cloud interface {
agentID string,
stemcellCID string,
cloudProperties biproperty.Map,
diskCIDS []string,
networksInterfaces map[string]biproperty.Map,
env biproperty.Map,
) (vmCID string, err error)
Expand Down Expand Up @@ -153,14 +154,14 @@ func (c cloud) CreateVM(
agentID string,
stemcellCID string,
cloudProperties biproperty.Map,
diskCIDS []string,
networksInterfaces map[string]biproperty.Map,
env biproperty.Map,
) (string, error) {
var (
ok bool
cidString string
method = "create_vm"
diskLocality = []interface{}{} // not used with bosh-init
ok bool
cidString string
method = "create_vm"
)

cpiInfo, err := c.Info()
Expand All @@ -176,7 +177,7 @@ func (c cloud) CreateVM(
stemcellCID,
cloudProperties,
networksInterfaces,
diskLocality,
diskCIDS,
env,
)

Expand Down
18 changes: 10 additions & 8 deletions cloud/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ var _ = Describe("Cloud", func() {
stemcellCID string
cloudProperties biproperty.Map
networkInterfaces map[string]biproperty.Map
diskCIDs []string
env biproperty.Map
)

Expand All @@ -387,6 +388,7 @@ var _ = Describe("Cloud", func() {
},
},
}
diskCIDs = []string{"fake-disk-cid"}
cloudProperties = biproperty.Map{
"fake-cloud-property-key": "fake-cloud-property-value",
}
Expand All @@ -408,7 +410,7 @@ var _ = Describe("Cloud", func() {
})

It("executes the cpi job script with the director UUID and stemcell CID", func() {
_, err := cloud.CreateVM(agentID, stemcellCID, cloudProperties, networkInterfaces, env)
_, err := cloud.CreateVM(agentID, stemcellCID, cloudProperties, diskCIDs, networkInterfaces, env)
Expect(err).NotTo(HaveOccurred())
Expect(fakeCPICmdRunner.CurrentRunInput).To(HaveLen(2))
Expect(fakeCPICmdRunner.CurrentRunInput[1]).To(Equal(fakebicloud.RunInput{
Expand All @@ -419,15 +421,15 @@ var _ = Describe("Cloud", func() {
stemcellCID,
cloudProperties,
networkInterfaces,
[]interface{}{},
diskCIDs,
env,
},
ApiVersion: 1,
}))
})

It("returns the cid returned from executing the cpi script", func() {
cid, err := cloud.CreateVM(agentID, stemcellCID, cloudProperties, networkInterfaces, env)
cid, err := cloud.CreateVM(agentID, stemcellCID, cloudProperties, diskCIDs, networkInterfaces, env)
Expect(err).NotTo(HaveOccurred())
Expect(cid).To(Equal("fake-vm-cid"))
})
Expand All @@ -448,7 +450,7 @@ var _ = Describe("Cloud", func() {
})

It("returns the vm cid", func() {
cid, err := cloud.CreateVM(agentID, stemcellCID, cloudProperties, networkInterfaces, env)
cid, err := cloud.CreateVM(agentID, stemcellCID, cloudProperties, diskCIDs, networkInterfaces, env)
Expect(err).NotTo(HaveOccurred())
Expect(cid).To(Equal("fake-vm-cid"))
})
Expand All @@ -468,7 +470,7 @@ var _ = Describe("Cloud", func() {
})

It("returns error", func() {
_, err := cloud.CreateVM(agentID, stemcellCID, cloudProperties, networkInterfaces, env)
_, err := cloud.CreateVM(agentID, stemcellCID, cloudProperties, diskCIDs, networkInterfaces, env)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("Unexpected external CPI command result: '[]interface {}"))
})
Expand All @@ -489,7 +491,7 @@ var _ = Describe("Cloud", func() {
})

It("returns an error", func() {
_, err := cloud.CreateVM(agentID, stemcellCID, cloudProperties, networkInterfaces, env)
_, err := cloud.CreateVM(agentID, stemcellCID, cloudProperties, diskCIDs, networkInterfaces, env)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("Unexpected external CPI command result: '1'"))
})
Expand All @@ -501,14 +503,14 @@ var _ = Describe("Cloud", func() {
})

It("returns an error", func() {
_, err := cloud.CreateVM(agentID, stemcellCID, cloudProperties, networkInterfaces, env)
_, err := cloud.CreateVM(agentID, stemcellCID, cloudProperties, diskCIDs, networkInterfaces, env)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("fake-run-error"))
})
})

itHandlesCPIErrors("create_vm", func() error {
_, err := cloud.CreateVM(agentID, stemcellCID, cloudProperties, networkInterfaces, env)
_, err := cloud.CreateVM(agentID, stemcellCID, cloudProperties, diskCIDs, networkInterfaces, env)
return err
})

Expand Down
3 changes: 3 additions & 0 deletions cloud/fakes/fake_cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ type CreateVMInput struct {
AgentID string
StemcellCID string
CloudProperties biproperty.Map
DiskCIDs []string
NetworksInterfaces map[string]biproperty.Map
Env biproperty.Map
}
Expand Down Expand Up @@ -130,13 +131,15 @@ func (c *FakeCloud) CreateVM(
agentID string,
stemcellCID string,
cloudProperties biproperty.Map,
diskCIDs []string,
networksInterfaces map[string]biproperty.Map,
env biproperty.Map,
) (string, error) {
c.CreateVMInput = CreateVMInput{
AgentID: agentID,
StemcellCID: stemcellCID,
CloudProperties: cloudProperties,
DiskCIDs: diskCIDs,
NetworksInterfaces: networksInterfaces,
Env: env,
}
Expand Down
8 changes: 4 additions & 4 deletions cloud/mocks/mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 31 additions & 3 deletions cmd/create_env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,8 +436,6 @@ var _ = Describe("CreateEnvCmd", func() {
}).Return(installation, nil).AnyTimes()
mockInstaller.EXPECT().Cleanup(installation).AnyTimes()

// mockDeployment := mock_deployment.NewMockDeployment(mockCtrl)

expectDeploy = mockDeployer.EXPECT().Deploy(
mockCloud,
boshDeploymentManifest,
Expand All @@ -446,7 +444,8 @@ var _ = Describe("CreateEnvCmd", func() {
mockBlobstore,
expectedSkipDrain,
gomock.Any(),
).Do(func(_, _, _, _, _, _ interface{}, stage boshui.Stage) {
gomock.Any(),
).Do(func(_, _, _, _, _, _ interface{}, _ interface{}, stage boshui.Stage) {
Expect(fakeStage.SubStages).To(ContainElement(stage))
}).Return(nil, expectedDeployError).AnyTimes()

Expand Down Expand Up @@ -1067,6 +1066,7 @@ var _ = Describe("CreateEnvCmd", func() {
mockBlobstore,
expectedSkipDrain,
gomock.Any(),
gomock.Any(),
).Return(nil, expectedDeployError).AnyTimes()

previousDeploymentState := biconfig.DeploymentState{
Expand Down Expand Up @@ -1117,5 +1117,33 @@ var _ = Describe("CreateEnvCmd", func() {
Expect(err).ToNot(HaveOccurred())
})
})

Context("the deployment state file exists", func() {
It("reads the disks out of the file and passes their CIDs to the deployer", func() {
err := fs.WriteFileString(deploymentStatePath, `
{
"disks": [
{
"id": "bc5dd497-b882-4397-6e9f-22f862277217",
"cid": "disk-cid-from-state",
"size": 51200,
"cloud_properties": {
"datastores": [
"sharedVmfs-0"
],
"type": "thin"
}
}
]
}`)
Expect(err).ToNot(HaveOccurred())

expectDeploy.Do(func(_, _, _, _, _, _ interface{}, diskCIDs []string, _ interface{}) {
Expect(diskCIDs).To(ConsistOf("disk-cid-from-state"))
})
err = command.Run(fakeStage, defaultCreateEnvOpts)
Expect(err).NotTo(HaveOccurred())
})
})
})
})
13 changes: 11 additions & 2 deletions cmd/deployment_preparer.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ func (c *DeploymentPreparer) PrepareDeployment(stage biui.Stage, recreate bool,

deploy := func() error {
return c.deploy(
installation,
deploymentState,
extractedStemcell,
installationManifest,
Expand Down Expand Up @@ -230,7 +229,6 @@ func (c *DeploymentPreparer) PrepareDeployment(stage biui.Stage, recreate bool,
}

func (c *DeploymentPreparer) deploy(
installation biinstall.Installation,
deploymentState biconfig.DeploymentState,
extractedStemcell bistemcell.ExtractedStemcell,
installationManifest biinstallmanifest.Manifest,
Expand Down Expand Up @@ -271,6 +269,7 @@ func (c *DeploymentPreparer) deploy(
vmManager,
blobstore,
skipDrain,
c.extractDiskCIDsFromState(deploymentState),
deployStage,
)
if err != nil {
Expand Down Expand Up @@ -305,3 +304,13 @@ func (c *DeploymentPreparer) stemcellApiVersion(stemcell bistemcell.ExtractedSte
}
return stemcellApiVersion
}

// These disk CIDs get passed all the way to the create_vm cpi call
func (c *DeploymentPreparer) extractDiskCIDsFromState(deploymentState biconfig.DeploymentState) []string {
var diskCIDs []string
for _, disk := range deploymentState.Disks {
diskCIDs = append(diskCIDs, disk.CID)
}

return diskCIDs
}
21 changes: 12 additions & 9 deletions deployment/deployer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ import (

type Deployer interface {
Deploy(
bicloud.Cloud,
bideplmanifest.Manifest,
bistemcell.CloudStemcell,
bivm.Manager,
biblobstore.Blobstore,
bool,
biui.Stage,
cloud bicloud.Cloud,
deploymentManifest bideplmanifest.Manifest,
cloudStemcell bistemcell.CloudStemcell,
vmManager bivm.Manager,
blobstore biblobstore.Blobstore,
skipDrain bool,
diskCIDs []string,
deployStage biui.Stage,
) (Deployment, error)
}

Expand Down Expand Up @@ -58,6 +59,7 @@ func (d *deployer) Deploy(
vmManager bivm.Manager,
blobstore biblobstore.Blobstore,
skipDrain bool,
diskCIDs []string,
deployStage biui.Stage,
) (Deployment, error) {
instanceManager := d.instanceManagerFactory.NewManager(cloud, vmManager, blobstore)
Expand All @@ -68,7 +70,7 @@ func (d *deployer) Deploy(
return nil, err
}

instances, disks, err := d.createAllInstances(deploymentManifest, instanceManager, cloudStemcell, deployStage)
instances, disks, err := d.createAllInstances(deploymentManifest, instanceManager, cloudStemcell, diskCIDs, deployStage)
if err != nil {
return nil, err
}
Expand All @@ -81,6 +83,7 @@ func (d *deployer) createAllInstances(
deploymentManifest bideplmanifest.Manifest,
instanceManager biinstance.Manager,
cloudStemcell bistemcell.CloudStemcell,
diskCIDs []string,
deployStage biui.Stage,
) ([]biinstance.Instance, []bidisk.Disk, error) {
instances := []biinstance.Instance{}
Expand All @@ -95,7 +98,7 @@ func (d *deployer) createAllInstances(
return instances, disks, bosherr.Errorf("Job '%s' must have only one instance, found %d", jobSpec.Name, jobSpec.Instances)
}
for instanceID := 0; instanceID < jobSpec.Instances; instanceID++ {
instance, instanceDisks, err := instanceManager.Create(jobSpec.Name, instanceID, deploymentManifest, cloudStemcell, deployStage)
instance, instanceDisks, err := instanceManager.Create(jobSpec.Name, instanceID, deploymentManifest, cloudStemcell, diskCIDs, deployStage)
if err != nil {
return instances, disks, bosherr.WrapErrorf(err, "Creating instance '%s/%d'", jobSpec.Name, instanceID)
}
Expand Down
Loading

0 comments on commit 1a367b9

Please sign in to comment.