Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 121 additions & 0 deletions inttest/addons/addons_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@
as.Run("Rename chart in Helm extension", func() { as.renameChart() })
as.Run("Secret-based authentication", func() { as.testSecretBasedAuth(kc) })
as.Run("Controller restart recovery", func() { as.testControllerRestartRecovery(kc) })
as.Run("Chart ordering", func() { as.testChartOrdering(kc) })

values := map[string]any{
"replicaCount": 2,
Expand Down Expand Up @@ -713,6 +714,126 @@
as.T().Logf("Successfully recovered from interrupted install: %s is now deployed", restartAddonName)
}

// testChartOrdering verifies that helm charts with inter-chart dependencies are
// reconciled correctly when the dependent chart (A) is submitted before the
// chart it depends on (B). This reproduces the ordering problem identified in
// https://github.com/k0sproject/k0s/issues/7305: with sequential (non-concurrent)
// reconciliation, chart A's pre-install hook blocks waiting for chart B's Service,
// while chart B never gets a chance to run. With concurrent reconciliation both
// charts run in parallel and chart A's hook succeeds once chart B creates the Service.
func (as *AddonsSuite) testChartOrdering(kc *k8s.Clientset) {

Check failure on line 724 in inttest/addons/addons_test.go

View workflow job for this annotation

GitHub Actions / Lint Go (darwin)

(*AddonsSuite).testChartOrdering - kc is unused (unparam)

Check failure on line 724 in inttest/addons/addons_test.go

View workflow job for this annotation

GitHub Actions / Lint Go (windows)

(*AddonsSuite).testChartOrdering - kc is unused (unparam)
ctx := as.TContext()

chartAReleaseName := "order-test-a"
chartBReleaseName := "order-test-b"
chartAName := "k0s-addon-chart-" + chartAReleaseName
chartBName := "k0s-addon-chart-" + chartBReleaseName

restConfig, err := as.GetKubeConfig(as.ControllerNode(0))
as.Require().NoError(err)
k0sClients, err := k0sclientset.NewForConfig(restConfig)
as.Require().NoError(err)

chartClient, err := client.New(restConfig, client.Options{Scheme: k0sscheme.Scheme})
as.Require().NoError(err)

// Create Chart A first — it has a pre-install hook waiting for Chart B's Service.
// This is the problematic ordering from the issue: submitting the dependent chart
// before the chart it depends on.
chartA := &helmv1beta1.Chart{
ObjectMeta: metav1.ObjectMeta{
Name: chartAName,
Namespace: metav1.NamespaceSystem,
Finalizers: []string{
"helm.k0sproject.io/uninstall-helm-release",
},
},
Spec: helmv1beta1.ChartSpec{
ChartName: as.uploadChart("order-test-chart-a"),
ReleaseName: chartAReleaseName,
Version: "0.1.0",
Namespace: metav1.NamespaceDefault,
Timeout: "6m0s",
},
}

as.T().Logf("Creating Chart A (%s) first — it depends on Chart B's Service", chartAName)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be missing something, but I'm not sure using non-existent service can break helm in any circumstances. The more correct check would be installing CRD and corresponding resource in different charts.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This setup is 100% artificial just for testing this cross-chart dependency scenario, for sure. The CRD way is probably closer to real-life scenarios but IMO it does not really matter how this gets tested, as long as we can prove that parallelism works and charts with cross deps gets properly installed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sorry, I got the idea wrong. I was thinking of a scenario where installation of one of the charts might fail.

_, err = k0sClients.HelmV1beta1().Charts(metav1.NamespaceSystem).Create(ctx, chartA, metav1.CreateOptions{})
as.Require().NoError(err)

// Create Chart B second — it creates the Service that Chart A's hook waits for.
chartB := &helmv1beta1.Chart{
ObjectMeta: metav1.ObjectMeta{
Name: chartBName,
Namespace: metav1.NamespaceSystem,
Finalizers: []string{
"helm.k0sproject.io/uninstall-helm-release",
},
},
Spec: helmv1beta1.ChartSpec{
ChartName: as.uploadChart("order-test-chart-b"),
ReleaseName: chartBReleaseName,
Version: "0.1.0",
Namespace: metav1.NamespaceDefault,
},
}

as.T().Logf("Creating Chart B (%s) second — it provides the Service chart A depends on", chartBName)
_, err = k0sClients.HelmV1beta1().Charts(metav1.NamespaceSystem).Create(ctx, chartB, metav1.CreateOptions{})
as.Require().NoError(err)

// With concurrent reconciliation both charts run in parallel: Chart B creates its
// Service and Chart A's pre-install hook finds it via the Kubernetes API.
// With sequential reconciliation Chart A would block the single reconciler worker
// forever (since its hook loops indefinitely), preventing Chart B from ever running.
as.T().Log("Waiting for Chart B to be deployed (creates the Service)...")
as.waitForTestRelease(chartBReleaseName, "1.0", metav1.NamespaceDefault, 1)

// Use a strict watch for Chart A: fail immediately if Status.Error is set.
// With concurrent reconciliation, chart A must install cleanly on the first
// attempt — a retry would mean the hook timed out, which only happens when
// chart B couldn't run in parallel (i.e. sequential deadlock).
as.T().Log("Waiting for Chart A to be deployed without errors (pre-install hook waits for the Service)...")
k0sKC, err := as.AutopilotClient(as.ControllerNode(0))
as.Require().NoError(err)
as.Require().NoError(watch.Charts(k0sKC.HelmV1beta1().Charts(metav1.NamespaceSystem)).
WithObjectName("k0s-addon-chart-"+chartAReleaseName).
Until(ctx, func(item *helmv1beta1.Chart) (bool, error) {
if item.Status.Error != "" {
return false, fmt.Errorf("chart A install failed (concurrent reconciliation should prevent this): %s", item.Status.Error)
}
if item.Status.ReleaseName == "" || item.Generation != 1 || item.Status.Revision != 1 {
as.T().Logf("Chart A not ready yet (version %q): releaseName=%q generation=%d revision=%d",
item.ResourceVersion, item.Status.ReleaseName, item.Generation, item.Status.Revision)
return false, nil
}
return true, nil
}),
)

as.T().Log("Both charts deployed successfully — chart ordering works correctly")

// Cleanup
for _, chart := range []*helmv1beta1.Chart{chartA, chartB} {
as.T().Logf("Deleting Chart %s/%s", chart.Namespace, chart.Name)
as.Require().NoError(chartClient.Delete(ctx, chart))
}
for _, chart := range []*helmv1beta1.Chart{chartA, chartB} {
as.Require().NoError(wait.PollUntilContextCancel(ctx, 1*time.Second, true, func(ctx context.Context) (bool, error) {
var found helmv1beta1.Chart
err := chartClient.Get(ctx, client.ObjectKey{Namespace: chart.Namespace, Name: chart.Name}, &found)
if apierrors.IsNotFound(err) {
as.T().Logf("Chart %s deleted", chart.Name)
return true, nil
}
if err != nil {
as.T().Log("Error while getting chart:", err)
}
return false, nil
}))
}
}

func (as *AddonsSuite) uploadChart(chartName string) string {
var chartArchive bytes.Buffer
gz := gzip.NewWriter(&chartArchive)
Expand Down
9 changes: 9 additions & 0 deletions inttest/addons/testdata/order-test-chart-a/Chart.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# SPDX-FileCopyrightText: 2026 k0s authors
# SPDX-License-Identifier: Apache-2.0

apiVersion: v2
name: order-test-chart
description: A test Helm chart with a pre-install hook that waits for a Service created by another chart
type: application
version: 0.1.0
appVersion: "1.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# SPDX-FileCopyrightText: 2026 k0s authors
# SPDX-License-Identifier: Apache-2.0

apiVersion: v1
kind: ConfigMap
metadata:
name: "{{ .Release.Name }}-installed"
namespace: {{ .Release.Namespace }}
data:
status: installed
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# SPDX-FileCopyrightText: 2026 k0s authors
# SPDX-License-Identifier: Apache-2.0

apiVersion: batch/v1
kind: Job
metadata:
name: "{{ .Release.Name }}-preinstall"
annotations:
helm.sh/hook: pre-install
helm.sh/hook-weight: "0"
helm.sh/hook-delete-policy: {{ .Values.hook.deletePolicy | quote }}
spec:
backoffLimit: 100
template:
spec:
serviceAccountName: order-test-hook-sa
restartPolicy: OnFailure
containers:
- name: wait-for-dependency
image: docker.io/library/busybox:1-musl
command:
- sh
- -c
- |
echo "Waiting for order-test-dependency service to appear in the Kubernetes API..."
APISERVER="https://${KUBERNETES_SERVICE_HOST}:${KUBERNETES_SERVICE_PORT}"
TOKEN="$(cat /var/run/secrets/kubernetes.io/serviceaccount/token)"
until wget -q -O - --no-check-certificate \
--header "Authorization: Bearer ${TOKEN}" \
"${APISERVER}/api/v1/namespaces/{{ .Release.Namespace }}/services/order-test-dependency" \
2>/dev/null | grep -q '"clusterIP"'; do
echo "Service not yet available via API, retrying in 5s..."
sleep 5
done
echo "Service is available, proceeding with installation."
16 changes: 16 additions & 0 deletions inttest/addons/testdata/order-test-chart-a/templates/role.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# SPDX-FileCopyrightText: 2026 k0s authors
# SPDX-License-Identifier: Apache-2.0

apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: order-test-hook-role
namespace: {{ .Release.Namespace }}
annotations:
helm.sh/hook: pre-install
helm.sh/hook-weight: "-10"
helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded
rules:
- apiGroups: [""]
resources: ["services"]
verbs: ["get"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# SPDX-FileCopyrightText: 2026 k0s authors
# SPDX-License-Identifier: Apache-2.0

apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: order-test-hook-rb
namespace: {{ .Release.Namespace }}
annotations:
helm.sh/hook: pre-install
helm.sh/hook-weight: "-10"
helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: order-test-hook-role
subjects:
- kind: ServiceAccount
name: order-test-hook-sa
namespace: {{ .Release.Namespace }}
12 changes: 12 additions & 0 deletions inttest/addons/testdata/order-test-chart-a/templates/sa.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# SPDX-FileCopyrightText: 2026 k0s authors
# SPDX-License-Identifier: Apache-2.0

apiVersion: v1
kind: ServiceAccount
metadata:
name: order-test-hook-sa
namespace: {{ .Release.Namespace }}
annotations:
helm.sh/hook: pre-install
helm.sh/hook-weight: "-10"
helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded
5 changes: 5 additions & 0 deletions inttest/addons/testdata/order-test-chart-a/values.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# SPDX-FileCopyrightText: 2026 k0s authors
# SPDX-License-Identifier: Apache-2.0

hook:
deletePolicy: before-hook-creation,hook-succeeded
9 changes: 9 additions & 0 deletions inttest/addons/testdata/order-test-chart-b/Chart.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# SPDX-FileCopyrightText: 2026 k0s authors
# SPDX-License-Identifier: Apache-2.0

apiVersion: v2
name: order-test-dependency
description: A test Helm chart that provides a Service that another chart depends on
type: application
version: 0.1.0
appVersion: "1.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# SPDX-FileCopyrightText: 2026 k0s authors
# SPDX-License-Identifier: Apache-2.0

apiVersion: v1
kind: Service
metadata:
name: order-test-dependency
namespace: {{ .Release.Namespace }}
spec:
type: ClusterIP
selector: {}
ports:
- port: 80
protocol: TCP
4 changes: 3 additions & 1 deletion pkg/component/controller/extensions_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -769,8 +769,10 @@
Metrics: metricsserver.Options{
BindAddress: "0",
},
Logger: logrusr.New(ec.L),

Check failure on line 772 in pkg/component/controller/extensions_controller.go

View workflow job for this annotation

GitHub Actions / Lint Go (darwin)

File is not properly formatted (gofmt)

Check failure on line 772 in pkg/component/controller/extensions_controller.go

View workflow job for this annotation

GitHub Actions / Lint Go (windows)

File is not properly formatted (gofmt)
Controller: ctrlconfig.Controller{},
Controller: ctrlconfig.Controller{
MaxConcurrentReconciles: 10,
},
})
if err != nil {
return nil, fmt.Errorf("can't build controller-runtime controller for helm extensions: %w", err)
Expand Down
Loading