Skip to content

Commit 7e511cc

Browse files
release-25.2: roachtest: avoid requesting local-SSD machine types when unnecessary (#169543)
release-25.2: roachtest: avoid requesting local-SSD machine types when unnecessary
2 parents 0bce347 + e0409b7 commit 7e511cc

2 files changed

Lines changed: 93 additions & 6 deletions

File tree

pkg/cmd/roachtest/spec/cluster_spec.go

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -442,12 +442,9 @@ func (s *ClusterSpec) RoachprodOpts(
442442
var err error
443443
switch cloud {
444444
case AWS:
445-
// We always pass true for shouldSupportLocalSSD here because the machine type selection
446-
// logic should not depend on the user's preference for local SSDs.
447-
// The actual decision to use provisioned local SSDs is handled in the disk configuration logic
448-
// at the provider level, and EBS volume have priority over local SSDs.
449-
// This means that if both EBS and local SSDs are available, EBS will be used.
450-
machineType, selectedArch, err = SelectAWSMachineType(s.CPUs, s.Mem, true, requestedArch)
445+
machineType, selectedArch, err = SelectAWSMachineType(
446+
s.CPUs, s.Mem, s.mayUseLocalSSD(params.Defaults.PreferLocalSSD), requestedArch,
447+
)
451448
case GCE:
452449
machineType, selectedArch = SelectGCEMachineType(s.CPUs, s.Mem, requestedArch)
453450
case Azure:
@@ -633,6 +630,27 @@ func (s *ClusterSpec) RoachprodOpts(
633630
return createVMOpts, providerOpts, workloadProviderOpts, selectedArch, nil
634631
}
635632

633+
// mayUseLocalSSD returns whether this spec could end up using local SSDs.
634+
// This is used to determine whether to select a machine type that supports
635+
// local SSDs (e.g. on AWS, machine families with a "d" suffix). When local
636+
// SSDs definitely won't be used, selecting a non-local-SSD machine type
637+
// avoids unnecessary capacity constraints.
638+
func (s *ClusterSpec) mayUseLocalSSD(defaultPreferLocalSSD bool) bool {
639+
if s.VolumeType != "" && s.VolumeType != "local-ssd" {
640+
return false
641+
}
642+
if s.LocalSSD == LocalSSDDisable {
643+
return false
644+
}
645+
if s.VolumeSize != 0 {
646+
return false
647+
}
648+
return s.VolumeType == "local-ssd" ||
649+
s.LocalSSD == LocalSSDPreferOn ||
650+
s.RandomizeVolumeType ||
651+
defaultPreferLocalSSD
652+
}
653+
636654
func (s *ClusterSpec) isLocalSSDAvailable(
637655
cloud Cloud, machineType string, selectedArch vm.CPUArch,
638656
) error {

pkg/cmd/roachtest/spec/cluster_spec_test.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,75 @@ func TestClustersCompatible(t *testing.T) {
4545
})
4646
}
4747

48+
func TestMayUseLocalSSD(t *testing.T) {
49+
tests := []struct {
50+
name string
51+
spec ClusterSpec
52+
defaultPreferLocalSSD bool
53+
expected bool
54+
}{
55+
{
56+
name: "default spec with PreferLocalSSD=true",
57+
defaultPreferLocalSSD: true,
58+
expected: true,
59+
},
60+
{
61+
name: "default spec with PreferLocalSSD=false",
62+
defaultPreferLocalSSD: false,
63+
expected: false,
64+
},
65+
{
66+
name: "explicit non-local-ssd volume type",
67+
spec: ClusterSpec{VolumeType: "gp3"},
68+
defaultPreferLocalSSD: true,
69+
expected: false,
70+
},
71+
{
72+
name: "explicit local-ssd volume type",
73+
spec: ClusterSpec{VolumeType: "local-ssd"},
74+
defaultPreferLocalSSD: false,
75+
expected: true,
76+
},
77+
{
78+
name: "LocalSSD disabled",
79+
spec: ClusterSpec{LocalSSD: LocalSSDDisable},
80+
defaultPreferLocalSSD: true,
81+
expected: false,
82+
},
83+
{
84+
name: "LocalSSD preferred",
85+
spec: ClusterSpec{LocalSSD: LocalSSDPreferOn},
86+
defaultPreferLocalSSD: false,
87+
expected: true,
88+
},
89+
{
90+
name: "RandomizeVolumeType enabled",
91+
spec: ClusterSpec{RandomizeVolumeType: true},
92+
defaultPreferLocalSSD: false,
93+
expected: true,
94+
},
95+
{
96+
name: "VolumeSize set overrides PreferLocalSSD",
97+
spec: ClusterSpec{VolumeSize: 100},
98+
defaultPreferLocalSSD: true,
99+
expected: false,
100+
},
101+
{
102+
name: "VolumeSize set overrides LocalSSDPreferOn",
103+
spec: ClusterSpec{VolumeSize: 100, LocalSSD: LocalSSDPreferOn},
104+
defaultPreferLocalSSD: true,
105+
expected: false,
106+
},
107+
}
108+
109+
for _, tc := range tests {
110+
t.Run(tc.name, func(t *testing.T) {
111+
result := tc.spec.mayUseLocalSSD(tc.defaultPreferLocalSSD)
112+
require.Equal(t, tc.expected, result)
113+
})
114+
}
115+
}
116+
48117
func TestClustersRetainClearedInfo(t *testing.T) {
49118
// Adding a test in case we switch the ClustersCompatible signature to take
50119
// pointers to ClusterSpec in the future.

0 commit comments

Comments
 (0)