From 2d1d6a9d773334e0fdd87e8e9f8b32a4bd4b9de0 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Tue, 25 Aug 2026 18:14:16 +0530 Subject: [PATCH] Propagate config reconciler errors so sidecar-not-ready races self-heal Reconcile() was swallowing errors from ApplyCACerts/ApplyProxy, which are RPC calls to the sidecarexec container. If that container's socket wasn't listening yet (a real container-start race, no ordering guarantee within the Pod), the failure was logged and discarded, so CA certs/proxy could go permanently unapplied for the Pod's lifetime with no retry. Now Reconcile() returns the error so controller-runtime requeues watch- triggered reconciles with backoff, and the one-shot startup reconcile in run.go retries with a bounded backoff instead of silently continuing. Signed-off-by: Himanshu Singh --- cmd/controller/run.go | 15 ++++- pkg/config/reconciler.go | 4 +- pkg/config/reconciler_test.go | 100 ++++++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 3 deletions(-) create mode 100644 pkg/config/reconciler_test.go diff --git a/cmd/controller/run.go b/cmd/controller/run.go index f971658a56..d3c275cf57 100644 --- a/cmd/controller/run.go +++ b/cmd/controller/run.go @@ -28,6 +28,7 @@ import ( "carvel.dev/kapp-controller/pkg/reftracker" "carvel.dev/kapp-controller/pkg/sidecarexec" "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" // Initialize gcp client auth plugin "k8s.io/component-base/cli/flag" @@ -181,7 +182,19 @@ func Run(opts Options, runLog logr.Logger) error { // Reconcile once synchronously to ensure controller configuration // (e.g. proxy, CA certs) is applied to sidecar before any tool execution happens. - _, err = reconciler.Reconcile(context.TODO(), reconcile.Request{}) + // The sidecarexec container may still be starting up when this process + // runs (there is no ordering guarantee between containers in the same + // Pod), so retry with backoff for a bounded amount of time instead of + // failing on the very first transient RPC error. + err = wait.PollUntilContextTimeout(context.TODO(), time.Second, time.Minute, true, + func(ctx context.Context) (bool, error) { + _, err := reconciler.Reconcile(ctx, reconcile.Request{}) + if err != nil { + runLog.Error(err, "Reconciling config once at startup; will retry") + return false, nil + } + return true, nil + }) if err != nil { return fmt.Errorf("Reconcile config reconciler once: %s", err) } diff --git a/pkg/config/reconciler.go b/pkg/config/reconciler.go index e59fd1c1df..fa4f4d0d5e 100644 --- a/pkg/config/reconciler.go +++ b/pkg/config/reconciler.go @@ -80,13 +80,13 @@ func (r *Reconciler) Reconcile(_ context.Context, request reconcile.Request) (re err = r.osConfig.ApplyCACerts(r.config.CACerts()) if err != nil { log.Error(err, "Failed applying CA certificates") - // continue on + return reconcile.Result{}, err } err = r.osConfig.ApplyProxy(r.config.ProxyOpts()) if err != nil { log.Error(err, "Failed applying proxy opts") - // continue on + return reconcile.Result{}, err } return reconcile.Result{}, nil // no re-queue diff --git a/pkg/config/reconciler_test.go b/pkg/config/reconciler_test.go new file mode 100644 index 0000000000..7592e2c4d8 --- /dev/null +++ b/pkg/config/reconciler_test.go @@ -0,0 +1,100 @@ +// Copyright 2024 The Carvel Authors. +// SPDX-License-Identifier: Apache-2.0 + +package config_test + +import ( + "context" + "errors" + "testing" + + kcconfig "carvel.dev/kapp-controller/pkg/config" + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8sfake "k8s.io/client-go/kubernetes/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +type fakeOSConfig struct { + applyCACertsErr error + applyProxyErr error + + applyCACertsCalled bool + applyProxyCalled bool +} + +func (f *fakeOSConfig) ApplyCACerts(string) error { + f.applyCACertsCalled = true + return f.applyCACertsErr +} + +func (f *fakeOSConfig) ApplyProxy(kcconfig.ProxyOpts) error { + f.applyProxyCalled = true + return f.applyProxyErr +} + +func newTestConfig(t *testing.T) *kcconfig.Config { + t.Helper() + + secret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kapp-controller-config", + Namespace: "default", + }, + Data: map[string][]byte{ + "caCerts": []byte("ca-certs"), + }, + } + + config, err := kcconfig.NewConfig(k8sfake.NewSimpleClientset(secret)) + require.NoError(t, err) + return config +} + +// Test_Reconciler_Reconcile_ReturnsErr_WhenApplyCACertsFails ensures that a +// transient failure applying CA certs (e.g. the sidecarexec RPC connection +// not being ready yet) is surfaced as an error so that controller-runtime +// requeues the request with backoff, instead of being silently swallowed. +func Test_Reconciler_Reconcile_ReturnsErr_WhenApplyCACertsFails(t *testing.T) { + osConfig := &fakeOSConfig{applyCACertsErr: errors.New("dial unix sidecarexec.sock: connect: no such file or directory")} + + reconciler := kcconfig.NewReconciler( + k8sfake.NewSimpleClientset(), newTestConfig(t), osConfig, logr.Discard()) + + _, err := reconciler.Reconcile(context.Background(), reconcile.Request{}) + assert.Error(t, err) + assert.True(t, osConfig.applyCACertsCalled) + assert.False(t, osConfig.applyProxyCalled, "should not attempt to apply proxy opts once CA certs failed to apply") +} + +// Test_Reconciler_Reconcile_ReturnsErr_WhenApplyProxyFails mirrors the CA +// certs case above for proxy configuration. +func Test_Reconciler_Reconcile_ReturnsErr_WhenApplyProxyFails(t *testing.T) { + osConfig := &fakeOSConfig{applyProxyErr: errors.New("dial unix sidecarexec.sock: connect: no such file or directory")} + + reconciler := kcconfig.NewReconciler( + k8sfake.NewSimpleClientset(), newTestConfig(t), osConfig, logr.Discard()) + + _, err := reconciler.Reconcile(context.Background(), reconcile.Request{}) + assert.Error(t, err) + assert.True(t, osConfig.applyCACertsCalled) + assert.True(t, osConfig.applyProxyCalled) +} + +// Test_Reconciler_Reconcile_Succeeds_WhenOSConfigApplied ensures the happy +// path continues to succeed with no requeue. +func Test_Reconciler_Reconcile_Succeeds_WhenOSConfigApplied(t *testing.T) { + osConfig := &fakeOSConfig{} + + reconciler := kcconfig.NewReconciler( + k8sfake.NewSimpleClientset(), newTestConfig(t), osConfig, logr.Discard()) + + result, err := reconciler.Reconcile(context.Background(), reconcile.Request{}) + assert.NoError(t, err) + assert.Equal(t, reconcile.Result{}, result) + assert.True(t, osConfig.applyCACertsCalled) + assert.True(t, osConfig.applyProxyCalled) +}