Skip to content

Commit a5d501b

Browse files
authored
fix(bundler): skip components with overrides.enabled: false (#382)
1 parent 3af8c01 commit a5d501b

File tree

4 files changed

+151
-0
lines changed

4 files changed

+151
-0
lines changed

pkg/bundler/bundler.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,33 @@ func (b *DefaultBundler) Make(ctx context.Context, input recipe.RecipeInput, dir
188188
"recipe must contain at least one component reference")
189189
}
190190

191+
// Filter out disabled components (overrides.enabled: false)
192+
enabledRefs := make([]recipe.ComponentRef, 0, len(recipeResult.ComponentRefs))
193+
enabledSet := make(map[string]struct{})
194+
for _, ref := range recipeResult.ComponentRefs {
195+
if !ref.IsEnabled() {
196+
slog.Info("skipping disabled component", "component", ref.Name)
197+
continue
198+
}
199+
enabledRefs = append(enabledRefs, ref)
200+
enabledSet[ref.Name] = struct{}{}
201+
}
202+
recipeResult.ComponentRefs = enabledRefs
203+
204+
// Filter DeploymentOrder to match enabled components
205+
filteredOrder := make([]string, 0, len(recipeResult.DeploymentOrder))
206+
for _, name := range recipeResult.DeploymentOrder {
207+
if _, ok := enabledSet[name]; ok {
208+
filteredOrder = append(filteredOrder, name)
209+
}
210+
}
211+
recipeResult.DeploymentOrder = filteredOrder
212+
213+
if len(enabledRefs) == 0 {
214+
return nil, errors.New(errors.ErrCodeInvalidRequest,
215+
"recipe has no enabled components after filtering")
216+
}
217+
191218
// Set default output directory
192219
if dir == "" {
193220
dir = "."

pkg/bundler/bundler_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,70 @@ func TestMake_Success(t *testing.T) {
271271
}
272272
}
273273

274+
func TestMake_DisabledComponentsFiltered(t *testing.T) {
275+
bundler, err := New()
276+
if err != nil {
277+
t.Fatalf("New() error = %v", err)
278+
}
279+
280+
ctx := context.Background()
281+
tmpDir := t.TempDir()
282+
283+
recipeResult := &recipe.RecipeResult{
284+
APIVersion: "aicr.nvidia.com/v1alpha1",
285+
Kind: "Recipe",
286+
Criteria: &recipe.Criteria{
287+
Service: "eks",
288+
Accelerator: "h100",
289+
Intent: "training",
290+
},
291+
ComponentRefs: []recipe.ComponentRef{
292+
{
293+
Name: "gpu-operator",
294+
Version: "v25.3.3",
295+
Type: "helm",
296+
Source: "https://helm.ngc.nvidia.com/nvidia",
297+
},
298+
{
299+
Name: "aws-ebs-csi-driver",
300+
Version: "2.55.0",
301+
Type: "helm",
302+
Source: "https://kubernetes-sigs.github.io/aws-ebs-csi-driver",
303+
Overrides: map[string]any{"enabled": false},
304+
},
305+
},
306+
DeploymentOrder: []string{"gpu-operator", "aws-ebs-csi-driver"},
307+
}
308+
309+
output, err := bundler.Make(ctx, recipeResult, tmpDir)
310+
if err != nil {
311+
t.Fatalf("Make() error = %v", err)
312+
}
313+
314+
if output == nil {
315+
t.Fatal("Make() returned nil output")
316+
}
317+
318+
// Enabled component should have a directory
319+
if _, statErr := os.Stat(filepath.Join(tmpDir, "gpu-operator", "values.yaml")); os.IsNotExist(statErr) {
320+
t.Error("expected gpu-operator/values.yaml to be created")
321+
}
322+
323+
// Disabled component should NOT have a directory
324+
if _, statErr := os.Stat(filepath.Join(tmpDir, "aws-ebs-csi-driver")); !os.IsNotExist(statErr) {
325+
t.Error("expected aws-ebs-csi-driver directory to NOT be created")
326+
}
327+
328+
// deploy.sh should not reference the disabled component
329+
deployScript, readErr := os.ReadFile(filepath.Join(tmpDir, "deploy.sh"))
330+
if readErr != nil {
331+
t.Fatalf("failed to read deploy.sh: %v", readErr)
332+
}
333+
if strings.Contains(string(deployScript), "aws-ebs-csi-driver") {
334+
t.Error("deploy.sh should not contain aws-ebs-csi-driver")
335+
}
336+
}
337+
274338
func TestMake_WithValueOverrides(t *testing.T) {
275339
cfg := config.NewConfig(
276340
config.WithValueOverrides(map[string]map[string]string{

pkg/recipe/metadata.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package recipe
1717

1818
import (
1919
"fmt"
20+
"log/slog"
2021
"maps"
2122
"sort"
2223
"strings"
@@ -119,6 +120,23 @@ type ComponentRef struct {
119120
HealthCheckAsserts string `json:"healthCheckAsserts,omitempty" yaml:"healthCheckAsserts,omitempty"`
120121
}
121122

123+
// IsEnabled returns whether this component is enabled for deployment.
124+
// A component is disabled when its Overrides map contains enabled: false.
125+
// Components without an explicit enabled override are enabled by default.
126+
func (c ComponentRef) IsEnabled() bool {
127+
v, ok := c.Overrides["enabled"]
128+
if !ok {
129+
return true
130+
}
131+
enabled, ok := v.(bool)
132+
if !ok {
133+
slog.Warn("overrides.enabled is not a bool, treating component as enabled",
134+
"component", c.Name, "value", v)
135+
return true
136+
}
137+
return enabled
138+
}
139+
122140
// ExpectedResource represents a Kubernetes resource that should exist after deployment.
123141
type ExpectedResource struct {
124142
// Kind is the resource kind (e.g., "Deployment", "DaemonSet").

pkg/recipe/metadata_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,48 @@ func TestRecipeMetadataSpecMerge(t *testing.T) {
286286
}
287287
}
288288

289+
func TestComponentRefIsEnabled(t *testing.T) {
290+
tests := []struct {
291+
name string
292+
ref ComponentRef
293+
expected bool
294+
}{
295+
{
296+
name: "no overrides",
297+
ref: ComponentRef{Name: "gpu-operator"},
298+
expected: true,
299+
},
300+
{
301+
name: "enabled true",
302+
ref: ComponentRef{Name: "gpu-operator", Overrides: map[string]any{"enabled": true}},
303+
expected: true,
304+
},
305+
{
306+
name: "enabled false",
307+
ref: ComponentRef{Name: "aws-ebs-csi-driver", Overrides: map[string]any{"enabled": false}},
308+
expected: false,
309+
},
310+
{
311+
name: "enabled string false is not recognized",
312+
ref: ComponentRef{Name: "test", Overrides: map[string]any{"enabled": "false"}},
313+
expected: true,
314+
},
315+
{
316+
name: "other overrides no enabled key",
317+
ref: ComponentRef{Name: "test", Overrides: map[string]any{"replicas": 3}},
318+
expected: true,
319+
},
320+
}
321+
for _, tt := range tests {
322+
t.Run(tt.name, func(t *testing.T) {
323+
got := tt.ref.IsEnabled()
324+
if got != tt.expected {
325+
t.Errorf("IsEnabled() = %v, want %v", got, tt.expected)
326+
}
327+
})
328+
}
329+
}
330+
289331
// TestComponentRefMergeInheritsFromBase verifies that when an overlay specifies
290332
// only partial fields for a component, the missing fields are inherited from base.
291333
func TestComponentRefMergeInheritsFromBase(t *testing.T) {

0 commit comments

Comments
 (0)