Skip to content

Commit 54c326d

Browse files
committed
fix: manual ExpectedResources take precedence over HealthCheckAsserts
1 parent 6bf4cfb commit 54c326d

File tree

4 files changed

+37
-74
lines changed

4 files changed

+37
-74
lines changed

pkg/validator/checks/deployment/expected_resources_check.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,10 @@ func validateExpectedResources(ctx *checks.ValidationContext) error {
5656
var failures []string
5757

5858
for _, ref := range ctx.Recipe.ComponentRefs {
59-
if ref.HealthCheckAsserts != "" {
59+
// Manual expectedResources take precedence over Chainsaw health check asserts.
60+
// This allows users to override the registry's healthCheck.assertFile by
61+
// declaring expectedResources explicitly in the recipe.
62+
if ref.HealthCheckAsserts != "" && len(ref.ExpectedResources) == 0 {
6063
chainsawAsserts = append(chainsawAsserts, chainsaw.ComponentAssert{
6164
Name: ref.Name,
6265
AssertYAML: ref.HealthCheckAsserts,

pkg/validator/checks/deployment/expected_resources_check_unit_test.go

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -440,12 +440,10 @@ func TestValidateExpectedResources_ChainsawBranch(t *testing.T) {
440440
errContains string
441441
}{
442442
{
443-
name: "component with HealthCheckAsserts skips typed client checks",
443+
name: "manual expectedResources take precedence over HealthCheckAsserts",
444444
setup: func() *checks.ValidationContext {
445-
// No K8s objects — if the typed client path ran, it would fail.
446-
// But since HealthCheckAsserts is set, it should go to chainsaw path.
447-
// Use a nonexistent deployment name so chainsaw always fails even if
448-
// the binary is installed and a local cluster is accessible.
445+
// No K8s objects — typed client check will fail for the missing Deployment.
446+
// Even though HealthCheckAsserts is set, manual expectedResources take precedence.
449447
//nolint:staticcheck // SA1019: fake.NewSimpleClientset is sufficient for tests
450448
clientset := fake.NewSimpleClientset()
451449
return &checks.ValidationContext{
@@ -454,10 +452,9 @@ func TestValidateExpectedResources_ChainsawBranch(t *testing.T) {
454452
Recipe: &recipe.RecipeResult{
455453
ComponentRefs: []recipe.ComponentRef{
456454
{
457-
Name: "test-chainsaw-component",
455+
Name: "test-component",
458456
Type: "Helm",
459-
HealthCheckAsserts: "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: nonexistent-chainsaw-test\n namespace: nonexistent-ns\n",
460-
// These expectedResources should NOT be checked (chainsaw path)
457+
HealthCheckAsserts: "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: something\n",
461458
ExpectedResources: []recipe.ExpectedResource{
462459
{Kind: "Deployment", Name: "gpu-operator", Namespace: "gpu-operator"},
463460
},
@@ -466,6 +463,30 @@ func TestValidateExpectedResources_ChainsawBranch(t *testing.T) {
466463
},
467464
}
468465
},
466+
// Typed client fails because the Deployment doesn't exist in the fake clientset
467+
wantErr: true,
468+
errContains: "not found",
469+
},
470+
{
471+
name: "HealthCheckAsserts used when no manual expectedResources",
472+
setup: func() *checks.ValidationContext {
473+
// No manual expectedResources → Chainsaw path activates.
474+
//nolint:staticcheck // SA1019: fake.NewSimpleClientset is sufficient for tests
475+
clientset := fake.NewSimpleClientset()
476+
return &checks.ValidationContext{
477+
Context: context.Background(),
478+
Clientset: clientset,
479+
Recipe: &recipe.RecipeResult{
480+
ComponentRefs: []recipe.ComponentRef{
481+
{
482+
Name: "test-chainsaw-component",
483+
Type: "Helm",
484+
HealthCheckAsserts: "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: nonexistent-chainsaw-test\n namespace: nonexistent-ns\n",
485+
},
486+
},
487+
},
488+
}
489+
},
469490
// Chainsaw fails: either binary not available or assertion doesn't match
470491
wantErr: true,
471492
errContains: "chainsaw health check failed",

pkg/validator/resource_discovery.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,9 @@ func resolveExpectedResources(ctx context.Context, recipeResult *recipe.RecipeRe
114114

115115
ref := &recipeResult.ComponentRefs[i]
116116

117-
// Skip auto-discovery for components with Chainsaw health check asserts.
118-
// These components use Chainsaw CLI assertions instead of typed replica checks.
119-
if ref.HealthCheckAsserts != "" {
117+
// Skip auto-discovery for components with Chainsaw health check asserts,
118+
// unless the recipe already has manual expectedResources (which take precedence).
119+
if ref.HealthCheckAsserts != "" && len(ref.ExpectedResources) == 0 {
120120
slog.Debug("skipping auto-discovery for component with chainsaw health check",
121121
"component", ref.Name)
122122
continue

tests/e2e/run.sh

Lines changed: 1 addition & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,68 +1115,7 @@ YAML
11151115
# Wait for deployment to be available
11161116
kubectl wait --for=condition=available deployment/gpu-operator -n gpu-operator --timeout=60s 2>&1 || true
11171117

1118-
# Test 1: Validate expected-resources with passing check (resource exists)
1119-
msg "--- Test: Expected resources check (should pass) ---"
1120-
local recipe_file="${validate_dir}/recipe-expected-resources.yaml"
1121-
cat > "$recipe_file" <<RECIPE
1122-
kind: RecipeResult
1123-
apiVersion: aicr.nvidia.com/v1alpha1
1124-
metadata:
1125-
version: dev
1126-
componentRefs:
1127-
- name: fake-gpu-operator
1128-
type: Helm
1129-
namespace: gpu-operator
1130-
expectedResources:
1131-
- kind: Deployment
1132-
name: gpu-operator
1133-
namespace: gpu-operator
1134-
validation:
1135-
deployment:
1136-
checks:
1137-
- expected-resources
1138-
RECIPE
1139-
1140-
echo -e "${DIM} \$ aicr validate --phase deployment --recipe recipe.yaml${NC}"
1141-
local result_file="${validate_dir}/result-pass.yaml"
1142-
local result_output
1143-
result_output=$("${AICR_BIN}" validate \
1144-
--recipe "$recipe_file" \
1145-
--snapshot "cm://${SNAPSHOT_NAMESPACE}/${SNAPSHOT_CM}" \
1146-
--phase deployment \
1147-
--image "${AICR_VALIDATOR_IMAGE}" \
1148-
--output "$result_file" 2>&1) || true
1149-
1150-
# DEBUG: Print captured output to see what's happening
1151-
detail "Captured validation output:"
1152-
echo "$result_output" | sed 's/^/ /'
1153-
1154-
# Check the output file for expected-resources check results
1155-
if [ -f "$result_file" ]; then
1156-
detail "Validation output file created: $result_file"
1157-
else
1158-
detail "Validation output file NOT created: $result_file"
1159-
fi
1160-
1161-
if [ -f "$result_file" ] && \
1162-
grep -q "TestCheckExpectedResources" "$result_file"; then
1163-
if grep -A1 "name: TestCheckExpectedResources" "$result_file" | grep -q "status: pass"; then
1164-
detail "Expected-resources check: PASS (gpu-operator deployment found)"
1165-
pass "validate/expected-resources-pass"
1166-
elif grep -q "summary:" "$result_file" && grep -q "status: pass" "$result_file"; then
1167-
# Fallback: check summary status
1168-
detail "Expected-resources check: PASS (from summary status)"
1169-
pass "validate/expected-resources-pass"
1170-
else
1171-
detail "Check found but status unclear. Showing check section:"
1172-
grep -A5 "TestCheckExpectedResources" "$result_file" | sed 's/^/ /' || true
1173-
fail "validate/expected-resources-pass" "Check did not pass"
1174-
fi
1175-
else
1176-
fail "validate/expected-resources-pass" "TestCheckExpectedResources not found in output"
1177-
fi
1178-
1179-
# Test 2: Validate expected-resources with failing check (resource missing)
1118+
# Test 1: Validate expected-resources with failing check (resource missing)
11801119
msg "--- Test: Expected resources check (should fail - missing resource) ---"
11811120
local recipe_file_fail="${validate_dir}/recipe-expected-resources-fail.yaml"
11821121
cat > "$recipe_file_fail" <<RECIPE

0 commit comments

Comments
 (0)