Skip to content

Commit 88745e5

Browse files
authored
fix: deduplicate resource rule scope to prevent resource overflow (#336)
## Issue Resolves #306 ## Description If the same scope is used in 2 or more resource validation rules, all rules after the 1st will automatically fail. The goal is to prevent potentially reusing the same set of resources to validate 2 rules. Two unit tests have been added to check for various cluster/scope combos.
1 parent bfb44b2 commit 88745e5

5 files changed

Lines changed: 116 additions & 5 deletions

File tree

Dockerfile.devspace

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Build the manager binary
2-
FROM --platform=$TARGETPLATFORM golang:alpine3.18 as builder
2+
FROM --platform=$TARGETPLATFORM golang:alpine3.19 as builder
33
ARG TARGETOS
44
ARG TARGETARCH
55

README.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,16 @@ make undeploy
6262
```
6363

6464
## Contributing
65-
// TODO(user): Add detailed information on how you would like others to contribute to this project
65+
All contributions are welcome! Feel free to reach out on the [Spectro Cloud community Slack](https://spectrocloudcommunity.slack.com/join/shared_invite/zt-g8gfzrhf-cKavsGD_myOh30K24pImLA#/shared-invite/email).
66+
67+
Make sure `pre-commit` is [installed](https://pre-commit.com#install).
68+
69+
Install the `pre-commit` scripts:
70+
71+
```console
72+
pre-commit install --hook-type commit-msg
73+
pre-commit install --hook-type pre-commit
74+
```
6675

6776
### How it works
6877
This project aims to follow the Kubernetes [Operator pattern](https://kubernetes.io/docs/concepts/extend-kubernetes/operator/).

pkg/validate/validate.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,13 +113,16 @@ func Validate(ctx context.Context, spec v1alpha1.VsphereValidatorSpec, vsphereAc
113113

114114
// Compute resource validation rules
115115
computeResourceValidationService := computeresources.NewValidationService(log, driver)
116+
seenScope := make(map[string]bool, 0)
116117
for _, rule := range spec.ComputeResourceRules {
117-
vrr, err := computeResourceValidationService.ReconcileComputeResourceValidationRule(rule, finder, driver)
118+
vrr, err := computeResourceValidationService.ReconcileComputeResourceValidationRule(rule, finder, driver, seenScope)
118119
if err != nil {
119120
log.Error(err, "failed to reconcile computeresources validation rule")
120121
}
121122
resp.AddResult(vrr, err)
122123
log.Info("Validated compute resources", "scope", rule.Scope, "entity name", rule.EntityName)
124+
key := computeresources.GetScopeKey(rule)
125+
seenScope[key] = true
123126
}
124127

125128
return resp

pkg/validators/computeresources/computeresources.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ var (
2929
// GetResourcePoolAndVMs is defined to enable monkey patching the getResourcePoolAndVMs function in integration tests
3030
GetResourcePoolAndVMs = getResourcePoolAndVMs
3131
errInsufficientComputeResources = errors.New("compute resources rule not satisfied")
32+
errRuleAlreadyProcessed = errors.New("rule for scope already processed")
3233
)
3334

3435
// ValidationService is a service that validates compute resource rules
@@ -103,10 +104,19 @@ type Usage struct {
103104
}
104105

105106
// ReconcileComputeResourceValidationRule reconciles the compute resource rule
106-
func (c *ValidationService) ReconcileComputeResourceValidationRule(rule v1alpha1.ComputeResourceRule, finder *find.Finder, driver *vsphere.CloudDriver) (*types.ValidationRuleResult, error) {
107+
func (c *ValidationService) ReconcileComputeResourceValidationRule(rule v1alpha1.ComputeResourceRule, finder *find.Finder, driver *vsphere.CloudDriver, seenScopes map[string]bool) (*types.ValidationRuleResult, error) {
107108

108109
vr := buildValidationResult(rule, constants.ValidationTypeComputeResources)
109110

111+
key := GetScopeKey(rule)
112+
if seenScopes[key] {
113+
vr.State = util.Ptr(vapi.ValidationFailed)
114+
vr.Condition.Message = "Rule for scope already processed"
115+
vr.Condition.Failures = append(vr.Condition.Failures, fmt.Sprintf("Rule for scope %s already processed", key))
116+
vr.Condition.Status = corev1.ConditionFalse
117+
return vr, errRuleAlreadyProcessed
118+
}
119+
110120
ctx, cancel := context.WithCancel(context.Background())
111121
defer cancel()
112122

@@ -348,3 +358,16 @@ func getTotalQuantity(quantity string, numberOfNodes int) resource.Quantity {
348358
}
349359
return totalQuantity
350360
}
361+
362+
// GetScopeKey returns a formatted key depending on the scope of a rule
363+
func GetScopeKey(rule v1alpha1.ComputeResourceRule) string {
364+
switch rule.Scope {
365+
case "cluster":
366+
return fmt.Sprintf("%s-%s", rule.Scope, rule.EntityName)
367+
case "host":
368+
return fmt.Sprintf("%s-%s", rule.Scope, rule.EntityName)
369+
case "resourcepool":
370+
return fmt.Sprintf("%s-%s", rule.Scope, rule.ClusterName)
371+
}
372+
return ""
373+
}

pkg/validators/computeresources/computeresources_test.go

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,77 @@ func TestComputeResourcesValidationService_ReconcileComputeResourceValidationRul
458458
State: util.Ptr(vapi.ValidationFailed),
459459
},
460460
},
461+
{
462+
name: "Duplicate scope resourcepool",
463+
expectedErr: errRuleAlreadyProcessed,
464+
rule: v1alpha1.ComputeResourceRule{
465+
Name: "Test Resourcepool Resource Validation rule",
466+
Scope: "resourcepool",
467+
ClusterName: "DC0_C1",
468+
EntityName: "DC0_C1_RP0",
469+
NodepoolResourceRequirements: []v1alpha1.NodepoolResourceRequirement{
470+
{
471+
Name: "masterpool",
472+
NumberOfNodes: 1,
473+
CPU: "1GHz",
474+
Memory: "500Mi",
475+
DiskSpace: "500Ti",
476+
},
477+
{
478+
Name: "workerpool",
479+
NumberOfNodes: 1,
480+
CPU: "1GHz",
481+
Memory: "1Gi",
482+
DiskSpace: "100Ti",
483+
},
484+
},
485+
},
486+
expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
487+
ValidationType: "vsphere-compute-resources",
488+
ValidationRule: "validation-resourcepool-DC0_C1_RP0",
489+
Message: "Rule for scope already processed",
490+
Details: []string{},
491+
Failures: []string{"Rule for scope resourcepool-DC0_C1 already processed"},
492+
Status: corev1.ConditionFalse,
493+
},
494+
State: util.Ptr(vapi.ValidationFailed),
495+
},
496+
},
497+
{
498+
name: "Duplicate scope cluster",
499+
expectedErr: errRuleAlreadyProcessed,
500+
rule: v1alpha1.ComputeResourceRule{
501+
Name: "Test Resourcepool Resource Validation rule",
502+
Scope: "cluster",
503+
EntityName: "DC0_C1",
504+
NodepoolResourceRequirements: []v1alpha1.NodepoolResourceRequirement{
505+
{
506+
Name: "masterpool",
507+
NumberOfNodes: 1,
508+
CPU: "1GHz",
509+
Memory: "500Mi",
510+
DiskSpace: "500Ti",
511+
},
512+
{
513+
Name: "workerpool",
514+
NumberOfNodes: 1,
515+
CPU: "1GHz",
516+
Memory: "1Gi",
517+
DiskSpace: "100Ti",
518+
},
519+
},
520+
},
521+
expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
522+
ValidationType: "vsphere-compute-resources",
523+
ValidationRule: "validation-cluster-DC0_C1",
524+
Message: "Rule for scope already processed",
525+
Details: []string{},
526+
Failures: []string{"Rule for scope cluster-DC0_C1 already processed"},
527+
Status: corev1.ConditionFalse,
528+
},
529+
State: util.Ptr(vapi.ValidationFailed),
530+
},
531+
},
461532
}
462533

463534
GetResourcePoolAndVMs = func(ctx context.Context, inventoryPath string, finder *find.Finder) (*mo.ResourcePool, *[]mo.VirtualMachine, error) {
@@ -487,8 +558,13 @@ func TestComputeResourcesValidationService_ReconcileComputeResourceValidationRul
487558
return &resourcePool, &virtualmachines, nil
488559
}
489560

561+
seenScopes := map[string]bool{
562+
"resourcepool-DC0_C1": true,
563+
"cluster-DC0_C1": true,
564+
}
565+
490566
for _, tc := range testCases {
491-
vr, err := validationService.ReconcileComputeResourceValidationRule(tc.rule, finder, vcSim.Driver)
567+
vr, err := validationService.ReconcileComputeResourceValidationRule(tc.rule, finder, vcSim.Driver, seenScopes)
492568
util.CheckTestCase(t, vr, tc.expectedResult, err, tc.expectedErr)
493569
}
494570
}

0 commit comments

Comments
 (0)