Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass existing disk CIDs to CPI in create-env command #664

Merged
merged 1 commit into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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