diff --git a/.github/workflows/build_dev.yml b/.github/workflows/build_dev.yml index 55da21a9..885b095a 100644 --- a/.github/workflows/build_dev.yml +++ b/.github/workflows/build_dev.yml @@ -31,6 +31,7 @@ jobs: name: Build and Push images outputs: modules_module_tag: ${{ steps.modules_module_tag.outputs.MODULES_MODULE_TAG }} + modules_module_digest: ${{ steps.modules_module_digest.outputs.MODULES_MODULE_DIGEST }} steps: - name: Set vars id: modules_module_tag @@ -65,6 +66,21 @@ jobs: module_tag: ${{ steps.modules_module_tag.outputs.MODULES_MODULE_TAG }} svace_enabled: false + # The build action exposes no outputs, so the digest of what it just pushed + # is resolved here and handed to the e2e job, which asserts the cluster is + # running exactly this artifact. + - name: Resolve module digest + id: modules_module_digest + run: | + IMAGE="dev-registry.deckhouse.io/sys/deckhouse-oss/modules/${{ vars.MODULES_MODULE_NAME }}:${{ steps.modules_module_tag.outputs.MODULES_MODULE_TAG }}" + DIGEST="$(crane digest "$IMAGE")" + if [[ ! "$DIGEST" =~ ^sha256:[0-9a-f]{64}$ ]]; then + echo "::error title=Cannot resolve module digest::crane digest $IMAGE returned '$DIGEST'" + exit 1 + fi + echo "$IMAGE -> $DIGEST" + echo "MODULES_MODULE_DIGEST=$DIGEST" >> "$GITHUB_OUTPUT" + show_dev_manifest: runs-on: [self-hosted, large] name: Show manifest @@ -142,6 +158,7 @@ jobs: KIND_CLUSTER_NAME: d8-operator-helm-${{ github.run_number }} DEV_REGISTRY_DOCKER_CONFIG: ${{ secrets.DEV_REGISTRY_DOCKER_CONFIG }} E2E_MODULE_TAG_NAME: ${{ needs.build_dev.outputs.modules_module_tag }} + E2E_MODULE_DIGEST: ${{ needs.build_dev.outputs.modules_module_digest }} E2E_MODULE_SOURCE: operator-helm - name: Delete kind cluster diff --git a/api/v1alpha1/conditions.go b/api/v1alpha1/conditions.go index fd34bd3e..efaffe7d 100644 --- a/api/v1alpha1/conditions.go +++ b/api/v1alpha1/conditions.go @@ -26,6 +26,10 @@ const ( ConditionTypeSynced = "Synced" ConditionTypeUninstallFailed = "UninstallFailed" + // kstatus abnormal-true conditions. Present only while applicable. + ConditionTypeReconciling = "Reconciling" + ConditionTypeStalled = "Stalled" + ReasonMaintenanceModeActive = "MaintenanceModeActive" ReasonMaintenanceModeInactive = "MaintenanceModeInactive" ReasonSyncFailed = "SyncFailed" @@ -36,6 +40,18 @@ const ( ReasonUninstallFailed = "UninstallFailed" ReasonChartClaimConflict = "ChartClaimConflict" + // HelmClusterAddonRepository condition reasons. + ReasonAuxiliaryResourcesFailed = "AuxiliaryResourcesFailed" + ReasonCatalogUpdateFailed = "CatalogUpdateFailed" + ReasonAwaitingInitialSync = "AwaitingInitialSync" + ReasonProgressingWithRetry = "ProgressingWithRetry" + ReasonRetriesExceeded = "RetriesExceeded" + ReasonAuthenticationFailed = "AuthenticationFailed" + ReasonSourceNotFound = "SourceNotFound" + ReasonSourceRejectedRequest = "SourceRejectedRequest" + ReasonInvalidRepositoryURL = "InvalidRepositoryURL" + ReasonUnsupportedRepositoryType = "UnsupportedRepositoryType" + // HelmRelease error reasons ReasonReleaseFailed = "ReleaseFailed" ReasonTestFailed = "TestFailed" diff --git a/api/v1alpha1/helm_cluster_addon_repository.go b/api/v1alpha1/helm_cluster_addon_repository.go index f4466faa..3dd8591c 100644 --- a/api/v1alpha1/helm_cluster_addon_repository.go +++ b/api/v1alpha1/helm_cluster_addon_repository.go @@ -28,6 +28,13 @@ const ( HelmClusterAddonRepositoryLabelSourceName = "helm.deckhouse.io/cluster-addon-repository" ) +// The "Next Sync" print column below is a string, not a date, on purpose: a date +// column prints how long ago its value was, and kubectl renders any instant more +// than a second in the future as . nextSyncTime is always in the future. +// +// This note is deliberately outside the doc comment below — controller-gen folds +// every non-marker line of that block into the resource's API description. + // HelmClusterAddonRepository represents a Helm or OCI-compliant repository containing Helm charts that can be referenced by HelmClusterAddon resources. // // +kubebuilder:object:root=true @@ -36,6 +43,10 @@ const ( // +kubebuilder:resource:singular=helmclusteraddonrepository,scope=Cluster // +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status",description="The readiness status of the repository" // +kubebuilder:printcolumn:name="Synced",type="string",JSONPath=".status.conditions[?(@.type=='Synced')].status",description="Repository synchronization status" +// +kubebuilder:printcolumn:name="Last Sync",type="date",JSONPath=".status.lastSuccessfulSyncTime",description="Time of the last successful catalog synchronization" +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" +// +kubebuilder:printcolumn:name="Next Sync",type="string",JSONPath=".status.nextSyncTime",priority=1,description="Scheduled time of the next synchronization attempt" +// +kubebuilder:printcolumn:name="Message",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].message",priority=1 // +genclient // +genclient:nonNamespaced // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -108,10 +119,31 @@ type HelmClusterAddonRepositoryAuth struct { type HelmClusterAddonRepositoryStatus struct { // Conditions represent the latest available observations of the repository state. + // + // Ready reports whether the repository is usable: auxiliary resources are in place, + // the internal source object is healthy and the repository has responded to a catalog + // read on the current spec. A transient read failure does not flip Ready to False. + // + // Synced reports whether the chart catalog is up to date. + // + // Reconciling and Stalled follow the kstatus convention: they are present only while + // applicable. Reconciling means work is in progress or a retry is scheduled; Stalled + // means the repository will not recover without a change. // +optional Conditions []metav1.Condition `json:"conditions,omitempty"` // Generation represents resource generation that was last processed by the controller. ObservedGeneration int64 `json:"observedGeneration,omitempty"` + // LastSuccessfulSyncTime is the last time the chart catalog was fully brought up to date, + // including creating and pruning chart resources. + // +optional + LastSuccessfulSyncTime *metav1.Time `json:"lastSuccessfulSyncTime,omitempty"` + // NextSyncTime is the scheduled time of the next synchronization attempt. + // +optional + NextSyncTime *metav1.Time `json:"nextSyncTime,omitempty"` + // ConsecutiveFetchFailures counts consecutive failures to read from the repository. + // It drives the retry backoff and resets on the first success. + // +optional + ConsecutiveFetchFailures int32 `json:"consecutiveFetchFailures,omitempty"` } // HelmClusterAddonRepositoryList contains a list of HelmClusterAddonRepositories. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 36a80652..e5ec01d8 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -332,6 +332,14 @@ func (in *HelmClusterAddonRepositoryStatus) DeepCopyInto(out *HelmClusterAddonRe (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.LastSuccessfulSyncTime != nil { + in, out := &in.LastSuccessfulSyncTime, &out.LastSuccessfulSyncTime + *out = (*in).DeepCopy() + } + if in.NextSyncTime != nil { + in, out := &in.NextSyncTime, &out.NextSyncTime + *out = (*in).DeepCopy() + } return } diff --git a/crds/doc-ru-helmclusteraddonrepositories.yaml b/crds/doc-ru-helmclusteraddonrepositories.yaml index f1d31128..c060c75f 100644 --- a/crds/doc-ru-helmclusteraddonrepositories.yaml +++ b/crds/doc-ru-helmclusteraddonrepositories.yaml @@ -31,6 +31,19 @@ spec: status: properties: conditions: - description: Условия отражают последние наблюдения за состоянием репозитория. + description: | + Условия отражают последние наблюдения за состоянием репозитория. + + `Ready` сообщает, пригоден ли репозиторий: вспомогательные ресурсы на месте, внутренний объект источника исправен, и репозиторий ответил на чтение каталога на текущей спецификации. Транзиентная ошибка чтения не переводит `Ready` в `False`. + + `Synced` сообщает, актуален ли каталог чартов. + + `Reconciling` и `Stalled` следуют соглашению kstatus: они присутствуют, только когда применимы. `Reconciling` означает, что работа выполняется или запланирован повтор; `Stalled` — что репозиторий не восстановится без вмешательства. observedGeneration: description: Поколение ресурса, обработанное контроллером последним. + lastSuccessfulSyncTime: + description: Время последнего успешного приведения каталога чартов в актуальное состояние. + nextSyncTime: + description: Запланированное время следующей попытки синхронизации. + consecutiveFetchFailures: + description: Число подряд идущих неудачных обращений к репозиторию. Определяет задержку повтора и обнуляется при первом успехе. diff --git a/crds/helmclusteraddonrepositories.yaml b/crds/helmclusteraddonrepositories.yaml index 4b20d5a3..5cd62350 100644 --- a/crds/helmclusteraddonrepositories.yaml +++ b/crds/helmclusteraddonrepositories.yaml @@ -26,6 +26,22 @@ spec: jsonPath: .status.conditions[?(@.type=='Synced')].status name: Synced type: string + - description: Time of the last successful catalog synchronization + jsonPath: .status.lastSuccessfulSyncTime + name: Last Sync + type: date + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + - description: Scheduled time of the next synchronization attempt + jsonPath: .status.nextSyncTime + name: Next Sync + priority: 1 + type: string + - jsonPath: .status.conditions[?(@.type=='Ready')].message + name: Message + priority: 1 + type: string name: v1alpha1 schema: openAPIV3Schema: @@ -88,8 +104,18 @@ spec: status: properties: conditions: - description: Conditions represent the latest available observations - of the repository state. + description: |- + Conditions represent the latest available observations of the repository state. + + Ready reports whether the repository is usable: auxiliary resources are in place, + the internal source object is healthy and the repository has responded to a catalog + read on the current spec. A transient read failure does not flip Ready to False. + + Synced reports whether the chart catalog is up to date. + + Reconciling and Stalled follow the kstatus convention: they are present only while + applicable. Reconciling means work is in progress or a retry is scheduled; Stalled + means the repository will not recover without a change. items: description: Condition contains details for one aspect of the current state of this API Resource. @@ -145,6 +171,23 @@ spec: - type type: object type: array + consecutiveFetchFailures: + description: |- + ConsecutiveFetchFailures counts consecutive failures to read from the repository. + It drives the retry backoff and resets on the first success. + format: int32 + type: integer + lastSuccessfulSyncTime: + description: |- + LastSuccessfulSyncTime is the last time the chart catalog was fully brought up to date, + including creating and pruning chart resources. + format: date-time + type: string + nextSyncTime: + description: NextSyncTime is the scheduled time of the next synchronization + attempt. + format: date-time + type: string observedGeneration: description: Generation represents resource generation that was last processed by the controller. diff --git a/docs/README.md b/docs/README.md index 929ebc31..6f43a7b4 100644 --- a/docs/README.md +++ b/docs/README.md @@ -32,3 +32,38 @@ The following custom resources are used to manage Helm charts in the module: - A HelmClusterAddon resource referencing a specific HelmClusterAddonChart can only be created as a single instance in the cluster. This is because Helm charts can contain custom resource definitions (CRDs), and installing them multiple times at the cluster level is not allowed. See [usage examples](example.html) for practical scenarios. + +## Repository Status + +`HelmClusterAddonRepository` reports four conditions. + +`Ready` tells whether the repository is usable: its auxiliary resources are in +place, its internal source object is healthy, and the repository responded to a +catalog read on the current spec. A transient read failure does not flip `Ready` +to `False` — installed addons keep working and only the catalog goes stale. + +`Synced` tells whether the chart catalog is up to date. + +`Reconciling` and `Stalled` follow the kstatus convention and are present only +while they apply. `Reconciling` means work is in progress or a retry is +scheduled; `Stalled` means the repository will not recover on its own. + +| Ready | Synced | What it means | What to do | +|---|---|---|---| +| True | True | The repository is healthy. | Nothing. | +| True | False | The catalog read failed but the repository was usable before. | Check the `Reconciling` message and `Last Sync`. Retries are already scheduled. | +| False | True | The catalog is fresh, but the source of chart artifacts is unhealthy. | Check the `Ready` message: it is translated from the internal source object. | +| False | False | The repository is unreachable or misconfigured. | Check `Stalled`: `AuthenticationFailed`, `SourceNotFound` and `InvalidRepositoryURL` need a change in `spec`. | +| Unknown | any | The first catalog read on the current spec has not succeeded yet. | Wait for the next attempt shown in `Next Sync`. | + +Synchronization runs every 5 minutes. After a failed read the delay doubles — +5m, 10m, 20m, 40m — up to one hour, and the repository is reported as `Stalled` +with reason `RetriesExceeded` once the delay reaches the cap. Retries continue +at that cadence, because the cause may disappear on the repository side. The +schedule is visible in `status.nextSyncTime` and with +`kubectl get helmclusteraddonrepository -o wide`. `kubectl get +helmclusteraddonrepository` shows the `Last Sync` and `Age` columns by +default, and `-o wide` adds `Next Sync` and the `Ready` message. The same +values are in the status itself as `lastSuccessfulSyncTime` and +`nextSyncTime`, alongside `consecutiveFetchFailures`, which counts the +consecutive failed reads driving the backoff. diff --git a/docs/README.ru.md b/docs/README.ru.md index c15c6cc1..0791e744 100644 --- a/docs/README.ru.md +++ b/docs/README.ru.md @@ -32,3 +32,38 @@ weight: 10 - Ресурс HelmClusterAddon, ссылающийся на заданный HelmClusterAddonChart, может быть создан в кластере только в единственном экземпляре. Это обусловлено тем, что Helm-чарты могут содержать определения кастомных ресурсов (CRD), повторная установка которых на уровне кластера недопустима. Примеры использования приведены в разделе [примеры использования](example.html). + +## Статус репозитория + +`HelmClusterAddonRepository` сообщает о себе четырьмя условиями. + +`Ready` показывает, пригоден ли репозиторий: вспомогательные ресурсы на месте, +внутренний объект источника исправен, и репозиторий ответил на чтение каталога +на текущей спецификации. Транзиентная ошибка чтения не переводит `Ready` в +`False` — установленные аддоны продолжают работать, устаревает только каталог. + +`Synced` показывает, актуален ли каталог чартов. + +`Reconciling` и `Stalled` следуют соглашению kstatus и присутствуют, только +когда применимы. `Reconciling` означает, что работа выполняется или запланирован +повтор; `Stalled` — что репозиторий сам не восстановится. + +| Ready | Synced | Что означает | Что делать | +|---|---|---|---| +| True | True | Репозиторий исправен. | Ничего. | +| True | False | Чтение каталога не удалось, но до этого репозиторий был пригоден. | Посмотреть сообщение `Reconciling` и колонку `Last Sync`. Повторы уже запланированы. | +| False | True | Каталог свежий, но источник артефактов чартов неисправен. | Посмотреть сообщение `Ready` — оно транслируется от внутреннего объекта источника. | +| False | False | Репозиторий недоступен или настроен неверно. | Посмотреть `Stalled`: `AuthenticationFailed`, `SourceNotFound` и `InvalidRepositoryURL` требуют правки `spec`. | +| Unknown | любое | Первое чтение каталога на текущей спецификации ещё не удалось. | Дождаться следующей попытки, время которой указано в `Next Sync`. | + +Синхронизация выполняется каждые 5 минут. После неудачного чтения задержка +удваивается — 5m, 10m, 20m, 40m — до часа, и по достижении потолка репозиторий +переводится в `Stalled` с причиной `RetriesExceeded`. Повторы при этом +продолжаются, потому что причина может уйти на стороне репозитория. Расписание +видно в `status.nextSyncTime` и через +`kubectl get helmclusteraddonrepository -o wide`. `kubectl get +helmclusteraddonrepository` по умолчанию показывает колонки `Last Sync` и +`Age`, а `-o wide` добавляет `Next Sync` и сообщение из `Ready`. Те же +значения лежат в самом статусе — `lastSuccessfulSyncTime` и `nextSyncTime`, — +рядом с `consecutiveFetchFailures`, который считает подряд идущие неудачные +чтения, определяющие задержку повтора. diff --git a/images/operator-helm-controller/go.mod b/images/operator-helm-controller/go.mod index f819642e..0a2e57d6 100644 --- a/images/operator-helm-controller/go.mod +++ b/images/operator-helm-controller/go.mod @@ -9,6 +9,7 @@ require ( github.com/deckhouse/operator-helm/api v0.0.0-00010101000000-000000000000 github.com/google/go-containerregistry v0.20.6 github.com/opencontainers/go-digest v1.0.0 + github.com/samber/lo v1.53.0 github.com/werf/3p-fluxcd-pkg/apis/meta v1.23.0-nelm.1 github.com/werf/3p-fluxcd-pkg/chartutil v1.17.0-nelm.1 github.com/werf/3p-helm-controller/api v0.1.5 @@ -19,6 +20,7 @@ require ( k8s.io/apimachinery v0.35.1 k8s.io/client-go v0.35.1 k8s.io/utils v0.0.0-20251002143259-bc988d571ff4 + sigs.k8s.io/cli-utils v0.37.2 sigs.k8s.io/controller-runtime v0.23.1 ) @@ -61,7 +63,6 @@ require ( github.com/prometheus/client_model v0.6.2 // indirect github.com/prometheus/common v0.66.1 // indirect github.com/prometheus/procfs v0.16.1 // indirect - github.com/samber/lo v1.53.0 // indirect github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 // indirect github.com/sirupsen/logrus v1.9.3 // indirect github.com/spf13/pflag v1.0.10 // indirect diff --git a/images/operator-helm-controller/go.sum b/images/operator-helm-controller/go.sum index 560d15e2..bbec7d6f 100644 --- a/images/operator-helm-controller/go.sum +++ b/images/operator-helm-controller/go.sum @@ -226,6 +226,8 @@ k8s.io/kube-openapi v0.0.0-20260127142750-a19766b6e2d4 h1:HhDfevmPS+OalTjQRKbTHp k8s.io/kube-openapi v0.0.0-20260127142750-a19766b6e2d4/go.mod h1:kdmbQkyfwUagLfXIad1y2TdrjPFWp2Q89B3qkRwf/pQ= k8s.io/utils v0.0.0-20251002143259-bc988d571ff4 h1:SjGebBtkBqHFOli+05xYbK8YF1Dzkbzn+gDM4X9T4Ck= k8s.io/utils v0.0.0-20251002143259-bc988d571ff4/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= +sigs.k8s.io/cli-utils v0.37.2 h1:GOfKw5RV2HDQZDJlru5KkfLO1tbxqMoyn1IYUxqBpNg= +sigs.k8s.io/cli-utils v0.37.2/go.mod h1:V+IZZr4UoGj7gMJXklWBg6t5xbdThFBcpj4MrZuCYco= sigs.k8s.io/controller-runtime v0.23.1 h1:TjJSM80Nf43Mg21+RCy3J70aj/W6KyvDtOlpKf+PupE= sigs.k8s.io/controller-runtime v0.23.1/go.mod h1:B6COOxKptp+YaUT5q4l6LqUJTRpizbgf9KSRNdQGns0= sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 h1:IpInykpT6ceI+QxKBbEflcR5EXP7sU1kvOlxwZh5txg= diff --git a/images/operator-helm-controller/internal/client/repository/errors.go b/images/operator-helm-controller/internal/client/repository/errors.go new file mode 100644 index 00000000..5cd70fd7 --- /dev/null +++ b/images/operator-helm-controller/internal/client/repository/errors.go @@ -0,0 +1,79 @@ +/* +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 repository + +import ( + "errors" + "fmt" + "net/http" + + helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" +) + +// TerminalError marks a repository failure that will not resolve by retrying: +// the remote rejected the request or the configuration is invalid. Callers use +// it to move the repository to Stalled instead of scheduling another attempt at +// the normal cadence. +type TerminalError struct { + Reason string + Message string + Err error +} + +func (e *TerminalError) Error() string { + if e.Err == nil { + return e.Message + } + + return fmt.Sprintf("%s: %s", e.Message, e.Err) +} + +func (e *TerminalError) Unwrap() error { return e.Err } + +// AsTerminal reports whether err wraps a TerminalError and returns it. +func AsTerminal(err error) (*TerminalError, bool) { + var terminal *TerminalError + if errors.As(err, &terminal) { + return terminal, true + } + + return nil, false +} + +// TerminalFromStatusCode maps a rejection status code to a terminal error and +// returns nil for codes that are worth retrying. +func TerminalFromStatusCode(code int, url string) *TerminalError { + switch { + case code == http.StatusUnauthorized, code == http.StatusForbidden: + return &TerminalError{ + Reason: helmv1alpha1.ReasonAuthenticationFailed, + Message: fmt.Sprintf("repository %s rejected the credentials (HTTP %d)", url, code), + } + case code == http.StatusNotFound: + return &TerminalError{ + Reason: helmv1alpha1.ReasonSourceNotFound, + Message: fmt.Sprintf("repository %s not found (HTTP %d)", url, code), + } + case code >= 400 && code < 500: + return &TerminalError{ + Reason: helmv1alpha1.ReasonSourceRejectedRequest, + Message: fmt.Sprintf("repository %s rejected the request (HTTP %d)", url, code), + } + default: + return nil + } +} diff --git a/images/operator-helm-controller/internal/client/repository/helm.go b/images/operator-helm-controller/internal/client/repository/helm.go index 0e1dc34e..685fa7e2 100644 --- a/images/operator-helm-controller/internal/client/repository/helm.go +++ b/images/operator-helm-controller/internal/client/repository/helm.go @@ -26,6 +26,7 @@ import ( "go.yaml.in/yaml/v3" "k8s.io/apimachinery/pkg/util/wait" + "sigs.k8s.io/controller-runtime/pkg/log" "github.com/Masterminds/semver/v3" ) @@ -68,6 +69,13 @@ func (c *helmRepositoryClient) FetchCharts(ctx context.Context, url string, conf ctx, cancel := context.WithTimeout(ctx, 8*time.Second) defer cancel() + // lastErr keeps the cause of the most recent retriable failure. The backoff + // helper reports only its own timeout once the steps are exhausted, and a + // transient read failure — 5xx, DNS, connection refused, TLS — is the most + // common one there is: without this the operator would surface "timed out + // waiting for the condition" as the whole diagnostic. + var lastErr error + err := wait.ExponentialBackoffWithContext(ctx, backoff, func(ctx context.Context) (done bool, err error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { @@ -80,16 +88,20 @@ func (c *helmRepositoryClient) FetchCharts(ctx context.Context, url string, conf resp, err := httpClient.Do(req) if err != nil { + lastErr = err + return false, nil } defer resp.Body.Close() if resp.StatusCode >= 500 { + lastErr = fmt.Errorf("repository %s is unavailable (HTTP %d)", url, resp.StatusCode) + return false, nil } - if resp.StatusCode >= 400 { - return true, fmt.Errorf("fatal client error: received status %d", resp.StatusCode) + if terminal := TerminalFromStatusCode(resp.StatusCode, url); terminal != nil { + return true, terminal } if err := yaml.NewDecoder(resp.Body).Decode(&indexFile); err != nil { @@ -99,6 +111,14 @@ func (c *helmRepositoryClient) FetchCharts(ctx context.Context, url string, conf return true, nil }) if err != nil { + if lastErr != nil && wait.Interrupted(err) { + // The loop ran out of steps or the context ended with every attempt + // failing retriably, so err is the bare timeout. Report the cause + // instead. A terminal error never reaches here: it stops the loop as + // the callback's own error and stays reachable through errors.As. + return nil, fmt.Errorf("helm repository index.yaml request failed: %w", lastErr) + } + return nil, fmt.Errorf("helm repository index.yaml request failed: %w", err) } @@ -114,7 +134,13 @@ func (c *helmRepositoryClient) FetchCharts(ctx context.Context, url string, conf semVersion, err := semver.NewVersion(chartVersion.Version) if err != nil { - return nil, fmt.Errorf("failed to parse chart %q version %q: %w", chartName, chartVersion.Version, err) + // A single malformed entry must not cost the whole catalog: the OCI + // client already skips such tags, and the repository owner may publish + // non-semver artifacts we simply cannot address. + log.FromContext(ctx).V(1).Info("Skipping chart version that is not valid semver", + "chart", chartName, "version", chartVersion.Version) + + continue } chart.Versions = append(chart.Versions, ChartVersion{Version: semVersion, IconURL: chartVersion.Icon}) diff --git a/images/operator-helm-controller/internal/client/repository/helm_test.go b/images/operator-helm-controller/internal/client/repository/helm_test.go new file mode 100644 index 00000000..f52d16e5 --- /dev/null +++ b/images/operator-helm-controller/internal/client/repository/helm_test.go @@ -0,0 +1,112 @@ +/* +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 repository + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + + helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" +) + +const testIndex = `apiVersion: v1 +entries: + podinfo: + - version: 6.7.1 + icon: https://example.invalid/icon.png + - version: not-a-semver + - version: 6.7.0 +` + +func TestFetchChartsTerminalStatusCodes(t *testing.T) { + cases := []struct { + name string + statusCode int + wantReason string + }{ + {"unauthorized", http.StatusUnauthorized, helmv1alpha1.ReasonAuthenticationFailed}, + {"forbidden", http.StatusForbidden, helmv1alpha1.ReasonAuthenticationFailed}, + {"not found", http.StatusNotFound, helmv1alpha1.ReasonSourceNotFound}, + {"teapot", http.StatusTeapot, helmv1alpha1.ReasonSourceRejectedRequest}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(tc.statusCode) + })) + defer srv.Close() + + _, err := HelmRepositoryDefaultClient.FetchCharts(context.Background(), srv.URL, nil) + if err == nil { + t.Fatalf("expected error for status %d", tc.statusCode) + } + + terminal, ok := AsTerminal(err) + if !ok { + t.Fatalf("expected terminal error, got %v", err) + } + if terminal.Reason != tc.wantReason { + t.Fatalf("expected reason %q, got %q", tc.wantReason, terminal.Reason) + } + }) + } +} + +func TestFetchChartsServerErrorIsNotTerminal(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + + _, err := HelmRepositoryDefaultClient.FetchCharts(context.Background(), srv.URL, nil) + if err == nil { + t.Fatal("expected error for repeated 500 responses") + } + if _, ok := AsTerminal(err); ok { + t.Fatalf("5xx must stay retriable, got terminal error: %v", err) + } + // The exhausted backoff must report what actually went wrong rather than the + // bare "timed out waiting for the condition" the wait helper returns. + if !strings.Contains(err.Error(), "500") { + t.Fatalf("expected the status code in the reported cause, got %v", err) + } +} + +func TestFetchChartsSkipsInvalidVersion(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(testIndex)) + })) + defer srv.Close() + + charts, err := HelmRepositoryDefaultClient.FetchCharts(context.Background(), srv.URL, nil) + if err != nil { + t.Fatalf("expected invalid version to be skipped, got error: %v", err) + } + if len(charts) != 1 { + t.Fatalf("expected 1 chart, got %d", len(charts)) + } + if len(charts[0].Versions) != 2 { + t.Fatalf("expected 2 valid versions, got %d", len(charts[0].Versions)) + } + if got := charts[0].Versions[0].Version.Original(); got != "6.7.1" { + t.Fatalf("expected newest version first, got %q", got) + } +} diff --git a/images/operator-helm-controller/internal/client/repository/oci.go b/images/operator-helm-controller/internal/client/repository/oci.go index 4e02b01c..e20e4e3d 100644 --- a/images/operator-helm-controller/internal/client/repository/oci.go +++ b/images/operator-helm-controller/internal/client/repository/oci.go @@ -27,6 +27,9 @@ import ( "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/remote/transport" + + helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" ) var OCIRepositoryDefaultClient ClientInterface = &ociRepositoryClient{} @@ -38,19 +41,29 @@ func (c *ociRepositoryClient) FetchCharts(ctx context.Context, url string, confi url = strings.TrimSuffix(url, "/") if !strings.Contains(url, "/") { - return nil, errors.New("url must contain chart/image name") + return nil, &TerminalError{ + Reason: helmv1alpha1.ReasonInvalidRepositoryURL, + Message: "repository url must contain the chart image name", + } } urlParts := strings.Split(url, "/") chartName := urlParts[len(urlParts)-1] if len(chartName) == 0 { - return nil, errors.New("failed to parse chart/image name from the url") + return nil, &TerminalError{ + Reason: helmv1alpha1.ReasonInvalidRepositoryURL, + Message: "cannot parse the chart image name from the repository url", + } } repo, err := name.NewRepository(url) if err != nil { - return nil, fmt.Errorf("failed to parse repository url: %w", err) + return nil, &TerminalError{ + Reason: helmv1alpha1.ReasonInvalidRepositoryURL, + Message: "cannot parse the repository url", + Err: err, + } } options := []remote.Option{ @@ -77,7 +90,7 @@ func (c *ociRepositoryClient) FetchCharts(ctx context.Context, url string, confi tags, err := remote.List(repo, options...) if err != nil { - return nil, fmt.Errorf("listing image tags: %w", err) + return nil, classifyRemoteError(err, url) } var chartVersions []ChartVersion @@ -125,3 +138,19 @@ func isCosignTag(tag string) bool { return false } + +// classifyRemoteError maps a registry rejection to a terminal error and leaves +// transport-level failures retriable. The original error is always wrapped so +// callers keep the full cause. +func classifyRemoteError(err error, url string) error { + var transportErr *transport.Error + if errors.As(err, &transportErr) { + if terminal := TerminalFromStatusCode(transportErr.StatusCode, url); terminal != nil { + terminal.Err = err + + return terminal + } + } + + return fmt.Errorf("listing image tags: %w", err) +} diff --git a/images/operator-helm-controller/internal/client/repository/oci_test.go b/images/operator-helm-controller/internal/client/repository/oci_test.go new file mode 100644 index 00000000..5884ac8d --- /dev/null +++ b/images/operator-helm-controller/internal/client/repository/oci_test.go @@ -0,0 +1,91 @@ +/* +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 repository + +import ( + "context" + "errors" + "net/http" + "testing" + + "github.com/google/go-containerregistry/pkg/v1/remote/transport" + + helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" +) + +func TestFetchChartsOCIRejectsURLWithoutImageName(t *testing.T) { + _, err := OCIRepositoryDefaultClient.FetchCharts(context.Background(), "oci://ghcr.io", nil) + if err == nil { + t.Fatal("expected error for url without an image name") + } + + terminal, ok := AsTerminal(err) + if !ok { + t.Fatalf("expected terminal error, got %v", err) + } + if terminal.Reason != helmv1alpha1.ReasonInvalidRepositoryURL { + t.Fatalf("expected reason %q, got %q", helmv1alpha1.ReasonInvalidRepositoryURL, terminal.Reason) + } +} + +func TestClassifyRemoteError(t *testing.T) { + cases := []struct { + name string + err error + wantTerminal bool + wantReason string + }{ + { + name: "unauthorized is terminal", + err: &transport.Error{StatusCode: http.StatusUnauthorized}, + wantTerminal: true, + wantReason: helmv1alpha1.ReasonAuthenticationFailed, + }, + { + name: "not found is terminal", + err: &transport.Error{StatusCode: http.StatusNotFound}, + wantTerminal: true, + wantReason: helmv1alpha1.ReasonSourceNotFound, + }, + { + name: "server error is retriable", + err: &transport.Error{StatusCode: http.StatusBadGateway}, + wantTerminal: false, + }, + { + name: "transport failure is retriable", + err: errors.New("dial tcp: connection refused"), + wantTerminal: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := classifyRemoteError(tc.err, "ghcr.io/example/chart") + terminal, ok := AsTerminal(got) + if ok != tc.wantTerminal { + t.Fatalf("terminal=%v, want %v (err %v)", ok, tc.wantTerminal, got) + } + if tc.wantTerminal && terminal.Reason != tc.wantReason { + t.Fatalf("expected reason %q, got %q", tc.wantReason, terminal.Reason) + } + if !errors.Is(got, tc.err) { + t.Fatalf("classified error must wrap the original, got %v", got) + } + }) + } +} diff --git a/images/operator-helm-controller/internal/controller/helmclusteraddonrepository/controller.go b/images/operator-helm-controller/internal/controller/helmclusteraddonrepository/controller.go index e96564c0..0e54e421 100644 --- a/images/operator-helm-controller/internal/controller/helmclusteraddonrepository/controller.go +++ b/images/operator-helm-controller/internal/controller/helmclusteraddonrepository/controller.go @@ -26,6 +26,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" + repoclient "github.com/deckhouse/operator-helm/internal/client/repository" "github.com/deckhouse/operator-helm/internal/manager/status" reconcile "github.com/deckhouse/operator-helm/internal/reconcile/helmclusteraddonrepository" "github.com/deckhouse/operator-helm/internal/services" @@ -43,7 +44,7 @@ func SetupWithManager(mgr ctrl.Manager) error { client, services.NewHelmRepoService(client, mgr.GetScheme(), helmv1alpha1.TargetNamespace), services.NewOCIRepoService(client, mgr.GetScheme(), helmv1alpha1.TargetNamespace), - services.NewRepoSyncService(client, mgr.GetScheme()), + services.NewRepoSyncService(client, mgr.GetScheme(), repoclient.NewClient), status.NewManager(client), ) diff --git a/images/operator-helm-controller/internal/manager/status/manager.go b/images/operator-helm-controller/internal/manager/status/manager.go index 78f92b93..db36d122 100644 --- a/images/operator-helm-controller/internal/manager/status/manager.go +++ b/images/operator-helm-controller/internal/manager/status/manager.go @@ -125,31 +125,20 @@ func (s *Manager) Update(ctx context.Context, obj ObjectWithConditions, mutatorF return s.Status().Patch(ctx, obj, client.MergeFrom(oldObj)) } -func (s *Manager) InitializeConditions(ctx context.Context, obj ObjectWithConditions, conditionTypes ...string) error { +// PatchStatus applies mutate to the object and patches the status subresource +// when it actually changed. It is the thin apply path used by reconcilers that +// compute the whole desired status themselves. +func (s *Manager) PatchStatus(ctx context.Context, obj ObjectWithConditions, mutate func()) error { oldObj := obj.DeepCopyObject().(ObjectWithConditions) - patchBase := client.MergeFrom(oldObj) - conditions := obj.GetConditions() - changed := false - - for _, t := range conditionTypes { - if meta.FindStatusCondition(*conditions, t) == nil { - meta.SetStatusCondition(conditions, metav1.Condition{ - Type: t, - Status: metav1.ConditionUnknown, - Reason: "Initialized", - }) - changed = true - } - } + mutate() - if changed { - logger := log.FromContext(ctx) - logger.Info("Initializing conditions", "name", obj.GetName(), "types", conditionTypes) + if reflect.DeepEqual(obj.GetStatus(), oldObj.GetStatus()) { + return nil + } - if err := s.Client.Status().Patch(ctx, obj, patchBase); err != nil { - return fmt.Errorf("initializing conditions: %w", err) - } + if err := s.Status().Patch(ctx, obj, client.MergeFrom(oldObj)); err != nil { + return fmt.Errorf("patching status: %w", err) } return nil diff --git a/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/evaluate.go b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/evaluate.go new file mode 100644 index 00000000..54257a39 --- /dev/null +++ b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/evaluate.go @@ -0,0 +1,442 @@ +/* +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 helmclusteraddonrepository + +import ( + "math/rand/v2" + "time" + + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" + "github.com/deckhouse/operator-helm/internal/services" +) + +// Inputs carries everything Evaluate needs. It holds no clients and no clock: +// Now and Jitter are supplied by the caller so the function is deterministic +// and unit testable. +type Inputs struct { + Generation int64 + Now time.Time + Jitter float64 + Current helmv1alpha1.HelmClusterAddonRepositoryStatus + + SecretsErr error + InternalRepositoryErr error + InternalRepository services.InternalRepositoryState + ConfigErr *services.ConfigOutcome + + // Attempted reports whether a synchronization attempt ran in this pass. + // Fetch and Catalog are nil when it did not. + Attempted bool + Fetch *services.FetchOutcome + Catalog *services.CatalogOutcome +} + +// Decision is the full desired status plus the scheduling verdict. Removing a +// condition is expressed by its absence from Status.Conditions. +type Decision struct { + Status helmv1alpha1.HelmClusterAddonRepositoryStatus + RequeueAfter time.Duration + Err error +} + +// abnormalCondition is an optional abnormal-true condition. +type abnormalCondition struct { + set bool + reason string + message string +} + +// Evaluate derives the desired repository status from the results of a single +// reconcile pass. +func Evaluate(in Inputs) Decision { + var status helmv1alpha1.HelmClusterAddonRepositoryStatus + in.Current.DeepCopyInto(&status) + status.ObservedGeneration = in.Generation + + internal := in.InternalRepository + if in.InternalRepositoryErr != nil { + // A failure to reconcile the internal object is a structural failure: + // report it the same way as an unhealthy object so Ready reflects it. + internal = services.InternalRepositoryState{ + Present: true, + Reason: helmv1alpha1.ReasonFailed, + Message: in.InternalRepositoryErr.Error(), + } + } + + fetchFailed := in.Attempted && in.Fetch != nil && in.Fetch.Err != nil + fetchSucceeded := in.Attempted && in.Fetch != nil && in.Fetch.Err == nil + catalogFailed := in.Attempted && in.Catalog != nil && in.Catalog.Err != nil + + failures := in.Current.ConsecutiveFetchFailures + if in.Generation != in.Current.ObservedGeneration { + // The spec changed: previous failures were about the previous source. + failures = 0 + } + failures = nextFailureCount(failures, in.Attempted, fetchFailed, in.Fetch) + + stalled := evaluateStalled(in, internal, failures, fetchFailed) + ready := evaluateReady(in, internal, stalled, fetchSucceeded) + reconciling := evaluateReconciling(in, internal, stalled, ready, failures, catalogFailed) + + setCondition(&status, in, helmv1alpha1.ConditionTypeReady, ready.Status, ready.Reason, ready.Message) + + if in.Attempted { + syncedStatus, syncedReason, syncedMessage := evaluateSynced(in, fetchFailed, catalogFailed) + setCondition(&status, in, helmv1alpha1.ConditionTypeSynced, syncedStatus, syncedReason, syncedMessage) + } + + applyAbnormal(&status, in, helmv1alpha1.ConditionTypeReconciling, reconciling) + applyAbnormal(&status, in, helmv1alpha1.ConditionTypeStalled, stalled) + + status.ConsecutiveFetchFailures = failures + + if in.Attempted { + if fetchSucceeded && !catalogFailed { + status.LastSuccessfulSyncTime = &metav1.Time{Time: in.Now} + } + + next := in.Now.Add(withJitter(syncDelay(failures), in.Jitter)) + status.NextSyncTime = &metav1.Time{Time: next} + } + + requeueAfter := time.Duration(0) + if in.ConfigErr == nil && status.NextSyncTime != nil { + requeueAfter = status.NextSyncTime.Sub(in.Now) + if requeueAfter < minRequeue { + requeueAfter = minRequeue + } + } + + return Decision{ + Status: status, + RequeueAfter: requeueAfter, + Err: firstErr(in.SecretsErr, in.InternalRepositoryErr, catalogErr(in)), + } +} + +func evaluateReady( + in Inputs, + internal services.InternalRepositoryState, + stalled abnormalCondition, + fetchSucceeded bool, +) metav1.Condition { + switch { + case in.SecretsErr != nil: + return metav1.Condition{ + Status: metav1.ConditionFalse, + Reason: helmv1alpha1.ReasonAuxiliaryResourcesFailed, + Message: "Failed to reconcile auxiliary resources: " + in.SecretsErr.Error(), + } + case internal.Present && !internal.Ready: + return metav1.Condition{ + Status: metav1.ConditionFalse, + Reason: internal.Reason, + Message: internal.Message, + } + case stalled.set: + return metav1.Condition{ + Status: metav1.ConditionFalse, + Reason: stalled.reason, + Message: stalled.message, + } + case fetchSucceeded: + return metav1.Condition{Status: metav1.ConditionTrue, Reason: helmv1alpha1.ReasonSuccess} + case hasEvidence(in.Current, in.Generation): + return metav1.Condition{Status: metav1.ConditionTrue, Reason: helmv1alpha1.ReasonSuccess} + default: + return metav1.Condition{ + Status: metav1.ConditionUnknown, + Reason: helmv1alpha1.ReasonAwaitingInitialSync, + Message: "Waiting for the first successful repository read", + } + } +} + +func evaluateSynced(in Inputs, fetchFailed, catalogFailed bool) (metav1.ConditionStatus, string, string) { + switch { + case fetchFailed: + return metav1.ConditionFalse, helmv1alpha1.ReasonSyncFailed, in.Fetch.Message + case catalogFailed: + return metav1.ConditionFalse, helmv1alpha1.ReasonCatalogUpdateFailed, + "Failed to update the chart catalog: " + in.Catalog.Err.Error() + default: + return metav1.ConditionTrue, helmv1alpha1.ReasonSuccess, "" + } +} + +func evaluateStalled( + in Inputs, + internal services.InternalRepositoryState, + failures int32, + fetchFailed bool, +) abnormalCondition { + switch { + case in.ConfigErr != nil: + return abnormalCondition{set: true, reason: in.ConfigErr.Reason, message: in.ConfigErr.Message} + case internal.Present && internal.Stalled: + return abnormalCondition{set: true, reason: internal.Reason, message: internal.Message} + case !in.Attempted: + // A pass without an attempt carries the previous verdict forward so the + // specific reason is not replaced by a generic one. Only carry it forward + // when it was set for the current generation: a generation bump voids + // evidence about the previous spec, mirroring hasEvidence. + if cond := apimeta.FindStatusCondition(in.Current.Conditions, helmv1alpha1.ConditionTypeStalled); cond != nil && + cond.Status == metav1.ConditionTrue && cond.ObservedGeneration == in.Generation { + return abnormalCondition{set: true, reason: cond.Reason, message: cond.Message} + } + + return abnormalCondition{} + case fetchFailed && in.Fetch.Terminal: + return abnormalCondition{set: true, reason: in.Fetch.Reason, message: in.Fetch.Message} + case failures >= MaxFetchFailures: + return abnormalCondition{ + set: true, + reason: helmv1alpha1.ReasonRetriesExceeded, + message: "Giving up on the repository after repeated read failures", + } + default: + return abnormalCondition{} + } +} + +func evaluateReconciling( + in Inputs, + internal services.InternalRepositoryState, + stalled abnormalCondition, + ready metav1.Condition, + failures int32, + catalogFailed bool, +) abnormalCondition { + switch { + case stalled.set: + return abnormalCondition{} + case internal.Present && !internal.Ready: + return abnormalCondition{set: true, reason: helmv1alpha1.ReasonReconciling, message: internal.Message} + case in.SecretsErr != nil: + return abnormalCondition{ + set: true, + reason: helmv1alpha1.ReasonProgressingWithRetry, + message: "Retrying after an auxiliary resource failure", + } + case catalogFailed: + return abnormalCondition{ + set: true, + reason: helmv1alpha1.ReasonProgressingWithRetry, + message: "Retrying after a chart catalog update failure", + } + case !in.Attempted && carriesCatalogFailure(in): + // The catalog write failure is handed to the work queue, whose immediate + // retry runs a pass with no attempt, so the case above cannot see it. Carry + // the retry forward the way evaluateStalled carries its verdict: without it + // the object would show Ready=True, Synced=False and no abnormal-true + // condition at all, which kstatus reads as healthy, and the fetch-failure + // counter — which deliberately ignores catalog failures — would never + // escalate it to Stalled either. + return abnormalCondition{ + set: true, + reason: helmv1alpha1.ReasonProgressingWithRetry, + message: "Retrying after a chart catalog update failure", + } + case failures > 0: + return abnormalCondition{ + set: true, + reason: helmv1alpha1.ReasonProgressingWithRetry, + message: "Retrying the repository read", + } + case ready.Status == metav1.ConditionUnknown: + return abnormalCondition{ + set: true, + reason: helmv1alpha1.ReasonAwaitingInitialSync, + message: ready.Message, + } + default: + return abnormalCondition{} + } +} + +// carriesCatalogFailure reports whether the current status still records an +// unresolved chart catalog write failure. Only a record made for the current +// generation counts: a generation bump voids evidence about the previous spec, +// mirroring hasEvidence and the carry-forward in evaluateStalled. +func carriesCatalogFailure(in Inputs) bool { + cond := apimeta.FindStatusCondition(in.Current.Conditions, helmv1alpha1.ConditionTypeSynced) + + return cond != nil && cond.Status == metav1.ConditionFalse && + cond.Reason == helmv1alpha1.ReasonCatalogUpdateFailed && + cond.ObservedGeneration == in.Generation +} + +// hasEvidence reports whether the repository is already proven usable on the +// current generation: a fetch succeeded for this spec. +func hasEvidence(current helmv1alpha1.HelmClusterAddonRepositoryStatus, generation int64) bool { + // Evidence is "a fetch succeeded on this spec". Ready alone cannot carry it: + // a higher-priority rule (an unhealthy internal repository, a failed secret) + // owns Ready on the very pass where the fetch succeeded, overwriting it. + // Synced is written only on a pass that attempted, and is True only when the + // fetch and the catalog write both succeeded, so Synced=True on the current + // generation means exactly that. + for _, conditionType := range []string{helmv1alpha1.ConditionTypeReady, helmv1alpha1.ConditionTypeSynced} { + if cond := apimeta.FindStatusCondition(current.Conditions, conditionType); cond != nil && + cond.Status == metav1.ConditionTrue && cond.ObservedGeneration == generation { + return true + } + } + + return false +} + +func setCondition( + status *helmv1alpha1.HelmClusterAddonRepositoryStatus, + in Inputs, + conditionType string, + conditionStatus metav1.ConditionStatus, + reason, message string, +) { + apimeta.SetStatusCondition(&status.Conditions, metav1.Condition{ + Type: conditionType, + Status: conditionStatus, + Reason: reason, + Message: message, + ObservedGeneration: in.Generation, + LastTransitionTime: metav1.NewTime(in.Now), + }) +} + +func applyAbnormal( + status *helmv1alpha1.HelmClusterAddonRepositoryStatus, + in Inputs, + conditionType string, + cond abnormalCondition, +) { + if !cond.set { + apimeta.RemoveStatusCondition(&status.Conditions, conditionType) + + return + } + + setCondition(status, in, conditionType, metav1.ConditionTrue, cond.reason, cond.message) +} + +func firstErr(errs ...error) error { + for _, err := range errs { + if err != nil { + return err + } + } + + return nil +} + +func catalogErr(in Inputs) error { + if in.Catalog == nil { + return nil + } + + return in.Catalog.Err +} + +const ( + // SyncInterval is the normal catalog synchronization cadence and the base of + // the retry backoff, so a broken repository is never polled more often than a + // healthy one. + SyncInterval = 5 * time.Minute + // MaxSyncBackoff caps the retry delay. + MaxSyncBackoff = 1 * time.Hour + // MaxFetchFailures is the number of consecutive read failures after which the + // repository is reported as Stalled. Reaching it does not stop the retries: + // the cause may disappear on the remote side. Moved here from task 4's + // standalone declaration — the value does not change. + MaxFetchFailures = 5 + // SyncBackoffJitter spreads the schedule of repositories that share a remote. + SyncBackoffJitter = 0.1 + // minRequeue keeps a due schedule from being reported as "no requeue", + // which is what a zero RequeueAfter means to controller-runtime. + minRequeue = time.Second +) + +// ShouldAttempt reports whether a synchronization attempt is due. The caller +// additionally requires the auxiliary resources to be in place. +func ShouldAttempt( + current helmv1alpha1.HelmClusterAddonRepositoryStatus, + generation int64, + now time.Time, + forced bool, +) bool { + switch { + case forced: + return true + case generation != current.ObservedGeneration: + return true + case current.NextSyncTime == nil: + return true + default: + return !now.Before(current.NextSyncTime.Time) + } +} + +// NewJitter returns the random factor Evaluate applies to the computed delay. +// It lives outside Evaluate to keep that function deterministic. +func NewJitter() float64 { + return (rand.Float64()*2 - 1) * SyncBackoffJitter +} + +func syncDelay(failures int32) time.Duration { + if failures <= 0 { + return SyncInterval + } + + delay := SyncInterval << (failures - 1) + if delay > MaxSyncBackoff || delay <= 0 { + return MaxSyncBackoff + } + + return delay +} + +func withJitter(delay time.Duration, jitter float64) time.Duration { + if jitter == 0 { + return delay + } + + return delay + time.Duration(float64(delay)*jitter) +} + +func nextFailureCount(failures int32, attempted, fetchFailed bool, fetch *services.FetchOutcome) int32 { + if !attempted { + return failures + } + + if !fetchFailed { + return 0 + } + + if fetch.Terminal { + // A terminal failure is reported immediately; saturating the counter keeps + // a single formula for the schedule and puts the retry at the cap. + return MaxFetchFailures + } + + if failures < MaxFetchFailures { + return failures + 1 + } + + return failures +} diff --git a/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/evaluate_test.go b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/evaluate_test.go new file mode 100644 index 00000000..b1614d45 --- /dev/null +++ b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/evaluate_test.go @@ -0,0 +1,500 @@ +/* +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 helmclusteraddonrepository + +import ( + "errors" + "testing" + "time" + + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" + "github.com/deckhouse/operator-helm/internal/services" +) + +var testNow = time.Date(2026, 9, 1, 12, 0, 0, 0, time.UTC) + +// readyStatus builds a status that already carries proven Ready for the given generation. +func readyStatus(generation int64) helmv1alpha1.HelmClusterAddonRepositoryStatus { + return helmv1alpha1.HelmClusterAddonRepositoryStatus{ + ObservedGeneration: generation, + Conditions: []metav1.Condition{ + { + Type: helmv1alpha1.ConditionTypeReady, + Status: metav1.ConditionTrue, + Reason: helmv1alpha1.ReasonSuccess, + ObservedGeneration: generation, + LastTransitionTime: metav1.NewTime(testNow.Add(-time.Hour)), + }, + { + Type: helmv1alpha1.ConditionTypeSynced, + Status: metav1.ConditionTrue, + Reason: helmv1alpha1.ReasonSuccess, + ObservedGeneration: generation, + LastTransitionTime: metav1.NewTime(testNow.Add(-time.Hour)), + }, + }, + } +} + +// stalledStatus builds a status like readyStatus, plus a Stalled=True condition +// recorded for staleGeneration — used to test that a generation bump voids a +// carried-forward Stalled reason that described the previous spec. +func stalledStatus(generation, staleGeneration int64, reason string) helmv1alpha1.HelmClusterAddonRepositoryStatus { + status := readyStatus(generation) + status.Conditions = append(status.Conditions, metav1.Condition{ + Type: helmv1alpha1.ConditionTypeStalled, + Status: metav1.ConditionTrue, + Reason: reason, + ObservedGeneration: staleGeneration, + LastTransitionTime: metav1.NewTime(testNow.Add(-time.Hour)), + }) + + return status +} + +// syncedNotReadyStatus builds the status left behind by the pass on which the +// first fetch succeeded while the internal repository was still unhealthy: +// Synced records the successful read, but Ready was written False by the +// higher-priority internal-repository rule, so Ready alone carries no evidence. +func syncedNotReadyStatus(generation int64) helmv1alpha1.HelmClusterAddonRepositoryStatus { + return helmv1alpha1.HelmClusterAddonRepositoryStatus{ + ObservedGeneration: generation, + Conditions: []metav1.Condition{ + { + Type: helmv1alpha1.ConditionTypeReady, + Status: metav1.ConditionFalse, + Reason: "FetchFailed", + Message: "failed to fetch index", + ObservedGeneration: generation, + LastTransitionTime: metav1.NewTime(testNow.Add(-time.Minute)), + }, + { + Type: helmv1alpha1.ConditionTypeSynced, + Status: metav1.ConditionTrue, + Reason: helmv1alpha1.ReasonSuccess, + ObservedGeneration: generation, + LastTransitionTime: metav1.NewTime(testNow.Add(-time.Minute)), + }, + { + Type: helmv1alpha1.ConditionTypeReconciling, + Status: metav1.ConditionTrue, + Reason: helmv1alpha1.ReasonReconciling, + Message: "failed to fetch index", + ObservedGeneration: generation, + LastTransitionTime: metav1.NewTime(testNow.Add(-time.Minute)), + }, + }, + } +} + +// catalogFailedStatus builds the status left behind by a pass whose fetch +// succeeded and whose catalog write failed: Ready stays latched True, Synced is +// False with CatalogUpdateFailed and Reconciling carries the retry. +func catalogFailedStatus(generation int64) helmv1alpha1.HelmClusterAddonRepositoryStatus { + status := readyStatus(generation) + apimeta.SetStatusCondition(&status.Conditions, metav1.Condition{ + Type: helmv1alpha1.ConditionTypeSynced, + Status: metav1.ConditionFalse, + Reason: helmv1alpha1.ReasonCatalogUpdateFailed, + Message: "Failed to update the chart catalog: etcdserver: request timed out", + ObservedGeneration: generation, + LastTransitionTime: metav1.NewTime(testNow.Add(-time.Minute)), + }) + apimeta.SetStatusCondition(&status.Conditions, metav1.Condition{ + Type: helmv1alpha1.ConditionTypeReconciling, + Status: metav1.ConditionTrue, + Reason: helmv1alpha1.ReasonProgressingWithRetry, + Message: "Retrying after a chart catalog update failure", + ObservedGeneration: generation, + LastTransitionTime: metav1.NewTime(testNow.Add(-time.Minute)), + }) + + return status +} + +func conditionOf(t *testing.T, status helmv1alpha1.HelmClusterAddonRepositoryStatus, conditionType string) *metav1.Condition { + t.Helper() + + return apimeta.FindStatusCondition(status.Conditions, conditionType) +} + +func TestEvaluateConditions(t *testing.T) { + fetchErr := errors.New("connection refused") + writeErr := errors.New("etcdserver: request timed out") + + cases := []struct { + name string + in Inputs + + wantReady metav1.ConditionStatus + wantReadyReason string + wantSynced metav1.ConditionStatus + wantReconciling string // reason; "" means the condition must be absent + wantStalled string // reason; "" means the condition must be absent + }{ + { + name: "healthy repository", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{}, + Catalog: &services.CatalogOutcome{}, + }, + wantReady: metav1.ConditionTrue, wantReadyReason: helmv1alpha1.ReasonSuccess, + wantSynced: metav1.ConditionTrue, + }, + { + name: "awaiting first sync on a fresh object", + in: Inputs{ + Generation: 1, Now: testNow, + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + }, + wantReady: metav1.ConditionUnknown, wantReadyReason: helmv1alpha1.ReasonAwaitingInitialSync, + wantSynced: "", + wantReconciling: helmv1alpha1.ReasonAwaitingInitialSync, + }, + { + name: "auxiliary resources failed", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + SecretsErr: writeErr, + }, + wantReady: metav1.ConditionFalse, wantReadyReason: helmv1alpha1.ReasonAuxiliaryResourcesFailed, + wantSynced: metav1.ConditionTrue, + wantReconciling: helmv1alpha1.ReasonProgressingWithRetry, + }, + { + name: "internal repository not ready", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + InternalRepository: services.InternalRepositoryState{ + Present: true, Ready: false, + Reason: "FetchFailed", Message: "failed to fetch index", + }, + Attempted: true, + Fetch: &services.FetchOutcome{}, + Catalog: &services.CatalogOutcome{}, + }, + wantReady: metav1.ConditionFalse, wantReadyReason: "FetchFailed", + wantSynced: metav1.ConditionTrue, + wantReconciling: helmv1alpha1.ReasonReconciling, + }, + { + name: "internal repository stalled", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + InternalRepository: services.InternalRepositoryState{ + Present: true, Ready: false, Stalled: true, + Reason: "InvalidSecretRef", Message: "secret not found", + }, + Attempted: true, + Fetch: &services.FetchOutcome{}, + Catalog: &services.CatalogOutcome{}, + }, + wantReady: metav1.ConditionFalse, wantReadyReason: "InvalidSecretRef", + wantSynced: metav1.ConditionTrue, + wantStalled: "InvalidSecretRef", + }, + { + name: "transient fetch failure keeps Ready latched", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{Err: fetchErr, Message: "cannot read index.yaml"}, + }, + wantReady: metav1.ConditionTrue, wantReadyReason: helmv1alpha1.ReasonSuccess, + wantSynced: metav1.ConditionFalse, + wantReconciling: helmv1alpha1.ReasonProgressingWithRetry, + }, + { + name: "transient fetch failure without evidence", + in: Inputs{ + Generation: 1, Now: testNow, + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{Err: fetchErr, Message: "cannot read index.yaml"}, + }, + wantReady: metav1.ConditionUnknown, wantReadyReason: helmv1alpha1.ReasonAwaitingInitialSync, + wantSynced: metav1.ConditionFalse, + wantReconciling: helmv1alpha1.ReasonProgressingWithRetry, + }, + { + name: "terminal fetch failure", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{ + Err: fetchErr, Terminal: true, + Reason: helmv1alpha1.ReasonAuthenticationFailed, + Message: "repository rejected the credentials (HTTP 401)", + }, + }, + wantReady: metav1.ConditionFalse, wantReadyReason: helmv1alpha1.ReasonAuthenticationFailed, + wantSynced: metav1.ConditionFalse, + wantStalled: helmv1alpha1.ReasonAuthenticationFailed, + }, + { + name: "unsupported repository type", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + ConfigErr: &services.ConfigOutcome{ + Reason: helmv1alpha1.ReasonUnsupportedRepositoryType, + Message: "unsupported repository schema in use: ftp", + }, + }, + wantReady: metav1.ConditionFalse, wantReadyReason: helmv1alpha1.ReasonUnsupportedRepositoryType, + wantSynced: metav1.ConditionTrue, + wantStalled: helmv1alpha1.ReasonUnsupportedRepositoryType, + }, + { + name: "catalog write failure", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{}, + Catalog: &services.CatalogOutcome{Err: writeErr}, + }, + wantReady: metav1.ConditionTrue, wantReadyReason: helmv1alpha1.ReasonSuccess, + wantSynced: metav1.ConditionFalse, + wantReconciling: helmv1alpha1.ReasonProgressingWithRetry, + }, + { + name: "generation bump voids the latch", + in: Inputs{ + Generation: 2, Now: testNow, Current: readyStatus(1), + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{Err: fetchErr, Message: "cannot read index.yaml"}, + }, + wantReady: metav1.ConditionUnknown, wantReadyReason: helmv1alpha1.ReasonAwaitingInitialSync, + wantSynced: metav1.ConditionFalse, + wantReconciling: helmv1alpha1.ReasonProgressingWithRetry, + }, + { + name: "oci repository has no internal object", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + InternalRepository: services.InternalRepositoryState{Present: false}, + Attempted: true, + Fetch: &services.FetchOutcome{}, + Catalog: &services.CatalogOutcome{}, + }, + wantReady: metav1.ConditionTrue, wantReadyReason: helmv1alpha1.ReasonSuccess, + wantSynced: metav1.ConditionTrue, + }, + { + // The fetch succeeded on an earlier pass, but the internal repository + // was unhealthy then, so the higher-priority rule owned Ready and wrote + // it False. Ready alone therefore carries no evidence; Synced=True on + // this generation does, and must keep the repository from falling back + // to Unknown/AwaitingInitialSync until the next scheduled sync. + name: "synced carries the evidence when Ready was owned by another rule", + in: Inputs{ + Generation: 1, Now: testNow, Current: syncedNotReadyStatus(1), + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + }, + wantReady: metav1.ConditionTrue, wantReadyReason: helmv1alpha1.ReasonSuccess, + wantSynced: metav1.ConditionTrue, + }, + { + // The work-queue retry after a catalog write failure runs a pass with no + // attempt. Without a carry-forward the repository would show Ready=True, + // Synced=False and no abnormal-true condition at all — Current to + // kstatus — and ConsecutiveFetchFailures never escalates it to Stalled. + name: "catalog write failure keeps Reconciling on a pass with no attempt", + in: Inputs{ + Generation: 1, Now: testNow, Current: catalogFailedStatus(1), + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + }, + wantReady: metav1.ConditionTrue, wantReadyReason: helmv1alpha1.ReasonSuccess, + wantSynced: metav1.ConditionFalse, + wantReconciling: helmv1alpha1.ReasonProgressingWithRetry, + }, + { + name: "generation bump voids a stale stalled reason when no attempt runs", + in: Inputs{ + Generation: 2, Now: testNow, + Current: stalledStatus(1, 1, helmv1alpha1.ReasonAuthenticationFailed), + SecretsErr: writeErr, + }, + wantReady: metav1.ConditionFalse, wantReadyReason: helmv1alpha1.ReasonAuxiliaryResourcesFailed, + wantSynced: metav1.ConditionTrue, + wantReconciling: helmv1alpha1.ReasonProgressingWithRetry, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := Evaluate(tc.in) + + ready := conditionOf(t, got.Status, helmv1alpha1.ConditionTypeReady) + if ready == nil { + t.Fatal("Ready condition must always be present") + } + if ready.Status != tc.wantReady { + t.Fatalf("Ready status is %q, want %q", ready.Status, tc.wantReady) + } + if ready.Reason != tc.wantReadyReason { + t.Fatalf("Ready reason is %q, want %q", ready.Reason, tc.wantReadyReason) + } + if ready.ObservedGeneration != tc.in.Generation { + t.Fatalf("Ready observedGeneration is %d, want %d", ready.ObservedGeneration, tc.in.Generation) + } + + synced := conditionOf(t, got.Status, helmv1alpha1.ConditionTypeSynced) + switch { + case tc.wantSynced == "" && synced != nil: + t.Fatalf("Synced must be absent, got %q", synced.Status) + case tc.wantSynced != "" && synced == nil: + t.Fatal("Synced condition is missing") + case tc.wantSynced != "" && synced.Status != tc.wantSynced: + t.Fatalf("Synced status is %q, want %q", synced.Status, tc.wantSynced) + } + + assertAbnormal(t, got.Status, helmv1alpha1.ConditionTypeReconciling, tc.wantReconciling) + assertAbnormal(t, got.Status, helmv1alpha1.ConditionTypeStalled, tc.wantStalled) + + // I3: the processed generation is always recorded. + if got.Status.ObservedGeneration != tc.in.Generation { + t.Fatalf("status.observedGeneration is %d, want %d", got.Status.ObservedGeneration, tc.in.Generation) + } + + // I1 and I2: exactly one abnormal-true condition while unhealthy, none while healthy. + healthy := ready.Status == metav1.ConditionTrue && synced != nil && synced.Status == metav1.ConditionTrue + abnormal := 0 + for _, conditionType := range []string{helmv1alpha1.ConditionTypeReconciling, helmv1alpha1.ConditionTypeStalled} { + if conditionOf(t, got.Status, conditionType) != nil { + abnormal++ + } + } + if healthy && abnormal != 0 { + t.Fatalf("healthy repository must carry no abnormal-true conditions, got %d", abnormal) + } + if !healthy && abnormal != 1 { + t.Fatalf("unhealthy repository must carry exactly one abnormal-true condition, got %d", abnormal) + } + }) + } +} + +// TestEvaluateDecisionErr pins the filter behind Decision.Err: only failures that +// belong on the controller-runtime work queue reach it (auxiliary resources, +// the internal repository object, the chart catalog write). Repository-read +// failures (Fetch, ConfigErr) are deliberately excluded — their retry is +// scheduled through nextSyncTime instead, and routing them into the work queue +// as well would double-schedule the retry. +func TestEvaluateDecisionErr(t *testing.T) { + secretsErr := errors.New("failed to reconcile secret") + internalErr := errors.New("failed to reconcile internal repository object") + catalogWriteErr := errors.New("etcdserver: request timed out") + fetchErr := errors.New("connection refused") + + cases := []struct { + name string + in Inputs + wantErr error + }{ + { + name: "auxiliary resource failure reaches Decision.Err", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + SecretsErr: secretsErr, + }, + wantErr: secretsErr, + }, + { + name: "internal repository reconcile failure reaches Decision.Err", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + InternalRepositoryErr: internalErr, + }, + wantErr: internalErr, + }, + { + name: "chart catalog write failure reaches Decision.Err", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{}, + Catalog: &services.CatalogOutcome{Err: catalogWriteErr}, + }, + wantErr: catalogWriteErr, + }, + { + name: "repository read failure never reaches Decision.Err", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{Err: fetchErr, Message: "cannot read index.yaml"}, + }, + wantErr: nil, + }, + { + name: "configuration failure never reaches Decision.Err", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + ConfigErr: &services.ConfigOutcome{ + Reason: helmv1alpha1.ReasonUnsupportedRepositoryType, + Message: "unsupported repository schema in use: ftp", + }, + }, + wantErr: nil, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := Evaluate(tc.in) + + if !errors.Is(got.Err, tc.wantErr) { + t.Fatalf("Decision.Err is %v, want %v", got.Err, tc.wantErr) + } + }) + } +} + +func assertAbnormal(t *testing.T, status helmv1alpha1.HelmClusterAddonRepositoryStatus, conditionType, wantReason string) { + t.Helper() + + cond := conditionOf(t, status, conditionType) + if wantReason == "" { + if cond != nil { + t.Fatalf("%s must be absent, got reason %q", conditionType, cond.Reason) + } + + return + } + + if cond == nil { + t.Fatalf("%s is missing, want reason %q", conditionType, wantReason) + } + if cond.Status != metav1.ConditionTrue { + t.Fatalf("%s status is %q, want True", conditionType, cond.Status) + } + if cond.Reason != wantReason { + t.Fatalf("%s reason is %q, want %q", conditionType, cond.Reason, wantReason) + } +} diff --git a/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/kstatus_test.go b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/kstatus_test.go new file mode 100644 index 00000000..2594c6f7 --- /dev/null +++ b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/kstatus_test.go @@ -0,0 +1,130 @@ +/* +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 helmclusteraddonrepository + +import ( + "errors" + "testing" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/cli-utils/pkg/kstatus/status" + + helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" + "github.com/deckhouse/operator-helm/internal/services" +) + +// computeStatus renders the decision as the repository object would look in the +// cluster and asks the real kstatus library for its verdict. +func computeStatus(t *testing.T, generation int64, decision Decision) status.Status { + t.Helper() + + repo := &helmv1alpha1.HelmClusterAddonRepository{} + repo.SetGroupVersionKind(helmv1alpha1.HelmClusterAddonRepositoryGVK) + repo.SetName("test-repo") + repo.SetGeneration(generation) + repo.Status = decision.Status + + content, err := runtime.DefaultUnstructuredConverter.ToUnstructured(repo) + if err != nil { + t.Fatalf("converting repository to unstructured: %v", err) + } + + result, err := status.Compute(&unstructured.Unstructured{Object: content}) + if err != nil { + t.Fatalf("computing kstatus: %v", err) + } + + return result.Status +} + +func TestKstatusVerdicts(t *testing.T) { + cases := []struct { + name string + in Inputs + want status.Status + }{ + { + name: "healthy repository is current", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{}, + Catalog: &services.CatalogOutcome{}, + }, + want: status.CurrentStatus, + }, + { + name: "retrying repository is in progress", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{Err: errors.New("connection refused"), Message: "cannot read index.yaml"}, + }, + want: status.InProgressStatus, + }, + { + name: "terminal failure is failed", + in: Inputs{ + Generation: 1, Now: testNow, Current: readyStatus(1), + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{ + Err: errors.New("unauthorized"), Terminal: true, + Reason: helmv1alpha1.ReasonAuthenticationFailed, Message: "rejected the credentials", + }, + }, + want: status.FailedStatus, + }, + { + // The work-queue retry after a catalog write failure runs a pass with no + // attempt. Without the carry-forward in evaluateReconciling the object + // would carry no abnormal-true condition and kstatus would call a + // permanently broken catalog write Current. + name: "catalog write failure stays in progress between attempts", + in: Inputs{ + Generation: 1, Now: testNow, Current: catalogFailedStatus(1), + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + }, + want: status.InProgressStatus, + }, + { + name: "stalled is not masked by a lagging observedGeneration", + in: Inputs{ + Generation: 3, Now: testNow, Current: readyStatus(1), + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{ + Err: errors.New("not found"), Terminal: true, + Reason: helmv1alpha1.ReasonSourceNotFound, Message: "repository not found", + }, + }, + want: status.FailedStatus, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := computeStatus(t, tc.in.Generation, Evaluate(tc.in)) + if got != tc.want { + t.Fatalf("kstatus verdict is %s, want %s", got, tc.want) + } + }) + } +} diff --git a/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/reconciler.go b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/reconciler.go index 6ef7ac9e..5b7dfdbe 100644 --- a/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/reconciler.go +++ b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/reconciler.go @@ -22,7 +22,6 @@ import ( "time" apierrors "k8s.io/apimachinery/pkg/api/errors" - apimeta "k8s.io/apimachinery/pkg/api/meta" "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -75,14 +74,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco if apierrors.IsNotFound(err) { return reconcile.Result{}, nil } + return reconcile.Result{}, fmt.Errorf("getting helm cluster addon repository: %w", err) } - repoType, err := utils.GetRepositoryType(repo.Spec.URL) - if err != nil { - logger.Error(err, "failed to determine repository type") - return reconcile.Result{}, err - } + repoType, repoTypeErr := utils.GetRepositoryType(repo.Spec.URL) if !repo.DeletionTimestamp.IsZero() { return r.reconcileDelete(ctx, &repo, repoType) @@ -100,69 +96,88 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco // would not trigger a follow-up reconcile. } - var helmRepoRes services.HelmRepoResult - var ociRepoRes services.OCIRepoResult - var chartSyncRes services.RepoSyncResult + in := Inputs{ + Generation: repo.Generation, + Now: time.Now().UTC(), + Jitter: NewJitter(), + Current: *repo.Status.DeepCopy(), + } - switch repoType { - case utils.InternalHelmRepository: - helmRepoRes = r.helmRepositoryService.EnsureInternalHelmRepository(ctx, &repo) - case utils.InternalOCIRepository: - if err := r.helmRepositoryService.RemoveHelmRepository(ctx, repo.Name); err != nil { - ociRepoRes = services.OCIRepoResult{ - Status: status.Failed(&repo, helmv1alpha1.ReasonFailed, "Repository change failed", err), - } - break + if repoTypeErr != nil { + in.ConfigErr = &services.ConfigOutcome{ + Reason: helmv1alpha1.ReasonUnsupportedRepositoryType, + Message: repoTypeErr.Error(), + Err: repoTypeErr, } - ociRepoRes = r.ociRepositoryService.EnsureRepositorySecrets(ctx, &repo) - default: - err := fmt.Errorf("unsupported repository type: %q", repoType) - helmRepoRes = services.HelmRepoResult{Status: status.Failed(&repo, "UnsupportedRepositoryType", err.Error(), err)} + + return r.finish(ctx, &repo, in, false) } - if helmRepoRes.IsReady() || ociRepoRes.IsReady() { - chartSyncRes = r.chartSyncService.EnsureAddonCharts(ctx, &repo, repoType) - } else { - chartSyncRes = services.RepoSyncResult{Status: status.Failed(&repo, helmv1alpha1.ReasonRepositoryNotReady, helmRepoRes.Status.Message, nil)} + // Both services embed the same BaseRepoService with the same target namespace, + // so one of them reconciles the auxiliary secrets for either repository type. + in.SecretsErr = r.helmRepositoryService.EnsureSecrets(ctx, &repo, repoType) + + if in.SecretsErr == nil { + switch repoType { + case utils.InternalHelmRepository: + in.InternalRepository, in.InternalRepositoryErr = r.helmRepositoryService.EnsureInternalHelmRepository(ctx, &repo) + case utils.InternalOCIRepository: + // The url may have changed from helm to oci: drop the internal object + // that is no longer used. OCI repositories have none of their own. + in.InternalRepositoryErr = r.helmRepositoryService.RemoveHelmRepository(ctx, repo.Name) + } } - if err := r.reconcileForceAnnotation(ctx, req); err != nil { - return reconcile.Result{}, fmt.Errorf("failed to reconcile force annotation: %w", err) + forced := repo.ForceReconcileRequired() + + if in.SecretsErr == nil && in.InternalRepositoryErr == nil && + ShouldAttempt(in.Current, in.Generation, in.Now, forced) { + outcome := r.chartSyncService.Sync(ctx, &repo, repoType) + + in.Attempted = true + in.Fetch = &outcome.Fetch + in.Catalog = &outcome.Catalog } - if err := r.statusManager.Update( - ctx, - &repo, - status.NoopStatusMutator, - status.NoopStatusMapper, - helmRepoRes, - ociRepoRes, - chartSyncRes, - ); client.IgnoreNotFound(err) != nil { - return reconcile.Result{}, fmt.Errorf("failed to update status: %w", err) + return r.finish(ctx, &repo, in, in.Attempted) +} + +// finish applies the decision and consumes the force annotation when an attempt +// actually ran. The annotation is removed after the status patch so a conflict +// does not lose the request. +func (r *Reconciler) finish( + ctx context.Context, + repo *helmv1alpha1.HelmClusterAddonRepository, + in Inputs, + attempted bool, +) (reconcile.Result, error) { + decision := Evaluate(in) + + if in.Fetch != nil && in.Fetch.Err != nil { + // A repository read failure is not returned to the work queue — its retry + // is carried by nextSyncTime — so this is the only place it is logged. + log.FromContext(ctx).Error(in.Fetch.Err, in.Fetch.Message, "repository", repo.Name) } - // EnsureAddonCharts is a two-phase state machine: the first pass only marks - // the Synced condition Reconciling, the second pass performs the actual chart - // fetch. Run the second pass in the same reconcile (the status update above - // already persisted the Reconciling state and advanced the condition's - // LastTransitionTime) instead of relying on the status-update watch event to - // trigger it — otherwise predicates that ignore status-only changes would - // stall the scheduled sync. - if chartSyncRes.InProgress() { - chartSyncRes = r.chartSyncService.EnsureAddonCharts(ctx, &repo, repoType) - if err := r.statusManager.Update( - ctx, - &repo, - status.NoopStatusMutator, - status.NoopStatusMapper, - chartSyncRes, - ); client.IgnoreNotFound(err) != nil { - return reconcile.Result{}, fmt.Errorf("failed to update sync status: %w", err) + if err := r.statusManager.PatchStatus(ctx, repo, func() { + repo.Status = decision.Status + }); client.IgnoreNotFound(err) != nil { + return reconcile.Result{}, err + } + + if attempted { + if err := r.reconcileForceAnnotation(ctx, client.ObjectKeyFromObject(repo)); err != nil { + return reconcile.Result{}, fmt.Errorf("failed to reconcile force annotation: %w", err) } } - return r.requeueAtSyncInterval(&repo) + if decision.Err != nil { + // Cluster write failures are handed to the work queue rate limiter; the + // schedule is re-established on the next pass. + return reconcile.Result{}, decision.Err + } + + return reconcile.Result{RequeueAfter: decision.RequeueAfter}, nil } func (r *Reconciler) reconcileDelete(ctx context.Context, repo *helmv1alpha1.HelmClusterAddonRepository, repoType utils.InternalRepositoryType) (reconcile.Result, error) { @@ -173,7 +188,19 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, repo *helmv1alpha1.Hel } switch repoType { - case utils.InternalHelmRepository: + case utils.InternalOCIRepository: + if err := r.ociRepositoryService.CleanupOCIRepository(ctx, repo.Name); err != nil && !apierrors.IsNotFound(err) { + _ = r.statusManager.MarkDeletionFailed(ctx, repo, "internal repository", err) + return reconcile.Result{}, err + } + default: + // The helm path is the default rather than a case of its own because an + // unknown repository type is a state a real repository can reach: the url + // validation regex on the CRD is looser than url.Parse, so a repository + // whose internal objects already exist can be edited to a url that no + // longer parses and then deleted. Cleaning up the helm way is safe for + // either type — it removes both auxiliary secrets and tolerates a missing + // internal repository — and leaving it out would orphan them. helmRepo, err := r.helmRepositoryService.CleanupHelmRepository(ctx, repo.Name) if err != nil && !apierrors.IsNotFound(err) { _ = r.statusManager.MarkDeletionFailed(ctx, repo, "internal repository", err) @@ -182,11 +209,6 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, repo *helmv1alpha1.Hel if helmRepo != nil { return r.awaitInternalResourceDeletion(ctx, repo, "internal repository", helmRepo) } - case utils.InternalOCIRepository: - if err := r.ociRepositoryService.CleanupOCIRepository(ctx, repo.Name); err != nil && !apierrors.IsNotFound(err) { - _ = r.statusManager.MarkDeletionFailed(ctx, repo, "internal repository", err) - return reconcile.Result{}, err - } } if err := retry.RetryOnConflict(retry.DefaultRetry, func() error { @@ -224,17 +246,21 @@ func (r *Reconciler) awaitInternalResourceDeletion(ctx context.Context, repo *he return reconcile.Result{RequeueAfter: internalResourceDeletionRequeueInterval}, nil } -func (r *Reconciler) reconcileForceAnnotation(ctx context.Context, req reconcile.Request) error { +func (r *Reconciler) reconcileForceAnnotation(ctx context.Context, key client.ObjectKey) error { var repo helmv1alpha1.HelmClusterAddonRepository - if err := r.Get(ctx, req.NamespacedName, &repo); err != nil { + if err := r.Get(ctx, key, &repo); err != nil { if apierrors.IsNotFound(err) { return nil } + return fmt.Errorf("getting helm cluster addon repository: %w", err) } - if repo.Annotations == nil { + if _, found := repo.Annotations[helmv1alpha1.AnnotationForceReconcile]; !found { + // Guard on the annotation itself, not on the map: a repository carrying + // any unrelated annotation would otherwise take an empty PATCH on every + // attempted pass. return nil } @@ -248,15 +274,3 @@ func (r *Reconciler) reconcileForceAnnotation(ctx context.Context, req reconcile return nil } - -func (r *Reconciler) requeueAtSyncInterval(repo *helmv1alpha1.HelmClusterAddonRepository) (reconcile.Result, error) { - repoSyncCond := apimeta.FindStatusCondition(repo.Status.Conditions, helmv1alpha1.ConditionTypeSynced) - if repoSyncCond != nil { - remaining := time.Until(repoSyncCond.LastTransitionTime.Add(services.ChartsSyncInterval)) - if remaining > 0 { - return reconcile.Result{RequeueAfter: remaining}, nil - } - } - - return reconcile.Result{RequeueAfter: services.ChartsSyncInterval}, nil -} diff --git a/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/reconciler_test.go b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/reconciler_test.go new file mode 100644 index 00000000..bc896aa7 --- /dev/null +++ b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/reconciler_test.go @@ -0,0 +1,327 @@ +/* +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 helmclusteraddonrepository + +import ( + "context" + "testing" + "time" + + "github.com/Masterminds/semver/v3" + sourcev1 "github.com/werf/nelm-source-controller/api/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" + repoclient "github.com/deckhouse/operator-helm/internal/client/repository" + "github.com/deckhouse/operator-helm/internal/manager/status" + "github.com/deckhouse/operator-helm/internal/services" + "github.com/deckhouse/operator-helm/internal/utils" +) + +type stubRepoClient struct { + charts []repoclient.Chart + err error +} + +// The receiver is a pointer so a test can change what the repository returns +// between reconcile passes. +func (s *stubRepoClient) FetchCharts(_ context.Context, _ string, _ *repoclient.RepoConfig) ([]repoclient.Chart, error) { + return s.charts, s.err +} + +func newReconciler(t *testing.T, stub *stubRepoClient, objects ...client.Object) (*Reconciler, client.Client) { + t.Helper() + + scheme := runtime.NewScheme() + for _, add := range []func(*runtime.Scheme) error{ + clientgoscheme.AddToScheme, + helmv1alpha1.AddToScheme, + sourcev1.AddToScheme, + } { + if err := add(scheme); err != nil { + t.Fatalf("registering scheme: %v", err) + } + } + + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objects...). + WithStatusSubresource( + &helmv1alpha1.HelmClusterAddonRepository{}, + &helmv1alpha1.HelmClusterAddonChart{}, + ). + Build() + + factory := func(_ utils.InternalRepositoryType) (repoclient.ClientInterface, error) { + return stub, nil + } + + r := New( + c, + services.NewHelmRepoService(c, scheme, helmv1alpha1.TargetNamespace), + services.NewOCIRepoService(c, scheme, helmv1alpha1.TargetNamespace), + services.NewRepoSyncService(c, scheme, factory), + status.NewManager(c), + ) + + return r, c +} + +func ociRepository() *helmv1alpha1.HelmClusterAddonRepository { + return &helmv1alpha1.HelmClusterAddonRepository{ + ObjectMeta: metav1.ObjectMeta{Name: "example", Generation: 1}, + Spec: helmv1alpha1.HelmClusterAddonRepositorySpec{URL: "oci://ghcr.io/example/podinfo"}, + } +} + +func reconcileUntilStable(t *testing.T, r *Reconciler, name string) reconcile.Result { + t.Helper() + + var result reconcile.Result + // The first pass only adds the finalizer path; two passes are enough to reach + // a stable state for a repository whose source responds. + for range 2 { + var err error + result, err = r.Reconcile(context.Background(), reconcile.Request{ + NamespacedName: types.NamespacedName{Name: name}, + }) + if err != nil { + t.Fatalf("Reconcile returned %v", err) + } + } + + return result +} + +func TestReconcileOCIRepositoryBecomesReady(t *testing.T) { + repo := ociRepository() + stub := &stubRepoClient{charts: []repoclient.Chart{{ + Name: "podinfo", + Versions: []repoclient.ChartVersion{{Version: semver.MustParse("6.7.1")}}, + }}} + + r, c := newReconciler(t, stub, repo) + result := reconcileUntilStable(t, r, repo.Name) + + if result.RequeueAfter <= 0 || result.RequeueAfter > time.Hour { + t.Fatalf("expected a scheduled requeue, got %s", result.RequeueAfter) + } + + updated := &helmv1alpha1.HelmClusterAddonRepository{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(repo), updated); err != nil { + t.Fatalf("getting repository: %v", err) + } + + if !apimeta.IsStatusConditionTrue(updated.Status.Conditions, helmv1alpha1.ConditionTypeReady) { + t.Fatalf("Ready must be True, conditions: %v", updated.Status.Conditions) + } + if !apimeta.IsStatusConditionTrue(updated.Status.Conditions, helmv1alpha1.ConditionTypeSynced) { + t.Fatal("Synced must be True") + } + if apimeta.FindStatusCondition(updated.Status.Conditions, helmv1alpha1.ConditionTypeReconciling) != nil { + t.Fatal("Reconciling must be absent on a healthy repository") + } + if apimeta.FindStatusCondition(updated.Status.Conditions, helmv1alpha1.ConditionTypeStalled) != nil { + t.Fatal("Stalled must be absent on a healthy repository") + } + if updated.Status.LastSuccessfulSyncTime == nil || updated.Status.NextSyncTime == nil { + t.Fatal("sync timestamps must be recorded") + } +} + +func TestReconcileSkipsFetchBeforeSchedule(t *testing.T) { + repo := ociRepository() + stub := &stubRepoClient{charts: []repoclient.Chart{{ + Name: "podinfo", + Versions: []repoclient.ChartVersion{{Version: semver.MustParse("6.7.1")}}, + }}} + + r, c := newReconciler(t, stub, repo) + reconcileUntilStable(t, r, repo.Name) + + before := &helmv1alpha1.HelmClusterAddonRepository{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(repo), before); err != nil { + t.Fatalf("getting repository: %v", err) + } + + // A watch-driven pass before nextSyncTime must not move the schedule. + if _, err := r.Reconcile(context.Background(), reconcile.Request{ + NamespacedName: types.NamespacedName{Name: repo.Name}, + }); err != nil { + t.Fatalf("Reconcile returned %v", err) + } + + after := &helmv1alpha1.HelmClusterAddonRepository{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(repo), after); err != nil { + t.Fatalf("getting repository: %v", err) + } + + if !after.Status.NextSyncTime.Time.Equal(before.Status.NextSyncTime.Time) { + t.Fatalf("nextSyncTime moved without a due schedule: %s -> %s", + before.Status.NextSyncTime.Time, after.Status.NextSyncTime.Time) + } +} + +func TestReconcileTerminalFetchFailureStalls(t *testing.T) { + repo := ociRepository() + stub := &stubRepoClient{err: &repoclient.TerminalError{ + Reason: helmv1alpha1.ReasonAuthenticationFailed, + Message: "repository rejected the credentials (HTTP 401)", + }} + + r, c := newReconciler(t, stub, repo) + reconcileUntilStable(t, r, repo.Name) + + updated := &helmv1alpha1.HelmClusterAddonRepository{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(repo), updated); err != nil { + t.Fatalf("getting repository: %v", err) + } + + stalled := apimeta.FindStatusCondition(updated.Status.Conditions, helmv1alpha1.ConditionTypeStalled) + if stalled == nil || stalled.Reason != helmv1alpha1.ReasonAuthenticationFailed { + t.Fatalf("expected Stalled=AuthenticationFailed, got %v", stalled) + } + if !apimeta.IsStatusConditionFalse(updated.Status.Conditions, helmv1alpha1.ConditionTypeReady) { + t.Fatalf("Ready must be False while Stalled, conditions: %v", updated.Status.Conditions) + } + if apimeta.FindStatusCondition(updated.Status.Conditions, helmv1alpha1.ConditionTypeReconciling) != nil { + t.Fatal("Reconciling and Stalled must be mutually exclusive") + } + // The fake client bumps generation on the finalizer update, so compare with + // the live object rather than with the fixture. + if updated.Status.ObservedGeneration != updated.Generation { + t.Fatalf("observedGeneration is %d, want %d", updated.Status.ObservedGeneration, updated.Generation) + } +} + +// TestReconcileRemovesStalledOnRecovery pins that an abnormal-true condition is +// removed from the STORED object and not merely from the in-memory status. +// Removal rides on the JSON merge patch client.MergeFrom produces, which +// replaces the whole conditions array; were that ever to stop holding, a +// repository would keep reporting Failed to kstatus forever after one stall. +func TestReconcileRemovesStalledOnRecovery(t *testing.T) { + repo := ociRepository() + stub := &stubRepoClient{err: &repoclient.TerminalError{ + Reason: helmv1alpha1.ReasonSourceNotFound, + Message: "repository not found (HTTP 404)", + }} + + r, c := newReconciler(t, stub, repo) + reconcileUntilStable(t, r, repo.Name) + + stalled := &helmv1alpha1.HelmClusterAddonRepository{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(repo), stalled); err != nil { + t.Fatalf("getting repository: %v", err) + } + if apimeta.FindStatusCondition(stalled.Status.Conditions, helmv1alpha1.ConditionTypeStalled) == nil { + t.Fatalf("the fixture must reach Stalled first, conditions: %v", stalled.Status.Conditions) + } + + // The source recovers. The force annotation makes the next pass attempt + // regardless of the schedule the stall left behind. + stub.err = nil + stub.charts = []repoclient.Chart{{ + Name: "podinfo", + Versions: []repoclient.ChartVersion{{Version: semver.MustParse("6.7.1")}}, + }} + + stalled.Annotations = map[string]string{helmv1alpha1.AnnotationForceReconcile: ""} + if err := c.Update(context.Background(), stalled); 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) + } + + recovered := &helmv1alpha1.HelmClusterAddonRepository{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(repo), recovered); err != nil { + t.Fatalf("getting repository: %v", err) + } + + if cond := apimeta.FindStatusCondition(recovered.Status.Conditions, helmv1alpha1.ConditionTypeStalled); cond != nil { + t.Fatalf("Stalled must be gone from the stored object, got %+v", cond) + } + if !apimeta.IsStatusConditionTrue(recovered.Status.Conditions, helmv1alpha1.ConditionTypeReady) { + t.Fatalf("Ready must be True after recovery, conditions: %v", recovered.Status.Conditions) + } + if apimeta.FindStatusCondition(recovered.Status.Conditions, helmv1alpha1.ConditionTypeReconciling) != nil { + t.Fatal("Reconciling must be absent on a recovered repository") + } + if _, found := recovered.Annotations[helmv1alpha1.AnnotationForceReconcile]; found { + t.Fatal("the force annotation must be consumed by the pass it triggered") + } +} + +// TestReconcileDeleteCleansUpWhenURLNoLongerParses covers a repository whose url +// satisfies the CRD's validation regex but is rejected by url.Parse, so the +// repository type cannot be determined. Its internal objects were created while +// the url still parsed, so the deletion path must still remove them. +func TestReconcileDeleteCleansUpWhenURLNoLongerParses(t *testing.T) { + now := metav1.Now() + repo := &helmv1alpha1.HelmClusterAddonRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Generation: 1, + Finalizers: []string{helmv1alpha1.FinalizerName}, + DeletionTimestamp: &now, + }, + // Passes the CRD rule ^(https?|oci)://.+$ and fails url.Parse. + Spec: helmv1alpha1.HelmClusterAddonRepositorySpec{URL: "https://exa mple.invalid/charts"}, + } + + if _, err := utils.GetRepositoryType(repo.Spec.URL); err == nil { + t.Fatal("the fixture url must be unparsable, otherwise the test proves nothing") + } + + internalRepo := &sourcev1.HelmRepository{ObjectMeta: metav1.ObjectMeta{ + Name: utils.GetInternalHelmRepositoryName(repo.Name), + Namespace: helmv1alpha1.TargetNamespace, + }} + authSecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{ + Name: utils.GetInternalRepositoryAuthSecretName(repo.Name), + Namespace: helmv1alpha1.TargetNamespace, + }} + tlsSecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{ + Name: utils.GetInternalRepositoryTLSSecretName(repo.Name), + Namespace: helmv1alpha1.TargetNamespace, + }} + + r, c := newReconciler(t, &stubRepoClient{}, repo, internalRepo, authSecret, tlsSecret) + + // The first pass deletes the internal objects and waits for the internal + // repository to disappear; the second removes the finalizer. + reconcileUntilStable(t, r, repo.Name) + + for _, obj := range []client.Object{internalRepo, authSecret, tlsSecret} { + key := client.ObjectKeyFromObject(obj) + if err := c.Get(context.Background(), key, obj.DeepCopyObject().(client.Object)); !apierrors.IsNotFound(err) { + t.Fatalf("%s must be deleted, got %v", key, err) + } + } +} diff --git a/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/schedule_test.go b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/schedule_test.go new file mode 100644 index 00000000..68913615 --- /dev/null +++ b/images/operator-helm-controller/internal/reconcile/helmclusteraddonrepository/schedule_test.go @@ -0,0 +1,291 @@ +/* +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 helmclusteraddonrepository + +import ( + "errors" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" + "github.com/deckhouse/operator-helm/internal/services" +) + +func TestBackoffProgression(t *testing.T) { + fetchErr := errors.New("connection refused") + + cases := []struct { + name string + failuresIn int32 + wantFailures int32 + wantRequeue time.Duration + }{ + {name: "first failure", failuresIn: 0, wantFailures: 1, wantRequeue: 5 * time.Minute}, + {name: "second failure", failuresIn: 1, wantFailures: 2, wantRequeue: 10 * time.Minute}, + {name: "third failure", failuresIn: 2, wantFailures: 3, wantRequeue: 20 * time.Minute}, + {name: "fourth failure", failuresIn: 3, wantFailures: 4, wantRequeue: 40 * time.Minute}, + {name: "fifth failure caps", failuresIn: 4, wantFailures: 5, wantRequeue: time.Hour}, + {name: "beyond cap stays capped", failuresIn: 5, wantFailures: 5, wantRequeue: time.Hour}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := Evaluate(Inputs{ + Generation: 1, + Now: testNow, + Current: helmv1alpha1.HelmClusterAddonRepositoryStatus{ + ObservedGeneration: 1, + ConsecutiveFetchFailures: tc.failuresIn, + }, + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{Err: fetchErr, Message: "cannot read index.yaml"}, + }) + + if got.Status.ConsecutiveFetchFailures != tc.wantFailures { + t.Fatalf("failures are %d, want %d", got.Status.ConsecutiveFetchFailures, tc.wantFailures) + } + if got.RequeueAfter != tc.wantRequeue { + t.Fatalf("requeue after %s, want %s", got.RequeueAfter, tc.wantRequeue) + } + if got.Status.NextSyncTime == nil { + t.Fatal("nextSyncTime must be set after an attempt") + } + if !got.Status.NextSyncTime.Time.Equal(testNow.Add(tc.wantRequeue)) { + t.Fatalf("nextSyncTime is %s, want %s", got.Status.NextSyncTime.Time, testNow.Add(tc.wantRequeue)) + } + }) + } +} + +func TestSuccessResetsCounterAndRecordsSyncTime(t *testing.T) { + got := Evaluate(Inputs{ + Generation: 1, + Now: testNow, + Current: helmv1alpha1.HelmClusterAddonRepositoryStatus{ + ObservedGeneration: 1, + ConsecutiveFetchFailures: 3, + }, + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{}, + Catalog: &services.CatalogOutcome{}, + }) + + if got.Status.ConsecutiveFetchFailures != 0 { + t.Fatalf("counter must reset on success, got %d", got.Status.ConsecutiveFetchFailures) + } + if got.RequeueAfter != SyncInterval { + t.Fatalf("requeue after %s, want %s", got.RequeueAfter, SyncInterval) + } + if got.Status.LastSuccessfulSyncTime == nil || !got.Status.LastSuccessfulSyncTime.Time.Equal(testNow) { + t.Fatalf("lastSuccessfulSyncTime is %v, want %s", got.Status.LastSuccessfulSyncTime, testNow) + } +} + +func TestCatalogFailureDoesNotRecordSyncTime(t *testing.T) { + previous := metav1.NewTime(testNow.Add(-time.Hour)) + + got := Evaluate(Inputs{ + Generation: 1, + Now: testNow, + Current: helmv1alpha1.HelmClusterAddonRepositoryStatus{ + ObservedGeneration: 1, + LastSuccessfulSyncTime: &previous, + }, + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{}, + Catalog: &services.CatalogOutcome{Err: errors.New("etcdserver: request timed out")}, + }) + + if !got.Status.LastSuccessfulSyncTime.Time.Equal(previous.Time) { + t.Fatalf("lastSuccessfulSyncTime must not advance on a catalog failure, got %s", got.Status.LastSuccessfulSyncTime.Time) + } + if got.Status.ConsecutiveFetchFailures != 0 { + t.Fatalf("a catalog failure must not count as a fetch failure, got %d", got.Status.ConsecutiveFetchFailures) + } + if got.Err == nil { + t.Fatal("a catalog failure must be returned for the work queue") + } +} + +func TestTerminalFetchSaturatesCounter(t *testing.T) { + got := Evaluate(Inputs{ + Generation: 1, + Now: testNow, + Current: helmv1alpha1.HelmClusterAddonRepositoryStatus{ObservedGeneration: 1}, + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{ + Err: errors.New("unauthorized"), Terminal: true, + Reason: helmv1alpha1.ReasonAuthenticationFailed, Message: "rejected the credentials", + }, + }) + + if got.Status.ConsecutiveFetchFailures != MaxFetchFailures { + t.Fatalf("terminal failure must saturate the counter, got %d", got.Status.ConsecutiveFetchFailures) + } + if got.RequeueAfter != MaxSyncBackoff { + t.Fatalf("requeue after %s, want %s", got.RequeueAfter, MaxSyncBackoff) + } +} + +func TestConfigErrorDoesNotRequeue(t *testing.T) { + got := Evaluate(Inputs{ + Generation: 1, + Now: testNow, + Current: helmv1alpha1.HelmClusterAddonRepositoryStatus{ObservedGeneration: 1}, + ConfigErr: &services.ConfigOutcome{ + Reason: helmv1alpha1.ReasonUnsupportedRepositoryType, + Message: "unsupported repository schema in use: ftp", + }, + }) + + if got.RequeueAfter != 0 { + t.Fatalf("a spec-only failure must not requeue, got %s", got.RequeueAfter) + } +} + +func TestGenerationBumpResetsCounter(t *testing.T) { + got := Evaluate(Inputs{ + Generation: 2, + Now: testNow, + Current: helmv1alpha1.HelmClusterAddonRepositoryStatus{ + ObservedGeneration: 1, + ConsecutiveFetchFailures: 4, + }, + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + Attempted: true, + Fetch: &services.FetchOutcome{}, + Catalog: &services.CatalogOutcome{}, + }) + + if got.Status.ConsecutiveFetchFailures != 0 { + t.Fatalf("counter must reset on a spec change, got %d", got.Status.ConsecutiveFetchFailures) + } +} + +func TestPassWithoutAttemptKeepsSchedule(t *testing.T) { + next := metav1.NewTime(testNow.Add(3 * time.Minute)) + + got := Evaluate(Inputs{ + Generation: 1, + Now: testNow, + Current: helmv1alpha1.HelmClusterAddonRepositoryStatus{ + ObservedGeneration: 1, + NextSyncTime: &next, + ConsecutiveFetchFailures: 2, + Conditions: []metav1.Condition{ + { + Type: helmv1alpha1.ConditionTypeReady, + Status: metav1.ConditionTrue, + Reason: helmv1alpha1.ReasonSuccess, + ObservedGeneration: 1, + LastTransitionTime: metav1.NewTime(testNow.Add(-time.Hour)), + }, + }, + }, + InternalRepository: services.InternalRepositoryState{Present: true, Ready: true}, + }) + + if !got.Status.NextSyncTime.Time.Equal(next.Time) { + t.Fatalf("nextSyncTime must not move without an attempt, got %s", got.Status.NextSyncTime.Time) + } + if got.Status.ConsecutiveFetchFailures != 2 { + t.Fatalf("counter must not move without an attempt, got %d", got.Status.ConsecutiveFetchFailures) + } + if got.RequeueAfter != 3*time.Minute { + t.Fatalf("requeue after %s, want the remaining 3m", got.RequeueAfter) + } +} + +func TestOverdueScheduleWithoutAttemptFloorsRequeue(t *testing.T) { + overdue := metav1.NewTime(testNow.Add(-time.Minute)) + + got := Evaluate(Inputs{ + Generation: 1, + Now: testNow, + Current: helmv1alpha1.HelmClusterAddonRepositoryStatus{ + ObservedGeneration: 1, + NextSyncTime: &overdue, + }, + // SecretsErr blocks the attempt: the reconciler never gets to Fetch/Catalog, + // so Attempted stays false while the schedule is already due. + SecretsErr: errors.New("failed to reconcile secret"), + }) + + if got.RequeueAfter != minRequeue { + t.Fatalf("requeue after %s, want the floor %s", got.RequeueAfter, minRequeue) + } +} + +func TestShouldAttempt(t *testing.T) { + future := metav1.NewTime(testNow.Add(time.Minute)) + past := metav1.NewTime(testNow.Add(-time.Minute)) + + cases := []struct { + name string + current helmv1alpha1.HelmClusterAddonRepositoryStatus + generation int64 + forced bool + want bool + }{ + {name: "fresh object", current: helmv1alpha1.HelmClusterAddonRepositoryStatus{}, generation: 1, want: true}, + { + name: "schedule not reached", + current: helmv1alpha1.HelmClusterAddonRepositoryStatus{ObservedGeneration: 1, NextSyncTime: &future}, + generation: 1, + want: false, + }, + { + name: "schedule reached", + current: helmv1alpha1.HelmClusterAddonRepositoryStatus{ObservedGeneration: 1, NextSyncTime: &past}, + generation: 1, + want: true, + }, + { + name: "forced beats the schedule", + current: helmv1alpha1.HelmClusterAddonRepositoryStatus{ObservedGeneration: 1, NextSyncTime: &future}, + generation: 1, + forced: true, + want: true, + }, + { + name: "spec change beats the schedule", + current: helmv1alpha1.HelmClusterAddonRepositoryStatus{ObservedGeneration: 1, NextSyncTime: &future}, + generation: 2, + want: true, + }, + { + name: "schedule never set on a matching generation", + current: helmv1alpha1.HelmClusterAddonRepositoryStatus{ObservedGeneration: 1, NextSyncTime: nil}, + generation: 1, + want: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := ShouldAttempt(tc.current, tc.generation, testNow, tc.forced); got != tc.want { + t.Fatalf("ShouldAttempt = %v, want %v", got, tc.want) + } + }) + } +} diff --git a/images/operator-helm-controller/internal/services/base.go b/images/operator-helm-controller/internal/services/base.go index 3e366f23..92a22448 100644 --- a/images/operator-helm-controller/internal/services/base.go +++ b/images/operator-helm-controller/internal/services/base.go @@ -65,6 +65,41 @@ type BaseRepoService struct { TargetNamespace string } +// EnsureSecrets reconciles every auxiliary secret the repository needs. Its +// success is the gate for attempting a catalog synchronization: without +// credentials there is nothing to try. +// +// The auth secret's shape depends on the repository kind and the two are not +// interchangeable: HelmRepository resolves HTTP basic auth from an Opaque +// secret, while OCIRepository accepts only a kubernetes.io/dockerconfigjson +// one. An unknown type is treated as helm, mirroring the deletion path — it is +// unreachable here anyway, because an unparsable url is reported before any +// secret is touched. +func (s *BaseRepoService) EnsureSecrets( + ctx context.Context, + repo *helmv1alpha1.HelmClusterAddonRepository, + repoType utils.InternalRepositoryType, +) error { + var err error + + switch repoType { + case utils.InternalOCIRepository: + err = s.reconcileDockerConfigAuthSecret(ctx, repo) + default: + err = s.reconcileBasicAuthSecret(ctx, repo) + } + + if err != nil { + return fmt.Errorf("reconciling auth secret: %w", err) + } + + if err := s.reconcileTLSSecret(ctx, repo); err != nil { + return fmt.Errorf("reconciling tls secret: %w", err) + } + + return nil +} + // reconcileBasicAuthSecret reconciles the internal auth secret as an Opaque secret // holding username/password keys, the shape HelmRepository expects for HTTP basic // auth. diff --git a/images/operator-helm-controller/internal/services/base_test.go b/images/operator-helm-controller/internal/services/base_test.go new file mode 100644 index 00000000..316f6a1c --- /dev/null +++ b/images/operator-helm-controller/internal/services/base_test.go @@ -0,0 +1,157 @@ +/* +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" + "strings" + "testing" + + 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" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "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" +) + +const testNamespace = "d8-operator-helm" + +func testScheme(t *testing.T) *runtime.Scheme { + t.Helper() + + scheme := runtime.NewScheme() + if err := clientgoscheme.AddToScheme(scheme); err != nil { + t.Fatalf("registering client-go scheme: %v", err) + } + if err := helmv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("registering helm scheme: %v", err) + } + + return scheme +} + +func newBaseRepoService(t *testing.T, objects ...client.Object) (*BaseRepoService, client.Client) { + t.Helper() + + scheme := testScheme(t) + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() + + return &BaseRepoService{ + BaseService: BaseService{Client: c, Scheme: scheme}, + TargetNamespace: testNamespace, + }, c +} + +func TestEnsureSecretsCreatesAuthAndTLS(t *testing.T) { + repo := &helmv1alpha1.HelmClusterAddonRepository{ + ObjectMeta: metav1.ObjectMeta{Name: "example"}, + Spec: helmv1alpha1.HelmClusterAddonRepositorySpec{ + URL: "https://example.invalid/charts", + Auth: &helmv1alpha1.HelmClusterAddonRepositoryAuth{Username: "user", Password: "secret"}, + CACertificate: "-----BEGIN CERTIFICATE-----", + }, + } + + service, c := newBaseRepoService(t, repo) + + if err := service.EnsureSecrets(context.Background(), repo, utils.InternalHelmRepository); err != nil { + t.Fatalf("EnsureSecrets returned %v", err) + } + + auth := &corev1.Secret{} + authKey := types.NamespacedName{Name: utils.GetInternalRepositoryAuthSecretName(repo.Name), Namespace: testNamespace} + if err := c.Get(context.Background(), authKey, auth); err != nil { + t.Fatalf("auth secret was not created: %v", err) + } + // The fake client stores what the controller wrote: unlike the API server it + // does not fold StringData into Data. + if got := auth.StringData["username"]; got != "user" { + t.Fatalf("auth secret username is %q, want %q", got, "user") + } + + tls := &corev1.Secret{} + tlsKey := types.NamespacedName{Name: utils.GetInternalRepositoryTLSSecretName(repo.Name), Namespace: testNamespace} + if err := c.Get(context.Background(), tlsKey, tls); err != nil { + t.Fatalf("tls secret was not created: %v", err) + } +} + +func TestEnsureSecretsRemovesObsoleteSecrets(t *testing.T) { + repo := &helmv1alpha1.HelmClusterAddonRepository{ + ObjectMeta: metav1.ObjectMeta{Name: "example"}, + Spec: helmv1alpha1.HelmClusterAddonRepositorySpec{URL: "https://example.invalid/charts"}, + } + obsolete := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: utils.GetInternalRepositoryAuthSecretName(repo.Name), + Namespace: testNamespace, + }, + } + + service, c := newBaseRepoService(t, repo, obsolete) + + if err := service.EnsureSecrets(context.Background(), repo, utils.InternalHelmRepository); err != nil { + t.Fatalf("EnsureSecrets returned %v", err) + } + + err := c.Get(context.Background(), client.ObjectKeyFromObject(obsolete), &corev1.Secret{}) + if !apierrors.IsNotFound(err) { + t.Fatalf("obsolete auth secret must be deleted, got %v", err) + } +} + +func TestEnsureSecretsUsesDockerConfigForOCIRepositories(t *testing.T) { + repo := &helmv1alpha1.HelmClusterAddonRepository{ + ObjectMeta: metav1.ObjectMeta{Name: "example"}, + Spec: helmv1alpha1.HelmClusterAddonRepositorySpec{ + URL: "oci://ghcr.io/example/podinfo", + Auth: &helmv1alpha1.HelmClusterAddonRepositoryAuth{Username: "user", Password: "secret"}, + }, + } + + service, c := newBaseRepoService(t, repo) + + if err := service.EnsureSecrets(context.Background(), repo, utils.InternalOCIRepository); err != nil { + t.Fatalf("EnsureSecrets returned %v", err) + } + + auth := &corev1.Secret{} + key := types.NamespacedName{Name: utils.GetInternalRepositoryAuthSecretName(repo.Name), Namespace: testNamespace} + if err := c.Get(context.Background(), key, auth); err != nil { + t.Fatalf("auth secret was not created: %v", err) + } + + // OCIRepository resolves credentials only from a dockerconfigjson secret; + // an Opaque username/password pair is silently ignored by the source controller. + if auth.Type != corev1.SecretTypeDockerConfigJson { + t.Fatalf("auth secret type is %q, want %q", auth.Type, corev1.SecretTypeDockerConfigJson) + } + + config, found := auth.StringData[corev1.DockerConfigJsonKey] + if !found { + t.Fatalf("auth secret has no %q key, got keys %v", corev1.DockerConfigJsonKey, auth.StringData) + } + if !strings.Contains(config, "ghcr.io") { + t.Fatalf("docker config does not mention the registry host: %s", config) + } +} diff --git a/images/operator-helm-controller/internal/services/helm_repo_service.go b/images/operator-helm-controller/internal/services/helm_repo_service.go index daed8d42..64f89763 100644 --- a/images/operator-helm-controller/internal/services/helm_repo_service.go +++ b/images/operator-helm-controller/internal/services/helm_repo_service.go @@ -24,6 +24,7 @@ import ( "github.com/werf/3p-fluxcd-pkg/apis/meta" sourcev1 "github.com/werf/nelm-source-controller/api/v1" corev1 "k8s.io/api/core/v1" + apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -36,10 +37,7 @@ import ( "github.com/deckhouse/operator-helm/internal/utils" ) -const ( - InternalRepositoryInterval = 5 * time.Minute - ChartsSyncInterval = 5 * time.Minute -) +const InternalRepositoryInterval = 5 * time.Minute type HelmRepoService struct { BaseRepoService @@ -57,35 +55,16 @@ func NewHelmRepoService(client client.Client, scheme *runtime.Scheme, namespace } } -var _ status.Provider = (*HelmRepoResult)(nil) - -type HelmRepoResult struct { - Status status.Status -} - -func (r HelmRepoResult) GetStatus() status.Status { - return r.Status -} - -func (r HelmRepoResult) IsReady() bool { - return r.Status.IsReady() -} - -func (r HelmRepoResult) GetConditionType() string { - return helmv1alpha1.ConditionTypeReady -} - -func (s *HelmRepoService) EnsureInternalHelmRepository(ctx context.Context, repo *helmv1alpha1.HelmClusterAddonRepository) HelmRepoResult { +// EnsureInternalHelmRepository reconciles the internal HelmRepository and +// reports its observed state. The returned error is an API failure that the +// caller must surface to the work queue; an unhealthy internal object is not an +// error and is reported through the state instead. +func (s *HelmRepoService) EnsureInternalHelmRepository( + ctx context.Context, + repo *helmv1alpha1.HelmClusterAddonRepository, +) (InternalRepositoryState, error) { logger := log.FromContext(ctx) - if err := s.reconcileBasicAuthSecret(ctx, repo); err != nil { - return HelmRepoResult{Status: status.Failed(repo, helmv1alpha1.ReasonFailed, "Failed to reconcile auth secret", err)} - } - - if err := s.reconcileTLSSecret(ctx, repo); err != nil { - return HelmRepoResult{Status: status.Failed(repo, helmv1alpha1.ReasonFailed, "Failed to reconcile tls secret", err)} - } - existing := &sourcev1.HelmRepository{ ObjectMeta: metav1.ObjectMeta{ Name: utils.GetInternalHelmRepositoryName(repo.Name), @@ -99,31 +78,37 @@ func (s *HelmRepoService) EnsureInternalHelmRepository(ctx context.Context, repo return nil }) if err != nil { - return HelmRepoResult{ - Status: status.Failed( - repo, - helmv1alpha1.ReasonFailed, - "Failed to reconcile helm repository", - fmt.Errorf("creating helm repository: %w", err), - ), - } + return InternalRepositoryState{Present: true}, fmt.Errorf("creating helm repository: %w", err) } if op != controllerutil.OperationResultNone { logger.Info("Reconciled helm repository", "operation", op) } - if cond, ok := status.IsConditionObserved(existing.Status.Conditions, helmv1alpha1.ConditionTypeReady, existing.Generation); ok { - return HelmRepoResult{Status: status.Status{ - Observed: ok, - Status: cond.Status, - ObservedGeneration: repo.Generation, - Reason: cond.Reason, - Message: cond.Message, - }} + state := InternalRepositoryState{Present: true} + + if stalled := apimeta.FindStatusCondition(existing.Status.Conditions, helmv1alpha1.ConditionTypeStalled); stalled != nil && + stalled.Status == metav1.ConditionTrue { + state.Stalled = true + state.Reason = stalled.Reason + state.Message = stalled.Message + + return state, nil + } + + cond, observed := status.IsConditionObserved(existing.Status.Conditions, helmv1alpha1.ConditionTypeReady, existing.Generation) + if !observed { + state.Reason = helmv1alpha1.ReasonReconciling + state.Message = "Waiting for the internal repository to be reconciled" + + return state, nil } - return HelmRepoResult{Status: status.Unknown(repo, helmv1alpha1.ReasonReconciling)} + state.Ready = cond.Status == metav1.ConditionTrue + state.Reason = cond.Reason + state.Message = cond.Message + + return state, nil } func (s *HelmRepoService) RemoveHelmRepository(ctx context.Context, repoName string) error { diff --git a/images/operator-helm-controller/internal/services/helm_repo_service_test.go b/images/operator-helm-controller/internal/services/helm_repo_service_test.go new file mode 100644 index 00000000..6fb33b2c --- /dev/null +++ b/images/operator-helm-controller/internal/services/helm_repo_service_test.go @@ -0,0 +1,264 @@ +/* +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" + "errors" + "testing" + + sourcev1 "github.com/werf/nelm-source-controller/api/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" + "github.com/deckhouse/operator-helm/internal/utils" +) + +func sourceScheme(t *testing.T) *runtime.Scheme { + t.Helper() + + scheme := runtime.NewScheme() + if err := clientgoscheme.AddToScheme(scheme); err != nil { + t.Fatalf("registering client-go scheme: %v", err) + } + if err := helmv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("registering helm scheme: %v", err) + } + if err := sourcev1.AddToScheme(scheme); err != nil { + t.Fatalf("registering source scheme: %v", err) + } + + return scheme +} + +func newHelmRepoService(t *testing.T, objects ...client.Object) *HelmRepoService { + t.Helper() + + scheme := sourceScheme(t) + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() + + return NewHelmRepoService(c, scheme, testNamespace) +} + +func testRepository() *helmv1alpha1.HelmClusterAddonRepository { + return &helmv1alpha1.HelmClusterAddonRepository{ + ObjectMeta: metav1.ObjectMeta{Name: "example", Generation: 1}, + Spec: helmv1alpha1.HelmClusterAddonRepositorySpec{URL: "https://example.invalid/charts"}, + } +} + +func TestEnsureInternalHelmRepositoryReportsNotObservedAsNotReady(t *testing.T) { + repo := testRepository() + service := newHelmRepoService(t, repo) + + state, err := service.EnsureInternalHelmRepository(context.Background(), repo) + if err != nil { + t.Fatalf("EnsureInternalHelmRepository returned %v", err) + } + if !state.Present { + t.Fatal("helm repositories must report an internal object") + } + if state.Ready { + t.Fatal("a freshly created internal object must not be reported ready") + } +} + +func TestEnsureInternalHelmRepositoryMirrorsConditions(t *testing.T) { + repo := testRepository() + // The spec and labels must already match what applyHelmRepositorySpec writes: + // otherwise CreateOrPatch mutates the object, the fake client bumps its + // generation, and the Ready condition stops counting as observed. + internal := &sourcev1.HelmRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: utils.GetInternalHelmRepositoryName(repo.Name), + Namespace: testNamespace, + Labels: map[string]string{ + helmv1alpha1.LabelManagedBy: helmv1alpha1.LabelManagedByValue, + helmv1alpha1.HelmClusterAddonRepositoryLabelSourceName: repo.Name, + }, + }, + Spec: sourcev1.HelmRepositorySpec{ + URL: repo.Spec.URL, + Interval: metav1.Duration{Duration: InternalRepositoryInterval}, + }, + Status: sourcev1.HelmRepositoryStatus{ + Conditions: []metav1.Condition{ + { + Type: helmv1alpha1.ConditionTypeReady, + Status: metav1.ConditionFalse, + Reason: "FetchFailed", + Message: "failed to fetch index", + LastTransitionTime: metav1.Now(), + }, + }, + }, + } + + service := newHelmRepoService(t, repo, internal) + + // The fixture is created with generation 0 and the condition observes 0, so + // the state must mirror the condition rather than report "not observed yet". + + state, err := service.EnsureInternalHelmRepository(context.Background(), repo) + if err != nil { + t.Fatalf("EnsureInternalHelmRepository returned %v", err) + } + if state.Ready { + t.Fatal("state must mirror Ready=False from the internal object") + } + if state.Reason != "FetchFailed" || state.Message != "failed to fetch index" { + t.Fatalf("state must translate reason and message, got %q / %q", state.Reason, state.Message) + } +} + +func TestEnsureInternalHelmRepositoryReportsStalled(t *testing.T) { + repo := testRepository() + internal := &sourcev1.HelmRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: utils.GetInternalHelmRepositoryName(repo.Name), + Namespace: testNamespace, + Generation: 1, + }, + Status: sourcev1.HelmRepositoryStatus{ + Conditions: []metav1.Condition{ + { + Type: helmv1alpha1.ConditionTypeStalled, + Status: metav1.ConditionTrue, + Reason: "InvalidSecretRef", + Message: "secret not found", + ObservedGeneration: 1, + LastTransitionTime: metav1.Now(), + }, + }, + }, + } + + service := newHelmRepoService(t, repo, internal) + + state, err := service.EnsureInternalHelmRepository(context.Background(), repo) + if err != nil { + t.Fatalf("EnsureInternalHelmRepository returned %v", err) + } + if !state.Stalled { + t.Fatal("state must report Stalled from the internal object") + } + if state.Reason != "InvalidSecretRef" { + t.Fatalf("state reason is %q, want %q", state.Reason, "InvalidSecretRef") + } +} + +// TestEnsureInternalHelmRepositoryStalledPrecedesReady pins the precedence rule: +// Stalled=True must win even when a healthy, observed Ready=True condition sits +// right next to it. The fixture's spec and labels already match what +// applyHelmRepositorySpec writes (same reason as the mirroring test above), so +// CreateOrPatch is a no-op and the internal object's generation stays at 1 - +// which is what lets the Ready condition below count as observed. +func TestEnsureInternalHelmRepositoryStalledPrecedesReady(t *testing.T) { + repo := testRepository() + internal := &sourcev1.HelmRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: utils.GetInternalHelmRepositoryName(repo.Name), + Namespace: testNamespace, + Generation: 1, + Labels: map[string]string{ + helmv1alpha1.LabelManagedBy: helmv1alpha1.LabelManagedByValue, + helmv1alpha1.HelmClusterAddonRepositoryLabelSourceName: repo.Name, + }, + }, + Spec: sourcev1.HelmRepositorySpec{ + URL: repo.Spec.URL, + Interval: metav1.Duration{Duration: InternalRepositoryInterval}, + }, + Status: sourcev1.HelmRepositoryStatus{ + Conditions: []metav1.Condition{ + { + Type: helmv1alpha1.ConditionTypeReady, + Status: metav1.ConditionTrue, + Reason: "Succeeded", + Message: "index fetched", + ObservedGeneration: 1, + LastTransitionTime: metav1.Now(), + }, + { + Type: helmv1alpha1.ConditionTypeStalled, + Status: metav1.ConditionTrue, + Reason: "InvalidSecretRef", + Message: "secret not found", + ObservedGeneration: 1, + LastTransitionTime: metav1.Now(), + }, + }, + }, + } + + service := newHelmRepoService(t, repo, internal) + + state, err := service.EnsureInternalHelmRepository(context.Background(), repo) + if err != nil { + t.Fatalf("EnsureInternalHelmRepository returned %v", err) + } + if !state.Stalled { + t.Fatal("Stalled=True must take precedence even when an observed Ready=True is also present") + } + if state.Ready { + t.Fatal("state must not report Ready when Stalled=True takes precedence") + } + if state.Reason != "InvalidSecretRef" { + t.Fatalf("state reason is %q, want the Stalled reason %q, not the Ready reason", state.Reason, "InvalidSecretRef") + } +} + +// TestEnsureInternalHelmRepositoryReturnsAPIError verifies the split this task +// exists to create: an API failure while reconciling the internal object is the +// caller's problem and comes back as a non-nil error (still with Present: true, +// since the internal object does exist as far as the caller is concerned), not +// swallowed into the state. The failure is injected on Create because the fixture +// has no pre-existing internal HelmRepository, so CreateOrPatch's Get finds +// nothing and falls through to Create. +func TestEnsureInternalHelmRepositoryReturnsAPIError(t *testing.T) { + repo := testRepository() + scheme := sourceScheme(t) + + sentinel := errors.New("synthetic create failure") + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(repo). + WithInterceptorFuncs(interceptor.Funcs{ + Create: func(ctx context.Context, _ client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + return sentinel + }, + }). + Build() + + service := NewHelmRepoService(c, scheme, testNamespace) + + state, err := service.EnsureInternalHelmRepository(context.Background(), repo) + if err == nil { + t.Fatal("EnsureInternalHelmRepository must return an error when the API call fails") + } + if !errors.Is(err, sentinel) { + t.Fatalf("returned error must wrap the underlying API failure, got %v", err) + } + if !state.Present { + t.Fatal("state must still report Present: true even when reconciling failed") + } +} 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 61a45e59..ce6ccbb6 100644 --- a/images/operator-helm-controller/internal/services/oci_repo_service.go +++ b/images/operator-helm-controller/internal/services/oci_repo_service.go @@ -123,47 +123,6 @@ func (s *OCIRepoService) EnsureInternalOCIRepository(ctx context.Context, addon } } -func (s *OCIRepoService) EnsureRepositorySecrets(ctx context.Context, repo *helmv1alpha1.HelmClusterAddonRepository) OCIRepoResult { - if err := s.reconcileDockerConfigAuthSecret(ctx, repo); err != nil { - return OCIRepoResult{ - Status: status.Status{ - ConditionType: helmv1alpha1.ConditionTypeReady, - Observed: true, - Status: metav1.ConditionFalse, - ObservedGeneration: repo.Generation, - Reason: helmv1alpha1.ReasonFailed, - Message: "Failed to reconcile auth secret", - Err: err, - }, - } - } - - if err := s.reconcileTLSSecret(ctx, repo); err != nil { - return OCIRepoResult{ - Status: status.Status{ - ConditionType: helmv1alpha1.ConditionTypeReady, - Observed: true, - Status: metav1.ConditionFalse, - ObservedGeneration: repo.Generation, - Reason: helmv1alpha1.ReasonFailed, - Message: "Failed to reconcile tls secret", - Err: err, - }, - } - } - - return OCIRepoResult{ - Artifact: &meta.Artifact{}, - Status: status.Status{ - ConditionType: helmv1alpha1.ConditionTypeReady, - Observed: true, - Status: metav1.ConditionTrue, - ObservedGeneration: repo.Generation, - Reason: helmv1alpha1.ReasonSuccess, - }, - } -} - func (s *OCIRepoService) CleanupOCIRepository(ctx context.Context, repoName string) error { resources := []struct { name string diff --git a/images/operator-helm-controller/internal/services/outcomes.go b/images/operator-helm-controller/internal/services/outcomes.go new file mode 100644 index 00000000..ca6477da --- /dev/null +++ b/images/operator-helm-controller/internal/services/outcomes.go @@ -0,0 +1,56 @@ +/* +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 + +// InternalRepositoryState describes the observed state of the internal FluxCD +// repository object. Present is false for OCI repositories: they have no +// internal object at the repository level, each addon creates its own. +type InternalRepositoryState struct { + Present bool + Ready bool + Stalled bool + Reason string + Message string +} + +// FetchOutcome is the result of reading the chart catalog from the remote +// repository. Terminal marks a failure that will not resolve by retrying. +type FetchOutcome struct { + Err error + Terminal bool + Reason string + Message string +} + +// CatalogOutcome is the result of writing the chart catalog into the cluster. +type CatalogOutcome struct { + Err error +} + +// ConfigOutcome is a terminal configuration failure detected before any attempt +// to reach the repository, such as an unsupported url scheme. +type ConfigOutcome struct { + Reason string + Message string + Err error +} + +// SyncOutcome carries both phases of a synchronization attempt. +type SyncOutcome struct { + Fetch FetchOutcome + Catalog CatalogOutcome +} diff --git a/images/operator-helm-controller/internal/services/repo_sync_service.go b/images/operator-helm-controller/internal/services/repo_sync_service.go index af552114..4ccf9744 100644 --- a/images/operator-helm-controller/internal/services/repo_sync_service.go +++ b/images/operator-helm-controller/internal/services/repo_sync_service.go @@ -19,9 +19,8 @@ package services import ( "context" "fmt" - "time" - apimeta "k8s.io/apimachinery/pkg/api/meta" + "github.com/samber/lo" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -32,9 +31,7 @@ import ( helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" repoclient "github.com/deckhouse/operator-helm/internal/client/repository" - "github.com/deckhouse/operator-helm/internal/manager/status" "github.com/deckhouse/operator-helm/internal/utils" - "github.com/samber/lo" ) const ( @@ -48,89 +45,104 @@ const ( type RepoSyncService struct { BaseService + + clientFactory RepoClientFactory } -func NewRepoSyncService(client client.Client, scheme *runtime.Scheme) *RepoSyncService { +// RepoClientFactory builds the client used to read a repository catalog. It is +// injected so the synchronization can be tested without a live repository. +type RepoClientFactory func(repoType utils.InternalRepositoryType) (repoclient.ClientInterface, error) + +func NewRepoSyncService(client client.Client, scheme *runtime.Scheme, factory RepoClientFactory) *RepoSyncService { + if factory == nil { + factory = repoclient.NewClient + } + return &RepoSyncService{ BaseService: BaseService{ Client: client, Scheme: scheme, }, + clientFactory: factory, } } -var _ status.Provider = (*RepoSyncResult)(nil) +// Sync reads the repository catalog and reconciles the HelmClusterAddonChart +// resources that mirror it. The two phases are reported separately: a fetch +// failure is about the remote, a catalog failure is about this cluster. +func (s *RepoSyncService) Sync( + ctx context.Context, + repo *helmv1alpha1.HelmClusterAddonRepository, + repoType utils.InternalRepositoryType, +) SyncOutcome { + charts, fetch := s.fetchCharts(ctx, repo, repoType) + if fetch.Err != nil { + return SyncOutcome{Fetch: fetch} + } -type RepoSyncResult struct { - Status status.Status + return SyncOutcome{Fetch: fetch, Catalog: s.reconcileCatalog(ctx, repo, charts)} } -func (r RepoSyncResult) GetStatus() status.Status { - return r.Status -} +func (s *RepoSyncService) fetchCharts( + ctx context.Context, + repo *helmv1alpha1.HelmClusterAddonRepository, + repoType utils.InternalRepositoryType, +) ([]repoclient.Chart, FetchOutcome) { + repoClient, err := s.clientFactory(repoType) + if err != nil { + return nil, FetchOutcome{ + Err: err, + Terminal: true, + Reason: helmv1alpha1.ReasonUnsupportedRepositoryType, + Message: "Unsupported repository type", + } + } -func (r RepoSyncResult) IsReady() bool { - return r.Status.IsReady() -} + charts, err := repoClient.FetchCharts(ctx, repo.Spec.URL, buildRepoConfig(repo)) + if err == nil { + return charts, FetchOutcome{} + } -func (r RepoSyncResult) GetConditionType() string { - return helmv1alpha1.ConditionTypeSynced -} + if terminal, ok := repoclient.AsTerminal(err); ok { + return nil, FetchOutcome{ + Err: err, + Terminal: true, + Reason: terminal.Reason, + Message: terminal.Message, + } + } -// InProgress reports the first phase of the sync state machine: the Synced -// condition has just been marked Reconciling and the actual chart fetch still -// needs to run. The caller performs that fetch in the same reconcile so progress -// does not depend on a status-update watch event. -func (r RepoSyncResult) InProgress() bool { - return r.Status.Status == metav1.ConditionUnknown && r.Status.Reason == helmv1alpha1.ReasonReconciling + return nil, FetchOutcome{ + Err: err, + Reason: helmv1alpha1.ReasonSyncFailed, + Message: "Failed to read the repository catalog: " + err.Error(), + } } -func (s *RepoSyncService) EnsureAddonCharts(ctx context.Context, repo *helmv1alpha1.HelmClusterAddonRepository, repoType utils.InternalRepositoryType) RepoSyncResult { - logger := log.FromContext(ctx) - - if !isRepoSyncRequired(repo) { - return RepoSyncResult{Status: status.Empty()} - } else if !isRepoSyncInProgress(repo) { - return RepoSyncResult{Status: status.Unknown(repo, helmv1alpha1.ReasonReconciling)} +func buildRepoConfig(repo *helmv1alpha1.HelmClusterAddonRepository) *repoclient.RepoConfig { + if repo.Spec.Auth == nil && repo.Spec.CACertificate == "" && !repo.Spec.InsecureSkipVerify { + return nil } - repoClient, err := repoclient.NewClient(repoType) - if err != nil { - return RepoSyncResult{ - Status: status.Failed( - repo, - helmv1alpha1.ReasonSyncFailed, - "Failed to get repository client on chart sync", - fmt.Errorf("getting repository client: %w", err), - ), - } + config := &repoclient.RepoConfig{ + Insecure: repo.Spec.InsecureSkipVerify, + CACertificate: repo.Spec.CACertificate, } - var repoConfig *repoclient.RepoConfig - if repo.Spec.Auth != nil || repo.Spec.CACertificate != "" || repo.Spec.InsecureSkipVerify { - repoConfig = &repoclient.RepoConfig{ - Insecure: repo.Spec.InsecureSkipVerify, - } - if repo.Spec.Auth != nil { - repoConfig.Username = repo.Spec.Auth.Username - repoConfig.Password = repo.Spec.Auth.Password - } - if repo.Spec.CACertificate != "" { - repoConfig.CACertificate = repo.Spec.CACertificate - } + if repo.Spec.Auth != nil { + config.Username = repo.Spec.Auth.Username + config.Password = repo.Spec.Auth.Password } - charts, err := repoClient.FetchCharts(ctx, repo.Spec.URL, repoConfig) - if err != nil { - return RepoSyncResult{ - Status: status.Failed( - repo, - helmv1alpha1.ReasonSyncFailed, - "Failed to fetch charts from repository", - fmt.Errorf("fetching charts: %w", err), - ), - } - } + return config +} + +func (s *RepoSyncService) reconcileCatalog( + ctx context.Context, + repo *helmv1alpha1.HelmClusterAddonRepository, + charts []repoclient.Chart, +) CatalogOutcome { + logger := log.FromContext(ctx) desiredCharts := make(map[string]struct{}, len(charts)) @@ -141,9 +153,7 @@ func (s *RepoSyncService) EnsureAddonCharts(ctx context.Context, repo *helmv1alp addonChartName := utils.GetHelmClusterAddonChartName(repo.Name, chart.Name) existing := &helmv1alpha1.HelmClusterAddonChart{ - ObjectMeta: metav1.ObjectMeta{ - Name: addonChartName, - }, + ObjectMeta: metav1.ObjectMeta{Name: addonChartName}, } desiredCharts[existing.Name] = struct{}{} @@ -164,17 +174,11 @@ func (s *RepoSyncService) EnsureAddonCharts(ctx context.Context, repo *helmv1alp LabelRepositoryName: repo.Name, LabelChartName: chart.Name, } + return nil }) if err != nil { - return RepoSyncResult{ - Status: status.Failed( - repo, - helmv1alpha1.ReasonSyncFailed, - fmt.Sprintf("Failed to create HelmClusterAddonChart %q", addonChartName), - fmt.Errorf("cannot create or update HelmClusterAddonChart: %w", err), - ), - } + return CatalogOutcome{Err: fmt.Errorf("creating or updating chart %q: %w", addonChartName, err)} } if op != controllerutil.OperationResultNone { @@ -189,29 +193,13 @@ func (s *RepoSyncService) EnsureAddonCharts(ctx context.Context, repo *helmv1alp }) if err := s.Client.Status().Patch(ctx, existing, client.MergeFrom(base)); err != nil { - return RepoSyncResult{ - Status: status.Failed( - repo, - helmv1alpha1.ReasonSyncFailed, - fmt.Sprintf("Failed to update HelmClusterAddonChart %q versions", addonChartName), - fmt.Errorf("updating chart versions: %w", err), - ), - } + return CatalogOutcome{Err: fmt.Errorf("updating versions of chart %q: %w", addonChartName, err)} } - - logger.Info("Successfully synced HelmClusterAddonChart versions", "operation", op, "addonChartName", addonChartName) } var existingCharts helmv1alpha1.HelmClusterAddonChartList if err := s.Client.List(ctx, &existingCharts, client.MatchingLabels{LabelRepositoryName: repo.Name}); err != nil { - return RepoSyncResult{ - Status: status.Failed( - repo, - helmv1alpha1.ReasonSyncFailed, - "Failed to list stale charts for pruning", - fmt.Errorf("listing existing charts for pruning: %w", err), - ), - } + return CatalogOutcome{Err: fmt.Errorf("listing charts for pruning: %w", err)} } for _, chart := range existingCharts.Items { @@ -220,41 +208,9 @@ func (s *RepoSyncService) EnsureAddonCharts(ctx context.Context, repo *helmv1alp } if err := s.ensureResourceDeleted(ctx, types.NamespacedName{Name: chart.Name}, &chart); err != nil { - return RepoSyncResult{ - Status: status.Failed( - repo, - helmv1alpha1.ReasonSyncFailed, - "Failed to delete stale charts", - fmt.Errorf("deleting stale charts: %w", err), - ), - } + return CatalogOutcome{Err: fmt.Errorf("deleting stale charts: %w", err)} } } - logger.Info(fmt.Sprintf("Scheduling next repo sync in %s", ChartsSyncInterval)) - - return RepoSyncResult{ - Status: status.Success(repo), - } -} - -func isRepoSyncRequired(repo *helmv1alpha1.HelmClusterAddonRepository) bool { - if repo.ForceReconcileRequired() { - return true - } - - syncCond := apimeta.FindStatusCondition(repo.Status.Conditions, helmv1alpha1.ConditionTypeSynced) - if syncCond != nil && syncCond.Status == metav1.ConditionTrue && syncCond.LastTransitionTime.UTC().Add(ChartsSyncInterval).After(time.Now().UTC()) { - return false - } - return true -} - -func isRepoSyncInProgress(repo *helmv1alpha1.HelmClusterAddonRepository) bool { - syncCond := apimeta.FindStatusCondition(repo.Status.Conditions, helmv1alpha1.ConditionTypeSynced) - if syncCond != nil && syncCond.Status == metav1.ConditionUnknown && syncCond.Reason == helmv1alpha1.ReasonReconciling { - return true - } - - return false + return CatalogOutcome{} } diff --git a/images/operator-helm-controller/internal/services/repo_sync_service_test.go b/images/operator-helm-controller/internal/services/repo_sync_service_test.go new file mode 100644 index 00000000..fcbb27e8 --- /dev/null +++ b/images/operator-helm-controller/internal/services/repo_sync_service_test.go @@ -0,0 +1,146 @@ +/* +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" + "errors" + "testing" + + "github.com/Masterminds/semver/v3" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + helmv1alpha1 "github.com/deckhouse/operator-helm/api/v1alpha1" + repoclient "github.com/deckhouse/operator-helm/internal/client/repository" + "github.com/deckhouse/operator-helm/internal/utils" +) + +type stubRepoClient struct { + charts []repoclient.Chart + err error +} + +func (s stubRepoClient) FetchCharts(_ context.Context, _ string, _ *repoclient.RepoConfig) ([]repoclient.Chart, error) { + return s.charts, s.err +} + +func newRepoSyncService(t *testing.T, stub stubRepoClient, objects ...client.Object) (*RepoSyncService, client.Client) { + t.Helper() + + scheme := testScheme(t) + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objects...). + WithStatusSubresource(&helmv1alpha1.HelmClusterAddonChart{}, &helmv1alpha1.HelmClusterAddonRepository{}). + Build() + + factory := func(_ utils.InternalRepositoryType) (repoclient.ClientInterface, error) { + return stub, nil + } + + return NewRepoSyncService(c, scheme, factory), c +} + +func chartFixture(name, version string) repoclient.Chart { + return repoclient.Chart{ + Name: name, + Versions: []repoclient.ChartVersion{{Version: semver.MustParse(version), IconURL: "https://example.invalid/icon.png"}}, + } +} + +func TestSyncCreatesChartsAndRecordsVersions(t *testing.T) { + repo := testRepository() + service, c := newRepoSyncService(t, stubRepoClient{charts: []repoclient.Chart{chartFixture("podinfo", "6.7.1")}}, repo) + + outcome := service.Sync(context.Background(), repo, utils.InternalHelmRepository) + if outcome.Fetch.Err != nil { + t.Fatalf("fetch failed: %v", outcome.Fetch.Err) + } + if outcome.Catalog.Err != nil { + t.Fatalf("catalog update failed: %v", outcome.Catalog.Err) + } + + chart := &helmv1alpha1.HelmClusterAddonChart{} + key := client.ObjectKey{Name: utils.GetHelmClusterAddonChartName(repo.Name, "podinfo")} + if err := c.Get(context.Background(), key, chart); err != nil { + t.Fatalf("chart was not created: %v", err) + } + if len(chart.Status.Versions) != 1 || chart.Status.Versions[0].Version != "6.7.1" { + t.Fatalf("chart versions are %v, want [6.7.1]", chart.Status.Versions) + } +} + +func TestSyncPrunesStaleCharts(t *testing.T) { + repo := testRepository() + stale := &helmv1alpha1.HelmClusterAddonChart{ + ObjectMeta: metav1.ObjectMeta{ + Name: utils.GetHelmClusterAddonChartName(repo.Name, "removed"), + Labels: map[string]string{LabelRepositoryName: repo.Name, LabelChartName: "removed"}, + }, + } + + service, c := newRepoSyncService(t, stubRepoClient{charts: []repoclient.Chart{chartFixture("podinfo", "6.7.1")}}, repo, stale) + + outcome := service.Sync(context.Background(), repo, utils.InternalHelmRepository) + if outcome.Catalog.Err != nil { + t.Fatalf("catalog update failed: %v", outcome.Catalog.Err) + } + + err := c.Get(context.Background(), client.ObjectKeyFromObject(stale), &helmv1alpha1.HelmClusterAddonChart{}) + if err == nil { + t.Fatal("stale chart must be pruned") + } +} + +func TestSyncReportsTerminalFetchFailure(t *testing.T) { + repo := testRepository() + terminal := &repoclient.TerminalError{ + Reason: helmv1alpha1.ReasonAuthenticationFailed, + Message: "repository rejected the credentials (HTTP 401)", + } + + service, _ := newRepoSyncService(t, stubRepoClient{err: terminal}, repo) + + outcome := service.Sync(context.Background(), repo, utils.InternalHelmRepository) + if outcome.Fetch.Err == nil { + t.Fatal("expected a fetch failure") + } + if !outcome.Fetch.Terminal { + t.Fatal("a TerminalError must be reported as terminal") + } + if outcome.Fetch.Reason != helmv1alpha1.ReasonAuthenticationFailed { + t.Fatalf("fetch reason is %q, want %q", outcome.Fetch.Reason, helmv1alpha1.ReasonAuthenticationFailed) + } +} + +func TestSyncReportsTransientFetchFailure(t *testing.T) { + repo := testRepository() + service, _ := newRepoSyncService(t, stubRepoClient{err: errors.New("connection refused")}, repo) + + outcome := service.Sync(context.Background(), repo, utils.InternalHelmRepository) + if outcome.Fetch.Err == nil { + t.Fatal("expected a fetch failure") + } + if outcome.Fetch.Terminal { + t.Fatal("a plain error must stay retriable") + } + if outcome.Fetch.Reason != helmv1alpha1.ReasonSyncFailed { + t.Fatalf("fetch reason is %q, want %q", outcome.Fetch.Reason, helmv1alpha1.ReasonSyncFailed) + } +} diff --git a/images/operator-helm-controller/internal/webhook/helmclusteraddon/webhook.go b/images/operator-helm-controller/internal/webhook/helmclusteraddon/webhook.go index 5f7a5a2a..ccb605e8 100644 --- a/images/operator-helm-controller/internal/webhook/helmclusteraddon/webhook.go +++ b/images/operator-helm-controller/internal/webhook/helmclusteraddon/webhook.go @@ -127,7 +127,7 @@ func isUniquenessBypassed(ctx context.Context) bool { 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) + return fmt.Errorf("failed to check if helmclusteraddon/%s owns chart claim: %w", addon.Name, err) } if owned { return nil diff --git a/tests/e2e/Taskfile.dist.yaml b/tests/e2e/Taskfile.dist.yaml index af0f7a2b..8c5d9e70 100644 --- a/tests/e2e/Taskfile.dist.yaml +++ b/tests/e2e/Taskfile.dist.yaml @@ -41,5 +41,6 @@ tasks: env: E2E_CLUSTERTRANSPORT_KUBECONFIG: "./kind/{{.KIND_CLUSTER_NAME}}/kubeconfig-external" E2E_MODULE_TAG_NAME: '{{.E2E_MODULE_TAG_NAME | default "main"}}' + E2E_MODULE_DIGEST: "{{.E2E_MODULE_DIGEST}}" E2E_MODULE_SOURCE: '{{.E2E_MODULE_SOURCE | default "operator-helm"}}' DEV_REGISTRY_DOCKER_CONFIG: "{{.DEV_REGISTRY_DOCKER_CONFIG}}" diff --git a/tests/e2e/default_config.yaml b/tests/e2e/default_config.yaml index eaefa374..ae4864fe 100644 --- a/tests/e2e/default_config.yaml +++ b/tests/e2e/default_config.yaml @@ -13,6 +13,9 @@ controllers: - "Failed to get internal repository" # Expected in the negative test that sets an invalid chart version; surfaced via status. - "failed to get desired chart version" + # Expected in the stall scenario that points a repository at a missing + # source; the reconciler logs every failed read with the repository name. + - "repo-source-not-found" # Transient controller-runtime cache reflector reconnects (watch/list drop, # unexpected EOF); self-recovering, not a real failure. - "Unexpected error when reading response body" diff --git a/tests/e2e/helmclusteraddonrepository/lifecycle.go b/tests/e2e/helmclusteraddonrepository/lifecycle.go index 96338772..d13fde19 100644 --- a/tests/e2e/helmclusteraddonrepository/lifecycle.go +++ b/tests/e2e/helmclusteraddonrepository/lifecycle.go @@ -79,6 +79,29 @@ func DefineLifecycleTests(repoType, repoURL string) { created, ) + By("Healthy repository must carry no abnormal-true conditions") + util.UntilConditionAbsent( + apiv1alpha1.ConditionTypeReconciling, + framework.LongTimeout, + created, + ) + util.UntilConditionAbsent( + apiv1alpha1.ConditionTypeStalled, + framework.LongTimeout, + created, + ) + + By("Repository must report its synchronization schedule") + Eventually(func(g Gomega) { + current, err := f.OperatorClient().HelmV1alpha1(). + HelmClusterAddonRepositories(). + Get(context.Background(), repoName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(current.Status.LastSuccessfulSyncTime).NotTo(BeNil()) + g.Expect(current.Status.NextSyncTime).NotTo(BeNil()) + g.Expect(current.Status.ConsecutiveFetchFailures).To(BeZero()) + }).WithTimeout(framework.LongTimeout).WithPolling(framework.PollingInterval).Should(Succeed()) + By("Should have existing HelmClusterAddonChart") labelSelector := fmt.Sprintf("repository=%s", repoName) charts, err := f.OperatorClient(). @@ -134,3 +157,103 @@ var _ = Describe("Create HelmClusterAddonRepository with invalid url", Ordered, Expect(err).To(MatchError(ContainSubstring("is invalid: spec.url"))) }) }) + +var _ = Describe("HelmClusterAddonRepository with an unreachable source", Ordered, func() { + f := framework.NewFramework("repository-lifecycle") + repoName := "repo-source-not-found" + + BeforeAll(func() { + DeferCleanup(f.After) + f.Before() + }) + + // No AssertNoErrorsFor here on purpose: this scenario breaks the repository + // deliberately, so error-level log lines from the controller are expected. + // Dropping the assertion is not enough on its own — the log watcher + // accumulates errors suite-wide and never resets, so every other scenario's + // assertion and the suite-teardown one would fail too. The repository name is + // excluded in default_config.yaml; keep the two in step if it ever changes. + + It("should stall on a missing source and recover after the url is fixed", func() { + repo := &apiv1alpha1.HelmClusterAddonRepository{ + ObjectMeta: metav1.ObjectMeta{Name: repoName}, + Spec: apiv1alpha1.HelmClusterAddonRepositorySpec{ + URL: "https://stefanprodan.github.io/podinfo-does-not-exist", + }, + } + + created, err := f.OperatorClient().HelmV1alpha1(). + HelmClusterAddonRepositories(). + Create(context.Background(), repo, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + f.DeferDelete(created) + + By("Waiting for the repository to become Stalled") + util.UntilConditionTrue( + apiv1alpha1.ConditionTypeStalled, + framework.LongTimeout, + created, + ) + + By("Stalled must name the terminal cause") + util.UntilConditionReason( + apiv1alpha1.ConditionTypeStalled, + apiv1alpha1.ReasonSourceNotFound, + framework.LongTimeout, + created, + ) + + By("Ready must be False while Stalled") + util.UntilConditionStatus( + apiv1alpha1.ConditionTypeReady, + string(metav1.ConditionFalse), + framework.LongTimeout, + created, + ) + + By("Reconciling must be absent while Stalled") + util.UntilConditionAbsent( + apiv1alpha1.ConditionTypeReconciling, + framework.LongTimeout, + created, + ) + + By("Fixing the url") + // The waits below must use the object as it exists AFTER the update: + // correcting the url bumps metadata.generation, and UntilConditionStatus + // compares the condition's observedGeneration against the generation of + // the object handed to it. Reusing `created`, frozen at generation 1 by + // the Create call, would compare 2 against 1 on every poll and fail the + // spec deterministically even though recovery worked. + var recovered *apiv1alpha1.HelmClusterAddonRepository + + Eventually(func(g Gomega) { + current, err := f.OperatorClient().HelmV1alpha1(). + HelmClusterAddonRepositories(). + Get(context.Background(), repoName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + + current.Spec.URL = "https://stefanprodan.github.io/podinfo" + + updated, err := f.OperatorClient().HelmV1alpha1(). + HelmClusterAddonRepositories(). + Update(context.Background(), current, metav1.UpdateOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + + recovered = updated + }).WithTimeout(framework.LongTimeout).WithPolling(framework.PollingInterval).Should(Succeed()) + + By("Waiting for the repository to recover") + util.UntilConditionTrue( + apiv1alpha1.ConditionTypeReady, + framework.LongTimeout, + recovered, + ) + util.UntilConditionAbsent( + apiv1alpha1.ConditionTypeStalled, + framework.LongTimeout, + recovered, + ) + }) +}) diff --git a/tests/e2e/internal/framework/config.go b/tests/e2e/internal/framework/config.go index 9648ab9d..25fd9cbc 100644 --- a/tests/e2e/internal/framework/config.go +++ b/tests/e2e/internal/framework/config.go @@ -76,6 +76,7 @@ type Config struct { ModuleSource string ModuleSourceDockerCfg string ModuleTagName string + ModuleDigest string } type ControllerConfig struct { @@ -147,6 +148,9 @@ func (c *Config) applyEnvOverrides() { if s, ok := os.LookupEnv("E2E_MODULE_TAG_NAME"); ok { c.ModuleTagName = s } + if s, ok := os.LookupEnv("E2E_MODULE_DIGEST"); ok { + c.ModuleDigest = s + } if s, ok := os.LookupEnv("E2E_MODULE_SOURCE"); ok { c.ModuleSource = s } else { diff --git a/tests/e2e/internal/util/moduleconfig.go b/tests/e2e/internal/util/moduleconfig.go index 8693ec4c..dfab6515 100644 --- a/tests/e2e/internal/util/moduleconfig.go +++ b/tests/e2e/internal/util/moduleconfig.go @@ -66,6 +66,15 @@ func EnsureModuleConfig(f *framework.Framework) { c := framework.GetConfig() + // An unknown digest fails the run instead of skipping the check. A skipped + // verification is indistinguishable from a passing one in the output, which is + // exactly how a suite ends up silently exercising whatever an older build left + // behind a mutable tag. + Expect(c.ModuleDigest).NotTo(BeEmpty(), + "E2E_MODULE_DIGEST is not set, so the suite cannot tell which module artifact the cluster will pull. "+ + "In CI the build job supplies it; locally resolve it with "+ + "crane digest dev-registry.deckhouse.io/sys/deckhouse-oss/modules/operator-helm:$E2E_MODULE_TAG_NAME") + moduleSource := &unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "deckhouse.io/v1alpha1", @@ -105,10 +114,6 @@ func EnsureModuleConfig(f *framework.Framework) { }, } - Eventually(func(g Gomega) { - g.Expect(f.EnsureDynamicWithoutCleanup(context.Background(), modulePullOverrideGVR, "", mpo, true)).To(Succeed()) - }).WithTimeout(framework.LongTimeout).WithPolling(framework.PollingInterval).Should(Succeed()) - mc := &unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "deckhouse.io/v1alpha1", @@ -126,6 +131,10 @@ func EnsureModuleConfig(f *framework.Framework) { Eventually(func(g Gomega) { g.Expect(f.EnsureDynamicWithoutCleanup(context.Background(), moduleConfigGVR, "", mc, true)).To(Succeed()) }).WithTimeout(framework.LongTimeout).WithPolling(framework.PollingInterval).Should(Succeed()) + + Eventually(func(g Gomega) { + g.Expect(f.EnsureDynamicWithoutCleanup(context.Background(), modulePullOverrideGVR, "", mpo, true)).To(Succeed()) + }).WithTimeout(framework.LongTimeout).WithPolling(framework.PollingInterval).Should(Succeed()) } func DisableModuleConfig(timeout time.Duration) { @@ -167,17 +176,36 @@ func UntilModuleEnabled(deployAt metav1.Time, timeout time.Duration) { UntilConditionStatusWithLastTransitionTime("EnabledByModuleManager", string(metav1.ConditionTrue), deployAt, framework.LongTimeout, module) UntilConditionStatusWithLastTransitionTime("IsReady", string(metav1.ConditionTrue), deployAt, framework.MaxTimeout, module) + // Only now is the digest meaningful: Deckhouse records which artifact the tag + // resolved to when it actually pulls the module, and it pulls it once the + // module is enabled. Checked here rather than right after the pull override is + // created, where status.imageDigest is still empty. + digestCfg := framework.GetConfig() + + By("Verifying the module pull override resolved to digest " + digestCfg.ModuleDigest) + + Eventually(func(g Gomega) { + override, err := framework.GetClients().DynamicClient(). + Resource(modulePullOverrideGVR). + Get(context.TODO(), moduleName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + + digest, found, err := unstructured.NestedString(override.Object, "status", "imageDigest") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(found).To(BeTrue(), "module pull override has no status.imageDigest yet") + g.Expect(digest).To(Equal(digestCfg.ModuleDigest), + "cluster is running module digest %q, expected %q", digest, digestCfg.ModuleDigest) + }).WithTimeout(framework.LongTimeout).WithPolling(framework.PollingInterval).Should(Succeed()) + Eventually(func(g Gomega) { webhook, err := framework.GetClients().KubeClient().AdmissionregistrationV1().ValidatingWebhookConfigurations().Get(context.TODO(), "operator-helm-controller-admission-webhook", metav1.GetOptions{}) g.Expect(err).NotTo(HaveOccurred()) - g.Expect(webhook.CreationTimestamp.After(deployAt.UTC().Add(-1 * time.Second))).To(BeTrue()) g.Expect(webhook.Webhooks).NotTo(BeEmpty()) caBundle := webhook.Webhooks[0].ClientConfig.CABundle secret, err := framework.GetClients().KubeClient().CoreV1().Secrets(moduleNamespace).Get(context.TODO(), "operator-helm-controller-tls", metav1.GetOptions{}) g.Expect(err).NotTo(HaveOccurred()) - g.Expect(secret.CreationTimestamp.After(deployAt.UTC().Add(-1 * time.Second))).To(BeTrue()) caCert, found := secret.Data["ca.crt"] g.Expect(found).To(BeTrue()) diff --git a/tests/e2e/internal/util/resource.go b/tests/e2e/internal/util/resource.go index 018dcc46..cfe04860 100644 --- a/tests/e2e/internal/util/resource.go +++ b/tests/e2e/internal/util/resource.go @@ -175,6 +175,34 @@ func UntilConditionReason(conditionType, expectedReason string, timeout time.Dur }).WithTimeout(timeout).WithPolling(framework.PollingInterval).Should(Succeed()) } +func UntilConditionAbsent(conditionType string, timeout time.Duration, objs ...client.Object) { + GinkgoHelper() + + Eventually(func(g Gomega) { + for _, obj := range objs { + u := toUnstructured(obj) + err := framework.GetClients().GenericClient().Get( + context.Background(), client.ObjectKeyFromObject(obj), u, + ) + g.Expect(err).NotTo(HaveOccurred()) + + conditions, _, err := unstructured.NestedSlice(u.Object, "status", "conditions") + g.Expect(err).NotTo(HaveOccurred(), + "failed to access status.conditions of %s", u.GetName()) + + for _, c := range conditions { + m, ok := c.(map[string]interface{}) + if !ok { + continue + } + t, _ := m["type"].(string) + g.Expect(t).NotTo(Equal(conditionType), + "condition %s must be absent on %s", conditionType, u.GetName()) + } + } + }).WithTimeout(timeout).WithPolling(framework.PollingInterval).Should(Succeed()) +} + func untilObjectField(fieldPath, expected string, timeout time.Duration, objs ...client.Object) { GinkgoHelper() Eventually(func(g Gomega) {