Skip to content

Commit

Permalink
roachprod: do not mutate EBSVolumes
Browse files Browse the repository at this point in the history
Previously, `roachprod create` appended volumes to
`providerOpts.EBSVolumes`, assuming it's done once per run. In reality,
the code is run every time when we create an instance, which leads to data
races and accumulation of extra EBS volumes.

This patch creates a copy of `providerOpts.EBSVolumes` before making any
changes to it.

Release note: None
  • Loading branch information
rail committed Jan 22, 2022
1 parent fdfd893 commit 936633b
Showing 1 changed file with 7 additions and 6 deletions.
13 changes: 7 additions & 6 deletions pkg/roachprod/vm/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -921,20 +921,21 @@ func (p *Provider) runInstance(
if p.IAMProfile != "" {
args = append(args, "--iam-instance-profile", "Name="+p.IAMProfile)
}

// Make a local copy of providerOpts.EBSVolumes to prevent data races
ebsVolumes := providerOpts.EBSVolumes
// The local NVMe devices are automatically mapped. Otherwise, we need to map an EBS data volume.
if !opts.SSDOpts.UseLocalSSD {
if len(providerOpts.EBSVolumes) == 0 && providerOpts.DefaultEBSVolume.Disk.VolumeType == "" {
if len(ebsVolumes) == 0 && providerOpts.DefaultEBSVolume.Disk.VolumeType == "" {
providerOpts.DefaultEBSVolume.Disk.VolumeType = defaultEBSVolumeType
providerOpts.DefaultEBSVolume.Disk.DeleteOnTermination = true
}

if providerOpts.DefaultEBSVolume.Disk.VolumeType != "" {
// Add default volume to the list of volumes we'll setup.
v := providerOpts.EBSVolumes.newVolume()
v := ebsVolumes.newVolume()
v.Disk = providerOpts.DefaultEBSVolume.Disk
v.Disk.DeleteOnTermination = true
providerOpts.EBSVolumes = append(providerOpts.EBSVolumes, v)
ebsVolumes = append(ebsVolumes, v)
}
}

Expand All @@ -946,9 +947,9 @@ func (p *Provider) runInstance(
DeleteOnTermination: true,
},
}
providerOpts.EBSVolumes = append(providerOpts.EBSVolumes, osDiskVolume)
ebsVolumes = append(ebsVolumes, osDiskVolume)

mapping, err := json.Marshal(providerOpts.EBSVolumes)
mapping, err := json.Marshal(ebsVolumes)
if err != nil {
return err
}
Expand Down

0 comments on commit 936633b

Please sign in to comment.