diff --git a/api/v1alpha1/conditions.go b/api/v1alpha1/conditions.go index ed5d37d..fd34bd3 100644 --- a/api/v1alpha1/conditions.go +++ b/api/v1alpha1/conditions.go @@ -34,6 +34,7 @@ const ( ReasonSuccess = "Success" ReasonFailed = "Failed" ReasonUninstallFailed = "UninstallFailed" + ReasonChartClaimConflict = "ChartClaimConflict" // HelmRelease error reasons ReasonReleaseFailed = "ReleaseFailed" diff --git a/images/operator-helm-controller/internal/controller/helmclusteraddon/controller.go b/images/operator-helm-controller/internal/controller/helmclusteraddon/controller.go index eb728f3..bec1053 100644 --- a/images/operator-helm-controller/internal/controller/helmclusteraddon/controller.go +++ b/images/operator-helm-controller/internal/controller/helmclusteraddon/controller.go @@ -45,6 +45,7 @@ func SetupWithManager(mgr ctrl.Manager) error { services.NewOCIRepoService(client, mgr.GetScheme(), helmv1alpha1.TargetNamespace), services.NewReleaseService(client, mgr.GetScheme(), helmv1alpha1.TargetNamespace), services.NewMaintenanceService(client, mgr.GetScheme(), helmv1alpha1.TargetNamespace), + services.NewClaimService(client, mgr.GetAPIReader(), helmv1alpha1.TargetNamespace), status.NewManager(client), ) diff --git a/images/operator-helm-controller/internal/reconcile/helmclusteraddon/reconciler.go b/images/operator-helm-controller/internal/reconcile/helmclusteraddon/reconciler.go index ca149c4..ded26ae 100644 --- a/images/operator-helm-controller/internal/reconcile/helmclusteraddon/reconciler.go +++ b/images/operator-helm-controller/internal/reconcile/helmclusteraddon/reconciler.go @@ -46,12 +46,19 @@ import ( // whose deletion is stuck and stops emitting events. const internalResourceDeletionRequeueInterval = 30 * time.Second +// chartClaimConflictRequeueInterval bounds how often an addon that lost the claim +// on its repository/chart pair re-checks whether the owner has released it. There +// is no watch on the claim Lease, so this periodic requeue is what lets a duplicate +// recover once the conflicting addon is deleted or repointed at another chart. +const chartClaimConflictRequeueInterval = 30 * time.Second + func New( client client.Client, chartService *services.ChartService, ociRepositoryService *services.OCIRepoService, releaseService *services.ReleaseService, maintenanceService *services.MaintenanceService, + claimService *services.ClaimService, statusManager *status.Manager, ) *Reconciler { return &Reconciler{ @@ -60,6 +67,7 @@ func New( ociRepositoryService: ociRepositoryService, releaseService: releaseService, maintenanceService: maintenanceService, + claimService: claimService, statusManager: statusManager, } } @@ -71,6 +79,7 @@ type Reconciler struct { ociRepositoryService *services.OCIRepoService releaseService *services.ReleaseService maintenanceService *services.MaintenanceService + claimService *services.ClaimService statusManager *status.Manager } @@ -90,6 +99,29 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco return r.reconcileDelete(ctx, addon) } + // Claim the repository/chart pair before anything else, including adding the + // finalizer. The claim is the authoritative, race-free guard on uniqueness (the + // webhook only fast-rejects the obvious duplicate on CREATE and cannot stop + // concurrent creates from racing past it). It must run before the finalizer + // because a duplicate that loses the race must not accrue a finalizer it would + // otherwise have to clean up: it simply surfaces the conflict on its status and + // requeues, recovering on its own once the owner releases the pair. + acquired, holder, err := r.claimService.Acquire(ctx, addon) + if err != nil { + return reconcile.Result{}, fmt.Errorf("acquiring chart claim: %w", err) + } + if !acquired { + return reconcile.Result{RequeueAfter: chartClaimConflictRequeueInterval}, r.statusManager.Update( + ctx, addon, status.NoopStatusMutator, status.NoopStatusMapper, + services.ReleaseResult{Status: status.Failed( + addon, + helmv1alpha1.ReasonChartClaimConflict, + fmt.Sprintf("chart %q is already used by helmclusteraddon/%s", addon.Spec.Chart.HelmClusterAddonChartName, holder), + nil, + )}, + ) + } + if utils.IsSystemNamespace(addon.Spec.Namespace) { return reconcile.Result{}, r.statusManager.Update(ctx, addon, status.NoopStatusMutator, status.NoopStatusMapper, services.ReleaseResult{Status: status.Failed( addon, @@ -110,6 +142,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco // would not trigger a follow-up reconcile. } + if err := r.claimService.ReleaseStale(ctx, addon); err != nil { + return reconcile.Result{}, fmt.Errorf("releasing stale chart claims: %w", err) + } + if r.maintenanceService.IsMaintenanceModeChangeRequired(addon) { maintenanceRes := r.maintenanceService.EnsureMaintenanceMode(ctx, addon) return reconcile.Result{}, r.statusManager.Update(ctx, addon, status.NoopStatusMutator, status.NoopStatusMapper, maintenanceRes, status.AsCondition(maintenanceRes, "Ready")) @@ -283,6 +319,14 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, addon *helmv1alpha1.He return r.awaitInternalResourceDeletion(ctx, addon, "internal repository", ociRepo) } + // Release the claim only once every downstream resource is gone: releasing it + // earlier would let another addon start reconciling the same chart while this + // one's release is still being uninstalled — exactly the collision the claim + // prevents. + if err := r.claimService.Release(ctx, addon); err != nil { + return reconcile.Result{}, fmt.Errorf("releasing chart claim: %w", err) + } + if err := retry.RetryOnConflict(retry.DefaultRetry, func() error { latestAddon := &helmv1alpha1.HelmClusterAddon{} if err := r.Get(ctx, client.ObjectKeyFromObject(addon), latestAddon); err != nil { diff --git a/images/operator-helm-controller/internal/services/chart_claim_service.go b/images/operator-helm-controller/internal/services/chart_claim_service.go new file mode 100644 index 0000000..ccccdee --- /dev/null +++ b/images/operator-helm-controller/internal/services/chart_claim_service.go @@ -0,0 +1,276 @@ +/* +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" + "fmt" + + coordinationv1 "k8s.io/api/coordination/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" + "github.com/deckhouse/operator-helm/internal/utils" +) + +// ClaimService enforces that at most one HelmClusterAddon uses a given +// repository/chart pair. A validating webhook cannot guarantee this: admission +// requests are served concurrently and before the object is persisted, so two +// simultaneous creates can both pass. Uniqueness is instead delegated to the API +// server's only atomic cross-object primitive — the uniqueness of an object name. +// Each repository/chart pair maps to a single Lease name; whoever creates that +// Lease first owns the pair, and every other addon reconciles into a conflict. +type ClaimService struct { + // reader reads from the API server directly (mgr.GetAPIReader()), bypassing the + // controller cache: an acquisition decision must never be made against stale data. + reader client.Reader + client client.Client + namespace string +} + +func NewClaimService(c client.Client, reader client.Reader, namespace string) *ClaimService { + return &ClaimService{ + reader: reader, + client: c, + namespace: namespace, + } +} + +// Acquire ensures the addon owns the claim for its repository/chart pair. It +// reports whether the claim is held by this addon; when it is not, holder names +// the addon that currently owns it so the caller can surface a conflict. +func (s *ClaimService) Acquire(ctx context.Context, addon *helmv1alpha1.HelmClusterAddon) (acquired bool, holder string, err error) { + nn := s.leaseKey(addon) + + lease := &coordinationv1.Lease{} + getErr := s.reader.Get(ctx, nn, lease) + switch { + case apierrors.IsNotFound(getErr): + created, createErr := s.create(ctx, nn, addon) + if createErr != nil { + return false, "", createErr + } + if created { + return true, addon.Name, nil + } + // Lost the create race against a concurrent reconcile; re-read the winning + // Lease authoritatively and fall through to the ownership check. + if err := s.reader.Get(ctx, nn, lease); err != nil { + return false, "", fmt.Errorf("getting chart claim lease: %w", err) + } + case getErr != nil: + return false, "", fmt.Errorf("getting chart claim lease: %w", getErr) + } + + holder = leaseHolder(lease) + if holder == addon.Name { + return true, holder, nil + } + + stale, err := s.isHolderStale(ctx, holder, nn.Name) + if err != nil { + return false, "", err + } + if !stale { + return false, holder, nil + } + + // The recorded holder is gone or no longer uses this chart; take the Lease over. + // The update is optimistic: if another addon takes it over first, our write is + // rejected with a conflict and we report the pair as claimed and requeue. + if err := s.takeOver(ctx, lease, addon); err != nil { + if apierrors.IsConflict(err) { + return false, holder, nil + } + return false, "", err + } + + return true, addon.Name, nil +} + +// OwnedBy reports whether the addon currently holds the claim Lease for its +// repository/chart pair. It reads through the direct reader for the same reason +// Acquire does: an ownership decision must not be made against stale cache data. +func (s *ClaimService) OwnedBy(ctx context.Context, addon *helmv1alpha1.HelmClusterAddon) (bool, error) { + lease := &coordinationv1.Lease{} + err := s.reader.Get(ctx, s.leaseKey(addon), lease) + if apierrors.IsNotFound(err) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("getting chart claim lease: %w", err) + } + + return leaseHolder(lease) == addon.Name, nil +} + +// Release deletes the claim Lease, but only if this addon still owns it, so a +// duplicate addon that never acquired the pair cannot free the real owner's claim. +func (s *ClaimService) Release(ctx context.Context, addon *helmv1alpha1.HelmClusterAddon) error { + nn := s.leaseKey(addon) + + lease := &coordinationv1.Lease{} + if err := s.reader.Get(ctx, nn, lease); err != nil { + return client.IgnoreNotFound(err) + } + + if leaseHolder(lease) != addon.Name { + return nil + } + + // Delete only this exact revision: if the Lease was taken over between the read + // and the delete, the precondition fails and we leave the new owner's claim intact. + err := s.client.Delete(ctx, lease, client.Preconditions{ResourceVersion: &lease.ResourceVersion}) + if apierrors.IsConflict(err) { + return nil + } + if client.IgnoreNotFound(err) != nil { + return fmt.Errorf("releasing chart claim lease: %w", err) + } + + return nil +} + +// ReleaseStale deletes claim Leases this addon still owns for repository/chart +// pairs it no longer references — e.g. after the chart ref changed. The Lease name +// is derived from the current spec, so a chart change makes Acquire claim a new +// Lease and orphan the old one; the previous pair is not stored anywhere. The +// orphans are found instead by the source-name label every claim already carries. +// Only the addon's own claims (by HolderIdentity) other than the current one are +// removed, so a pair taken over by another addon is left intact. +func (s *ClaimService) ReleaseStale(ctx context.Context, addon *helmv1alpha1.HelmClusterAddon) error { + current := s.leaseKey(addon).Name + + // Direct reader, not the cached client: claim Leases are not watched, so the + // informer cache has no data for them (same reason Acquire/Release use reader). + leases := &coordinationv1.LeaseList{} + if err := s.reader.List(ctx, leases, + client.InNamespace(s.namespace), + client.MatchingLabels{ + helmv1alpha1.LabelManagedBy: helmv1alpha1.LabelManagedByValue, + helmv1alpha1.HelmClusterAddonLabelSourceName: addon.Name, + }, + ); err != nil { + return fmt.Errorf("listing chart claim leases: %w", err) + } + + for i := range leases.Items { + lease := &leases.Items[i] + if lease.Name == current || leaseHolder(lease) != addon.Name { + continue + } + + // Delete only this exact revision: if the Lease was taken over between the + // list and the delete, the precondition fails and we leave it intact. + err := s.client.Delete(ctx, lease, client.Preconditions{ResourceVersion: &lease.ResourceVersion}) + if apierrors.IsConflict(err) { + continue + } + if client.IgnoreNotFound(err) != nil { + return fmt.Errorf("releasing stale chart claim lease %q: %w", lease.Name, err) + } + } + + return nil +} + +func (s *ClaimService) create(ctx context.Context, nn types.NamespacedName, addon *helmv1alpha1.HelmClusterAddon) (created bool, err error) { + holder := addon.Name + lease := &coordinationv1.Lease{ + ObjectMeta: metav1.ObjectMeta{ + Name: nn.Name, + Namespace: nn.Namespace, + Labels: claimLabels(addon), + }, + Spec: coordinationv1.LeaseSpec{ + HolderIdentity: &holder, + }, + } + + err = s.client.Create(ctx, lease) + if apierrors.IsAlreadyExists(err) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("creating chart claim lease: %w", err) + } + + return true, nil +} + +func (s *ClaimService) takeOver(ctx context.Context, lease *coordinationv1.Lease, addon *helmv1alpha1.HelmClusterAddon) error { + holder := addon.Name + lease.Spec.HolderIdentity = &holder + + if lease.Labels == nil { + lease.Labels = map[string]string{} + } + for k, v := range claimLabels(addon) { + lease.Labels[k] = v + } + + return s.client.Update(ctx, lease) +} + +// isHolderStale reports whether the recorded holder no longer keeps this claim: it +// either no longer exists or its current spec points at a different chart, in which +// case the Lease is an orphan left behind by a deletion or a chart change. +func (s *ClaimService) isHolderStale(ctx context.Context, holder, leaseName string) (bool, error) { + if holder == "" { + return true, nil + } + + other := &helmv1alpha1.HelmClusterAddon{} + err := s.reader.Get(ctx, types.NamespacedName{Name: holder}, other) + if apierrors.IsNotFound(err) { + return true, nil + } + if err != nil { + return false, fmt.Errorf("getting chart claim holder: %w", err) + } + + holderLease := utils.GetChartClaimLeaseName( + other.Spec.Chart.HelmClusterAddonRepository, other.Spec.Chart.HelmClusterAddonChartName, + ) + + return holderLease != leaseName, nil +} + +func (s *ClaimService) leaseKey(addon *helmv1alpha1.HelmClusterAddon) types.NamespacedName { + return types.NamespacedName{ + Name: utils.GetChartClaimLeaseName(addon.Spec.Chart.HelmClusterAddonRepository, addon.Spec.Chart.HelmClusterAddonChartName), + Namespace: s.namespace, + } +} + +func leaseHolder(lease *coordinationv1.Lease) string { + if lease.Spec.HolderIdentity == nil { + return "" + } + + return *lease.Spec.HolderIdentity +} + +func claimLabels(addon *helmv1alpha1.HelmClusterAddon) map[string]string { + return map[string]string{ + helmv1alpha1.LabelManagedBy: helmv1alpha1.LabelManagedByValue, + helmv1alpha1.HelmClusterAddonLabelSourceName: addon.Name, + } +} diff --git a/images/operator-helm-controller/internal/utils/name.go b/images/operator-helm-controller/internal/utils/name.go index ae40712..29b4e13 100644 --- a/images/operator-helm-controller/internal/utils/name.go +++ b/images/operator-helm-controller/internal/utils/name.go @@ -127,6 +127,18 @@ func GetInternalOCIRepositoryName(addonName string) string { return strings.TrimRight(result, "-") + postfix } +// GetChartClaimLeaseName derives the name of the Lease that guards uniqueness of a +// repository/chart pair across HelmClusterAddon objects. The name is a pure hash of +// the pair: repository and chart names may contain characters that are invalid in an +// object name (uppercase, dots for OCI references), and the hash is always a valid +// DNS subdomain. +func GetChartClaimLeaseName(repoName, chartName string) string { + prefix := "hca-claim" + hash := GetHash(fmt.Sprintf("%s-%s-%s", prefix, repoName, chartName)) + + return prefix + "-" + hash +} + func GetInternalHelmRepositoryName(addonRepositoryName string) string { prefix := "hcar" hash := GetHash(fmt.Sprintf("%s-%s", prefix, addonRepositoryName)) diff --git a/images/operator-helm-controller/internal/webhook/helmclusteraddon/webhook.go b/images/operator-helm-controller/internal/webhook/helmclusteraddon/webhook.go index 941946d..5f7a5a2 100644 --- a/images/operator-helm-controller/internal/webhook/helmclusteraddon/webhook.go +++ b/images/operator-helm-controller/internal/webhook/helmclusteraddon/webhook.go @@ -19,17 +19,23 @@ package helmclusteraddon import ( "context" "fmt" + "slices" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" + "github.com/deckhouse/operator-helm/internal/services" "github.com/deckhouse/operator-helm/internal/utils" ) const addonChartIndex = ".spec.chart.repoAndChart" +var uniquenessBypassUsernames = []string{ + "system:serviceaccount:d8-operator-helm:operator-helm-controller", +} + func SetupIndexes(mgr ctrl.Manager) error { return mgr.GetFieldIndexer().IndexField( context.Background(), &helmv1alpha1.HelmClusterAddon{}, addonChartIndex, @@ -42,14 +48,18 @@ func SetupIndexes(mgr ctrl.Manager) error { func SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr, &helmv1alpha1.HelmClusterAddon{}). - WithValidator(&HelmClusterAddonWebhookValidator{Client: mgr.GetClient()}). + WithValidator(&HelmClusterAddonWebhookValidator{ + Client: mgr.GetClient(), + claimService: services.NewClaimService(mgr.GetClient(), mgr.GetAPIReader(), helmv1alpha1.TargetNamespace), + }). Complete() } var _ admission.Validator[*helmv1alpha1.HelmClusterAddon] = (*HelmClusterAddonWebhookValidator)(nil) type HelmClusterAddonWebhookValidator struct { - Client client.Client + Client client.Client + claimService *services.ClaimService } func (v *HelmClusterAddonWebhookValidator) ValidateCreate(ctx context.Context, addon *helmv1alpha1.HelmClusterAddon) (admission.Warnings, error) { @@ -57,14 +67,26 @@ func (v *HelmClusterAddonWebhookValidator) ValidateCreate(ctx context.Context, a return nil, err } + if isUniquenessBypassed(ctx) { + return nil, nil + } + return nil, v.checkUniqueness(ctx, addon) } -func (v *HelmClusterAddonWebhookValidator) ValidateUpdate(ctx context.Context, _, newObj *helmv1alpha1.HelmClusterAddon) (admission.Warnings, error) { +func (v *HelmClusterAddonWebhookValidator) ValidateUpdate(ctx context.Context, oldObj, newObj *helmv1alpha1.HelmClusterAddon) (admission.Warnings, error) { if err := validateNotSystemNamespace(newObj); err != nil { return nil, err } + if isUniquenessBypassed(ctx) { + return nil, nil + } + + if forceReconcileToggled(oldObj, newObj) && !chartRefChanged(oldObj, newObj) { + return nil, nil + } + return nil, v.checkUniqueness(ctx, newObj) } @@ -84,7 +106,33 @@ func validateNotSystemNamespace(addon *helmv1alpha1.HelmClusterAddon) error { return nil } +func forceReconcileToggled(oldObj, newObj *helmv1alpha1.HelmClusterAddon) bool { + return oldObj.Annotations[helmv1alpha1.AnnotationForceReconcile] != newObj.Annotations[helmv1alpha1.AnnotationForceReconcile] +} + +func chartRefChanged(oldObj, newObj *helmv1alpha1.HelmClusterAddon) bool { + return oldObj.Spec.Chart.HelmClusterAddonRepository != newObj.Spec.Chart.HelmClusterAddonRepository || + oldObj.Spec.Chart.HelmClusterAddonChartName != newObj.Spec.Chart.HelmClusterAddonChartName +} + +func isUniquenessBypassed(ctx context.Context) bool { + req, err := admission.RequestFromContext(ctx) + if err != nil { + return false + } + + return slices.Contains(uniquenessBypassUsernames, req.UserInfo.Username) +} + func (v *HelmClusterAddonWebhookValidator) checkUniqueness(ctx context.Context, addon *helmv1alpha1.HelmClusterAddon) error { + owned, err := v.claimService.OwnedBy(ctx, addon) + if err != nil { + return fmt.Errorf("failed to check if helmclusteraddon/%s owns chart claim: %w", err) + } + if owned { + return nil + } + list := &helmv1alpha1.HelmClusterAddonList{} indexValue := addon.Spec.Chart.HelmClusterAddonRepository + "/" + addon.Spec.Chart.HelmClusterAddonChartName diff --git a/tests/e2e/helmclusteraddon/chartclaim.go b/tests/e2e/helmclusteraddon/chartclaim.go new file mode 100644 index 0000000..ce5150b --- /dev/null +++ b/tests/e2e/helmclusteraddon/chartclaim.go @@ -0,0 +1,228 @@ +/* +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 helmclusteraddon + +import ( + "context" + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + apiv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" + "github.com/deckhouse/operator-helm/tests/e2e/internal/controller" + "github.com/deckhouse/operator-helm/tests/e2e/internal/framework" + "github.com/deckhouse/operator-helm/tests/e2e/internal/util" +) + +// claimLeaseNames returns the names of the claim Leases the controller currently +// records for an addon. Each claim Lease carries the source-name label, so the +// leases an addon owns can be listed without recomputing the hashed Lease name. +func claimLeaseNames(addonName string) []string { + GinkgoHelper() + + selector := fmt.Sprintf("%s=%s,%s=%s", + apiv1alpha1.LabelManagedBy, apiv1alpha1.LabelManagedByValue, + apiv1alpha1.HelmClusterAddonLabelSourceName, addonName, + ) + + list, err := framework.GetClients().KubeClient().CoordinationV1(). + Leases(apiv1alpha1.TargetNamespace). + List(context.Background(), metav1.ListOptions{LabelSelector: selector}) + Expect(err).NotTo(HaveOccurred()) + + names := make([]string, 0, len(list.Items)) + for _, lease := range list.Items { + names = append(names, lease.Name) + } + return names +} + +var _ = Describe("HelmClusterAddon chart claim", Ordered, func() { + f := framework.NewFramework("addon-chart-claim") + + const ( + repoURL = "https://stefanprodan.github.io/podinfo" + chartName = "podinfo" + version = "6.10.2" + ) + + repoAName := "e2e-claim-repo-a" + repoBName := "e2e-claim-repo-b" + addonAName := "e2e-claim-addon-a" + addonBName := "e2e-claim-addon-b" + + // A dedicated namespace for addon B: it installs the same chart as addon A, so + // it must land in a separate namespace to avoid colliding on release resources. + var addonBNamespace string + + // The name of the claim Lease addon A holds while it points at repoA; captured + // before the chart reference change so its release can be asserted afterwards. + var repoAClaimLease string + + BeforeAll(func() { + DeferCleanup(f.After) + f.Before() + + nsB := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: fmt.Sprintf("%s-addon-chart-claim-b-", framework.NamespacePrefix), + Labels: map[string]string{framework.E2ELabel: "true"}, + }, + } + Expect(f.GenericClient().Create(context.Background(), nsB)).To(Succeed()) + f.DeferDelete(nsB) + addonBNamespace = nsB.Name + }) + + AfterEach(func() { + By("Verifying no errors in operator-helm-controller logs") + controller.AssertNoErrorsFor("operator-helm-controller") + }) + + It("should create two repositories pointing at the same source", func() { + for _, name := range []string{repoAName, repoBName} { + repo := &apiv1alpha1.HelmClusterAddonRepository{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: apiv1alpha1.HelmClusterAddonRepositorySpec{ + URL: repoURL, + InsecureSkipVerify: false, + }, + } + + created, err := f.OperatorClient().HelmV1alpha1(). + HelmClusterAddonRepositories(). + Create(context.Background(), repo, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + f.DeferDelete(created) + + By(fmt.Sprintf("Waiting for repository %q to become Ready and Synced", name)) + util.UntilConditionTrue(apiv1alpha1.ConditionTypeReady, framework.LongTimeout, created) + util.UntilConditionTrue(apiv1alpha1.ConditionTypeSynced, framework.LongTimeout, created) + } + }) + + It("should create addon A and claim the repository/chart pair", func() { + addon := &apiv1alpha1.HelmClusterAddon{ + ObjectMeta: metav1.ObjectMeta{Name: addonAName}, + Spec: apiv1alpha1.HelmClusterAddonSpec{ + Chart: apiv1alpha1.HelmClusterAddonChartRef{ + HelmClusterAddonChartName: chartName, + HelmClusterAddonRepository: repoAName, + Version: version, + }, + Namespace: f.NamespaceName(), + }, + } + + created, err := f.OperatorClient().HelmV1alpha1(). + HelmClusterAddons(). + Create(context.Background(), addon, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + f.DeferDelete(created) + + By("Waiting for addon A to become Ready") + util.UntilConditionTrue(apiv1alpha1.ConditionTypeReady, framework.LongTimeout, created) + + By("Verifying addon A holds exactly one claim Lease") + Eventually(func(g Gomega) { + leases := claimLeaseNames(addonAName) + g.Expect(leases).To(HaveLen(1)) + repoAClaimLease = leases[0] + }).WithTimeout(framework.LongTimeout).WithPolling(framework.PollingInterval).Should(Succeed()) + }) + + It("should reject a second addon that reuses the same repository and chart", func() { + duplicate := &apiv1alpha1.HelmClusterAddon{ + ObjectMeta: metav1.ObjectMeta{Name: addonBName}, + Spec: apiv1alpha1.HelmClusterAddonSpec{ + Chart: apiv1alpha1.HelmClusterAddonChartRef{ + HelmClusterAddonChartName: chartName, + HelmClusterAddonRepository: repoAName, + Version: version, + }, + Namespace: addonBNamespace, + }, + } + + By("Attempting to create a duplicate addon") + _, err := f.OperatorClient().HelmV1alpha1(). + HelmClusterAddons(). + Create(context.Background(), duplicate, metav1.CreateOptions{}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("already used by helmclusteraddon/" + addonAName)) + + By("Verifying addon A still holds a single claim Lease") + Expect(claimLeaseNames(addonAName)).To(ConsistOf(repoAClaimLease)) + }) + + It("should release the stale claim when addon A changes its repository", func() { + By("Repointing addon A to the second repository") + updated := util.UpdateHelmClusterAddon(addonAName, func(addon *apiv1alpha1.HelmClusterAddon) { + addon.Spec.Chart.HelmClusterAddonRepository = repoBName + }) + + By("Waiting for addon A to reconcile against the new repository") + util.UntilConditionTrue(apiv1alpha1.ConditionTypeReady, framework.LongTimeout, updated) + + By("Verifying the old claim Lease is released and a single new one is held") + Eventually(func(g Gomega) { + leases := claimLeaseNames(addonAName) + g.Expect(leases).To(HaveLen(1)) + g.Expect(leases).NotTo(ContainElement(repoAClaimLease)) + }).WithTimeout(framework.LongTimeout).WithPolling(framework.PollingInterval).Should(Succeed()) + + By("Verifying the old Lease object no longer exists") + Eventually(func(g Gomega) { + _, err := f.KubeClient().CoordinationV1(). + Leases(apiv1alpha1.TargetNamespace). + Get(context.Background(), repoAClaimLease, metav1.GetOptions{}) + g.Expect(err).To(HaveOccurred()) + }).WithTimeout(framework.LongTimeout).WithPolling(framework.PollingInterval).Should(Succeed()) + }) + + It("should let a new addon claim the released repository/chart pair", func() { + reuse := &apiv1alpha1.HelmClusterAddon{ + ObjectMeta: metav1.ObjectMeta{Name: addonBName}, + Spec: apiv1alpha1.HelmClusterAddonSpec{ + Chart: apiv1alpha1.HelmClusterAddonChartRef{ + HelmClusterAddonChartName: chartName, + HelmClusterAddonRepository: repoAName, + Version: version, + }, + Namespace: addonBNamespace, + }, + } + + By("Creating addon B on the freed repository/chart pair") + created, err := f.OperatorClient().HelmV1alpha1(). + HelmClusterAddons(). + Create(context.Background(), reuse, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + f.DeferDelete(created) + + By("Waiting for addon B to become Ready") + util.UntilConditionTrue(apiv1alpha1.ConditionTypeReady, framework.LongTimeout, created) + + By("Verifying addon B did not surface a chart claim conflict") + claimLease := claimLeaseNames(addonBName) + Expect(claimLease).To(HaveLen(1)) + Expect(claimLease).To(ConsistOf(repoAClaimLease)) + }) +})