diff --git a/go.mod b/go.mod index 118374e07..233acaa62 100644 --- a/go.mod +++ b/go.mod @@ -27,7 +27,7 @@ require ( github.com/pkg/errors v0.9.1 github.com/stretchr/testify v1.11.1 github.com/tidwall/gjson v1.19.0 - github.com/werf/nelm v1.26.1 + github.com/werf/nelm v1.26.2 google.golang.org/protobuf v1.36.11 gopkg.in/yaml.v3 v3.0.1 helm.sh/helm/v3 v3.19.5 diff --git a/go.sum b/go.sum index c84e18995..133fe3d58 100644 --- a/go.sum +++ b/go.sum @@ -548,8 +548,8 @@ github.com/werf/lockgate v0.1.1 h1:S400JFYjtWfE4i4LY9FA8zx0fMdfui9DPrBiTciCrx4= github.com/werf/lockgate v0.1.1/go.mod h1:0yIFSLq9ausy6ejNxF5uUBf/Ib6daMAfXuCaTMZJzIE= github.com/werf/logboek v0.6.1 h1:oEe6FkmlKg0z0n80oZjLplj6sXcBeLleCkjfOOZEL2g= github.com/werf/logboek v0.6.1/go.mod h1:Gez5J4bxekyr6MxTmIJyId1F61rpO+0/V4vjCIEIZmk= -github.com/werf/nelm v1.26.1 h1:0gH8JuJlKZtPLpeRAt4H9mVLRzDQwNqhcllxcr2UTDg= -github.com/werf/nelm v1.26.1/go.mod h1:D3uU5e1dSBc0l0oo/iB6SjM2sLaXICqRDSWXAYF+vpk= +github.com/werf/nelm v1.26.2 h1:HKTYpMHGHecfQnHJ3QyK9zu+d3G3GoSjFWXZxhHIVco= +github.com/werf/nelm v1.26.2/go.mod h1:D3uU5e1dSBc0l0oo/iB6SjM2sLaXICqRDSWXAYF+vpk= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= diff --git a/pkg/helm/fallback_client.go b/pkg/helm/fallback_client.go index e26eb4e0f..f11abf81c 100644 --- a/pkg/helm/fallback_client.go +++ b/pkg/helm/fallback_client.go @@ -1,8 +1,13 @@ package helm import ( + "bytes" "errors" + "fmt" + "io/fs" "log/slog" + "os" + "path/filepath" "github.com/deckhouse/deckhouse/pkg/log" "github.com/werf/nelm/pkg/action" @@ -30,11 +35,15 @@ const ( // that helm3lib is able to recover from: action.ErrBuildPlan and // resource.ErrResourceDuplicatesFound. // +// Upgrades of charts that use werf.io/* annotations are never handed to helm3lib: it does +// not implement those semantics, so nelm's error is reported instead of a wrong deploy. +// // Read-only methods are always served by the primary client, both clients receive // label/annotation updates so the fallback is ready to take over at any moment. // -// When MetricStorage is configured the client increments -// metrics.HelmFallbackTotal{module, operation, error_type} on every fallback. +// When MetricStorage is configured the client increments both +// metrics.HelmFallbackTotal{module, operation, error_type} and its telemetry mirror on +// every fallback. type FallbackClient struct { primary client.HelmClient fallback client.HelmClient @@ -65,6 +74,67 @@ func shouldFallback(err error) bool { return errors.Is(err, action.ErrBuildPlan) || errors.Is(err, resource.ErrResourceDuplicatesFound) } +// werfAnnotationPrefix marks resources whose deploy semantics only nelm implements: +// ordering (werf.io/weight, werf.io/deploy-dependency-*), tracking +// (werf.io/track-termination-mode) and the like. helm3lib ignores such annotations. +const werfAnnotationPrefix = "werf.io/" + +// chartRenderSources are the parts of a chart helm renders manifests from, and therefore the +// only places an annotation can reach the release from. Hooks and image sources are left out +// on purpose: a module's hook is a compiled binary, so reading it would be slow and would +// match werf.io/ inside compiled code, disabling the fallback for a chart that never asked +// for werf semantics. +var chartRenderSources = []string{"Chart.yaml", "values.yaml", "templates", "charts"} + +// chartUsesWerfAnnotations reports whether the chart declares a werf.io/* annotation. It +// scans the raw sources rather than rendered manifests: the check runs on the fallback path, +// where nelm has just failed, so it must not depend on another render. A false positive only +// disables the fallback, which is the safe way to be wrong. +func chartUsesWerfAnnotations(chartPath string) (bool, error) { + if _, err := os.Stat(chartPath); err != nil { + return false, fmt.Errorf("scan chart for werf annotations: %w", err) + } + + found := false + + scan := func(path string, entry fs.DirEntry, err error) error { + switch { + case errors.Is(err, fs.ErrNotExist): + // this part of the chart is optional, nothing to scan + return fs.SkipDir + case err != nil: + return err + case entry.IsDir(): + return nil + } + + data, err := os.ReadFile(path) + if err != nil { + return err + } + + if bytes.Contains(data, []byte(werfAnnotationPrefix)) { + found = true + + return fs.SkipAll + } + + return nil + } + + for _, source := range chartRenderSources { + if err := filepath.WalkDir(filepath.Join(chartPath, source), scan); err != nil { + return false, fmt.Errorf("scan chart for werf annotations: %w", err) + } + + if found { + return true, nil + } + } + + return false, nil +} + // fallbackErrorType returns a low-cardinality label value describing which // recoverable nelm error triggered the fallback. func fallbackErrorType(err error) string { @@ -78,19 +148,23 @@ func fallbackErrorType(err error) string { } } -// recordFallback emits the helm_fallback_total counter when metric storage is -// configured. releaseName is used as the module label since within addon-operator -// helm release name and module name are equivalent. +// recordFallback emits the fallback counters when metric storage is configured. The +// telemetry mirror carries the same value and labels, it only differs in the prefix +// flant-integration collects by. releaseName is used as the module label since within +// addon-operator helm release name and module name are equivalent. func (c *FallbackClient) recordFallback(operation, releaseName string, err error) { if c.metricStorage == nil { return } - c.metricStorage.CounterAdd(metrics.HelmFallbackTotal, 1.0, map[string]string{ + labels := map[string]string{ pkg.MetricKeyModule: releaseName, pkg.MetricKeyOperation: operation, pkg.MetricKeyErrorType: fallbackErrorType(err), - }) + } + + c.metricStorage.CounterAdd(metrics.HelmFallbackTotal, 1.0, labels) + c.metricStorage.CounterAdd(metrics.HelmFallbackTelemetryTotal, 1.0, labels) } func (c *FallbackClient) UpgradeRelease(releaseName, chart string, valuesPaths, setValues []string, releaseLabels map[string]string, namespace string) error { @@ -99,6 +173,30 @@ func (c *FallbackClient) UpgradeRelease(releaseName, chart string, valuesPaths, return err } + // helm3lib does not implement werf.io/* semantics, so handing such a chart to it would + // deploy the release in a different order and track it differently. Report nelm's error + // instead of silently deploying the module wrong. + usesWerf, scanErr := chartUsesWerfAnnotations(chart) + if scanErr != nil { + c.logger.Warn("cannot check chart for werf.io annotations, not falling back to helm", + slog.String(pkg.LogKeyRelease, releaseName), + slog.String(pkg.LogKeyChart, chart), + log.Err(scanErr), + ) + + return err + } + + if usesWerf { + c.logger.Warn("chart uses werf.io annotations unsupported by helm, not falling back to helm", + slog.String(pkg.LogKeyRelease, releaseName), + slog.String(pkg.LogKeyChart, chart), + log.Err(err), + ) + + return err + } + c.recordFallback(fallbackOperationUpgrade, releaseName, err) c.logger.Warn("nelm UpgradeRelease failed with a recoverable error, falling back to helm", diff --git a/pkg/helm/fallback_client_test.go b/pkg/helm/fallback_client_test.go index 2532e01cf..9bb490c50 100644 --- a/pkg/helm/fallback_client_test.go +++ b/pkg/helm/fallback_client_test.go @@ -3,6 +3,8 @@ package helm import ( "errors" "fmt" + "os" + "path/filepath" "sync" "testing" @@ -120,6 +122,65 @@ func (c *fakeHelmClient) WithExtraLabels(labels map[string]string) { c.extraLabels = labels } +// chartDir writes a single-template chart and returns its path. The fallback path scans the +// chart on disk, so tests must point at a real directory laid out the way helm expects. +func chartDir(t *testing.T, template string) string { + t.Helper() + + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, "templates"), 0o700); err != nil { + t.Fatalf("create chart templates dir: %v", err) + } + + if err := os.WriteFile(filepath.Join(dir, "templates", "manifest.yaml"), []byte(template), 0o600); err != nil { + t.Fatalf("write chart template: %v", err) + } + + return dir +} + +// writeHook drops a file into the chart's hooks directory, standing in for a module's +// compiled hook binary. +func writeHook(t *testing.T, chartPath, content string) { + t.Helper() + + if err := os.MkdirAll(filepath.Join(chartPath, "hooks"), 0o700); err != nil { + t.Fatalf("create chart hooks dir: %v", err) + } + + if err := os.WriteFile(filepath.Join(chartPath, "hooks", "hook"), []byte(content), 0o700); err != nil { + t.Fatalf("write chart hook: %v", err) + } +} + +const ( + plainTemplate = "kind: Deployment\nmetadata:\n name: test\n" + werfTemplate = "kind: Deployment\nmetadata:\n name: test\n annotations:\n werf.io/weight: \"-1\"\n" +) + +func TestChartUsesWerfAnnotations(t *testing.T) { + g := NewWithT(t) + + uses, err := chartUsesWerfAnnotations(chartDir(t, werfTemplate)) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(uses).To(BeTrue()) + + uses, err = chartUsesWerfAnnotations(chartDir(t, plainTemplate)) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(uses).To(BeFalse()) + + // A hook binary can carry the annotation string in compiled code without the chart ever + // declaring it, so hooks must not be scanned. + withHook := chartDir(t, plainTemplate) + writeHook(t, withHook, "\x7fELF...werf.io/weight...") + uses, err = chartUsesWerfAnnotations(withHook) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(uses).To(BeFalse()) + + _, err = chartUsesWerfAnnotations(filepath.Join(t.TempDir(), "missing")) + g.Expect(err).Should(HaveOccurred()) +} + func TestShouldFallback(t *testing.T) { g := NewWithT(t) @@ -141,7 +202,7 @@ func TestFallbackClient_UpgradeRelease(t *testing.T) { ms := &fakeMetricStorage{} c := NewFallbackClient(primary, fallback, log.NewNop(), ms) - err := c.UpgradeRelease("r", "c", nil, nil, nil, "ns") + err := c.UpgradeRelease("r", chartDir(t, plainTemplate), nil, nil, nil, "ns") g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(primary.upgradeCalls).To(Equal(1)) g.Expect(fallback.upgradeCalls).To(Equal(0)) @@ -155,7 +216,7 @@ func TestFallbackClient_UpgradeRelease(t *testing.T) { ms := &fakeMetricStorage{} c := NewFallbackClient(primary, fallback, log.NewNop(), ms) - err := c.UpgradeRelease("r", "c", nil, nil, nil, "ns") + err := c.UpgradeRelease("r", chartDir(t, plainTemplate), nil, nil, nil, "ns") g.Expect(err).Should(MatchError("boom")) g.Expect(primary.upgradeCalls).To(Equal(1)) g.Expect(fallback.upgradeCalls).To(Equal(0)) @@ -172,18 +233,23 @@ func TestFallbackClient_UpgradeRelease(t *testing.T) { ms := &fakeMetricStorage{} c := NewFallbackClient(primary, fallback, log.NewNop(), ms) - err := c.UpgradeRelease("my-release", "c", nil, nil, nil, "ns") + err := c.UpgradeRelease("my-release", chartDir(t, plainTemplate), nil, nil, nil, "ns") g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(primary.upgradeCalls).To(Equal(1)) g.Expect(fallback.upgradeCalls).To(Equal(1)) + // the telemetry mirror is emitted alongside, with the same value and labels calls := ms.calls() - g.Expect(calls).To(HaveLen(1)) - g.Expect(calls[0].metric).To(Equal(metrics.HelmFallbackTotal)) - g.Expect(calls[0].value).To(Equal(1.0)) - g.Expect(calls[0].labels).To(HaveKeyWithValue(pkg.MetricKeyModule, "my-release")) - g.Expect(calls[0].labels).To(HaveKeyWithValue(pkg.MetricKeyOperation, "upgrade")) - g.Expect(calls[0].labels).To(HaveKeyWithValue(pkg.MetricKeyErrorType, "build_plan")) + g.Expect(calls).To(HaveLen(2)) + g.Expect([]string{calls[0].metric, calls[1].metric}).To(ConsistOf( + metrics.HelmFallbackTotal, metrics.HelmFallbackTelemetryTotal)) + + for _, call := range calls { + g.Expect(call.value).To(Equal(1.0)) + g.Expect(call.labels).To(HaveKeyWithValue(pkg.MetricKeyModule, "my-release")) + g.Expect(call.labels).To(HaveKeyWithValue(pkg.MetricKeyOperation, "upgrade")) + g.Expect(call.labels).To(HaveKeyWithValue(pkg.MetricKeyErrorType, "build_plan")) + } }) t.Run("falls back on resource.ErrResourceDuplicatesFound", func(t *testing.T) { @@ -196,15 +262,51 @@ func TestFallbackClient_UpgradeRelease(t *testing.T) { ms := &fakeMetricStorage{} c := NewFallbackClient(primary, fallback, log.NewNop(), ms) - err := c.UpgradeRelease("my-release", "c", nil, nil, nil, "ns") + err := c.UpgradeRelease("my-release", chartDir(t, plainTemplate), nil, nil, nil, "ns") g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(primary.upgradeCalls).To(Equal(1)) g.Expect(fallback.upgradeCalls).To(Equal(1)) calls := ms.calls() - g.Expect(calls).To(HaveLen(1)) - g.Expect(calls[0].labels).To(HaveKeyWithValue(pkg.MetricKeyErrorType, "resource_duplicates")) - g.Expect(calls[0].labels).To(HaveKeyWithValue(pkg.MetricKeyOperation, "upgrade")) + g.Expect(calls).To(HaveLen(2)) + + for _, call := range calls { + g.Expect(call.labels).To(HaveKeyWithValue(pkg.MetricKeyErrorType, "resource_duplicates")) + g.Expect(call.labels).To(HaveKeyWithValue(pkg.MetricKeyOperation, "upgrade")) + } + }) + + t.Run("chart with werf annotations is never handed to helm", func(t *testing.T) { + g := NewWithT(t) + primary := &fakeHelmClient{ + name: "primary", + upgradeErr: fmt.Errorf("install: %w", action.ErrBuildPlan), + } + fallback := &fakeHelmClient{name: "fallback"} + ms := &fakeMetricStorage{} + c := NewFallbackClient(primary, fallback, log.NewNop(), ms) + + err := c.UpgradeRelease("my-release", chartDir(t, werfTemplate), nil, nil, nil, "ns") + g.Expect(err).Should(MatchError(action.ErrBuildPlan)) + g.Expect(primary.upgradeCalls).To(Equal(1)) + g.Expect(fallback.upgradeCalls).To(Equal(0)) + g.Expect(ms.calls()).To(BeEmpty()) + }) + + t.Run("unscannable chart is never handed to helm", func(t *testing.T) { + g := NewWithT(t) + primary := &fakeHelmClient{ + name: "primary", + upgradeErr: fmt.Errorf("install: %w", action.ErrBuildPlan), + } + fallback := &fakeHelmClient{name: "fallback"} + ms := &fakeMetricStorage{} + c := NewFallbackClient(primary, fallback, log.NewNop(), ms) + + err := c.UpgradeRelease("my-release", filepath.Join(t.TempDir(), "missing"), nil, nil, nil, "ns") + g.Expect(err).Should(MatchError(action.ErrBuildPlan)) + g.Expect(fallback.upgradeCalls).To(Equal(0)) + g.Expect(ms.calls()).To(BeEmpty()) }) t.Run("fallback error is propagated and still counted", func(t *testing.T) { @@ -217,11 +319,11 @@ func TestFallbackClient_UpgradeRelease(t *testing.T) { ms := &fakeMetricStorage{} c := NewFallbackClient(primary, fallback, log.NewNop(), ms) - err := c.UpgradeRelease("r", "c", nil, nil, nil, "ns") + err := c.UpgradeRelease("r", chartDir(t, plainTemplate), nil, nil, nil, "ns") g.Expect(err).Should(MatchError("fallback failed")) g.Expect(primary.upgradeCalls).To(Equal(1)) g.Expect(fallback.upgradeCalls).To(Equal(1)) - g.Expect(ms.calls()).To(HaveLen(1)) + g.Expect(ms.calls()).To(HaveLen(2)) }) t.Run("nil metric storage is safe", func(t *testing.T) { @@ -233,7 +335,7 @@ func TestFallbackClient_UpgradeRelease(t *testing.T) { fallback := &fakeHelmClient{name: "fallback"} c := NewFallbackClient(primary, fallback, log.NewNop(), nil) - err := c.UpgradeRelease("r", "c", nil, nil, nil, "ns") + err := c.UpgradeRelease("r", chartDir(t, plainTemplate), nil, nil, nil, "ns") g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(fallback.upgradeCalls).To(Equal(1)) }) @@ -269,7 +371,7 @@ func TestFallbackClient_Render(t *testing.T) { g.Expect(out).To(Equal("fallback")) calls := ms.calls() - g.Expect(calls).To(HaveLen(1)) + g.Expect(calls).To(HaveLen(2)) g.Expect(calls[0].labels).To(HaveKeyWithValue(pkg.MetricKeyOperation, "render")) g.Expect(calls[0].labels).To(HaveKeyWithValue(pkg.MetricKeyErrorType, "resource_duplicates")) }) @@ -291,7 +393,7 @@ func TestFallbackClient_DeleteRelease(t *testing.T) { g.Expect(fallback.deleteCalls).To(Equal(1)) calls := ms.calls() - g.Expect(calls).To(HaveLen(1)) + g.Expect(calls).To(HaveLen(2)) g.Expect(calls[0].labels).To(HaveKeyWithValue(pkg.MetricKeyOperation, "delete")) g.Expect(calls[0].labels).To(HaveKeyWithValue(pkg.MetricKeyErrorType, "build_plan")) } diff --git a/pkg/helm/helm3lib/helm3lib.go b/pkg/helm/helm3lib/helm3lib.go index ae1f5f032..ea8e39837 100644 --- a/pkg/helm/helm3lib/helm3lib.go +++ b/pkg/helm/helm3lib/helm3lib.go @@ -214,6 +214,11 @@ func (h *LibClient) upgradeRelease(releaseName, modulePath string, valuesPaths [ upg.MaxHistory = int(options.HistoryMax) upg.Timeout = options.Timeout upg.Labels = labels + // The operator applies some of a module's resources (e.g. a ConversionWebhook) + // before the release runs, so they pre-exist without Helm ownership metadata and + // helm would refuse to import them. Adopt them instead of failing — this mirrors + // ForceAdoption in the nelm client, so both helm clients behave the same. + upg.TakeOwnership = true var resultValues chartutil.Values @@ -268,6 +273,8 @@ func (h *LibClient) upgradeRelease(releaseName, modulePath string, valuesPaths [ instClient.ReleaseName = releaseName instClient.UseReleaseName = true instClient.Labels = labels + // Adopt pre-applied resources on a first install too, see upg.TakeOwnership above. + instClient.TakeOwnership = true _, err = instClient.Run(loaded, resultValues) diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 326e77727..eebbdd85c 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -107,6 +107,10 @@ var ( HelmOperationSeconds = "{PREFIX}helm_operation_seconds" // HelmFallbackTotal counts how many times nelm operations had to fall back to helm3lib HelmFallbackTotal = "{PREFIX}helm_fallback_total" + // HelmFallbackTelemetryTotal mirrors HelmFallbackTotal under the d8_telemetry_ prefix so + // flant-integration ships the metric to DOP. + // The prefix is literal here (no {PREFIX} placeholder) and must stay that way. + HelmFallbackTelemetryTotal = "d8_telemetry_helm_fallback_total" // ============================================================================ // Task Queue Metrics @@ -202,6 +206,7 @@ func InitMetrics(prefix string) { ModuleHelmSeconds = ReplacePrefix(ModuleHelmSeconds, prefix) HelmOperationSeconds = ReplacePrefix(HelmOperationSeconds, prefix) HelmFallbackTotal = ReplacePrefix(HelmFallbackTotal, prefix) + // HelmFallbackTelemetryTotal is intentionally left alone: it keeps its d8_telemetry_ name. // ============================================================================ // Task Queue Metrics @@ -556,6 +561,15 @@ func registerHelmMetrics(metricStorage metricsstorage.Storage) error { return fmt.Errorf("can not register %s: %w", HelmFallbackTotal, err) } + _, err = metricStorage.RegisterCounter( + HelmFallbackTelemetryTotal, + []string{pkg.MetricKeyModule, pkg.MetricKeyOperation, pkg.MetricKeyErrorType}, + options.WithHelp("Counter of nelm Helm operations that fell back to helm3lib, labeled by module, operation and error type"), + ) + if err != nil { + return fmt.Errorf("can not register %s: %w", HelmFallbackTelemetryTotal, err) + } + return nil }