Skip to content

Commit aa037ae

Browse files
mblosmblos
authored
Commitment resources use new naming schema (#629)
1 parent b19510a commit aa037ae

13 files changed

Lines changed: 132 additions & 99 deletions

‎internal/scheduling/reservations/commitments/api_change_commitments_monitor_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -172,24 +172,24 @@ func TestCountCommitments(t *testing.T) {
172172
{
173173
name: "Single commitment",
174174
request: newCommitmentRequest("az-a", false, 1234,
175-
createCommitment("ram_hana_1", "project-A", "uuid-1", "confirmed", 2),
175+
createCommitment("hw_version_hana_1_ram", "project-A", "uuid-1", "confirmed", 2),
176176
),
177177
expected: 1,
178178
},
179179
{
180180
name: "Multiple commitments same project",
181181
request: newCommitmentRequest("az-a", false, 1234,
182-
createCommitment("ram_hana_1", "project-A", "uuid-1", "confirmed", 2),
183-
createCommitment("ram_hana_2", "project-A", "uuid-2", "confirmed", 2),
182+
createCommitment("hw_version_hana_1_ram", "project-A", "uuid-1", "confirmed", 2),
183+
createCommitment("hw_version_hana_2_ram", "project-A", "uuid-2", "confirmed", 2),
184184
),
185185
expected: 2,
186186
},
187187
{
188188
name: "Multiple commitments multiple projects",
189189
request: newCommitmentRequest("az-a", false, 1234,
190-
createCommitment("ram_hana_1", "project-A", "uuid-1", "confirmed", 2),
191-
createCommitment("ram_hana_1", "project-B", "uuid-2", "confirmed", 3),
192-
createCommitment("ram_gp_1", "project-C", "uuid-3", "confirmed", 1),
190+
createCommitment("hw_version_hana_1_ram", "project-A", "uuid-1", "confirmed", 2),
191+
createCommitment("hw_version_hana_1_ram", "project-B", "uuid-2", "confirmed", 3),
192+
createCommitment("hw_version_gp_1_ram", "project-C", "uuid-3", "confirmed", 1),
193193
),
194194
expected: 3,
195195
},

‎internal/scheduling/reservations/commitments/api_change_commitments_test.go‎

Lines changed: 39 additions & 39 deletions
Large diffs are not rendered by default.

‎internal/scheduling/reservations/commitments/api_info.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ func (api *HTTPAPI) buildServiceInfo(ctx context.Context, logger logr.Logger) (l
107107
// Build resources map
108108
resources := make(map[liquid.ResourceName]liquid.ResourceInfo)
109109
for groupName, groupData := range flavorGroups {
110-
resourceName := liquid.ResourceName(commitmentResourceNamePrefix + groupName)
110+
resourceName := liquid.ResourceName(ResourceNameFromFlavorGroup(groupName))
111111

112112
flavorNames := make([]string, 0, len(groupData.Flavors))
113113
for _, flavor := range groupData.Flavors {

‎internal/scheduling/reservations/commitments/api_info_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -218,38 +218,38 @@ func TestHandleInfo_HasCapacityEqualsHandlesCommitments(t *testing.T) {
218218
t.Fatalf("expected 2 resources, got %d", len(serviceInfo.Resources))
219219
}
220220

221-
// Test fixed ratio group: ram_hana_fixed
222-
fixedResource, ok := serviceInfo.Resources["ram_hana_fixed"]
221+
// Test fixed ratio group: hw_version_hana_fixed_ram
222+
fixedResource, ok := serviceInfo.Resources["hw_version_hana_fixed_ram"]
223223
if !ok {
224-
t.Fatal("expected ram_hana_fixed resource to exist")
224+
t.Fatal("expected hw_version_hana_fixed_ram resource to exist")
225225
}
226226
if !fixedResource.HasCapacity {
227-
t.Error("ram_hana_fixed: expected HasCapacity=true")
227+
t.Error("hw_version_hana_fixed_ram: expected HasCapacity=true")
228228
}
229229
if !fixedResource.HandlesCommitments {
230-
t.Error("ram_hana_fixed: expected HandlesCommitments=true (fixed ratio group)")
230+
t.Error("hw_version_hana_fixed_ram: expected HandlesCommitments=true (fixed ratio group)")
231231
}
232232
if fixedResource.HasCapacity != fixedResource.HandlesCommitments {
233-
t.Errorf("ram_hana_fixed: HasCapacity (%v) should equal HandlesCommitments (%v)",
233+
t.Errorf("hw_version_hana_fixed_ram: HasCapacity (%v) should equal HandlesCommitments (%v)",
234234
fixedResource.HasCapacity, fixedResource.HandlesCommitments)
235235
}
236236

237-
// Test variable ratio group: ram_v2_variable
238-
variableResource, ok := serviceInfo.Resources["ram_v2_variable"]
237+
// Test variable ratio group: hw_version_v2_variable_ram
238+
variableResource, ok := serviceInfo.Resources["hw_version_v2_variable_ram"]
239239
if !ok {
240-
t.Fatal("expected ram_v2_variable resource to exist")
240+
t.Fatal("expected hw_version_v2_variable_ram resource to exist")
241241
}
242242
// Variable ratio groups don't accept commitments, and we only report capacity for groups
243243
// that accept commitments, so both HasCapacity and HandlesCommitments should be false
244244
if variableResource.HasCapacity {
245-
t.Error("ram_v2_variable: expected HasCapacity=false (variable ratio groups don't report capacity)")
245+
t.Error("hw_version_v2_variable_ram: expected HasCapacity=false (variable ratio groups don't report capacity)")
246246
}
247247
if variableResource.HandlesCommitments {
248-
t.Error("ram_v2_variable: expected HandlesCommitments=false (variable ratio group)")
248+
t.Error("hw_version_v2_variable_ram: expected HandlesCommitments=false (variable ratio group)")
249249
}
250250
// Verify HasCapacity == HandlesCommitments for consistency
251251
if variableResource.HasCapacity != variableResource.HandlesCommitments {
252-
t.Errorf("ram_v2_variable: HasCapacity (%v) should equal HandlesCommitments (%v)",
252+
t.Errorf("hw_version_v2_variable_ram: HasCapacity (%v) should equal HandlesCommitments (%v)",
253253
variableResource.HasCapacity, variableResource.HandlesCommitments)
254254
}
255255
}

‎internal/scheduling/reservations/commitments/api_report_capacity_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,9 @@ func TestCapacityCalculator(t *testing.T) {
187187
t.Fatalf("Expected 1 resource, got %d", len(report.Resources))
188188
}
189189

190-
resource := report.Resources[liquid.ResourceName("ram_test-group")]
190+
resource := report.Resources[liquid.ResourceName("hw_version_test-group_ram")]
191191
if resource == nil {
192-
t.Fatal("Expected ram_test-group resource to exist")
192+
t.Fatal("Expected hw_version_test-group_ram resource to exist")
193193
}
194194

195195
// Should have empty perAZ map when no host details

‎internal/scheduling/reservations/commitments/api_report_usage_test.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func TestReportUsageIntegration(t *testing.T) {
5757
Reservations: []*UsageTestReservation{},
5858
AllAZs: []string{"az-a"},
5959
Expected: map[string]ExpectedResourceUsage{
60-
"ram_hana_1": {
60+
"hw_version_hana_1_ram": {
6161
PerAZ: map[string]ExpectedAZUsage{
6262
"az-a": {Usage: 0, VMs: []ExpectedVMUsage{}},
6363
},
@@ -77,7 +77,7 @@ func TestReportUsageIntegration(t *testing.T) {
7777
},
7878
AllAZs: []string{"az-a"},
7979
Expected: map[string]ExpectedResourceUsage{
80-
"ram_hana_1": {
80+
"hw_version_hana_1_ram": {
8181
PerAZ: map[string]ExpectedAZUsage{
8282
"az-a": {
8383
Usage: 4, // 4096 MB / 1024 MB = 4 units
@@ -99,7 +99,7 @@ func TestReportUsageIntegration(t *testing.T) {
9999
Reservations: []*UsageTestReservation{}, // No commitments
100100
AllAZs: []string{"az-a"},
101101
Expected: map[string]ExpectedResourceUsage{
102-
"ram_hana_1": {
102+
"hw_version_hana_1_ram": {
103103
PerAZ: map[string]ExpectedAZUsage{
104104
"az-a": {
105105
Usage: 4,
@@ -127,7 +127,7 @@ func TestReportUsageIntegration(t *testing.T) {
127127
},
128128
AllAZs: []string{"az-a"},
129129
Expected: map[string]ExpectedResourceUsage{
130-
"ram_hana_1": {
130+
"hw_version_hana_1_ram": {
131131
PerAZ: map[string]ExpectedAZUsage{
132132
"az-a": {
133133
Usage: 12, // 12 units total
@@ -157,7 +157,7 @@ func TestReportUsageIntegration(t *testing.T) {
157157
},
158158
AllAZs: []string{"az-a"},
159159
Expected: map[string]ExpectedResourceUsage{
160-
"ram_hana_1": {
160+
"hw_version_hana_1_ram": {
161161
PerAZ: map[string]ExpectedAZUsage{
162162
"az-a": {
163163
Usage: 12,
@@ -187,7 +187,7 @@ func TestReportUsageIntegration(t *testing.T) {
187187
},
188188
AllAZs: []string{"az-a"},
189189
Expected: map[string]ExpectedResourceUsage{
190-
"ram_hana_1": {
190+
"hw_version_hana_1_ram": {
191191
PerAZ: map[string]ExpectedAZUsage{
192192
"az-a": {
193193
Usage: 13, // 1 + 4 + 8 = 13 units
@@ -216,7 +216,7 @@ func TestReportUsageIntegration(t *testing.T) {
216216
},
217217
AllAZs: []string{"az-a"},
218218
Expected: map[string]ExpectedResourceUsage{
219-
"ram_hana_1": {
219+
"hw_version_hana_1_ram": {
220220
PerAZ: map[string]ExpectedAZUsage{
221221
"az-a": {
222222
Usage: 8,
@@ -243,7 +243,7 @@ func TestReportUsageIntegration(t *testing.T) {
243243
},
244244
AllAZs: []string{"az-a", "az-b"},
245245
Expected: map[string]ExpectedResourceUsage{
246-
"ram_hana_1": {
246+
"hw_version_hana_1_ram": {
247247
PerAZ: map[string]ExpectedAZUsage{
248248
"az-a": {
249249
Usage: 4,
@@ -275,7 +275,7 @@ func TestReportUsageIntegration(t *testing.T) {
275275
},
276276
AllAZs: []string{"az-a"},
277277
Expected: map[string]ExpectedResourceUsage{
278-
"ram_hana_1": {
278+
"hw_version_hana_1_ram": {
279279
PerAZ: map[string]ExpectedAZUsage{
280280
"az-a": {
281281
Usage: 4, // 4096 MB / 1024 MB = 4 units
@@ -285,7 +285,7 @@ func TestReportUsageIntegration(t *testing.T) {
285285
},
286286
},
287287
},
288-
"ram_gp_1": {
288+
"hw_version_gp_1_ram": {
289289
PerAZ: map[string]ExpectedAZUsage{
290290
"az-a": {
291291
Usage: 4, // 2048 MB / 512 MB = 4 units
@@ -332,7 +332,7 @@ func TestReportUsageIntegration(t *testing.T) {
332332
},
333333
AllAZs: []string{"az-a"},
334334
Expected: map[string]ExpectedResourceUsage{
335-
"ram_hana_1": {
335+
"hw_version_hana_1_ram": {
336336
PerAZ: map[string]ExpectedAZUsage{
337337
"az-a": {
338338
Usage: 4,

‎internal/scheduling/reservations/commitments/capacity.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ func (c *CapacityCalculator) CalculateCapacity(ctx context.Context) (liquid.Serv
5353
continue
5454
}
5555

56-
// Resource name follows pattern: ram_<flavorgroup>
57-
resourceName := liquid.ResourceName(commitmentResourceNamePrefix + groupName)
56+
// Resource name follows pattern: hw_version_<flavorgroup>_ram
57+
resourceName := liquid.ResourceName(ResourceNameFromFlavorGroup(groupName))
5858

5959
// Calculate per-AZ capacity and usage
6060
azCapacity, err := c.calculateAZCapacity(ctx, groupName, groupData)

‎internal/scheduling/reservations/commitments/state.go‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,30 @@ import (
1818
// commitmentUUIDPattern validates commitment UUID format.
1919
var commitmentUUIDPattern = regexp.MustCompile(`^[a-zA-Z0-9-]{6,40}$`)
2020

21-
// Limes LIQUID resource naming convention: ram_<flavorgroup>
22-
const commitmentResourceNamePrefix = "ram_"
21+
// Limes LIQUID resource naming convention: hw_version_<flavorgroup>_ram
22+
const (
23+
resourceNamePrefix = "hw_version_"
24+
resourceNameSuffix = "_ram"
25+
)
26+
27+
// ResourceNameFromFlavorGroup creates a LIQUID resource name from a flavor group name.
28+
// Format: hw_version_<flavorgroup>_ram
29+
func ResourceNameFromFlavorGroup(flavorGroup string) string {
30+
return resourceNamePrefix + flavorGroup + resourceNameSuffix
31+
}
2332

2433
func getFlavorGroupNameFromResource(resourceName string) (string, error) {
25-
if !strings.HasPrefix(resourceName, commitmentResourceNamePrefix) {
34+
if !strings.HasPrefix(resourceName, resourceNamePrefix) || !strings.HasSuffix(resourceName, resourceNameSuffix) {
2635
return "", fmt.Errorf("invalid resource name: %s", resourceName)
2736
}
28-
return strings.TrimPrefix(resourceName, commitmentResourceNamePrefix), nil
37+
// Remove prefix and suffix
38+
name := strings.TrimPrefix(resourceName, resourceNamePrefix)
39+
name = strings.TrimSuffix(name, resourceNameSuffix)
40+
// Validate that the extracted group name is not empty
41+
if name == "" {
42+
return "", fmt.Errorf("invalid resource name: %s (empty group name)", resourceName)
43+
}
44+
return name, nil
2945
}
3046

3147
// CommitmentState represents desired or current commitment resource allocation.

‎internal/scheduling/reservations/commitments/state_test.go‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ func TestFromCommitment_CalculatesMemoryCorrectly(t *testing.T) {
3636
commitment := Commitment{
3737
UUID: "test-uuid",
3838
ProjectID: "project-1",
39-
ResourceName: "ram_test-group",
39+
ResourceName: "hw_version_test-group_ram",
4040
Amount: 5, // 5 multiples of smallest flavor
4141
}
4242

@@ -236,7 +236,8 @@ func TestFromReservations_NonCommittedResourceType(t *testing.T) {
236236
}
237237

238238
func TestGetFlavorGroupNameFromResource_Valid(t *testing.T) {
239-
name, err := getFlavorGroupNameFromResource("ram_hana_medium_v2")
239+
// Test valid resource names with underscores in flavor group
240+
name, err := getFlavorGroupNameFromResource("hw_version_hana_medium_v2_ram")
240241
if err != nil {
241242
t.Fatalf("unexpected error: %v", err)
242243
}
@@ -246,8 +247,29 @@ func TestGetFlavorGroupNameFromResource_Valid(t *testing.T) {
246247
}
247248

248249
func TestGetFlavorGroupNameFromResource_Invalid(t *testing.T) {
249-
_, err := getFlavorGroupNameFromResource("invalid_resource")
250-
if err == nil {
251-
t.Fatal("expected error for invalid resource name, got nil")
250+
invalidCases := []string{
251+
"ram_2101", // old format
252+
"invalid", // completely wrong
253+
"hw_version__ram", // empty group name
254+
"hw_version_2101", // missing suffix
255+
}
256+
for _, input := range invalidCases {
257+
if _, err := getFlavorGroupNameFromResource(input); err == nil {
258+
t.Errorf("expected error for %q, got nil", input)
259+
}
260+
}
261+
}
262+
263+
func TestResourceNameRoundTrip(t *testing.T) {
264+
// Test that ResourceNameFromFlavorGroup and getFlavorGroupNameFromResource are inverses
265+
for _, groupName := range []string{"2101", "hana_1", "hana_medium_v2"} {
266+
resourceName := ResourceNameFromFlavorGroup(groupName)
267+
recovered, err := getFlavorGroupNameFromResource(resourceName)
268+
if err != nil {
269+
t.Fatalf("round-trip failed for %q: %v", groupName, err)
270+
}
271+
if recovered != groupName {
272+
t.Errorf("round-trip mismatch: %q -> %q -> %q", groupName, resourceName, recovered)
273+
}
252274
}
253275
}

‎internal/scheduling/reservations/commitments/syncer.go‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package commitments
66
import (
77
"context"
88
"fmt"
9-
"strings"
109
"time"
1110

1211
"github.com/cobaltcore-dev/cortex/api/v1alpha1"
@@ -90,12 +89,8 @@ func (s *Syncer) getCommitmentStates(ctx context.Context, log logr.Logger, flavo
9089
log.Info("skipping non-compute commitment", "id", id, "serviceType", commitment.ServiceType)
9190
continue
9291
}
93-
if !strings.HasPrefix(commitment.ResourceName, commitmentResourceNamePrefix) {
94-
log.Info("skipping non-RAM-flavor-group commitment", "id", id, "resourceName", commitment.ResourceName)
95-
continue
96-
}
9792

98-
// Extract flavor group name from resource name
93+
// Extract flavor group name from resource name (validates format: hw_version_<group>_ram)
9994
flavorGroupName, err := getFlavorGroupNameFromResource(commitment.ResourceName)
10095
if err != nil {
10196
log.Info("skipping commitment with invalid resource name",

0 commit comments

Comments
 (0)