From c65ab08f894145ebeab7fe7d6a64bd559908dfb9 Mon Sep 17 00:00:00 2001 From: Ilya Drey Date: Thu, 3 Sep 2026 15:19:49 +0300 Subject: [PATCH] fix(core): propagate force reconcile to internal sources The force annotation reached only the HelmRelease. applyOCIRepositorySpec read it off the repository instead of the addon, and applyHelmChartSpec set no reconcile annotations at all, so forcing an addon re-ran the release against the artifact the source already had. The repository branch it replaces was a race: the addon controller watches HelmClusterAddonRepository with GenerationChangedPredicate, so an annotation change enqueues no addons, and the repository reconciler consumes the annotation right after its own attempt. An oci:// repository owns no internal source object - the artifact is pulled by a per-addon OCIRepository - so a force on it is now pushed onto those, from finish(), before the annotation is consumed. The helm:// path needs no equivalent: there the internal HelmRepository carries the request and its HelmCharts follow the re-indexed source on their own. setReconcileRequestAnnotations moves to base.go and takes a metav1.Object, replacing three copies. Signed-off-by: Ilya Drey --- .../helmclusteraddonrepository/reconciler.go | 16 ++- .../reconciler_test.go | 101 +++++++++++++ .../internal/services/base.go | 20 +++ .../internal/services/chart_service.go | 4 + .../internal/services/chart_service_test.go | 88 ++++++++++++ .../internal/services/oci_repo_service.go | 56 ++++++-- .../services/oci_repo_service_test.go | 135 +++++++++++++++++- .../internal/services/release_service.go | 13 -- 8 files changed, 407 insertions(+), 26 deletions(-) create mode 100644 images/operator-helm-controller/internal/services/chart_service_test.go diff --git a/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/reconciler.go b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/reconciler.go index fb3bdc3d..1f91fff0 100644 --- a/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/reconciler.go +++ b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/reconciler.go @@ -110,7 +110,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco Err: repoTypeErr, } - return r.finish(ctx, &repo, in, false) + return r.finish(ctx, &repo, in, repoType, false) } // Both services embed the same BaseRepoService with the same target namespace, @@ -145,7 +145,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco in.Catalog = &outcome.Catalog } - return r.finish(ctx, &repo, in, in.Attempted) + return r.finish(ctx, &repo, in, repoType, in.Attempted) } // finish applies the decision and consumes the force annotation when an attempt @@ -155,6 +155,7 @@ func (r *Reconciler) finish( ctx context.Context, repo *helmv1alpha1.HelmClusterAddonRepository, in Inputs, + repoType utils.InternalRepositoryType, attempted bool, ) (reconcile.Result, error) { decision := Evaluate(in) @@ -172,6 +173,17 @@ func (r *Reconciler) finish( } if attempted { + // An oci:// repository has no internal source object of its own, so a force + // request reaches the artifacts only through the addons' OCIRepositories. + // This runs before the annotation is consumed: a failure leaves the request + // in place to be retried. The helm:// path needs no equivalent - there the + // internal HelmRepository carries the request. + if repoType == utils.InternalOCIRepository && repo.ForceReconcileRequired() { + if err := r.ociRepositoryService.ForceReconcileInternalRepositories(ctx, repo.Name); err != nil { + return reconcile.Result{}, fmt.Errorf("failed to force reconcile internal oci repositories: %w", err) + } + } + if err := r.reconcileForceAnnotation(ctx, client.ObjectKeyFromObject(repo)); err != nil { return reconcile.Result{}, fmt.Errorf("failed to reconcile force annotation: %w", err) } diff --git a/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/reconciler_test.go b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/reconciler_test.go index 284e0e29..cb12b47b 100644 --- a/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/reconciler_test.go +++ b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/reconciler_test.go @@ -22,6 +22,7 @@ import ( "time" "github.com/Masterminds/semver/v3" + "github.com/werf/3p-fluxcd-pkg/apis/meta" sourcev1 "github.com/werf/nelm-source-controller/api/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -82,6 +83,11 @@ func newReconciler(t *testing.T, stub *stubRepoClient, objects ...client.Object) addon.Spec.Chart.HelmClusterAddonChartName, )} }). + WithIndex(&helmv1alpha1.HelmClusterAddon{}, index.AddonRepository, func(obj client.Object) []string { + addon := obj.(*helmv1alpha1.HelmClusterAddon) + + return []string{addon.Spec.Chart.HelmClusterAddonRepository} + }). Build() factory := func(_ utils.InternalRepositoryType) (repoclient.ClientInterface, error) { @@ -334,3 +340,98 @@ func TestReconcileDeleteCleansUpWhenURLNoLongerParses(t *testing.T) { } } } + +// TestReconcileForcedOCIRepositoryForcesAddonSources covers a force request on an +// oci:// repository. Unlike the helm:// path, where the internal HelmRepository +// carries the request and the HelmCharts follow it, an OCI repository has no +// internal source object of its own: the artifacts are pulled by the per-addon +// OCIRepositories, so the request must be pushed onto those. +func TestReconcileForcedOCIRepositoryForcesAddonSources(t *testing.T) { + repo := ociRepository() + addon := &helmv1alpha1.HelmClusterAddon{ + ObjectMeta: metav1.ObjectMeta{Name: "consumer", Generation: 1}, + Spec: helmv1alpha1.HelmClusterAddonSpec{ + Namespace: "app", + Chart: helmv1alpha1.HelmClusterAddonChartRef{ + HelmClusterAddonRepository: repo.Name, + HelmClusterAddonChartName: "podinfo", + Version: "6.7.1", + }, + }, + } + source := &sourcev1.OCIRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: utils.GetInternalOCIRepositoryName(addon.Name), + Namespace: helmv1alpha1.TargetNamespace, + }, + } + stub := &stubRepoClient{charts: []repoclient.Chart{{ + Name: "podinfo", + Versions: []repoclient.ChartVersion{{Version: semver.MustParse("6.7.1")}}, + }}} + + r, c := newReconciler(t, stub, repo, addon, source) + reconcileUntilStable(t, r, repo.Name) + + stored := &helmv1alpha1.HelmClusterAddonRepository{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(repo), stored); err != nil { + t.Fatalf("getting repository: %v", err) + } + stored.Annotations = map[string]string{helmv1alpha1.AnnotationForceReconcile: "2026-01-01T00:00:00Z"} + if err := c.Update(context.Background(), stored); err != nil { + t.Fatalf("annotating repository: %v", err) + } + + if _, err := r.Reconcile(context.Background(), reconcile.Request{ + NamespacedName: types.NamespacedName{Name: repo.Name}, + }); err != nil { + t.Fatalf("Reconcile returned %v", err) + } + + forced := &sourcev1.OCIRepository{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(source), forced); err != nil { + t.Fatalf("getting internal oci repository: %v", err) + } + if forced.Annotations[meta.ReconcileRequestAnnotation] == "" { + t.Errorf("%s must be pushed onto the addon source by a forced repository", meta.ReconcileRequestAnnotation) + } +} + +// TestReconcileUnforcedOCIRepositoryLeavesAddonSources is the complement: a +// scheduled synchronization must not stamp the addon sources, or every pass would +// make the source controller re-pull every artifact of the repository. +func TestReconcileUnforcedOCIRepositoryLeavesAddonSources(t *testing.T) { + repo := ociRepository() + addon := &helmv1alpha1.HelmClusterAddon{ + ObjectMeta: metav1.ObjectMeta{Name: "consumer", Generation: 1}, + Spec: helmv1alpha1.HelmClusterAddonSpec{ + Namespace: "app", + Chart: helmv1alpha1.HelmClusterAddonChartRef{ + HelmClusterAddonRepository: repo.Name, + HelmClusterAddonChartName: "podinfo", + Version: "6.7.1", + }, + }, + } + source := &sourcev1.OCIRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: utils.GetInternalOCIRepositoryName(addon.Name), + Namespace: helmv1alpha1.TargetNamespace, + }, + } + stub := &stubRepoClient{charts: []repoclient.Chart{{ + Name: "podinfo", + Versions: []repoclient.ChartVersion{{Version: semver.MustParse("6.7.1")}}, + }}} + + r, c := newReconciler(t, stub, repo, addon, source) + reconcileUntilStable(t, r, repo.Name) + + untouched := &sourcev1.OCIRepository{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(source), untouched); err != nil { + t.Fatalf("getting internal oci repository: %v", err) + } + if _, found := untouched.Annotations[meta.ReconcileRequestAnnotation]; found { + t.Errorf("%s must not be pushed onto the addon source by a scheduled synchronization", meta.ReconcileRequestAnnotation) + } +} diff --git a/images/operator-helm-controller/internal/services/base.go b/images/operator-helm-controller/internal/services/base.go index 92a22448..87f51ff9 100644 --- a/images/operator-helm-controller/internal/services/base.go +++ b/images/operator-helm-controller/internal/services/base.go @@ -19,7 +19,9 @@ package services import ( "context" "fmt" + "time" + "github.com/werf/3p-fluxcd-pkg/apis/meta" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -267,3 +269,21 @@ func (s *BaseRepoService) reconcileTLSSecret(ctx context.Context, repo *helmv1al return nil } + +// setReconcileRequestAnnotations stamps the flux reconcile/force request +// annotations so the controller owning obj reconciles it immediately instead of +// waiting for its next interval. Both are stamped with the same timestamp: +// ForceRequestAnnotation is only honoured when it matches +// ReconcileRequestAnnotation. +func setReconcileRequestAnnotations(obj metav1.Object) { + annotations := obj.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + + ts := time.Now().UTC().Format(time.RFC3339) + annotations[meta.ForceRequestAnnotation] = ts + annotations[meta.ReconcileRequestAnnotation] = ts + + obj.SetAnnotations(annotations) +} diff --git a/images/operator-helm-controller/internal/services/chart_service.go b/images/operator-helm-controller/internal/services/chart_service.go index 59f3c7b2..eeb1a058 100644 --- a/images/operator-helm-controller/internal/services/chart_service.go +++ b/images/operator-helm-controller/internal/services/chart_service.go @@ -140,6 +140,10 @@ func (s *ChartService) CleanupHelmChart(ctx context.Context, addon *helmv1alpha1 } func applyHelmChartSpec(addon *helmv1alpha1.HelmClusterAddon, existing *sourcev1.HelmChart) { + if addon.ForceReconcileRequired() { + setReconcileRequestAnnotations(existing) + } + if existing.Labels == nil { existing.Labels = map[string]string{} } diff --git a/images/operator-helm-controller/internal/services/chart_service_test.go b/images/operator-helm-controller/internal/services/chart_service_test.go new file mode 100644 index 00000000..f5e844b6 --- /dev/null +++ b/images/operator-helm-controller/internal/services/chart_service_test.go @@ -0,0 +1,88 @@ +/* +Copyright 2026 Flant JSC. + +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 services + +import ( + "context" + "testing" + + "github.com/werf/3p-fluxcd-pkg/apis/meta" + sourcev1 "github.com/werf/nelm-source-controller/api/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" + "github.com/deckhouse/operator-helm/internal/utils" +) + +func newChartService(t *testing.T, objects ...client.Object) (*ChartService, client.Client) { + t.Helper() + + scheme := testScheme(t) + if err := sourcev1.AddToScheme(scheme); err != nil { + t.Fatalf("registering source scheme: %v", err) + } + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() + + return NewChartService(c, scheme, testNamespace), c +} + +// TestEnsureHelmChartForcesReconcileFromAddon covers the force reconcile +// annotation applied to the HelmClusterAddon: on the internal Helm repository +// path it must reach the HelmChart, so that a forced addon re-pulls its source +// instead of only nudging the HelmRelease. +func TestEnsureHelmChartForcesReconcileFromAddon(t *testing.T) { + addon := testAddon() + addon.Annotations = map[string]string{helmv1alpha1.AnnotationForceReconcile: "2026-01-01T00:00:00Z"} + service, c := newChartService(t, addon) + + service.EnsureHelmChart(context.Background(), addon) + + chart := &sourcev1.HelmChart{} + key := client.ObjectKey{Name: utils.GetInternalHelmChartName(addon.Name), Namespace: testNamespace} + if err := c.Get(context.Background(), key, chart); err != nil { + t.Fatalf("helm chart was not created: %v", err) + } + + if chart.Annotations[meta.ReconcileRequestAnnotation] == "" { + t.Errorf("%s must be stamped on the helm chart", meta.ReconcileRequestAnnotation) + } + if chart.Annotations[meta.ForceRequestAnnotation] == "" { + t.Errorf("%s must be stamped on the helm chart", meta.ForceRequestAnnotation) + } +} + +// TestEnsureHelmChartDoesNotForceReconcileWithoutAnnotation is the complement: +// an unannotated addon must not stamp a fresh timestamp on every pass, which +// would make the source controller re-reconcile continuously. +func TestEnsureHelmChartDoesNotForceReconcileWithoutAnnotation(t *testing.T) { + addon := testAddon() + service, c := newChartService(t, addon) + + service.EnsureHelmChart(context.Background(), addon) + + chart := &sourcev1.HelmChart{} + key := client.ObjectKey{Name: utils.GetInternalHelmChartName(addon.Name), Namespace: testNamespace} + if err := c.Get(context.Background(), key, chart); err != nil { + t.Fatalf("helm chart was not created: %v", err) + } + + if _, found := chart.Annotations[meta.ReconcileRequestAnnotation]; found { + t.Errorf("%s must not be stamped without a force request", meta.ReconcileRequestAnnotation) + } +} diff --git a/images/operator-helm-controller/internal/services/oci_repo_service.go b/images/operator-helm-controller/internal/services/oci_repo_service.go index 1a64b043..e1678fe8 100644 --- a/images/operator-helm-controller/internal/services/oci_repo_service.go +++ b/images/operator-helm-controller/internal/services/oci_repo_service.go @@ -19,11 +19,11 @@ package services import ( "context" "fmt" - "time" "github.com/werf/3p-fluxcd-pkg/apis/meta" sourcev1 "github.com/werf/nelm-source-controller/api/v1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -32,6 +32,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" + "github.com/deckhouse/operator-helm/internal/index" "github.com/deckhouse/operator-helm/internal/manager/status" "github.com/deckhouse/operator-helm/internal/utils" ) @@ -140,6 +141,50 @@ func (s *OCIRepoService) EnsureInternalOCIRepository( } } +// ForceReconcileInternalRepositories stamps the reconcile request annotations on +// the internal OCIRepository of every addon that references repoName. +// +// An oci:// repository has no internal source object of its own: the artifact is +// pulled per addon, so a force request on the repository reaches the artifacts +// only through its addons' OCIRepositories. The helm:// path needs no equivalent - +// there the internal HelmRepository carries the request and its HelmCharts follow +// the re-indexed source on their own. +// +// An addon whose internal OCIRepository does not exist yet is skipped: the force +// request must not be blocked by an addon that has not reached the point of +// building one. +func (s *OCIRepoService) ForceReconcileInternalRepositories(ctx context.Context, repoName string) error { + addons := &helmv1alpha1.HelmClusterAddonList{} + if err := s.Client.List(ctx, addons, client.MatchingFields{index.AddonRepository: repoName}); err != nil { + return fmt.Errorf("listing addons of repository %s: %w", repoName, err) + } + + for i := range addons.Items { + name := utils.GetInternalOCIRepositoryName(addons.Items[i].Name) + nn := types.NamespacedName{Name: name, Namespace: s.TargetNamespace} + + ociRepo := &sourcev1.OCIRepository{} + if err := s.Client.Get(ctx, nn, ociRepo); err != nil { + if apierrors.IsNotFound(err) { + continue + } + + return fmt.Errorf("getting internal oci repository %s: %w", name, err) + } + + base := ociRepo.DeepCopy() + setReconcileRequestAnnotations(ociRepo) + + // The internal repository may be removed between the get and the patch, + // which is the same case as the one skipped above. + if err := s.Client.Patch(ctx, ociRepo, client.MergeFrom(base)); client.IgnoreNotFound(err) != nil { + return fmt.Errorf("requesting reconciliation of internal oci repository %s: %w", name, err) + } + } + + return nil +} + func (s *OCIRepoService) CleanupOCIRepository(ctx context.Context, repoName string) error { resources := []struct { name string @@ -190,13 +235,8 @@ func applyOCIRepositorySpec( mediaType string, existing *sourcev1.OCIRepository, ) { - if repo.ForceReconcileRequired() { - if existing.Annotations == nil { - existing.Annotations = map[string]string{} - } - ts := time.Now().UTC().Format(time.RFC3339) - existing.Annotations[meta.ForceRequestAnnotation] = ts - existing.Annotations[meta.ReconcileRequestAnnotation] = ts + if addon.ForceReconcileRequired() { + setReconcileRequestAnnotations(existing) } existing.Spec.URL = repo.Spec.URL diff --git a/images/operator-helm-controller/internal/services/oci_repo_service_test.go b/images/operator-helm-controller/internal/services/oci_repo_service_test.go index ee6cce40..ea9f0006 100644 --- a/images/operator-helm-controller/internal/services/oci_repo_service_test.go +++ b/images/operator-helm-controller/internal/services/oci_repo_service_test.go @@ -20,13 +20,14 @@ import ( "context" "testing" + "github.com/werf/3p-fluxcd-pkg/apis/meta" + sourcev1 "github.com/werf/nelm-source-controller/api/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - sourcev1 "github.com/werf/nelm-source-controller/api/v1" - helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" + "github.com/deckhouse/operator-helm/internal/index" "github.com/deckhouse/operator-helm/internal/utils" ) @@ -38,11 +39,28 @@ func newOCIRepoService(t *testing.T, objects ...client.Object) (*OCIRepoService, t.Fatalf("registering source scheme: %v", err) } - c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objects...). + WithIndex(&helmv1alpha1.HelmClusterAddon{}, index.AddonRepository, func(obj client.Object) []string { + addon := obj.(*helmv1alpha1.HelmClusterAddon) + + return []string{addon.Spec.Chart.HelmClusterAddonRepository} + }). + Build() return NewOCIRepoService(c, scheme, testNamespace), c } +func internalOCIRepository(addonName string) *sourcev1.OCIRepository { + return &sourcev1.OCIRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: utils.GetInternalOCIRepositoryName(addonName), + Namespace: testNamespace, + }, + } +} + func testAddon() *helmv1alpha1.HelmClusterAddon { return &helmv1alpha1.HelmClusterAddon{ ObjectMeta: metav1.ObjectMeta{Name: "consumer", Generation: 1}, @@ -180,3 +198,114 @@ func TestEnsureInternalOCIRepositoryDoesNotRelabelReadyChildOnRemovedVersion(t * t.Fatalf("message is %q, want the child's own message untouched", result.Status.Message) } } + +// TestEnsureInternalOCIRepositoryForcesReconcileFromAddon covers the force +// reconcile annotation applied to the HelmClusterAddon itself: it must reach the +// internal OCIRepository, otherwise the source is never re-pulled and only the +// HelmRelease is nudged. +func TestEnsureInternalOCIRepositoryForcesReconcileFromAddon(t *testing.T) { + addon, repo := testAddon(), ociTestRepository() + addon.Annotations = map[string]string{helmv1alpha1.AnnotationForceReconcile: "2026-01-01T00:00:00Z"} + service, c := newOCIRepoService(t, addon, repo) + + version := &helmv1alpha1.HelmClusterAddonChartVersion{ + Version: "6.7.1", + MediaType: "application/tar+gzip", + } + + service.EnsureInternalOCIRepository(context.Background(), addon, repo, version) + + ociRepo := &sourcev1.OCIRepository{} + key := client.ObjectKey{Name: utils.GetInternalOCIRepositoryName(addon.Name), Namespace: testNamespace} + if err := c.Get(context.Background(), key, ociRepo); err != nil { + t.Fatalf("oci repository was not created: %v", err) + } + + if ociRepo.Annotations[meta.ReconcileRequestAnnotation] == "" { + t.Errorf("%s must be stamped on the oci repository", meta.ReconcileRequestAnnotation) + } + if ociRepo.Annotations[meta.ForceRequestAnnotation] == "" { + t.Errorf("%s must be stamped on the oci repository", meta.ForceRequestAnnotation) + } +} + +// TestEnsureInternalOCIRepositoryDoesNotForceReconcileWithoutAnnotation is the +// complement: an unannotated addon must not stamp a fresh timestamp on every +// pass, which would make the source controller re-reconcile continuously. +func TestEnsureInternalOCIRepositoryDoesNotForceReconcileWithoutAnnotation(t *testing.T) { + addon, repo := testAddon(), ociTestRepository() + service, c := newOCIRepoService(t, addon, repo) + + version := &helmv1alpha1.HelmClusterAddonChartVersion{ + Version: "6.7.1", + MediaType: "application/tar+gzip", + } + + service.EnsureInternalOCIRepository(context.Background(), addon, repo, version) + + ociRepo := &sourcev1.OCIRepository{} + key := client.ObjectKey{Name: utils.GetInternalOCIRepositoryName(addon.Name), Namespace: testNamespace} + if err := c.Get(context.Background(), key, ociRepo); err != nil { + t.Fatalf("oci repository was not created: %v", err) + } + + if _, found := ociRepo.Annotations[meta.ReconcileRequestAnnotation]; found { + t.Errorf("%s must not be stamped without a force request", meta.ReconcileRequestAnnotation) + } +} + +// TestForceReconcileInternalRepositoriesStampsOnlyItsOwnAddons covers the force +// reconcile annotation applied to an oci:// HelmClusterAddonRepository: unlike the +// helm:// path, where the internal HelmRepository re-indexes and the HelmCharts +// follow, an OCI repository has no intermediate source object, so the request must +// be pushed onto the internal OCIRepository of each addon that references it - and +// only of those addons. +func TestForceReconcileInternalRepositoriesStampsOnlyItsOwnAddons(t *testing.T) { + addon := testAddon() + foreign := testAddon() + foreign.Name = "foreign" + foreign.Spec.Chart.HelmClusterAddonRepository = "another" + + service, c := newOCIRepoService(t, + addon, foreign, + internalOCIRepository(addon.Name), internalOCIRepository(foreign.Name), + ) + + if err := service.ForceReconcileInternalRepositories(context.Background(), "example"); err != nil { + t.Fatalf("forcing internal repositories: %v", err) + } + + ociRepo := &sourcev1.OCIRepository{} + key := client.ObjectKey{Name: utils.GetInternalOCIRepositoryName(addon.Name), Namespace: testNamespace} + if err := c.Get(context.Background(), key, ociRepo); err != nil { + t.Fatalf("getting oci repository: %v", err) + } + if ociRepo.Annotations[meta.ReconcileRequestAnnotation] == "" { + t.Errorf("%s must be stamped on the oci repository of the addon", meta.ReconcileRequestAnnotation) + } + if ociRepo.Annotations[meta.ForceRequestAnnotation] == "" { + t.Errorf("%s must be stamped on the oci repository of the addon", meta.ForceRequestAnnotation) + } + + foreignRepo := &sourcev1.OCIRepository{} + key = client.ObjectKey{Name: utils.GetInternalOCIRepositoryName(foreign.Name), Namespace: testNamespace} + if err := c.Get(context.Background(), key, foreignRepo); err != nil { + t.Fatalf("getting foreign oci repository: %v", err) + } + if _, found := foreignRepo.Annotations[meta.ReconcileRequestAnnotation]; found { + t.Errorf("%s must not be stamped on an addon of another repository", meta.ReconcileRequestAnnotation) + } +} + +// TestForceReconcileInternalRepositoriesToleratesMissingSource covers the addon +// that has no internal OCIRepository yet - it has just been created, or it never +// reached the point of building one. A force on the repository must not fail +// because of it, otherwise the request is retried forever. +func TestForceReconcileInternalRepositoriesToleratesMissingSource(t *testing.T) { + addon := testAddon() + service, _ := newOCIRepoService(t, addon) + + if err := service.ForceReconcileInternalRepositories(context.Background(), "example"); err != nil { + t.Fatalf("a missing internal oci repository must not fail the force request: %v", err) + } +} diff --git a/images/operator-helm-controller/internal/services/release_service.go b/images/operator-helm-controller/internal/services/release_service.go index aac9c5a4..8528c8e2 100644 --- a/images/operator-helm-controller/internal/services/release_service.go +++ b/images/operator-helm-controller/internal/services/release_service.go @@ -22,7 +22,6 @@ import ( "strings" "time" - "github.com/werf/3p-fluxcd-pkg/apis/meta" helmv2 "github.com/werf/3p-helm-controller/api/v2" sourcev1 "github.com/werf/nelm-source-controller/api/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -172,18 +171,6 @@ func (s *ReleaseService) SyncReleaseSpec(ctx context.Context, addon *helmv1alpha return nil } -// setReconcileRequestAnnotations stamps the flux reconcile/force request -// annotations so helm-controller reconciles the release immediately. -func setReconcileRequestAnnotations(release *helmv2.HelmRelease) { - if release.Annotations == nil { - release.Annotations = map[string]string{} - } - - ts := time.Now().UTC().Format(time.RFC3339) - release.Annotations[meta.ForceRequestAnnotation] = ts - release.Annotations[meta.ReconcileRequestAnnotation] = ts -} - func applyHelmReleaseSpec(addon *helmv1alpha1.HelmClusterAddon, existing *helmv2.HelmRelease, repoType utils.InternalRepositoryType, targetNamespace string) error { if addon.ForceReconcileRequired() { setReconcileRequestAnnotations(existing)