diff --git a/pkg/action/install.go b/pkg/action/install.go index 0fe1f1a6e..0826f0b83 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -492,15 +492,134 @@ func (i *Install) performInstall(rel *release.Release, toBeAdopted kube.Resource } } - // At this point, we can do the install. Note that before we were detecting whether to - // do an update, but it's not clear whether we WANT to do an update if the reuse is set - // to true, since that is basically an upgrade operation. + // HIP-0025: ordered sequencing — deploy resources in subchart dependency order + if i.WaitStrategy == kube.OrderedStrategy { + if err := i.performOrderedInstall(rel, toBeAdopted, resources); err != nil { + return rel, err + } + } else { + // Default: install all resources at once + if len(toBeAdopted) == 0 && len(resources) > 0 { + _, err = i.cfg.KubeClient.Create( + resources, + kube.ClientCreateOptionServerSideApply(i.ServerSideApply, false)) + } else if len(resources) > 0 { + updateThreeWayMergeForUnstructured := i.TakeOwnership && !i.ServerSideApply + _, err = i.cfg.KubeClient.Update( + toBeAdopted, + resources, + kube.ClientUpdateOptionForceReplace(i.ForceReplace), + kube.ClientUpdateOptionServerSideApply(i.ServerSideApply, i.ForceConflicts), + kube.ClientUpdateOptionThreeWayMergeForUnstructured(updateThreeWayMergeForUnstructured), + kube.ClientUpdateOptionUpgradeClientSideFieldManager(true)) + } + if err != nil { + return rel, err + } + + var waiter kube.Waiter + if c, supportsOptions := i.cfg.KubeClient.(kube.InterfaceWaitOptions); supportsOptions { + waiter, err = c.GetWaiterWithOptions(i.WaitStrategy, i.WaitOptions...) + } else { + waiter, err = i.cfg.KubeClient.GetWaiter(i.WaitStrategy) + } + if err != nil { + return rel, fmt.Errorf("failed to get waiter: %w", err) + } + + if i.WaitForJobs { + err = waiter.WaitWithJobs(resources, i.Timeout) + } else { + err = waiter.Wait(resources, i.Timeout) + } + if err != nil { + return rel, err + } + } + + if !i.DisableHooks { + if err := i.cfg.execHook(rel, release.HookPostInstall, i.WaitStrategy, i.WaitOptions, i.Timeout, i.ServerSideApply); err != nil { + return rel, fmt.Errorf("failed post-install: %s", err) + } + } + + if len(i.Description) > 0 { + rel.SetStatus(rcommon.StatusDeployed, i.Description) + } else { + rel.SetStatus(rcommon.StatusDeployed, "Install complete") + } + + if err := i.recordRelease(rel); err != nil { + i.cfg.Logger().Error("failed to record the release", slog.Any("error", err)) + } + + return rel, nil +} + +// performOrderedInstall deploys resources in subchart dependency order as defined +// by HIP-0025. Resources are grouped by subchart, then deployed in batches +// according to the subchart DAG. Each batch is deployed and waited for before +// the next batch begins. +func (i *Install) performOrderedInstall(rel *release.Release, toBeAdopted kube.ResourceList, resources kube.ResourceList) error { + batches, err := BuildInstallBatches(rel.Chart) + if err != nil { + return fmt.Errorf("failed to build installation order: %w", err) + } + + // If no sequencing declared, fall back to deploying all at once + if batches == nil { + return i.createAndWaitResources(toBeAdopted, resources) + } + + // Split the manifest by subchart + subchartManifests := SplitManifestsBySubchart(rel.Manifest, rel.Chart.Metadata.Name) + + 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)) + + // Collect manifests for this batch + var batchManifest string + for _, subchartName := range batch { + if m, ok := subchartManifests[subchartName]; ok { + if batchManifest != "" { + batchManifest += "\n---\n" + } + batchManifest += m + } + } + + if batchManifest == "" { + continue + } + + // Build resource list from batch manifest + 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) + } + + if err := i.createAndWaitResources(nil, batchResources); err != nil { + return fmt.Errorf("batch %d (%v) failed: %w", batchIdx+1, batch, err) + } + } + + return nil +} + +// createAndWaitResources creates resources and waits for readiness. +func (i *Install) createAndWaitResources(toBeAdopted kube.ResourceList, resources kube.ResourceList) error { + var err error if len(toBeAdopted) == 0 && len(resources) > 0 { _, err = i.cfg.KubeClient.Create( resources, kube.ClientCreateOptionServerSideApply(i.ServerSideApply, false)) } else if len(resources) > 0 { - updateThreeWayMergeForUnstructured := i.TakeOwnership && !i.ServerSideApply // Use three-way merge when taking ownership (and not using server-side apply) + updateThreeWayMergeForUnstructured := i.TakeOwnership && !i.ServerSideApply _, err = i.cfg.KubeClient.Update( toBeAdopted, resources, @@ -510,7 +629,7 @@ func (i *Install) performInstall(rel *release.Release, toBeAdopted kube.Resource kube.ClientUpdateOptionUpgradeClientSideFieldManager(true)) } if err != nil { - return rel, err + return err } var waiter kube.Waiter @@ -520,7 +639,7 @@ func (i *Install) performInstall(rel *release.Release, toBeAdopted kube.Resource waiter, err = i.cfg.KubeClient.GetWaiter(i.WaitStrategy) } if err != nil { - return rel, fmt.Errorf("failed to get waiter: %w", err) + return fmt.Errorf("failed to get waiter: %w", err) } if i.WaitForJobs { @@ -528,34 +647,7 @@ func (i *Install) performInstall(rel *release.Release, toBeAdopted kube.Resource } else { err = waiter.Wait(resources, i.Timeout) } - if err != nil { - return rel, err - } - - if !i.DisableHooks { - if err := i.cfg.execHook(rel, release.HookPostInstall, i.WaitStrategy, i.WaitOptions, i.Timeout, i.ServerSideApply); err != nil { - return rel, fmt.Errorf("failed post-install: %s", err) - } - } - - if len(i.Description) > 0 { - rel.SetStatus(rcommon.StatusDeployed, i.Description) - } else { - rel.SetStatus(rcommon.StatusDeployed, "Install complete") - } - - // This is a tricky case. The release has been created, but the result - // cannot be recorded. The truest thing to tell the user is that the - // release was created. However, the user will not be able to do anything - // further with this release. - // - // One possible strategy would be to do a timed retry to see if we can get - // this stored in the future. - if err := i.recordRelease(rel); err != nil { - i.cfg.Logger().Error("failed to record the release", slog.Any("error", err)) - } - - return rel, nil + return err } func (i *Install) failRelease(rel *release.Release, err error) (*release.Release, error) { diff --git a/pkg/action/sequencing.go b/pkg/action/sequencing.go new file mode 100644 index 000000000..04b6c5d2a --- /dev/null +++ b/pkg/action/sequencing.go @@ -0,0 +1,124 @@ +/* +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 ( + "fmt" + "strings" + + chart "helm.sh/helm/v4/pkg/chart/v2" + chartutil "helm.sh/helm/v4/pkg/chart/v2/util" +) + +// SubchartManifests groups rendered manifest content by subchart name. +// The parent chart's own resources are keyed by the chart name itself. +type SubchartManifests map[string]string + +// SplitManifestsBySubchart splits a rendered manifest string into per-subchart +// manifest strings based on the # Source: comments that Helm inserts during rendering. +// +// Each manifest document is attributed to a subchart by parsing the Source path: +// - "parentchart/templates/foo.yaml" → parent chart +// - "parentchart/charts/subchart/templates/bar.yaml" → "subchart" +// - "parentchart/charts/sub1/charts/sub2/templates/baz.yaml" → "sub1" (immediate child) +// +// The chartName parameter is the root chart name used to identify parent resources. +func SplitManifestsBySubchart(manifest string, chartName string) SubchartManifests { + result := make(SubchartManifests) + + // Split on YAML document separator + docs := strings.Split(manifest, "---") + for _, doc := range docs { + doc = strings.TrimSpace(doc) + if doc == "" { + continue + } + + owner := chartName // default: parent chart owns this resource + // Look for # Source: comment to determine subchart + for line := range strings.SplitSeq(doc, "\n") { + trimmed := strings.TrimSpace(line) + if sourcePath, ok := strings.CutPrefix(trimmed, "# Source: "); ok { + owner = subchartFromSourcePath(sourcePath, chartName) + break + } + } + + if existing, ok := result[owner]; ok { + result[owner] = existing + "\n---\n" + doc + } else { + result[owner] = doc + } + } + + return result +} + +// subchartFromSourcePath extracts the immediate subchart name from a Source path. +// Path format: "chartname/charts/subchartname/templates/file.yaml" +// or: "chartname/templates/file.yaml" (parent chart) +func subchartFromSourcePath(sourcePath, chartName string) string { + parts := strings.Split(sourcePath, "/") + // Find "charts" segment — the next segment is the subchart name + for i, part := range parts { + if part == "charts" && i+1 < len(parts) { + return parts[i+1] + } + } + // No "charts" segment → belongs to parent chart + return chartName +} + +// 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. +// +// If the chart has no sequencing metadata, returns nil (all resources deployed at once). +func BuildInstallBatches(chrt *chart.Chart) ([][]string, error) { + if chrt.Metadata == nil { + return nil, nil + } + + // Check if any sequencing declarations exist + hasAnnotation := false + if chrt.Metadata.Annotations != nil { + _, hasAnnotation = chrt.Metadata.Annotations[chartutil.AnnotationDependsOnSubcharts] + } + + hasDependsOn := false + for _, dep := range chrt.Metadata.Dependencies { + if len(dep.DependsOn) > 0 { + hasDependsOn = true + break + } + } + + if !hasAnnotation && !hasDependsOn { + return nil, nil + } + + dag, err := chartutil.BuildSubchartDAG(chrt) + if err != nil { + return nil, fmt.Errorf("failed to build subchart dependency graph: %w", err) + } + + batches, err := dag.Batches() + if err != nil { + return nil, fmt.Errorf("failed to compute installation order: %w", err) + } + + return batches, nil +} diff --git a/pkg/action/sequencing_test.go b/pkg/action/sequencing_test.go new file mode 100644 index 000000000..624c69cce --- /dev/null +++ b/pkg/action/sequencing_test.go @@ -0,0 +1,162 @@ +/* +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" +) + +func TestSplitManifestsBySubchart(t *testing.T) { + 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/redis/templates/service.yaml +apiVersion: v1 +kind: Service +metadata: + name: redis-svc +--- +# Source: myapp/charts/nginx/templates/deployment.yaml +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx` + + result := SplitManifestsBySubchart(manifest, "myapp") + + assert.Len(t, result, 3) + assert.Contains(t, result, "myapp") + assert.Contains(t, result, "redis") + assert.Contains(t, result, "nginx") + + // Parent chart has 1 resource + assert.Contains(t, result["myapp"], "myapp-svc") + + // Redis has 2 resources + assert.Contains(t, result["redis"], "redis") + assert.Contains(t, result["redis"], "redis-svc") + + // Nginx has 1 resource + assert.Contains(t, result["nginx"], "nginx") +} + +func TestSplitManifestsBySubchartNested(t *testing.T) { + manifest := `--- +# Source: parent/charts/redis/charts/sentinel/templates/deploy.yaml +apiVersion: apps/v1 +kind: Deployment +metadata: + name: sentinel` + + result := SplitManifestsBySubchart(manifest, "parent") + + // Nested subchart sentinel under redis → immediate child is "redis" + assert.Contains(t, result, "redis") + assert.Contains(t, result["redis"], "sentinel") +} + +func TestSplitManifestsBySubchartEmpty(t *testing.T) { + result := SplitManifestsBySubchart("", "myapp") + assert.Empty(t, result) +} + +func TestSplitManifestsBySubchartNoSource(t *testing.T) { + manifest := `--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: orphan` + + result := SplitManifestsBySubchart(manifest, "myapp") + + // No Source comment → attributed to parent + assert.Contains(t, result, "myapp") + assert.Contains(t, result["myapp"], "orphan") +} + +func TestBuildInstallBatchesNoSequencing(t *testing.T) { + chrt := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "myapp", + Dependencies: []*chart.Dependency{ + {Name: "redis"}, + {Name: "nginx"}, + }, + }, + } + + batches, err := BuildInstallBatches(chrt) + require.NoError(t, err) + assert.Nil(t, batches, "no batches when no sequencing declared") +} + +func TestBuildInstallBatchesWithDependsOn(t *testing.T) { + chrt := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "foo", + Annotations: map[string]string{ + chartutil.AnnotationDependsOnSubcharts: `["bar", "rabbitmq"]`, + }, + Dependencies: []*chart.Dependency{ + {Name: "nginx"}, + {Name: "rabbitmq"}, + { + Name: "bar", + DependsOn: []string{"nginx", "rabbitmq"}, + }, + }, + }, + } + + batches, err := BuildInstallBatches(chrt) + require.NoError(t, err) + require.Len(t, batches, 3) + assert.Equal(t, []string{"nginx", "rabbitmq"}, batches[0]) + assert.Equal(t, []string{"bar"}, batches[1]) + assert.Equal(t, []string{"foo"}, batches[2]) +} + +func TestBuildInstallBatchesCircularDep(t *testing.T) { + chrt := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "parent", + Dependencies: []*chart.Dependency{ + {Name: "A", DependsOn: []string{"B"}}, + {Name: "B", DependsOn: []string{"A"}}, + }, + }, + } + + _, err := BuildInstallBatches(chrt) + require.Error(t, err) + assert.Contains(t, err.Error(), "circular dependency") +}