From 55e74f064483b76fba0c6944c5ce7cac7895b1fb Mon Sep 17 00:00:00 2001 From: caretak3r <50377477+caretak3r@users.noreply.github.com> Date: Thu, 19 Feb 2026 20:23:47 -0500 Subject: [PATCH] feat(hip-0025): implement resource-group sequencing, custom readiness, upgrade/rollback/uninstall ordering, template output, and lint rules Complete HIP-0025 implementation across worktrees 2-7: - Resource-group sequencing: parse helm.sh/resource-group and helm.sh/depends-on/resource-groups annotations, build per-chart DAG, deploy in batch order with unsequenced resources last - Custom readiness: JSONPath-based readiness-success/readiness-failure evaluation engine with --readiness-timeout flag - Upgrade/rollback/uninstall: ordered upgrade mirrors install batching, reverse-order deletion for uninstall/rollback using stored SequencingMetadata - helm template: reorder manifest output by DAG batch order with resource-group delimiter comments - Lint rules: validate subchart depends-on references and detect DAG cycles at lint time - DAG visualization: String() method for human-readable batch output Co-Authored-By: Claude Opus 4.6 --- pkg/action/install.go | 77 +++++- pkg/action/readiness.go | 282 +++++++++++++++++++++ pkg/action/readiness_test.go | 221 ++++++++++++++++ pkg/action/rollback.go | 193 ++++++++++---- pkg/action/sequencing.go | 135 ++++++++++ pkg/action/sequencing_ordered_test.go | 105 ++++++++ pkg/action/sequencing_test.go | 132 ++++++++++ pkg/action/uninstall.go | 78 +++++- pkg/action/upgrade.go | 179 +++++++++++-- pkg/chart/v2/lint/lint.go | 1 + pkg/chart/v2/lint/rules/sequencing.go | 134 ++++++++++ pkg/chart/v2/lint/rules/sequencing_test.go | 145 +++++++++++ pkg/chart/v2/util/dag.go | 22 ++ pkg/chart/v2/util/dag_test.go | 12 + pkg/chart/v2/util/sequencing.go | 119 +++++++++ pkg/chart/v2/util/sequencing_test.go | 137 ++++++++++ pkg/cmd/flags.go | 9 + pkg/cmd/template.go | 10 +- pkg/release/v1/release.go | 3 + 19 files changed, 1912 insertions(+), 82 deletions(-) create mode 100644 pkg/action/readiness.go create mode 100644 pkg/action/readiness_test.go create mode 100644 pkg/action/sequencing_ordered_test.go create mode 100644 pkg/chart/v2/lint/rules/sequencing.go create mode 100644 pkg/chart/v2/lint/rules/sequencing_test.go diff --git a/pkg/action/install.go b/pkg/action/install.go index 9d224d2bb..d449e3443 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -584,7 +584,7 @@ func (i *Install) performOrderedInstall(rel *release.Release, toBeAdopted kube.R i.cfg.Logger().Info(fmt.Sprintf("ordered install: %d batches to deploy", len(batches))) for batchIdx, batch := range batches { - i.cfg.Logger().Info(fmt.Sprintf("deploying batch %d: %v", batchIdx+1, batch)) + i.cfg.Logger().Info(fmt.Sprintf("deploying subchart batch %d: %v", batchIdx+1, batch)) // Collect manifests for this batch var batchManifest string @@ -601,17 +601,84 @@ func (i *Install) performOrderedInstall(rel *release.Release, toBeAdopted kube.R continue } - // Build resource list from batch manifest + // Check for resource-group sequencing within this subchart batch + if err := i.deployWithResourceGroupSequencing(batchManifest, batchIdx+1, batch); err != nil { + return err + } + } + + return nil +} + +// deployWithResourceGroupSequencing deploys a batch manifest, applying resource-group +// ordering if resource-group annotations are present. If no annotations are found, +// deploys all resources at once. +func (i *Install) deployWithResourceGroupSequencing(batchManifest string, batchIdx int, batchNames []string) error { + rgBatches, err := BuildResourceGroupBatches(batchManifest) + if err != nil { + return fmt.Errorf("subchart batch %d (%v): %w", batchIdx, batchNames, err) + } + + if rgBatches == nil { + // No resource-group annotations — deploy all at once batchResources, err := i.cfg.KubeClient.Build( bytes.NewBufferString(batchManifest), !i.DisableOpenAPIValidation, ) if err != nil { - return fmt.Errorf("failed to build resources for batch %d: %w", batchIdx+1, err) + return fmt.Errorf("failed to build resources for batch %d: %w", batchIdx, err) } - if err := i.createAndWaitResources(nil, batchResources); err != nil { - return fmt.Errorf("batch %d (%v) failed: %w", batchIdx+1, batch, err) + return fmt.Errorf("batch %d (%v) failed: %w", batchIdx, batchNames, err) + } + return nil + } + + // Deploy resource groups in DAG batch order + for rgIdx, rgBatch := range rgBatches.Batches { + i.cfg.Logger().Info(fmt.Sprintf("deploying resource-group batch %d.%d: %v", batchIdx, rgIdx+1, rgBatch)) + + var rgManifest string + for _, groupName := range rgBatch { + if m, ok := rgBatches.GroupedManifests[groupName]; ok { + if rgManifest != "" { + rgManifest += "\n---\n" + } + rgManifest += m + } + } + + if rgManifest == "" { + continue + } + + rgResources, err := i.cfg.KubeClient.Build( + bytes.NewBufferString(rgManifest), + !i.DisableOpenAPIValidation, + ) + if err != nil { + return fmt.Errorf("failed to build resources for resource-group batch %d.%d: %w", batchIdx, rgIdx+1, err) + } + + if err := i.createAndWaitResources(nil, rgResources); err != nil { + return fmt.Errorf("resource-group batch %d.%d (%v) failed: %w", batchIdx, rgIdx+1, rgBatch, err) + } + } + + // Deploy unsequenced resources last + if rgBatches.UnsequencedManifest != "" { + i.cfg.Logger().Info(fmt.Sprintf("deploying unsequenced resources for batch %d", batchIdx)) + + unResources, err := i.cfg.KubeClient.Build( + bytes.NewBufferString(rgBatches.UnsequencedManifest), + !i.DisableOpenAPIValidation, + ) + if err != nil { + return fmt.Errorf("failed to build unsequenced resources for batch %d: %w", batchIdx, err) + } + + if err := i.createAndWaitResources(nil, unResources); err != nil { + return fmt.Errorf("unsequenced resources for batch %d (%v) failed: %w", batchIdx, batchNames, err) } } diff --git a/pkg/action/readiness.go b/pkg/action/readiness.go new file mode 100644 index 000000000..4f56100d1 --- /dev/null +++ b/pkg/action/readiness.go @@ -0,0 +1,282 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "encoding/json" + "fmt" + "log/slog" + "strconv" + "strings" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/util/jsonpath" +) + +const ( + // AnnotationReadinessSuccess is the annotation key for custom readiness success conditions. + // Value is a JSON array of "{.jsonpath} op value" expressions. + AnnotationReadinessSuccess = "helm.sh/readiness-success" + + // AnnotationReadinessFailure is the annotation key for custom readiness failure conditions. + // Value is a JSON array of "{.jsonpath} op value" expressions. + // Failure takes precedence over success. + AnnotationReadinessFailure = "helm.sh/readiness-failure" +) + +// ReadinessResult represents the outcome of a custom readiness evaluation. +type ReadinessResult int + +const ( + // ReadinessUnknown means readiness could not be determined (fall back to kstatus). + ReadinessUnknown ReadinessResult = iota + // ReadinessReady means all success conditions are met and no failure conditions triggered. + ReadinessReady + // ReadinessFailed means a failure condition was triggered. + ReadinessFailed + // ReadinessPending means no failure triggered but success conditions not yet met. + ReadinessPending +) + +// ReadinessExpression represents a parsed "{.jsonpath} op value" expression. +type ReadinessExpression struct { + // JSONPath is the path expression scoped to the resource object (e.g., "{.status.phase}"). + JSONPath string + // Operator is the comparison operator (==, !=, <, <=, >, >=). + Operator string + // Value is the expected scalar value to compare against. + Value string +} + +// ParseReadinessExpressions parses a JSON array of readiness expressions. +// Each expression has the format: "{.jsonpath} op value" +func ParseReadinessExpressions(raw string) ([]ReadinessExpression, error) { + if raw == "" { + return nil, nil + } + + var exprs []string + if err := json.Unmarshal([]byte(raw), &exprs); err != nil { + return nil, fmt.Errorf("invalid readiness expression JSON: %w", err) + } + + result := make([]ReadinessExpression, 0, len(exprs)) + for _, expr := range exprs { + parsed, err := parseOneExpression(expr) + if err != nil { + return nil, fmt.Errorf("invalid readiness expression %q: %w", expr, err) + } + result = append(result, parsed) + } + return result, nil +} + +// parseOneExpression parses a single "{.jsonpath} op value" string. +func parseOneExpression(expr string) (ReadinessExpression, error) { + expr = strings.TrimSpace(expr) + if expr == "" { + return ReadinessExpression{}, fmt.Errorf("empty expression") + } + + // Find the end of the JSONPath (closing brace) + braceEnd := strings.Index(expr, "}") + if braceEnd < 0 || !strings.HasPrefix(expr, "{") { + return ReadinessExpression{}, fmt.Errorf("expression must start with a JSONPath like {.status.phase}") + } + + jp := expr[:braceEnd+1] + rest := strings.TrimSpace(expr[braceEnd+1:]) + + // Parse operator + var op string + for _, candidate := range []string{"==", "!=", "<=", ">=", "<", ">"} { + if strings.HasPrefix(rest, candidate) { + op = candidate + rest = strings.TrimSpace(rest[len(candidate):]) + break + } + } + if op == "" { + return ReadinessExpression{}, fmt.Errorf("no valid operator found (expected ==, !=, <, <=, >, >=)") + } + + // Remaining is the value (trimmed) + val := strings.TrimSpace(rest) + if val == "" { + return ReadinessExpression{}, fmt.Errorf("missing comparison value") + } + + return ReadinessExpression{ + JSONPath: jp, + Operator: op, + Value: val, + }, nil +} + +// EvaluateReadiness evaluates custom readiness for an unstructured Kubernetes object. +// Returns ReadinessUnknown if the object has no readiness annotations (or only one of the pair). +// Failure conditions take precedence over success conditions per the spec. +func EvaluateReadiness(obj *unstructured.Unstructured) (ReadinessResult, string, error) { + annotations := obj.GetAnnotations() + if annotations == nil { + return ReadinessUnknown, "", nil + } + + successRaw := annotations[AnnotationReadinessSuccess] + failureRaw := annotations[AnnotationReadinessFailure] + + // Both must be present; if only one, warn and fall back + hasSuccess := successRaw != "" + hasFailure := failureRaw != "" + + if !hasSuccess && !hasFailure { + return ReadinessUnknown, "", nil + } + if hasSuccess != hasFailure { + which := AnnotationReadinessSuccess + if hasFailure { + which = AnnotationReadinessFailure + } + slog.Warn("only one readiness annotation present, both required; falling back to kstatus", + "annotation", which, + "resource", obj.GetName(), + ) + return ReadinessUnknown, "incomplete readiness annotations", nil + } + + // Parse failure expressions (evaluate first — failure takes precedence) + failureExprs, err := ParseReadinessExpressions(failureRaw) + if err != nil { + return ReadinessUnknown, "", fmt.Errorf("resource %s: %w", obj.GetName(), err) + } + + successExprs, err := ParseReadinessExpressions(successRaw) + if err != nil { + return ReadinessUnknown, "", fmt.Errorf("resource %s: %w", obj.GetName(), err) + } + + // Check failure conditions first + for _, expr := range failureExprs { + match, err := evaluateExpression(obj.Object, expr) + if err != nil { + // If we can't evaluate, skip this expression + continue + } + if match { + return ReadinessFailed, fmt.Sprintf("failure condition met: %s %s %s", expr.JSONPath, expr.Operator, expr.Value), nil + } + } + + // Check success conditions + allSuccess := true + for _, expr := range successExprs { + match, err := evaluateExpression(obj.Object, expr) + if err != nil { + allSuccess = false + continue + } + if !match { + allSuccess = false + } + } + + if allSuccess && len(successExprs) > 0 { + return ReadinessReady, "all success conditions met", nil + } + + return ReadinessPending, "waiting for success conditions", nil +} + +// evaluateExpression evaluates a single readiness expression against a resource object. +func evaluateExpression(obj map[string]interface{}, expr ReadinessExpression) (bool, error) { + // Parse and execute the JSONPath + jp := jsonpath.New("readiness") + if err := jp.Parse(expr.JSONPath); err != nil { + return false, fmt.Errorf("invalid JSONPath %q: %w", expr.JSONPath, err) + } + + results, err := jp.FindResults(obj) + if err != nil { + return false, fmt.Errorf("JSONPath %q evaluation failed: %w", expr.JSONPath, err) + } + + if len(results) == 0 || len(results[0]) == 0 { + return false, fmt.Errorf("JSONPath %q returned no results", expr.JSONPath) + } + + // Get the actual value as a string for comparison + actual := fmt.Sprintf("%v", results[0][0].Interface()) + + return compareValues(actual, expr.Operator, expr.Value) +} + +// compareValues compares two string values using the given operator. +// Attempts numeric comparison first; falls back to string comparison. +func compareValues(actual, op, expected string) (bool, error) { + // Try numeric comparison + actualNum, aErr := strconv.ParseFloat(actual, 64) + expectedNum, eErr := strconv.ParseFloat(expected, 64) + if aErr == nil && eErr == nil { + return compareNumeric(actualNum, op, expectedNum) + } + + // String comparison (only == and != are valid for strings) + switch op { + case "==": + return actual == expected, nil + case "!=": + return actual != expected, nil + case "<", "<=", ">", ">=": + // For non-numeric values with ordering operators, compare lexicographically + return compareLexicographic(actual, op, expected) + default: + return false, fmt.Errorf("unknown operator %q", op) + } +} + +func compareNumeric(actual float64, op string, expected float64) (bool, error) { + switch op { + case "==": + return actual == expected, nil + case "!=": + return actual != expected, nil + case "<": + return actual < expected, nil + case "<=": + return actual <= expected, nil + case ">": + return actual > expected, nil + case ">=": + return actual >= expected, nil + default: + return false, fmt.Errorf("unknown operator %q", op) + } +} + +func compareLexicographic(actual, op, expected string) (bool, error) { + switch op { + case "<": + return actual < expected, nil + case "<=": + return actual <= expected, nil + case ">": + return actual > expected, nil + case ">=": + return actual >= expected, nil + default: + return false, fmt.Errorf("unknown operator %q", op) + } +} diff --git a/pkg/action/readiness_test.go b/pkg/action/readiness_test.go new file mode 100644 index 000000000..b7d39262b --- /dev/null +++ b/pkg/action/readiness_test.go @@ -0,0 +1,221 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +func TestParseReadinessExpressions(t *testing.T) { + tests := []struct { + name string + raw string + expected []ReadinessExpression + expectError bool + }{ + { + name: "empty string", + raw: "", + expected: nil, + }, + { + name: "single expression", + raw: `["{.status.phase} == Running"]`, + expected: []ReadinessExpression{ + {JSONPath: "{.status.phase}", Operator: "==", Value: "Running"}, + }, + }, + { + name: "multiple expressions", + raw: `["{.status.replicas} >= 3", "{.status.phase} != Failed"]`, + expected: []ReadinessExpression{ + {JSONPath: "{.status.replicas}", Operator: ">=", Value: "3"}, + {JSONPath: "{.status.phase}", Operator: "!=", Value: "Failed"}, + }, + }, + { + name: "invalid JSON", + raw: "not-json", + expectError: true, + }, + { + name: "invalid expression format", + raw: `["no-jsonpath here"]`, + expectError: true, + }, + { + name: "missing operator", + raw: `["{.status.phase} Running"]`, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := ParseReadinessExpressions(tt.raw) + if tt.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + }) + } +} + +func TestEvaluateReadiness(t *testing.T) { + tests := []struct { + name string + obj *unstructured.Unstructured + expectedResult ReadinessResult + }{ + { + name: "no annotations - unknown", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "test", + }, + }, + }, + expectedResult: ReadinessUnknown, + }, + { + name: "only success annotation - unknown (incomplete pair)", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "test", + "annotations": map[string]interface{}{ + AnnotationReadinessSuccess: `["{.status.phase} == Running"]`, + }, + }, + }, + }, + expectedResult: ReadinessUnknown, + }, + { + name: "success conditions met", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "test", + "annotations": map[string]interface{}{ + AnnotationReadinessSuccess: `["{.status.phase} == Running"]`, + AnnotationReadinessFailure: `["{.status.phase} == Failed"]`, + }, + }, + "status": map[string]interface{}{ + "phase": "Running", + }, + }, + }, + expectedResult: ReadinessReady, + }, + { + name: "failure condition met - takes precedence", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "test", + "annotations": map[string]interface{}{ + AnnotationReadinessSuccess: `["{.status.phase} == Running"]`, + AnnotationReadinessFailure: `["{.status.phase} == Failed"]`, + }, + }, + "status": map[string]interface{}{ + "phase": "Failed", + }, + }, + }, + expectedResult: ReadinessFailed, + }, + { + name: "pending - conditions not yet met", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "test", + "annotations": map[string]interface{}{ + AnnotationReadinessSuccess: `["{.status.phase} == Running"]`, + AnnotationReadinessFailure: `["{.status.phase} == Failed"]`, + }, + }, + "status": map[string]interface{}{ + "phase": "Pending", + }, + }, + }, + expectedResult: ReadinessPending, + }, + { + name: "numeric comparison", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "test", + "annotations": map[string]interface{}{ + AnnotationReadinessSuccess: `["{.status.readyReplicas} >= 3"]`, + AnnotationReadinessFailure: `["{.status.readyReplicas} < 0"]`, + }, + }, + "status": map[string]interface{}{ + "readyReplicas": int64(3), + }, + }, + }, + expectedResult: ReadinessReady, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, _, err := EvaluateReadiness(tt.obj) + require.NoError(t, err) + assert.Equal(t, tt.expectedResult, result) + }) + } +} + +func TestCompareValues(t *testing.T) { + tests := []struct { + actual string + op string + expected string + result bool + }{ + {"Running", "==", "Running", true}, + {"Running", "!=", "Failed", true}, + {"3", ">=", "3", true}, + {"5", ">", "3", true}, + {"2", "<", "3", true}, + {"3", "<=", "3", true}, + {"abc", "<", "bcd", true}, + } + + for _, tt := range tests { + t.Run(tt.actual+" "+tt.op+" "+tt.expected, func(t *testing.T) { + result, err := compareValues(tt.actual, tt.op, tt.expected) + require.NoError(t, err) + assert.Equal(t, tt.result, result) + }) + } +} diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go index 459569781..b6b7515a9 100644 --- a/pkg/action/rollback.go +++ b/pkg/action/rollback.go @@ -224,57 +224,72 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas if err != nil { return targetRelease, fmt.Errorf("unable to set metadata visitor from target release: %w", err) } - results, err := r.cfg.KubeClient.Update( - current, - target, - kube.ClientUpdateOptionForceReplace(r.ForceReplace), - kube.ClientUpdateOptionServerSideApply(serverSideApply, r.ForceConflicts), - kube.ClientUpdateOptionThreeWayMergeForUnstructured(false), - kube.ClientUpdateOptionUpgradeClientSideFieldManager(true)) - if err != nil { - msg := fmt.Sprintf("Rollback %q failed: %s", targetRelease.Name, err) - r.cfg.Logger().Warn(msg) - currentRelease.Info.Status = common.StatusSuperseded - targetRelease.Info.Status = common.StatusFailed - targetRelease.Info.Description = msg - r.cfg.recordRelease(currentRelease) - r.cfg.recordRelease(targetRelease) - if r.CleanupOnFail { - r.cfg.Logger().Debug("cleanup on fail set, cleaning up resources", "count", len(results.Created)) - _, errs := r.cfg.KubeClient.Delete(results.Created, metav1.DeletePropagationBackground) - if errs != nil { - return targetRelease, fmt.Errorf( - "an error occurred while cleaning up resources. original rollback error: %w", - fmt.Errorf("unable to cleanup resources: %w", joinErrors(errs, ", "))) - } - r.cfg.Logger().Debug("resource cleanup complete") - } - return targetRelease, err - } - - var waiter kube.Waiter - if c, supportsOptions := r.cfg.KubeClient.(kube.InterfaceWaitOptions); supportsOptions { - waiter, err = c.GetWaiterWithOptions(r.WaitStrategy, r.WaitOptions...) - } else { - waiter, err = r.cfg.KubeClient.GetWaiter(r.WaitStrategy) - } - if err != nil { - return nil, fmt.Errorf("unable to get waiter: %w", err) - } - if r.WaitForJobs { - if err := waiter.WaitWithJobs(target, r.Timeout); err != nil { - targetRelease.SetStatus(common.StatusFailed, fmt.Sprintf("Release %q failed: %s", targetRelease.Name, err.Error())) + // HIP-0025: If target release has sequencing metadata, apply in stored batch order + if r.WaitStrategy == kube.OrderedStrategy && targetRelease.Sequencing != nil && targetRelease.Sequencing.Enabled { + if err := r.performOrderedRollback(currentRelease, targetRelease, serverSideApply); err != nil { + msg := fmt.Sprintf("Rollback %q failed: %s", targetRelease.Name, err) + r.cfg.Logger().Warn(msg) + currentRelease.Info.Status = common.StatusSuperseded + targetRelease.Info.Status = common.StatusFailed + targetRelease.Info.Description = msg r.cfg.recordRelease(currentRelease) r.cfg.recordRelease(targetRelease) - return targetRelease, fmt.Errorf("release %s failed: %w", targetRelease.Name, err) + return targetRelease, err } } else { - if err := waiter.Wait(target, r.Timeout); err != nil { - targetRelease.SetStatus(common.StatusFailed, fmt.Sprintf("Release %q failed: %s", targetRelease.Name, err.Error())) + results, err := r.cfg.KubeClient.Update( + current, + target, + kube.ClientUpdateOptionForceReplace(r.ForceReplace), + kube.ClientUpdateOptionServerSideApply(serverSideApply, r.ForceConflicts), + kube.ClientUpdateOptionThreeWayMergeForUnstructured(false), + kube.ClientUpdateOptionUpgradeClientSideFieldManager(true)) + + if err != nil { + msg := fmt.Sprintf("Rollback %q failed: %s", targetRelease.Name, err) + r.cfg.Logger().Warn(msg) + currentRelease.Info.Status = common.StatusSuperseded + targetRelease.Info.Status = common.StatusFailed + targetRelease.Info.Description = msg r.cfg.recordRelease(currentRelease) r.cfg.recordRelease(targetRelease) - return targetRelease, fmt.Errorf("release %s failed: %w", targetRelease.Name, err) + if r.CleanupOnFail { + r.cfg.Logger().Debug("cleanup on fail set, cleaning up resources", "count", len(results.Created)) + _, errs := r.cfg.KubeClient.Delete(results.Created, metav1.DeletePropagationBackground) + if errs != nil { + return targetRelease, fmt.Errorf( + "an error occurred while cleaning up resources. original rollback error: %w", + fmt.Errorf("unable to cleanup resources: %w", joinErrors(errs, ", "))) + } + r.cfg.Logger().Debug("resource cleanup complete") + } + return targetRelease, err + } + + var waiter kube.Waiter + if c, supportsOptions := r.cfg.KubeClient.(kube.InterfaceWaitOptions); supportsOptions { + waiter, err = c.GetWaiterWithOptions(r.WaitStrategy, r.WaitOptions...) + } else { + waiter, err = r.cfg.KubeClient.GetWaiter(r.WaitStrategy) + } + if err != nil { + return nil, fmt.Errorf("unable to get waiter: %w", err) + } + if r.WaitForJobs { + if err := waiter.WaitWithJobs(target, r.Timeout); err != nil { + targetRelease.SetStatus(common.StatusFailed, fmt.Sprintf("Release %q failed: %s", targetRelease.Name, err.Error())) + r.cfg.recordRelease(currentRelease) + r.cfg.recordRelease(targetRelease) + return targetRelease, fmt.Errorf("release %s failed: %w", targetRelease.Name, err) + } + } else { + if err := waiter.Wait(target, r.Timeout); err != nil { + targetRelease.SetStatus(common.StatusFailed, fmt.Sprintf("Release %q failed: %s", targetRelease.Name, err.Error())) + r.cfg.recordRelease(currentRelease) + r.cfg.recordRelease(targetRelease) + return targetRelease, fmt.Errorf("release %s failed: %w", targetRelease.Name, err) + } } } @@ -304,3 +319,93 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas return targetRelease, nil } + +// performOrderedRollback applies the target release's resources in stored batch order +// and deletes the current release's resources in reverse batch order. +func (r *Rollback) performOrderedRollback(currentRelease, targetRelease *release.Release, serverSideApply bool) error { + batches := targetRelease.Sequencing.Batches + + // Delete current release resources in reverse order (if current was also sequenced) + if currentRelease.Sequencing != nil && currentRelease.Sequencing.Enabled { + currentSubcharts := SplitManifestsBySubchart(currentRelease.Manifest, currentRelease.Chart.Metadata.Name) + currentBatches := currentRelease.Sequencing.Batches + + r.cfg.Logger().Info(fmt.Sprintf("ordered rollback: deleting current release in reverse order (%d batches)", len(currentBatches))) + + for i := len(currentBatches) - 1; i >= 0; i-- { + batch := currentBatches[i] + var batchManifest string + for _, name := range batch { + if m, ok := currentSubcharts[name]; ok { + if batchManifest != "" { + batchManifest += "\n---\n" + } + batchManifest += m + } + } + if batchManifest == "" { + continue + } + resources, err := r.cfg.KubeClient.Build(bytes.NewBufferString(batchManifest), false) + if err != nil { + continue // best-effort deletion + } + r.cfg.KubeClient.Delete(resources, metav1.DeletePropagationBackground) + } + } + + // Install target release resources in forward batch order + targetSubcharts := SplitManifestsBySubchart(targetRelease.Manifest, targetRelease.Chart.Metadata.Name) + + r.cfg.Logger().Info(fmt.Sprintf("ordered rollback: installing target release (%d batches)", len(batches))) + + for batchIdx, batch := range batches { + var batchManifest string + for _, name := range batch { + if m, ok := targetSubcharts[name]; ok { + if batchManifest != "" { + batchManifest += "\n---\n" + } + batchManifest += m + } + } + if batchManifest == "" { + continue + } + + batchResources, err := r.cfg.KubeClient.Build(bytes.NewBufferString(batchManifest), false) + if err != nil { + return fmt.Errorf("failed to build target resources for rollback batch %d: %w", batchIdx+1, err) + } + + err = batchResources.Visit(setMetadataVisitor(targetRelease.Name, targetRelease.Namespace, true)) + if err != nil { + return err + } + + // Use Update with empty current to handle create-or-update + _, err = r.cfg.KubeClient.Update( + nil, batchResources, + kube.ClientUpdateOptionForceReplace(r.ForceReplace), + kube.ClientUpdateOptionServerSideApply(serverSideApply, r.ForceConflicts), + kube.ClientUpdateOptionUpgradeClientSideFieldManager(true)) + if err != nil { + return fmt.Errorf("rollback batch %d (%v) failed: %w", batchIdx+1, batch, err) + } + + var waiter kube.Waiter + if wc, supportsOptions := r.cfg.KubeClient.(kube.InterfaceWaitOptions); supportsOptions { + waiter, err = wc.GetWaiterWithOptions(r.WaitStrategy, r.WaitOptions...) + } else { + waiter, err = r.cfg.KubeClient.GetWaiter(r.WaitStrategy) + } + if err != nil { + return err + } + if err := waiter.Wait(batchResources, r.Timeout); err != nil { + return fmt.Errorf("rollback batch %d (%v) wait failed: %w", batchIdx+1, batch, err) + } + } + + return nil +} diff --git a/pkg/action/sequencing.go b/pkg/action/sequencing.go index 04b6c5d2a..5fb99e10d 100644 --- a/pkg/action/sequencing.go +++ b/pkg/action/sequencing.go @@ -17,6 +17,7 @@ package action import ( "fmt" + "log/slog" "strings" chart "helm.sh/helm/v4/pkg/chart/v2" @@ -82,6 +83,140 @@ func subchartFromSourcePath(sourcePath, chartName string) string { return chartName } +// SplitManifestDocs splits a rendered manifest string into individual YAML documents. +func SplitManifestDocs(manifest string) []string { + var docs []string + for _, doc := range strings.Split(manifest, "---") { + doc = strings.TrimSpace(doc) + if doc == "" { + continue + } + docs = append(docs, doc) + } + return docs +} + +// ResourceGroupBatches holds the ordered deployment plan for resource-group sequencing. +type ResourceGroupBatches struct { + // Batches is the ordered list of resource-group batches. + // Each batch is a list of group names that can be deployed in parallel. + Batches [][]string + // GroupedManifests maps group name to concatenated manifest YAML. + GroupedManifests map[string]string + // UnsequencedManifest is the combined manifest for resources without a resource-group. + UnsequencedManifest string + // Warnings contains any non-fatal issues found during parsing. + Warnings []string +} + +// BuildResourceGroupBatches analyzes rendered manifests for a single chart/subchart +// and produces ordered batches based on resource-group annotations. +// Returns nil if no resource-group annotations are found. +func BuildResourceGroupBatches(manifest string) (*ResourceGroupBatches, error) { + docs := SplitManifestDocs(manifest) + if len(docs) == 0 { + return nil, nil + } + + dag, grouped, unsequenced, warnings, err := chartutil.BuildResourceGroupDAG(docs) + if err != nil { + return nil, fmt.Errorf("failed to build resource-group DAG: %w", err) + } + + // No resource-group annotations found + if dag.Len() == 0 { + return nil, nil + } + + batches, err := dag.Batches() + if err != nil { + return nil, fmt.Errorf("failed to compute resource-group batch order: %w", err) + } + + // Build concatenated manifests per group + groupedManifests := make(map[string]string, len(grouped)) + for group, groupDocs := range grouped { + groupedManifests[group] = strings.Join(groupDocs, "\n---\n") + } + + var unsequencedManifest string + if len(unsequenced) > 0 { + unsequencedManifest = strings.Join(unsequenced, "\n---\n") + } + + // Log warnings + for _, w := range warnings { + slog.Warn("resource-group sequencing warning", "message", w) + } + + return &ResourceGroupBatches{ + Batches: batches, + GroupedManifests: groupedManifests, + UnsequencedManifest: unsequencedManifest, + Warnings: warnings, + }, nil +} + +// ReorderManifestForTemplate reorders a rendered manifest string according to +// HIP-0025 sequencing metadata. Adds resource-group delimiters as comments. +// Returns the original manifest unchanged if no sequencing metadata exists. +func ReorderManifestForTemplate(manifest string, chrt *chart.Chart) string { + batches, err := BuildInstallBatches(chrt) + if err != nil || batches == nil { + return manifest + } + + subchartManifests := SplitManifestsBySubchart(manifest, chrt.Metadata.Name) + + var result strings.Builder + for _, batch := range batches { + for _, subchartName := range batch { + m, ok := subchartManifests[subchartName] + if !ok { + continue + } + + // Check for resource-group sequencing within this subchart + rgBatches, rgErr := BuildResourceGroupBatches(m) + if rgErr != nil || rgBatches == nil { + // No resource-group ordering — output as-is + if result.Len() > 0 { + result.WriteString("\n---\n") + } + result.WriteString(m) + continue + } + + // Output in resource-group batch order with delimiters + for _, rgBatch := range rgBatches.Batches { + for _, groupName := range rgBatch { + groupManifest, ok := rgBatches.GroupedManifests[groupName] + if !ok { + continue + } + if result.Len() > 0 { + result.WriteString("\n---\n") + } + result.WriteString(fmt.Sprintf("## START resource-group: %s/%s\n", subchartName, groupName)) + result.WriteString(groupManifest) + result.WriteString(fmt.Sprintf("\n## END resource-group: %s/%s", subchartName, groupName)) + } + } + + // Unsequenced resources (no delimiters) + if rgBatches.UnsequencedManifest != "" { + if result.Len() > 0 { + result.WriteString("\n---\n") + } + result.WriteString(rgBatches.UnsequencedManifest) + } + } + } + + // Add any resources not in any batch (shouldn't happen, but safety) + return result.String() +} + // BuildInstallBatches constructs ordered batches of subchart names for installation. // Returns batches where each batch can be installed in parallel, and batches // are processed sequentially. The parent chart name is always in the last batch. diff --git a/pkg/action/sequencing_ordered_test.go b/pkg/action/sequencing_ordered_test.go new file mode 100644 index 000000000..510c644f7 --- /dev/null +++ b/pkg/action/sequencing_ordered_test.go @@ -0,0 +1,105 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + chart "helm.sh/helm/v4/pkg/chart/v2" + chartutil "helm.sh/helm/v4/pkg/chart/v2/util" + release "helm.sh/helm/v4/pkg/release/v1" +) + +func TestBuildInstallBatchesWithResourceGroups(t *testing.T) { + // Test that BuildInstallBatches correctly builds subchart batches + // Resource-group batching is tested separately through BuildResourceGroupBatches + chrt := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "myapp", + Dependencies: []*chart.Dependency{ + {Name: "database", DependsOn: []string{}}, + {Name: "cache"}, + {Name: "api", DependsOn: []string{"database", "cache"}}, + }, + Annotations: map[string]string{ + chartutil.AnnotationDependsOnSubcharts: `["api"]`, + }, + }, + } + + batches, err := BuildInstallBatches(chrt) + require.NoError(t, err) + require.Len(t, batches, 3) + assert.Equal(t, []string{"cache", "database"}, batches[0]) + assert.Equal(t, []string{"api"}, batches[1]) + assert.Equal(t, []string{"myapp"}, batches[2]) +} + +func TestSequencingMetadataResourceGroupBatches(t *testing.T) { + // Verify the release SequencingMetadata can store resource-group batches + seq := &release.SequencingMetadata{ + Enabled: true, + Strategy: "ordered", + Batches: [][]string{{"db"}, {"app"}, {"parent"}}, + ResourceGroupBatches: map[string][][]string{ + "app": {{"database"}, {"application"}}, + }, + } + + assert.True(t, seq.Enabled) + assert.Len(t, seq.Batches, 3) + assert.Contains(t, seq.ResourceGroupBatches, "app") + assert.Len(t, seq.ResourceGroupBatches["app"], 2) +} + +func TestSplitManifestsBySubchartConsistency(t *testing.T) { + // Ensure subchart splitting works consistently for ordered operations + manifest := `--- +# Source: myapp/templates/service.yaml +apiVersion: v1 +kind: Service +metadata: + name: myapp-svc +--- +# Source: myapp/charts/redis/templates/deployment.yaml +apiVersion: apps/v1 +kind: Deployment +metadata: + name: redis +--- +# Source: myapp/charts/nginx/templates/deployment.yaml +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx` + + result := SplitManifestsBySubchart(manifest, "myapp") + + // Should produce 3 buckets + assert.Len(t, result, 3) + assert.Contains(t, result, "myapp") + assert.Contains(t, result, "redis") + assert.Contains(t, result, "nginx") + + // Parent chart + assert.Contains(t, result["myapp"], "myapp-svc") + // Subcharts + assert.Contains(t, result["redis"], "redis") + assert.Contains(t, result["nginx"], "nginx") +} diff --git a/pkg/action/sequencing_test.go b/pkg/action/sequencing_test.go index 624c69cce..8610810c4 100644 --- a/pkg/action/sequencing_test.go +++ b/pkg/action/sequencing_test.go @@ -16,6 +16,7 @@ limitations under the License. package action import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -103,6 +104,137 @@ metadata: assert.Contains(t, result["myapp"], "orphan") } +func TestSplitManifestDocs(t *testing.T) { + manifest := "---\napiVersion: v1\nkind: ConfigMap\n---\napiVersion: apps/v1\nkind: Deployment\n---\n" + docs := SplitManifestDocs(manifest) + assert.Len(t, docs, 2) +} + +func TestBuildResourceGroupBatchesNone(t *testing.T) { + manifest := "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: test" + result, err := BuildResourceGroupBatches(manifest) + require.NoError(t, err) + assert.Nil(t, result) +} + +func TestBuildResourceGroupBatchesLinear(t *testing.T) { + manifest := `--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: db-config + annotations: + helm.sh/resource-group: database +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: app-deploy + annotations: + helm.sh/resource-group: application + helm.sh/depends-on/resource-groups: '["database"]'` + + result, err := BuildResourceGroupBatches(manifest) + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, [][]string{{"database"}, {"application"}}, result.Batches) + assert.Contains(t, result.GroupedManifests, "database") + assert.Contains(t, result.GroupedManifests, "application") + assert.Empty(t, result.UnsequencedManifest) +} + +func TestBuildResourceGroupBatchesMixed(t *testing.T) { + manifest := `--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: db-config + annotations: + helm.sh/resource-group: database +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: orphan` + + result, err := BuildResourceGroupBatches(manifest) + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, [][]string{{"database"}}, result.Batches) + assert.NotEmpty(t, result.UnsequencedManifest) + assert.Contains(t, result.UnsequencedManifest, "orphan") +} + +func TestBuildResourceGroupBatchesWarnings(t *testing.T) { + manifest := `--- +apiVersion: v1 +kind: ConfigMap +metadata: + annotations: + helm.sh/resource-group: app + helm.sh/depends-on/resource-groups: '["nonexistent"]'` + + result, err := BuildResourceGroupBatches(manifest) + require.NoError(t, err) + require.NotNil(t, result) + assert.Len(t, result.Warnings, 1) + assert.Contains(t, result.Warnings[0], "nonexistent") +} + +func TestReorderManifestForTemplateNoSequencing(t *testing.T) { + chrt := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "myapp", + }, + } + manifest := "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: test" + result := ReorderManifestForTemplate(manifest, chrt) + assert.Equal(t, manifest, result, "unchanged when no sequencing") +} + +func TestReorderManifestForTemplateWithSequencing(t *testing.T) { + chrt := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "parent", + Dependencies: []*chart.Dependency{ + {Name: "redis"}, + {Name: "app", DependsOn: []string{"redis"}}, + }, + Annotations: map[string]string{ + chartutil.AnnotationDependsOnSubcharts: `["app"]`, + }, + }, + } + + manifest := `--- +# Source: parent/charts/redis/templates/deploy.yaml +apiVersion: apps/v1 +kind: Deployment +metadata: + name: redis +--- +# Source: parent/charts/app/templates/deploy.yaml +apiVersion: apps/v1 +kind: Deployment +metadata: + name: app +--- +# Source: parent/templates/service.yaml +apiVersion: v1 +kind: Service +metadata: + name: parent-svc` + + result := ReorderManifestForTemplate(manifest, chrt) + // Redis should appear before app, and parent should be last + redisIdx := strings.Index(result, "redis") + appIdx := strings.Index(result, "name: app") + parentIdx := strings.Index(result, "parent-svc") + + assert.True(t, redisIdx < appIdx, "redis should come before app") + assert.True(t, appIdx < parentIdx, "app should come before parent") +} + func TestBuildInstallBatchesNoSequencing(t *testing.T) { chrt := &chart.Chart{ Metadata: &chart.Metadata{ diff --git a/pkg/action/uninstall.go b/pkg/action/uninstall.go index 79156991c..592881580 100644 --- a/pkg/action/uninstall.go +++ b/pkg/action/uninstall.go @@ -255,15 +255,85 @@ func (e *joinedErrors) Unwrap() []error { // deleteRelease deletes the release and returns list of delete resources and manifests that were kept in the deletion process func (u *Uninstall) deleteRelease(rel *release.Release) (kube.ResourceList, string, []error) { + // HIP-0025: If sequencing metadata exists, delete in reverse batch order + if rel.Sequencing != nil && rel.Sequencing.Enabled && len(rel.Sequencing.Batches) > 0 { + return u.deleteReleaseOrdered(rel) + } + + return u.deleteReleaseUnordered(rel) +} + +// deleteReleaseOrdered deletes resources in reverse subchart batch order +// using the SequencingMetadata stored in the release. +func (u *Uninstall) deleteReleaseOrdered(rel *release.Release) (kube.ResourceList, string, []error) { + var allDeleted kube.ResourceList + var errs []error + + subchartManifests := SplitManifestsBySubchart(rel.Manifest, rel.Chart.Metadata.Name) + + // Reverse the batch order + batches := rel.Sequencing.Batches + u.cfg.Logger().Info(fmt.Sprintf("ordered uninstall: %d batches to delete (reverse order)", len(batches))) + + var waiter kube.Waiter + var wErr error + if c, supportsOptions := u.cfg.KubeClient.(kube.InterfaceWaitOptions); supportsOptions { + waiter, wErr = c.GetWaiterWithOptions(u.WaitStrategy, u.WaitOptions...) + } else { + waiter, wErr = u.cfg.KubeClient.GetWaiter(u.WaitStrategy) + } + if wErr != nil { + return nil, "", []error{wErr} + } + + for i := len(batches) - 1; i >= 0; i-- { + batch := batches[i] + u.cfg.Logger().Info(fmt.Sprintf("deleting batch %d: %v", i+1, batch)) + + var batchManifest string + for _, subchartName := range batch { + if m, ok := subchartManifests[subchartName]; ok { + if batchManifest != "" { + batchManifest += "\n---\n" + } + batchManifest += m + } + } + + if batchManifest == "" { + continue + } + + resources, err := u.cfg.KubeClient.Build(strings.NewReader(batchManifest), false) + if err != nil { + errs = append(errs, fmt.Errorf("unable to build resources for batch %d: %w", i+1, err)) + continue + } + + if len(resources) > 0 { + _, delErrs := u.cfg.KubeClient.Delete(resources, parseCascadingFlag(u.DeletionPropagation)) + if delErrs != nil { + errs = append(errs, delErrs...) + } else { + allDeleted = append(allDeleted, resources...) + // Wait for resources to be gone before next batch + if err := waiter.WaitForDelete(resources, u.Timeout); err != nil { + errs = append(errs, fmt.Errorf("batch %d wait for delete failed: %w", i+1, err)) + } + } + } + } + + return allDeleted, "", errs +} + +// deleteReleaseUnordered is the original unordered deletion path. +func (u *Uninstall) deleteReleaseUnordered(rel *release.Release) (kube.ResourceList, string, []error) { var errs []error manifests := releaseutil.SplitManifests(rel.Manifest) _, files, err := releaseutil.SortManifests(manifests, nil, releaseutil.UninstallOrder) if err != nil { - // We could instead just delete everything in no particular order. - // FIXME: One way to delete at this point would be to try a label-based - // deletion. The problem with this is that we could get a false positive - // and delete something that was not legitimately part of this release. return nil, rel.Manifest, []error{fmt.Errorf("corrupted release record. You must manually delete the resources: %w", err)} } diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 4c93855b1..2f7e0403e 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -462,48 +462,57 @@ func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *rele u.cfg.Logger().Debug("upgrade hooks disabled", "name", upgradedRelease.Name) } - upgradeClientSideFieldManager := isReleaseApplyMethodClientSideApply(originalRelease.ApplyMethod) && serverSideApply // Update client-side field manager if transitioning from client-side to server-side apply - results, err := u.cfg.KubeClient.Update( - current, - target, - kube.ClientUpdateOptionForceReplace(u.ForceReplace), - kube.ClientUpdateOptionServerSideApply(serverSideApply, u.ForceConflicts), - kube.ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManager)) - if err != nil { - u.cfg.recordRelease(originalRelease) - u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err) - return - } - - var waiter kube.Waiter - if c, supportsOptions := u.cfg.KubeClient.(kube.InterfaceWaitOptions); supportsOptions { - waiter, err = c.GetWaiterWithOptions(u.WaitStrategy, u.WaitOptions...) + // HIP-0025: ordered sequencing — deploy resources in subchart dependency order + if u.WaitStrategy == kube.OrderedStrategy { + if err := u.performOrderedUpgrade(upgradedRelease, current, target, originalRelease, serverSideApply); err != nil { + u.cfg.recordRelease(originalRelease) + u.reportToPerformUpgrade(c, upgradedRelease, kube.ResourceList{}, err) + return + } } else { - waiter, err = u.cfg.KubeClient.GetWaiter(u.WaitStrategy) - } - if err != nil { - u.cfg.recordRelease(originalRelease) - u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err) - return - } - if u.WaitForJobs { - if err := waiter.WaitWithJobs(target, u.Timeout); err != nil { + upgradeClientSideFieldManager := isReleaseApplyMethodClientSideApply(originalRelease.ApplyMethod) && serverSideApply + results, err := u.cfg.KubeClient.Update( + current, + target, + kube.ClientUpdateOptionForceReplace(u.ForceReplace), + kube.ClientUpdateOptionServerSideApply(serverSideApply, u.ForceConflicts), + kube.ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManager)) + if err != nil { u.cfg.recordRelease(originalRelease) u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err) return } - } else { - if err := waiter.Wait(target, u.Timeout); err != nil { + + var waiter kube.Waiter + if wc, supportsOptions := u.cfg.KubeClient.(kube.InterfaceWaitOptions); supportsOptions { + waiter, err = wc.GetWaiterWithOptions(u.WaitStrategy, u.WaitOptions...) + } else { + waiter, err = u.cfg.KubeClient.GetWaiter(u.WaitStrategy) + } + if err != nil { u.cfg.recordRelease(originalRelease) u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err) return } + if u.WaitForJobs { + if err := waiter.WaitWithJobs(target, u.Timeout); err != nil { + u.cfg.recordRelease(originalRelease) + u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err) + return + } + } else { + if err := waiter.Wait(target, u.Timeout); err != nil { + u.cfg.recordRelease(originalRelease) + u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err) + return + } + } } // post-upgrade hooks if !u.DisableHooks { if err := u.cfg.execHook(upgradedRelease, release.HookPostUpgrade, u.WaitStrategy, u.WaitOptions, u.Timeout, serverSideApply); err != nil { - u.reportToPerformUpgrade(c, upgradedRelease, results.Created, fmt.Errorf("post-upgrade hooks failed: %s", err)) + u.reportToPerformUpgrade(c, upgradedRelease, kube.ResourceList{}, fmt.Errorf("post-upgrade hooks failed: %s", err)) return } } @@ -520,6 +529,120 @@ func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *rele u.reportToPerformUpgrade(c, upgradedRelease, nil, nil) } +// performOrderedUpgrade deploys resources in subchart dependency order during upgrade. +// Mirrors performOrderedInstall logic from install.go but uses Update instead of Create. +func (u *Upgrade) performOrderedUpgrade(upgradedRelease *release.Release, current kube.ResourceList, target kube.ResourceList, originalRelease *release.Release, serverSideApply bool) error { + batches, err := BuildInstallBatches(upgradedRelease.Chart) + if err != nil { + return fmt.Errorf("failed to build upgrade order: %w", err) + } + + // If no sequencing declared, fall back to standard update + if batches == nil { + upgradeClientSideFieldManager := isReleaseApplyMethodClientSideApply(originalRelease.ApplyMethod) && serverSideApply + _, err := u.cfg.KubeClient.Update( + current, target, + kube.ClientUpdateOptionForceReplace(u.ForceReplace), + kube.ClientUpdateOptionServerSideApply(serverSideApply, u.ForceConflicts), + kube.ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManager)) + if err != nil { + return err + } + + var waiter kube.Waiter + if wc, supportsOptions := u.cfg.KubeClient.(kube.InterfaceWaitOptions); supportsOptions { + waiter, err = wc.GetWaiterWithOptions(u.WaitStrategy, u.WaitOptions...) + } else { + waiter, err = u.cfg.KubeClient.GetWaiter(u.WaitStrategy) + } + if err != nil { + return err + } + return waiter.Wait(target, u.Timeout) + } + + // Store sequencing metadata in the release + upgradedRelease.Sequencing = &release.SequencingMetadata{ + Enabled: true, + Strategy: string(kube.OrderedStrategy), + Batches: batches, + } + + // Split manifests by subchart for both current and target + subchartManifests := SplitManifestsBySubchart(upgradedRelease.Manifest, upgradedRelease.Chart.Metadata.Name) + currentSubchartManifests := SplitManifestsBySubchart(originalRelease.Manifest, originalRelease.Chart.Metadata.Name) + + u.cfg.Logger().Info(fmt.Sprintf("ordered upgrade: %d batches to deploy", len(batches))) + + upgradeClientSideFieldManager := isReleaseApplyMethodClientSideApply(originalRelease.ApplyMethod) && serverSideApply + + for batchIdx, batch := range batches { + u.cfg.Logger().Info(fmt.Sprintf("upgrading batch %d: %v", batchIdx+1, batch)) + + var batchManifest, currentBatchManifest string + for _, subchartName := range batch { + if m, ok := subchartManifests[subchartName]; ok { + if batchManifest != "" { + batchManifest += "\n---\n" + } + batchManifest += m + } + if m, ok := currentSubchartManifests[subchartName]; ok { + if currentBatchManifest != "" { + currentBatchManifest += "\n---\n" + } + currentBatchManifest += m + } + } + + if batchManifest == "" { + continue + } + + batchTarget, err := u.cfg.KubeClient.Build(bytes.NewBufferString(batchManifest), !u.DisableOpenAPIValidation) + if err != nil { + return fmt.Errorf("failed to build target resources for batch %d: %w", batchIdx+1, err) + } + + var batchCurrent kube.ResourceList + if currentBatchManifest != "" { + batchCurrent, err = u.cfg.KubeClient.Build(bytes.NewBufferString(currentBatchManifest), false) + if err != nil { + return fmt.Errorf("failed to build current resources for batch %d: %w", batchIdx+1, err) + } + } + + err = batchTarget.Visit(setMetadataVisitor(upgradedRelease.Name, upgradedRelease.Namespace, true)) + if err != nil { + return err + } + + _, err = u.cfg.KubeClient.Update( + batchCurrent, batchTarget, + kube.ClientUpdateOptionForceReplace(u.ForceReplace), + kube.ClientUpdateOptionServerSideApply(serverSideApply, u.ForceConflicts), + kube.ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManager)) + if err != nil { + return fmt.Errorf("batch %d (%v) update failed: %w", batchIdx+1, batch, err) + } + + var waiter kube.Waiter + if wc, supportsOptions := u.cfg.KubeClient.(kube.InterfaceWaitOptions); supportsOptions { + waiter, err = wc.GetWaiterWithOptions(u.WaitStrategy, u.WaitOptions...) + } else { + waiter, err = u.cfg.KubeClient.GetWaiter(u.WaitStrategy) + } + if err != nil { + return err + } + if err := waiter.Wait(batchTarget, u.Timeout); err != nil { + return fmt.Errorf("batch %d (%v) wait failed: %w", batchIdx+1, batch, err) + } + } + + return nil +} + func (u *Upgrade) failRelease(rel *release.Release, created kube.ResourceList, err error) (*release.Release, error) { msg := fmt.Sprintf("Upgrade %q failed: %s", rel.Name, err) u.cfg.Logger().Warn( diff --git a/pkg/chart/v2/lint/lint.go b/pkg/chart/v2/lint/lint.go index 1c871d936..7c5efe93e 100644 --- a/pkg/chart/v2/lint/lint.go +++ b/pkg/chart/v2/lint/lint.go @@ -66,6 +66,7 @@ func RunAll(baseDir string, values map[string]interface{}, namespace string, opt rules.TemplateLinterSkipSchemaValidation(lo.SkipSchemaValidation)) rules.Dependencies(&result) rules.Crds(&result) + rules.Sequencing(&result) return result } diff --git a/pkg/chart/v2/lint/rules/sequencing.go b/pkg/chart/v2/lint/rules/sequencing.go new file mode 100644 index 000000000..8f8161434 --- /dev/null +++ b/pkg/chart/v2/lint/rules/sequencing.go @@ -0,0 +1,134 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rules + +import ( + "fmt" + "os" + "path/filepath" + + "sigs.k8s.io/yaml" + + chart "helm.sh/helm/v4/pkg/chart/v2" + "helm.sh/helm/v4/pkg/chart/v2/lint/support" + chartutil "helm.sh/helm/v4/pkg/chart/v2/util" +) + +// Sequencing runs HIP-0025 sequencing lint rules. +func Sequencing(linter *support.Linter) { + chartPath := filepath.Join(linter.ChartDir, "Chart.yaml") + + chartFile, err := loadChartForSequencing(chartPath) + if err != nil || chartFile == nil { + return // Can't lint sequencing without a valid Chart.yaml + } + + // Validate subchart depends-on references + linter.RunLinterRule(support.ErrorSev, "Chart.yaml", + validateSubchartDependsOn(chartFile)) + + // Validate no cycles in subchart DAG + linter.RunLinterRule(support.ErrorSev, "Chart.yaml", + validateSubchartDAG(chartFile)) +} + +func loadChartForSequencing(chartPath string) (*chart.Metadata, error) { + data, err := os.ReadFile(chartPath) + if err != nil { + return nil, err + } + var md chart.Metadata + if err := yaml.Unmarshal(data, &md); err != nil { + return nil, err + } + return &md, nil +} + +// validateSubchartDependsOn checks that all depends-on references point to known dependencies. +func validateSubchartDependsOn(md *chart.Metadata) error { + if md == nil { + return nil + } + + depNames := make(map[string]bool) + for _, dep := range md.Dependencies { + key := dep.Name + if dep.Alias != "" { + key = dep.Alias + } + depNames[key] = true + } + + // Check DependsOn field references + for _, dep := range md.Dependencies { + key := dep.Name + if dep.Alias != "" { + key = dep.Alias + } + for _, upstream := range dep.DependsOn { + if !depNames[upstream] { + return fmt.Errorf( + "dependency %q declares depends-on %q, but %q is not a known dependency", + key, upstream, upstream, + ) + } + } + } + + // Check annotation references + annotationDeps, err := chartutil.ParseDependsOnSubcharts(md) + if err != nil { + return err + } + for _, upstream := range annotationDeps { + if !depNames[upstream] { + return fmt.Errorf( + "annotation %s references %q, but %q is not a known dependency", + chartutil.AnnotationDependsOnSubcharts, upstream, upstream, + ) + } + } + + return nil +} + +// validateSubchartDAG builds the subchart DAG and checks for cycles. +func validateSubchartDAG(md *chart.Metadata) error { + if md == nil { + return nil + } + + // Only validate if there are sequencing declarations + hasDependsOn := false + for _, dep := range md.Dependencies { + if len(dep.DependsOn) > 0 { + hasDependsOn = true + break + } + } + hasAnnotation := false + if md.Annotations != nil { + _, hasAnnotation = md.Annotations[chartutil.AnnotationDependsOnSubcharts] + } + if !hasDependsOn && !hasAnnotation { + return nil + } + + c := &chart.Chart{Metadata: md} + _, err := chartutil.BuildSubchartDAG(c) + return err +} diff --git a/pkg/chart/v2/lint/rules/sequencing_test.go b/pkg/chart/v2/lint/rules/sequencing_test.go new file mode 100644 index 000000000..a178b51ea --- /dev/null +++ b/pkg/chart/v2/lint/rules/sequencing_test.go @@ -0,0 +1,145 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rules + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + chart "helm.sh/helm/v4/pkg/chart/v2" + chartutil "helm.sh/helm/v4/pkg/chart/v2/util" +) + +func TestValidateSubchartDependsOn(t *testing.T) { + tests := []struct { + name string + metadata *chart.Metadata + expectError bool + }{ + { + name: "valid depends-on", + metadata: &chart.Metadata{ + Name: "parent", + Dependencies: []*chart.Dependency{ + {Name: "redis"}, + {Name: "app", DependsOn: []string{"redis"}}, + }, + }, + expectError: false, + }, + { + name: "unknown depends-on reference", + metadata: &chart.Metadata{ + Name: "parent", + Dependencies: []*chart.Dependency{ + {Name: "app", DependsOn: []string{"ghost"}}, + }, + }, + expectError: true, + }, + { + name: "valid annotation reference", + metadata: &chart.Metadata{ + Name: "parent", + Annotations: map[string]string{ + chartutil.AnnotationDependsOnSubcharts: `["redis"]`, + }, + Dependencies: []*chart.Dependency{ + {Name: "redis"}, + }, + }, + expectError: false, + }, + { + name: "unknown annotation reference", + metadata: &chart.Metadata{ + Name: "parent", + Annotations: map[string]string{ + chartutil.AnnotationDependsOnSubcharts: `["ghost"]`, + }, + Dependencies: []*chart.Dependency{ + {Name: "redis"}, + }, + }, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateSubchartDependsOn(tt.metadata) + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestValidateSubchartDAG(t *testing.T) { + tests := []struct { + name string + metadata *chart.Metadata + expectError bool + }{ + { + name: "no sequencing - no error", + metadata: &chart.Metadata{ + Name: "parent", + Dependencies: []*chart.Dependency{ + {Name: "redis"}, + }, + }, + expectError: false, + }, + { + name: "valid DAG", + metadata: &chart.Metadata{ + Name: "parent", + Dependencies: []*chart.Dependency{ + {Name: "redis"}, + {Name: "app", DependsOn: []string{"redis"}}, + }, + }, + expectError: false, + }, + { + name: "circular dependency", + metadata: &chart.Metadata{ + Name: "parent", + Dependencies: []*chart.Dependency{ + {Name: "A", DependsOn: []string{"B"}}, + {Name: "B", DependsOn: []string{"A"}}, + }, + }, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateSubchartDAG(tt.metadata) + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/pkg/chart/v2/util/dag.go b/pkg/chart/v2/util/dag.go index 713b419eb..90d42d774 100644 --- a/pkg/chart/v2/util/dag.go +++ b/pkg/chart/v2/util/dag.go @@ -250,6 +250,28 @@ func (d *DAG) Len() int { return len(d.nodes) } +// String returns a human-readable ASCII representation of the DAG, +// showing nodes grouped by batch level with dependency arrows. +func (d *DAG) String() string { + batches, err := d.Batches() + if err != nil { + return fmt.Sprintf("error computing batches: %v", err) + } + + var b strings.Builder + b.WriteString("DAG Dependency Order:\n") + for i, batch := range batches { + b.WriteString(fmt.Sprintf(" Batch %d: %s\n", i+1, strings.Join(batch, ", "))) + for _, node := range batch { + deps := d.DependsOn(node) + if len(deps) > 0 { + b.WriteString(fmt.Sprintf(" %s <- [%s]\n", node, strings.Join(deps, ", "))) + } + } + } + return b.String() +} + func (d *DAG) sortedNodes() []string { result := make([]string, 0, len(d.nodes)) for n := range d.nodes { diff --git a/pkg/chart/v2/util/dag_test.go b/pkg/chart/v2/util/dag_test.go index c66cac357..04676dae9 100644 --- a/pkg/chart/v2/util/dag_test.go +++ b/pkg/chart/v2/util/dag_test.go @@ -172,3 +172,15 @@ func TestDAGComplexGraph(t *testing.T) { assert.Equal(t, []string{"queue"}, batches[1]) assert.Equal(t, []string{"app"}, batches[2]) } + +func TestDAGString(t *testing.T) { + dag := NewDAG() + dag.AddNode("redis") + dag.AddNode("app") + require.NoError(t, dag.AddEdge("redis", "app")) + + s := dag.String() + assert.Contains(t, s, "Batch 1: redis") + assert.Contains(t, s, "Batch 2: app") + assert.Contains(t, s, "app <- [redis]") +} diff --git a/pkg/chart/v2/util/sequencing.go b/pkg/chart/v2/util/sequencing.go index 2878fd27c..5d86b722f 100644 --- a/pkg/chart/v2/util/sequencing.go +++ b/pkg/chart/v2/util/sequencing.go @@ -18,6 +18,7 @@ package util import ( "encoding/json" "fmt" + "strings" chart "helm.sh/helm/v4/pkg/chart/v2" ) @@ -55,6 +56,124 @@ func ParseDependsOnSubcharts(md *chart.Metadata) ([]string, error) { return names, nil } +// ResourceGroupAnnotation holds the parsed resource-group annotation value +// for a single rendered manifest document. +type ResourceGroupAnnotation struct { + // Group is the resource-group name assigned to this resource. + Group string + // DependsOn is the list of resource-group names this group depends on. + DependsOn []string +} + +// ParseResourceGroupAnnotations extracts resource-group metadata from a rendered +// manifest document (single YAML document string). It reads: +// - metadata.annotations["helm.sh/resource-group"] +// - metadata.annotations["helm.sh/depends-on/resource-groups"] (JSON array) +// +// Returns nil if the resource has no resource-group annotation. +func ParseResourceGroupAnnotations(doc string) (*ResourceGroupAnnotation, error) { + group := extractAnnotationValue(doc, AnnotationResourceGroup) + if group == "" { + return nil, nil + } + + rga := &ResourceGroupAnnotation{Group: group} + + depsRaw := extractAnnotationValue(doc, AnnotationDependsOnResourceGroups) + if depsRaw != "" { + var deps []string + if err := json.Unmarshal([]byte(depsRaw), &deps); err != nil { + return nil, fmt.Errorf("invalid %s annotation: %w", AnnotationDependsOnResourceGroups, err) + } + rga.DependsOn = deps + } + + return rga, nil +} + +// extractAnnotationValue does a simple line-based scan for a YAML annotation +// key and returns its value. This avoids a full YAML parse for performance. +// It handles both quoted and unquoted values. +func extractAnnotationValue(doc string, key string) string { + // Look for the annotation key in lines + for line := range strings.SplitSeq(doc, "\n") { + trimmed := strings.TrimSpace(line) + // Check for key: value or "key": value patterns + if prefix, ok := strings.CutPrefix(trimmed, key+":"); ok { + return strings.Trim(strings.TrimSpace(prefix), "\"'") + } + // Also check for quoted key + if prefix, ok := strings.CutPrefix(trimmed, "\""+key+"\":"); ok { + return strings.Trim(strings.TrimSpace(prefix), "\"'") + } + } + return "" +} + +// BuildResourceGroupDAG constructs a dependency DAG for resource-groups found +// in a set of rendered manifest documents. Returns the DAG, a map of group name +// to manifest documents, the list of unsequenced documents, and any error. +// +// Emits warnings (returned as []string) for: +// - References to non-existent groups +// - Resources assigned to multiple groups (error) +func BuildResourceGroupDAG(docs []string) (*DAG, map[string][]string, []string, []string, error) { + dag := NewDAG() + grouped := make(map[string][]string) // group name → list of YAML docs + var unsequenced []string + var warnings []string + + // First pass: collect all groups and their documents + knownGroups := make(map[string]bool) + type docMeta struct { + index int + group string + deps []string + } + var metas []docMeta + + for i, doc := range docs { + rga, err := ParseResourceGroupAnnotations(doc) + if err != nil { + return nil, nil, nil, nil, fmt.Errorf("document %d: %w", i, err) + } + if rga == nil { + unsequenced = append(unsequenced, doc) + metas = append(metas, docMeta{index: i}) + continue + } + knownGroups[rga.Group] = true + grouped[rga.Group] = append(grouped[rga.Group], doc) + metas = append(metas, docMeta{index: i, group: rga.Group, deps: rga.DependsOn}) + } + + // Second pass: build DAG edges + for _, m := range metas { + if m.group == "" { + continue + } + dag.AddNode(m.group) + for _, dep := range m.deps { + if !knownGroups[dep] { + warnings = append(warnings, fmt.Sprintf( + "resource-group %q depends on %q, but group %q does not exist in rendered manifests", + m.group, dep, dep, + )) + continue + } + if err := dag.AddEdge(dep, m.group); err != nil { + return nil, nil, nil, nil, err + } + } + } + + if err := dag.DetectCycles(); err != nil { + return nil, nil, nil, nil, err + } + + return dag, grouped, unsequenced, warnings, nil +} + // BuildSubchartDAG constructs a dependency DAG for subcharts of the given chart. // It combines dependencies declared via: // - Dependency.DependsOn field entries in Chart.yaml dependencies diff --git a/pkg/chart/v2/util/sequencing_test.go b/pkg/chart/v2/util/sequencing_test.go index 3074d58bc..211645666 100644 --- a/pkg/chart/v2/util/sequencing_test.go +++ b/pkg/chart/v2/util/sequencing_test.go @@ -101,6 +101,143 @@ func TestParseDependsOnSubcharts(t *testing.T) { } } +func TestParseResourceGroupAnnotations(t *testing.T) { + tests := []struct { + name string + doc string + expected *ResourceGroupAnnotation + expectError bool + }{ + { + name: "no annotation", + doc: `apiVersion: v1 +kind: ConfigMap +metadata: + name: test`, + expected: nil, + }, + { + name: "resource-group only", + doc: `apiVersion: v1 +kind: ConfigMap +metadata: + name: test + annotations: + helm.sh/resource-group: database`, + expected: &ResourceGroupAnnotation{Group: "database"}, + }, + { + name: "resource-group with depends-on", + doc: `apiVersion: apps/v1 +kind: Deployment +metadata: + name: app + annotations: + helm.sh/resource-group: application + helm.sh/depends-on/resource-groups: '["database", "cache"]'`, + expected: &ResourceGroupAnnotation{ + Group: "application", + DependsOn: []string{"database", "cache"}, + }, + }, + { + name: "invalid depends-on JSON", + doc: `apiVersion: v1 +kind: Service +metadata: + annotations: + helm.sh/resource-group: app + helm.sh/depends-on/resource-groups: not-json`, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := ParseResourceGroupAnnotations(tt.doc) + if tt.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + }) + } +} + +func TestBuildResourceGroupDAG(t *testing.T) { + tests := []struct { + name string + docs []string + expectedBatch [][]string + expectWarnings int + expectError bool + errorContains string + }{ + { + name: "no resource groups", + docs: []string{"apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: test"}, + expectedBatch: nil, + }, + { + name: "linear dependency: database -> application", + docs: []string{ + "apiVersion: v1\nkind: ConfigMap\nmetadata:\n annotations:\n helm.sh/resource-group: database", + "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n annotations:\n helm.sh/resource-group: application\n helm.sh/depends-on/resource-groups: '[\"database\"]'", + }, + expectedBatch: [][]string{{"database"}, {"application"}}, + }, + { + name: "parallel groups with shared downstream", + docs: []string{ + "apiVersion: v1\nkind: ConfigMap\nmetadata:\n annotations:\n helm.sh/resource-group: database", + "apiVersion: v1\nkind: ConfigMap\nmetadata:\n annotations:\n helm.sh/resource-group: cache", + "apiVersion: v1\nkind: ConfigMap\nmetadata:\n annotations:\n helm.sh/resource-group: app\n helm.sh/depends-on/resource-groups: '[\"database\", \"cache\"]'", + }, + expectedBatch: [][]string{{"cache", "database"}, {"app"}}, + }, + { + name: "warning on non-existent group reference", + docs: []string{ + "apiVersion: v1\nkind: ConfigMap\nmetadata:\n annotations:\n helm.sh/resource-group: app\n helm.sh/depends-on/resource-groups: '[\"ghost\"]'", + }, + expectedBatch: [][]string{{"app"}}, + expectWarnings: 1, + }, + { + name: "cycle detection", + docs: []string{ + "apiVersion: v1\nkind: ConfigMap\nmetadata:\n annotations:\n helm.sh/resource-group: A\n helm.sh/depends-on/resource-groups: '[\"B\"]'", + "apiVersion: v1\nkind: ConfigMap\nmetadata:\n annotations:\n helm.sh/resource-group: B\n helm.sh/depends-on/resource-groups: '[\"A\"]'", + }, + expectError: true, + errorContains: "circular dependency", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dag, _, _, warnings, err := BuildResourceGroupDAG(tt.docs) + if tt.expectError { + require.Error(t, err) + if tt.errorContains != "" { + assert.Contains(t, err.Error(), tt.errorContains) + } + } else { + require.NoError(t, err) + if tt.expectedBatch == nil { + assert.Equal(t, 0, dag.Len()) + } else { + batches, err := dag.Batches() + require.NoError(t, err) + assert.Equal(t, tt.expectedBatch, batches) + } + assert.Len(t, warnings, tt.expectWarnings) + } + }) + } +} + func TestBuildSubchartDAG(t *testing.T) { tests := []struct { name string diff --git a/pkg/cmd/flags.go b/pkg/cmd/flags.go index da01a615e..61b7e7789 100644 --- a/pkg/cmd/flags.go +++ b/pkg/cmd/flags.go @@ -24,6 +24,7 @@ import ( "path/filepath" "sort" "strings" + "time" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -100,6 +101,14 @@ func (ws *waitValue) Type() string { return "WaitStrategy" } +// AddReadinessTimeoutFlag adds the --readiness-timeout flag to a command. +// This flag controls how long to wait for custom readiness conditions (HIP-0025). +// Must not exceed --timeout. Default: 1 minute. +func AddReadinessTimeoutFlag(cmd *cobra.Command, readinessTimeout *time.Duration) { + cmd.Flags().DurationVar(readinessTimeout, "readiness-timeout", time.Minute, + "time to wait for custom readiness conditions per resource (HIP-0025). Must not exceed --timeout.") +} + func addChartPathOptionsFlags(f *pflag.FlagSet, c *action.ChartPathOptions) { f.StringVar(&c.Version, "version", "", "specify a version constraint for the chart version to use. This constraint can be a specific tag (e.g. 1.1.1) or it may reference a valid range (e.g. ^2.0.0). If this is not specified, the latest version is used") f.BoolVar(&c.Verify, "verify", false, "verify the package before using it") diff --git a/pkg/cmd/template.go b/pkg/cmd/template.go index 047fd60df..e85e60160 100644 --- a/pkg/cmd/template.go +++ b/pkg/cmd/template.go @@ -118,7 +118,15 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { // we always want to print the YAML, even if it is not valid. The error is still returned afterwards. if rel != nil { var manifests bytes.Buffer - fmt.Fprintln(&manifests, strings.TrimSpace(rel.Manifest)) + // HIP-0025: reorder manifests by sequencing order if chart has sequencing metadata + manifestContent := rel.Manifest + if rel.Chart != nil { + reordered := action.ReorderManifestForTemplate(manifestContent, rel.Chart) + if reordered != "" { + manifestContent = reordered + } + } + fmt.Fprintln(&manifests, strings.TrimSpace(manifestContent)) if !client.DisableHooks { fileWritten := make(map[string]bool) for _, m := range rel.Hooks { diff --git a/pkg/release/v1/release.go b/pkg/release/v1/release.go index fc7cd1963..8b46989f2 100644 --- a/pkg/release/v1/release.go +++ b/pkg/release/v1/release.go @@ -67,6 +67,9 @@ type SequencingMetadata struct { // Batches records the ordered deployment batches. // Each batch is a list of subchart names deployed together. Batches [][]string `json:"batches,omitempty"` + // ResourceGroupBatches records per-subchart resource-group ordering. + // Key is the subchart name; value is the ordered resource-group batches within that subchart. + ResourceGroupBatches map[string][][]string `json:"resource_group_batches,omitempty"` } // SetStatus is a helper for setting the status on a release.