Fix error propagation in helmfile diff when Kubernetes is unreachable (#2149)

* Fix error propagation in helmfile diff when Kubernetes is unreachable

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* Fix golangci-lint issue: replace custom contains function with strings.Contains

Co-authored-by: zhaque44 <20215376+zhaque44@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
Co-authored-by: zhaque44 <20215376+zhaque44@users.noreply.github.com>

pkg/state/diff_error_propagation_test.go

@@ -0,0 +1,64 @@
+package state
+
+import (
+	"errors"
+	"strings"
+	"sync"
+	"testing"
+
+	"go.uber.org/zap"
+
+	"github.com/helmfile/helmfile/pkg/exectest"
+	"github.com/helmfile/helmfile/pkg/helmexec"
+)
+
+func TestIsReleaseInstalled_HandlesConnectionError(t *testing.T) {
+	logger := zap.NewNop().Sugar()
+
+	state := &HelmState{
+		logger: logger,
+	}
+
+	// Create a custom helm mock that fails on List operations
+	helm := &CustomFailingHelm{
+		Helm: &exectest.Helm{
+			DiffMutex:     &sync.Mutex{},
+			ChartsMutex:   &sync.Mutex{},
+			ReleasesMutex: &sync.Mutex{},
+			Helm3:         true,
+		},
+	}
+
+	release := ReleaseSpec{
+		Name:      "test-release",
+		Chart:     "test/chart",
+		Namespace: "default",
+	}
+
+	// This should return an error due to connection failure
+	_, err := state.isReleaseInstalled(helmexec.HelmContext{}, helm, release)
+
+	// Verify that error was propagated
+	if err == nil {
+		t.Fatalf("expected isReleaseInstalled to return error when Kubernetes is unreachable, but got no error")
+	}
+
+	if err.Error() == "" {
+		t.Fatalf("expected isReleaseInstalled to return meaningful error when Kubernetes is unreachable, but got empty error")
+	}
+
+	// Check if the error contains the expected message
+	expectedMsg := "Kubernetes cluster unreachable"
+	if err.Error() != expectedMsg && !strings.Contains(err.Error(), "Kubernetes cluster unreachable") {
+		t.Fatalf("expected error to contain 'Kubernetes cluster unreachable', but got: %v", err.Error())
+	}
+}
+
+// CustomFailingHelm wraps exectest.Helm and overrides List to simulate failures
+type CustomFailingHelm struct {
+	*exectest.Helm
+}
+
+func (h *CustomFailingHelm) List(context helmexec.HelmContext, filter string, flags ...string) (string, error) {
+	return "", errors.New("Kubernetes cluster unreachable: Get \"http://localhost:8080/version\": dial tcp [::1]:8080: connect: connection refused")
+}

pkg/state/state.go

@@ -1887,7 +1887,7 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu
 	mu := &sync.RWMutex{}
 	installedReleases := map[string]bool{}
 
-	isInstalled := func(r *ReleaseSpec) bool {
+	isInstalled := func(r *ReleaseSpec) (bool, error) {
 		id := ReleaseToID(r)
 
 		mu.RLock()
@@ -1895,19 +1895,19 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu
 		mu.RUnlock()
 
 		if ok {
-			return v
+			return v, nil
 		}
 
 		v, err := st.isReleaseInstalled(st.createHelmContext(r, 0), helm, *r)
 		if err != nil {
-			st.logger.Warnf("confirming if the release is already installed or not: %v", err)
-		} else {
-			mu.Lock()
-			installedReleases[id] = v
-			mu.Unlock()
+			return false, err
 		}
 
-		return v
+		mu.Lock()
+		installedReleases[id] = v
+		mu.Unlock()
+
+		return v, nil
 	}
 
 	releases := []*ReleaseSpec{}
@@ -1952,12 +1952,25 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu
 					suppressDiff = true
 				}
 
-				if opt.SkipDiffOnInstall && !isInstalled(release) {
-					results <- diffPrepareResult{release: release, upgradeDueToSkippedDiff: true, suppressDiff: suppressDiff}
-					continue
+				if opt.SkipDiffOnInstall {
+					installed, err := isInstalled(release)
+					if err != nil {
+						errs = append(errs, err)
+					} else if !installed {
+						results <- diffPrepareResult{release: release, upgradeDueToSkippedDiff: true, suppressDiff: suppressDiff}
+						continue
+					}
 				}
 
-				disableValidation := release.DisableValidationOnInstall != nil && *release.DisableValidationOnInstall && !isInstalled(release)
+				var disableValidation bool
+				if release.DisableValidationOnInstall != nil && *release.DisableValidationOnInstall {
+					installed, err := isInstalled(release)
+					if err != nil {
+						errs = append(errs, err)
+					} else {
+						disableValidation = !installed
+					}
+				}
 
 				// TODO We need a long-term fix for this :)
 				// See https://github.com/roboll/helmfile/issues/737