Skip to content

Commit

Permalink
roachprod: minor fixes for snapshots
Browse files Browse the repository at this point in the history
Support for disk snapshots has been recently added in [1].
This change adds minor fixes which were necessary to
run in a different environment which was previously tested.
Specifically, several commands were missing `--project`
which resulted in (silent) failures. Error propagation
from `c.Parallel` was signaling an internal error
instead of passing command's error. (Granted, the existing
mechanism for error propagation is to be revisited in [2].)

[1] cockroachdb#103757
[2] cockroachdb#104312

Epic: none

Release note: None
  • Loading branch information
srosenberg committed Jun 8, 2023
1 parent e1253c3 commit f05dcfa
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 3 deletions.
13 changes: 10 additions & 3 deletions pkg/roachprod/roachprod.go
Original file line number Diff line number Diff line change
Expand Up @@ -1626,6 +1626,13 @@ func CreateSnapshot(
crdbVersion = "unknown"
}
crdbVersion = strings.TrimPrefix(crdbVersion, "cockroach-")
// N.B. snapshot name cannot exceed 63 characters, so we use short sha for dev version.
if index := strings.Index(crdbVersion, "dev-"); index != -1 {
sha := crdbVersion[index+4:]
if len(sha) > 7 {
crdbVersion = crdbVersion[:index+4] + sha[:7]
}
}

labels := map[string]string{
"roachprod-node-src-spec": cVM.MachineType,
Expand Down Expand Up @@ -1655,6 +1662,9 @@ func CreateSnapshot(
snapshotFingerprintInfix := strings.ReplaceAll(
fmt.Sprintf("%s-n%d", crdbVersion, len(nodes)), ".", "-")
snapshotName := fmt.Sprintf("%s-%s-%04d", vsco.Name, snapshotFingerprintInfix, node)
if len(snapshotName) > 63 {
return fmt.Errorf("snapshot name %q exceeds 63 characters; shorten name prefix and use description arg. for more context", snapshotName)
}
volumeSnapshot, err := provider.CreateVolumeSnapshot(l, volume,
vm.VolumeSnapshotCreateOpts{
Name: snapshotName,
Expand All @@ -1673,7 +1683,6 @@ func CreateSnapshot(
return nil
}); err != nil {
res.Err = err
return res, res.Err
}
return res, nil
}); err != nil {
Expand Down Expand Up @@ -1746,7 +1755,6 @@ func ApplySnapshots(
return nil
}); err != nil {
res.Err = err
return res, res.Err
}
return res, nil
}); err != nil {
Expand Down Expand Up @@ -1817,7 +1825,6 @@ func ApplySnapshots(
return nil
}); err != nil {
res.Err = err
return res, res.Err
}
return res, nil
})
Expand Down
10 changes: 10 additions & 0 deletions pkg/roachprod/vm/gce/gcloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ func (p *Provider) CreateVolumeSnapshot(
) (vm.VolumeSnapshot, error) {
args := []string{
"compute",
"--project", p.GetProject(),
"snapshots",
"create", vsco.Name,
"--source-disk", volume.ProviderResourceID,
Expand All @@ -336,6 +337,7 @@ func (p *Provider) CreateVolumeSnapshot(

args = []string{
"compute",
"--project", p.GetProject(),
"snapshots",
"add-labels", vsco.Name,
"--labels", s[:len(s)-1],
Expand All @@ -355,6 +357,7 @@ func (p *Provider) ListVolumeSnapshots(
) ([]vm.VolumeSnapshot, error) {
args := []string{
"compute",
"--project", p.GetProject(),
"snapshots",
"list",
"--format", "json(name,id)",
Expand Down Expand Up @@ -398,6 +401,7 @@ func (p *Provider) DeleteVolumeSnapshots(l *logger.Logger, snapshots ...vm.Volum
}
args := []string{
"compute",
"--project", p.GetProject(),
"snapshots",
"delete",
}
Expand Down Expand Up @@ -438,6 +442,7 @@ func (p *Provider) CreateVolume(
}
args := []string{
"compute",
"--project", p.GetProject(),
"disks",
"create", vco.Name,
"--size", strconv.Itoa(vco.Size),
Expand Down Expand Up @@ -496,6 +501,7 @@ func (p *Provider) CreateVolume(
s := sb.String()
args = []string{
"compute",
"--project", p.GetProject(),
"disks",
"add-labels", vco.Name,
"--labels", s[:len(s)-1],
Expand All @@ -522,6 +528,7 @@ func (p *Provider) DeleteVolume(l *logger.Logger, volume vm.Volume, vm *vm.VM) e
{ // Detach disks.
args := []string{
"compute",
"--project", p.GetProject(),
"instances",
"detach-disk", vm.Name,
"--disk", volume.ProviderResourceID,
Expand All @@ -535,6 +542,7 @@ func (p *Provider) DeleteVolume(l *logger.Logger, volume vm.Volume, vm *vm.VM) e
{ // Delete disks.
args := []string{
"compute",
"--project", p.GetProject(),
"disks",
"delete",
volume.ProviderResourceID,
Expand Down Expand Up @@ -646,6 +654,7 @@ func (p *Provider) AttachVolume(l *logger.Logger, volume vm.Volume, vm *vm.VM) (
// Volume attach.
args := []string{
"compute",
"--project", p.GetProject(),
"instances",
"attach-disk",
vm.ProviderID,
Expand Down Expand Up @@ -675,6 +684,7 @@ func (p *Provider) AttachVolume(l *logger.Logger, volume vm.Volume, vm *vm.VM) (
// Volume auto delete.
args = []string{
"compute",
"--project", p.GetProject(),
"instances",
"set-disk-auto-delete", vm.ProviderID,
"--auto-delete",
Expand Down

0 comments on commit f05dcfa

Please sign in to comment.