From 570c24e38e058041e86e7683f23bd7a86bf3cc3f Mon Sep 17 00:00:00 2001 From: Shyukri Shyukriev Date: Thu, 2 Nov 2023 17:21:56 +0200 Subject: [PATCH 1/3] chore: Upgrade Redis to redis:7.0.14 (#16164) Fixes https://github.com/argoproj/argo-cd/issues/15448 Signed-off-by: Shyukri Shyukriev Co-authored-by: Shyukri Shyukriev --- .github/workflows/ci-build.yaml | 2 +- manifests/base/redis/argocd-redis-deployment.yaml | 2 +- manifests/core-install.yaml | 2 +- manifests/ha/base/redis-ha/chart/upstream.yaml | 8 ++++---- manifests/ha/base/redis-ha/chart/values.yaml | 2 +- manifests/ha/install.yaml | 8 ++++---- manifests/ha/namespace-install.yaml | 8 ++++---- manifests/install.yaml | 2 +- manifests/namespace-install.yaml | 2 +- 9 files changed, 18 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci-build.yaml b/.github/workflows/ci-build.yaml index adffe526da728e..afdfa61822344f 100644 --- a/.github/workflows/ci-build.yaml +++ b/.github/workflows/ci-build.yaml @@ -429,7 +429,7 @@ jobs: run: | docker pull ghcr.io/dexidp/dex:v2.37.0 docker pull argoproj/argo-cd-ci-builder:v1.0.0 - docker pull redis:7.0.11-alpine + docker pull redis:7.0.14-alpine - name: Create target directory for binaries in the build-process run: | mkdir -p dist diff --git a/manifests/base/redis/argocd-redis-deployment.yaml b/manifests/base/redis/argocd-redis-deployment.yaml index 8d649e3995ebc9..6fc776785185fe 100644 --- a/manifests/base/redis/argocd-redis-deployment.yaml +++ b/manifests/base/redis/argocd-redis-deployment.yaml @@ -23,7 +23,7 @@ spec: serviceAccountName: argocd-redis containers: - name: redis - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: Always args: - "--save" diff --git a/manifests/core-install.yaml b/manifests/core-install.yaml index 86e636e12ef59f..69514971c03e9e 100644 --- a/manifests/core-install.yaml +++ b/manifests/core-install.yaml @@ -20843,7 +20843,7 @@ spec: - "" - --appendonly - "no" - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: Always name: redis ports: diff --git a/manifests/ha/base/redis-ha/chart/upstream.yaml b/manifests/ha/base/redis-ha/chart/upstream.yaml index 80e3bfa21dcdfb..1d0e4b3c247f85 100644 --- a/manifests/ha/base/redis-ha/chart/upstream.yaml +++ b/manifests/ha/base/redis-ha/chart/upstream.yaml @@ -1207,7 +1207,7 @@ spec: automountServiceAccountToken: false initContainers: - name: config-init - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: IfNotPresent resources: {} @@ -1241,7 +1241,7 @@ spec: containers: - name: redis - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: IfNotPresent command: - redis-server @@ -1298,7 +1298,7 @@ spec: - /bin/sh - /readonly-config/trigger-failover-if-master.sh - name: sentinel - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: IfNotPresent command: - redis-sentinel @@ -1349,7 +1349,7 @@ spec: {} - name: split-brain-fix - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: IfNotPresent command: - sh diff --git a/manifests/ha/base/redis-ha/chart/values.yaml b/manifests/ha/base/redis-ha/chart/values.yaml index e2788777e980de..5606daac34bb37 100644 --- a/manifests/ha/base/redis-ha/chart/values.yaml +++ b/manifests/ha/base/redis-ha/chart/values.yaml @@ -20,7 +20,7 @@ redis-ha: metrics: enabled: true image: - tag: 7.0.11-alpine + tag: 7.0.14-alpine containerSecurityContext: null sentinel: bind: "0.0.0.0" diff --git a/manifests/ha/install.yaml b/manifests/ha/install.yaml index cd532b40b13ad9..8576a7d651f67a 100644 --- a/manifests/ha/install.yaml +++ b/manifests/ha/install.yaml @@ -23211,7 +23211,7 @@ spec: - /data/conf/redis.conf command: - redis-server - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: IfNotPresent lifecycle: preStop: @@ -23265,7 +23265,7 @@ spec: - /data/conf/sentinel.conf command: - redis-sentinel - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: IfNotPresent lifecycle: {} livenessProbe: @@ -23318,7 +23318,7 @@ spec: value: 40000915ab58c3fa8fd888fb8b24711944e6cbb4 - name: SENTINEL_ID_2 value: 2bbec7894d954a8af3bb54d13eaec53cb024e2ca - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: IfNotPresent name: split-brain-fix resources: {} @@ -23348,7 +23348,7 @@ spec: value: 40000915ab58c3fa8fd888fb8b24711944e6cbb4 - name: SENTINEL_ID_2 value: 2bbec7894d954a8af3bb54d13eaec53cb024e2ca - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: IfNotPresent name: config-init securityContext: diff --git a/manifests/ha/namespace-install.yaml b/manifests/ha/namespace-install.yaml index 524ec8ace3e6c2..4b33a7ceb867eb 100644 --- a/manifests/ha/namespace-install.yaml +++ b/manifests/ha/namespace-install.yaml @@ -2866,7 +2866,7 @@ spec: - /data/conf/redis.conf command: - redis-server - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: IfNotPresent lifecycle: preStop: @@ -2920,7 +2920,7 @@ spec: - /data/conf/sentinel.conf command: - redis-sentinel - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: IfNotPresent lifecycle: {} livenessProbe: @@ -2973,7 +2973,7 @@ spec: value: 40000915ab58c3fa8fd888fb8b24711944e6cbb4 - name: SENTINEL_ID_2 value: 2bbec7894d954a8af3bb54d13eaec53cb024e2ca - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: IfNotPresent name: split-brain-fix resources: {} @@ -3003,7 +3003,7 @@ spec: value: 40000915ab58c3fa8fd888fb8b24711944e6cbb4 - name: SENTINEL_ID_2 value: 2bbec7894d954a8af3bb54d13eaec53cb024e2ca - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: IfNotPresent name: config-init securityContext: diff --git a/manifests/install.yaml b/manifests/install.yaml index 211cdf31d92de7..45d1864c556914 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -21376,7 +21376,7 @@ spec: - "" - --appendonly - "no" - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: Always name: redis ports: diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml index cfdf7756ff1345..89cb5ed4ff5eda 100644 --- a/manifests/namespace-install.yaml +++ b/manifests/namespace-install.yaml @@ -1031,7 +1031,7 @@ spec: - "" - --appendonly - "no" - image: redis:7.0.11-alpine + image: redis:7.0.14-alpine imagePullPolicy: Always name: redis ports: From 162c2d3001f52325dd722c4e7a0683f93b08c743 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Thu, 2 Nov 2023 11:49:10 -0400 Subject: [PATCH 2/3] chore: add SECURITY-INSIGHTS.yml (#16135) automation Update SECURITY-INSIGHTS.yml Update SECURITY-INSIGHTS.yml Update SECURITY-INSIGHTS.yml Update SECURITY-INSIGHTS.yml add snyk as security tester reorganize Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- .github/workflows/release.yaml | 6 + SECURITY-INSIGHTS.yml | 128 ++++++++++++++++++ .../release-process-and-cadence.md | 18 +++ 3 files changed, 152 insertions(+) create mode 100644 SECURITY-INSIGHTS.yml diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 9448b98e5c6310..ae5174659cf406 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -265,11 +265,13 @@ jobs: set -xue SOURCE_TAG=${{ github.ref_name }} VERSION_REF="${SOURCE_TAG#*v}" + COMMIT_HASH=$(git rev-parse HEAD) if echo "$VERSION_REF" | grep -E -- '^[0-9]+\.[0-9]+\.0-rc1';then VERSION=$(awk 'BEGIN {FS=OFS="."} {$2++; print}' <<< "${VERSION_REF%-rc1}") echo "Updating VERSION to: $VERSION" echo "UPDATE_VERSION=true" >> $GITHUB_ENV echo "NEW_VERSION=$VERSION" >> $GITHUB_ENV + echo "COMMIT_HASH=$COMMIT_HASH" >> $GITHUB_ENV else echo "Not updating VERSION" echo "UPDATE_VERSION=false" >> $GITHUB_ENV @@ -278,6 +280,10 @@ jobs: - name: Update VERSION on master branch run: | echo ${{ env.NEW_VERSION }} > VERSION + # Replace the 'project-release: vX.X.X-rcX' line in SECURITY-INSIGHTS.yml + sed -i "s/project-release: v.*$/project-release: v${{ env.NEW_VERSION }}/" SECURITY-INSIGHTS.yml + # Update the 'commit-hash: XXXXXXX' line in SECURITY-INSIGHTS.yml + sed -i "s/commit-hash: .*/commit-hash: ${{ env.NEW_VERSION }}/" SECURITY-INSIGHTS.yml if: ${{ env.UPDATE_VERSION == 'true' }} - name: Create PR to update VERSION on master branch diff --git a/SECURITY-INSIGHTS.yml b/SECURITY-INSIGHTS.yml new file mode 100644 index 00000000000000..8ac4bc36b04ae5 --- /dev/null +++ b/SECURITY-INSIGHTS.yml @@ -0,0 +1,128 @@ +header: + schema-version: 1.0.0 + expiration-date: '2024-10-31T00:00:00.000Z' # One year from initial release. + last-updated: '2023-10-27' + last-reviewed: '2023-10-27' + commit-hash: b71277c6beb949d0199d647a582bc25822b88838 + project-url: https://github.com/argoproj/argo-cd + project-release: v2.9.0-rc3 + changelog: https://github.com/argoproj/argo-cd/releases + license: https://github.com/argoproj/argo-cd/blob/master/LICENSE +project-lifecycle: + status: active + roadmap: https://github.com/orgs/argoproj/projects/25 + bug-fixes-only: false + core-maintainers: + - https://github.com/argoproj/argoproj/blob/master/MAINTAINERS.md + release-cycle: https://argo-cd.readthedocs.io/en/stable/developer-guide/release-process-and-cadence/ + release-process: https://argo-cd.readthedocs.io/en/stable/developer-guide/release-process-and-cadence/#release-process +contribution-policy: + accepts-pull-requests: true + accepts-automated-pull-requests: true + automated-tools-list: + - automated-tool: dependabot + action: allowed + path: + - / + - automated-tool: snyk-report + action: allowed + path: + - docs/snyk + comment: | + This tool runs Snyk and generates a report of vulnerabilities in the project's dependencies. The report is + placed in the project's documentation. The workflow is defined here: + https://github.com/argoproj/argo-cd/blob/master/.github/workflows/update-snyk.yaml + contributing-policy: https://argo-cd.readthedocs.io/en/stable/developer-guide/code-contributions/ + code-of-conduct: https://github.com/cncf/foundation/blob/master/code-of-conduct.md +documentation: + - https://argo-cd.readthedocs.io/ +distribution-points: + - https://github.com/argoproj/argo-cd/releases + - https://quay.io/repository/argoproj/argocd +security-artifacts: + threat-model: + threat-model-created: true + evidence-url: + - https://github.com/argoproj/argoproj/blob/master/docs/argo_threat_model.pdf + - https://github.com/argoproj/argoproj/blob/master/docs/end_user_threat_model.pdf + self-assessment: + self-assessment-created: false + comment: | + An extensive self-assessment was performed for CNCF graduation. Because the self-assessment process was evolving + at the time, no standardized document has been published. +security-testing: + - tool-type: sca + tool-name: Dependabot + tool-version: "2" + tool-url: https://github.com/dependabot + integration: + ad-hoc: false + ci: false + before-release: false + tool-rulesets: + - https://github.com/argoproj/argo-cd/blob/master/.github/dependabot.yml + - tool-type: sca + tool-name: Snyk + tool-version: latest + tool-url: https://snyk.io/ + integration: + ad-hoc: true + ci: true + before-release: false + - tool-type: sast + tool-name: CodeQL + tool-version: latest + tool-url: https://codeql.github.com/ + integration: + ad-hoc: false + ci: true + before-release: false + comment: | + We use the default configuration with the latest version. +security-assessments: + - auditor-name: Trail of Bits + auditor-url: https://trailofbits.com + auditor-report: https://github.com/argoproj/argoproj/blob/master/docs/argo_security_final_report.pdf + report-year: 2021 + - auditor-name: Ada Logics + auditor-url: https://adalogics.com + auditor-report: https://github.com/argoproj/argoproj/blob/master/docs/argo_security_audit_2022.pdf + report-year: 2022 + - auditor-name: Ada Logics + auditor-url: https://adalogics.com + auditor-report: https://github.com/argoproj/argoproj/blob/master/docs/audit_fuzzer_adalogics_2022.pdf + report-year: 2022 + comment: | + Part of the audit was performed by Ada Logics, focussed on fuzzing. + - auditor-name: Chainguard + auditor-url: https://chainguard.dev + auditor-report: https://github.com/argoproj/argoproj/blob/master/docs/software_supply_chain_slsa_assessment_chainguard_2023.pdf + report-year: 2023 + comment: | + Confirmed the project's release process as achieving SLSA (v0.1) level 3. +security-contacts: + - type: email + value: cncf-argo-security@lists.cncf.io + primary: true +vulnerability-reporting: + accepts-vulnerability-reports: true + email-contact: cncf-argo-security@lists.cncf.io + security-policy: https://github.com/argoproj/argo-cd/security/policy + bug-bounty-available: true + bug-bounty-url: https://hackerone.com/ibb/policy_scopes + out-scope: + - vulnerable and outdated components # See https://github.com/argoproj/argo-cd/blob/master/SECURITY.md#a-word-about-security-scanners + - security logging and monitoring failures +dependencies: + third-party-packages: true + dependencies-lists: + - https://github.com/argoproj/argo-cd/blob/master/go.mod + - https://github.com/argoproj/argo-cd/blob/master/Dockerfile + - https://github.com/argoproj/argo-cd/blob/master/ui/package.json + sbom: + - sbom-file: https://github.com/argoproj/argo-cd/releases # Every release's assets include SBOMs. + sbom-format: SPDX + dependencies-lifecycle: + policy-url: https://argo-cd.readthedocs.io/en/stable/developer-guide/release-process-and-cadence/#dependencies-lifecycle-policy + env-dependencies-policy: + policy-url: https://argo-cd.readthedocs.io/en/stable/developer-guide/release-process-and-cadence/#dependencies-lifecycle-policy diff --git a/docs/developer-guide/release-process-and-cadence.md b/docs/developer-guide/release-process-and-cadence.md index 051de617f07762..337d5bafc35282 100644 --- a/docs/developer-guide/release-process-and-cadence.md +++ b/docs/developer-guide/release-process-and-cadence.md @@ -78,3 +78,21 @@ The feature PR must include: If these criteria are not met by the RC date, the feature will be ineligible for inclusion in the RC series or GA for that minor release. It will have to wait for the next minor release. + +### Security Patch Policy + +CVEs in Argo CD code will be patched for all [supported versions](../operator-manual/installation.md#supported-versions). + +### Dependencies Lifecycle Policy + +Dependencies are evaluated before being introduced to ensure they: + +1) are actively maintained +2) are maintained by trustworthy maintainers + +These evaluations vary from dependency to dependencies. + +Dependencies are also scheduled for removal if the project has been deprecated or if the project is no longer maintained. + +CVEs in dependencies will be patched for all supported versions if the CVE is applicable and is assessed by Snyk to be +of high or critical severity. Automation generates a [new Snyk scan weekly](../snyk). From be2a01c231e09b9056f115ef10f22f34f0290d9e Mon Sep 17 00:00:00 2001 From: gdsoumya <44349253+gdsoumya@users.noreply.github.com> Date: Thu, 2 Nov 2023 21:21:16 +0530 Subject: [PATCH 3/3] feat: grace period for repo errors to prevent aggressive unknown sync state (#16085) * feat: added grace period for repo errors to prevent aggressive sync state unknowns Signed-off-by: Soumya Ghosh Dastidar * feat: updated docs Signed-off-by: Soumya Ghosh Dastidar * fix: e2e test Signed-off-by: Soumya Ghosh Dastidar * feat: resolved review comments Signed-off-by: Soumya Ghosh Dastidar * feat: update unit test Signed-off-by: Soumya Ghosh Dastidar * fix: codegen Signed-off-by: Soumya Ghosh Dastidar --------- Signed-off-by: Soumya Ghosh Dastidar Signed-off-by: gdsoumya <44349253+gdsoumya@users.noreply.github.com> --- .../commands/argocd_application_controller.go | 3 + cmd/argocd/commands/admin/app.go | 7 +- controller/appcontroller.go | 10 +- controller/appcontroller_test.go | 113 ++++++------ controller/state.go | 38 +++- controller/state_test.go | 166 ++++++++++++------ controller/sync.go | 9 +- controller/sync_test.go | 10 +- .../operator-manual/argocd-cmd-params-cm.yaml | 2 + .../argocd-application-controller.md | 1 + ...cd-application-controller-statefulset.yaml | 6 + manifests/core-install.yaml | 6 + manifests/ha/install.yaml | 6 + manifests/ha/namespace-install.yaml | 6 + manifests/install.yaml | 6 + manifests/namespace-install.yaml | 6 + 16 files changed, 269 insertions(+), 126 deletions(-) diff --git a/cmd/argocd-application-controller/commands/argocd_application_controller.go b/cmd/argocd-application-controller/commands/argocd_application_controller.go index 391751b0d95d19..4f7e587e36564c 100644 --- a/cmd/argocd-application-controller/commands/argocd_application_controller.go +++ b/cmd/argocd-application-controller/commands/argocd_application_controller.go @@ -50,6 +50,7 @@ func NewCommand() *cobra.Command { clientConfig clientcmd.ClientConfig appResyncPeriod int64 appHardResyncPeriod int64 + repoErrorGracePeriod int64 repoServerAddress string repoServerTimeoutSeconds int selfHealTimeoutSeconds int @@ -154,6 +155,7 @@ func NewCommand() *cobra.Command { resyncDuration, hardResyncDuration, time.Duration(selfHealTimeoutSeconds)*time.Second, + time.Duration(repoErrorGracePeriod)*time.Second, metricsPort, metricsCacheExpiration, metricsAplicationLabels, @@ -188,6 +190,7 @@ func NewCommand() *cobra.Command { clientConfig = cli.AddKubectlFlagsToCmd(&command) command.Flags().Int64Var(&appResyncPeriod, "app-resync", int64(env.ParseDurationFromEnv("ARGOCD_RECONCILIATION_TIMEOUT", defaultAppResyncPeriod*time.Second, 0, math.MaxInt64).Seconds()), "Time period in seconds for application resync.") command.Flags().Int64Var(&appHardResyncPeriod, "app-hard-resync", int64(env.ParseDurationFromEnv("ARGOCD_HARD_RECONCILIATION_TIMEOUT", defaultAppHardResyncPeriod*time.Second, 0, math.MaxInt64).Seconds()), "Time period in seconds for application hard resync.") + command.Flags().Int64Var(&repoErrorGracePeriod, "repo-error-grace-period-seconds", int64(env.ParseDurationFromEnv("ARGOCD_REPO_ERROR_GRACE_PERIOD_SECONDS", defaultAppResyncPeriod*time.Second, 0, math.MaxInt64).Seconds()), "Grace period in seconds for ignoring consecutive errors while communicating with repo server.") command.Flags().StringVar(&repoServerAddress, "repo-server", env.StringFromEnv("ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER", common.DefaultRepoServerAddr), "Repo server address.") command.Flags().IntVar(&repoServerTimeoutSeconds, "repo-server-timeout-seconds", env.ParseNumFromEnv("ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER_TIMEOUT_SECONDS", 60, 0, math.MaxInt64), "Repo server RPC call timeout seconds.") command.Flags().IntVar(&statusProcessors, "status-processors", env.ParseNumFromEnv("ARGOCD_APPLICATION_CONTROLLER_STATUS_PROCESSORS", 20, 0, math.MaxInt32), "Number of application status processors") diff --git a/cmd/argocd/commands/admin/app.go b/cmd/argocd/commands/admin/app.go index a4f4557858a226..10e4effe8797aa 100644 --- a/cmd/argocd/commands/admin/app.go +++ b/cmd/argocd/commands/admin/app.go @@ -384,7 +384,7 @@ func reconcileApplications( ) appStateManager := controller.NewAppStateManager( - argoDB, appClientset, repoServerClient, namespace, kubeutil.NewKubectl(), settingsMgr, stateCache, projInformer, server, cache, time.Second, argo.NewResourceTracking(), false) + argoDB, appClientset, repoServerClient, namespace, kubeutil.NewKubectl(), settingsMgr, stateCache, projInformer, server, cache, time.Second, argo.NewResourceTracking(), false, 0) appsList, err := appClientset.ArgoprojV1alpha1().Applications(namespace).List(ctx, v1.ListOptions{LabelSelector: selector}) if err != nil { @@ -419,7 +419,10 @@ func reconcileApplications( sources = append(sources, app.Spec.GetSource()) revisions = append(revisions, app.Spec.GetSource().TargetRevision) - res := appStateManager.CompareAppState(&app, proj, revisions, sources, false, false, nil, false) + res, err := appStateManager.CompareAppState(&app, proj, revisions, sources, false, false, nil, false) + if err != nil { + return nil, err + } items = append(items, appReconcileResult{ Name: app.Name, Conditions: app.Status.Conditions, diff --git a/controller/appcontroller.go b/controller/appcontroller.go index 3582d907e5f5b3..964bcc1aa0dec9 100644 --- a/controller/appcontroller.go +++ b/controller/appcontroller.go @@ -143,6 +143,7 @@ func NewApplicationController( appResyncPeriod time.Duration, appHardResyncPeriod time.Duration, selfHealTimeout time.Duration, + repoErrorGracePeriod time.Duration, metricsPort int, metricsCacheExpiration time.Duration, metricsApplicationLabels []string, @@ -259,7 +260,7 @@ func NewApplicationController( } } stateCache := statecache.NewLiveStateCache(db, appInformer, ctrl.settingsMgr, kubectl, ctrl.metricsServer, ctrl.handleObjectUpdated, clusterFilter, argo.NewResourceTracking()) - appStateManager := NewAppStateManager(db, applicationClientset, repoClientset, namespace, kubectl, ctrl.settingsMgr, stateCache, projInformer, ctrl.metricsServer, argoCache, ctrl.statusRefreshTimeout, argo.NewResourceTracking(), persistResourceHealth) + appStateManager := NewAppStateManager(db, applicationClientset, repoClientset, namespace, kubectl, ctrl.settingsMgr, stateCache, projInformer, ctrl.metricsServer, argoCache, ctrl.statusRefreshTimeout, argo.NewResourceTracking(), persistResourceHealth, repoErrorGracePeriod) ctrl.appInformer = appInformer ctrl.appLister = appLister ctrl.projInformer = projInformer @@ -1512,10 +1513,15 @@ func (ctrl *ApplicationController) processAppRefreshQueueItem() (processNext boo } now := metav1.Now() - compareResult := ctrl.appStateManager.CompareAppState(app, project, revisions, sources, + compareResult, err := ctrl.appStateManager.CompareAppState(app, project, revisions, sources, refreshType == appv1.RefreshTypeHard, comparisonLevel == CompareWithLatestForceResolve, localManifests, hasMultipleSources) + if goerrors.Is(err, CompareStateRepoError) { + logCtx.Warnf("Ignoring temporary failed attempt to compare app state against repo: %v", err) + return // short circuit if git error is encountered + } + for k, v := range compareResult.timings { logCtx = logCtx.WithField(k, v.Milliseconds()) } diff --git a/controller/appcontroller_test.go b/controller/appcontroller_test.go index a47114f5c2527a..9c2933feeb6df4 100644 --- a/controller/appcontroller_test.go +++ b/controller/appcontroller_test.go @@ -59,7 +59,7 @@ type fakeData struct { applicationNamespaces []string } -func newFakeController(data *fakeData) *ApplicationController { +func newFakeController(data *fakeData, repoErr error) *ApplicationController { var clust corev1.Secret err := yaml.Unmarshal([]byte(fakeCluster), &clust) if err != nil { @@ -71,10 +71,18 @@ func newFakeController(data *fakeData) *ApplicationController { if len(data.manifestResponses) > 0 { for _, response := range data.manifestResponses { - mockRepoClient.On("GenerateManifest", mock.Anything, mock.Anything).Return(response, nil).Once() + if repoErr != nil { + mockRepoClient.On("GenerateManifest", mock.Anything, mock.Anything).Return(response, repoErr).Once() + } else { + mockRepoClient.On("GenerateManifest", mock.Anything, mock.Anything).Return(response, nil).Once() + } } } else { - mockRepoClient.On("GenerateManifest", mock.Anything, mock.Anything).Return(data.manifestResponse, nil) + if repoErr != nil { + mockRepoClient.On("GenerateManifest", mock.Anything, mock.Anything).Return(data.manifestResponse, repoErr).Once() + } else { + mockRepoClient.On("GenerateManifest", mock.Anything, mock.Anything).Return(data.manifestResponse, nil).Once() + } } mockRepoClientset := mockrepoclient.Clientset{RepoServerServiceClient: &mockRepoClient} @@ -116,6 +124,7 @@ func newFakeController(data *fakeData) *ApplicationController { time.Minute, time.Hour, time.Minute, + time.Second*10, common.DefaultPortArgoCDMetrics, data.metricsCacheExpiration, []string{}, @@ -364,7 +373,7 @@ func newFakeCM() map[string]interface{} { func TestAutoSync(t *testing.T) { app := newFakeApp() - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) syncStatus := v1alpha1.SyncStatus{ Status: v1alpha1.SyncStatusCodeOutOfSync, Revision: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", @@ -381,7 +390,7 @@ func TestAutoSync(t *testing.T) { func TestAutoSyncNotAllowEmpty(t *testing.T) { app := newFakeApp() app.Spec.SyncPolicy.Automated.Prune = true - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) syncStatus := v1alpha1.SyncStatus{ Status: v1alpha1.SyncStatusCodeOutOfSync, Revision: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", @@ -394,7 +403,7 @@ func TestAutoSyncAllowEmpty(t *testing.T) { app := newFakeApp() app.Spec.SyncPolicy.Automated.Prune = true app.Spec.SyncPolicy.Automated.AllowEmpty = true - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) syncStatus := v1alpha1.SyncStatus{ Status: v1alpha1.SyncStatusCodeOutOfSync, Revision: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", @@ -408,7 +417,7 @@ func TestSkipAutoSync(t *testing.T) { // Set current to 'aaaaa', desired to 'aaaa' and mark system OutOfSync t.Run("PreviouslySyncedToRevision", func(t *testing.T) { app := newFakeApp() - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) syncStatus := v1alpha1.SyncStatus{ Status: v1alpha1.SyncStatusCodeOutOfSync, Revision: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", @@ -423,7 +432,7 @@ func TestSkipAutoSync(t *testing.T) { // Verify we skip when we are already Synced (even if revision is different) t.Run("AlreadyInSyncedState", func(t *testing.T) { app := newFakeApp() - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) syncStatus := v1alpha1.SyncStatus{ Status: v1alpha1.SyncStatusCodeSynced, Revision: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", @@ -439,7 +448,7 @@ func TestSkipAutoSync(t *testing.T) { t.Run("AutoSyncIsDisabled", func(t *testing.T) { app := newFakeApp() app.Spec.SyncPolicy = nil - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) syncStatus := v1alpha1.SyncStatus{ Status: v1alpha1.SyncStatusCodeOutOfSync, Revision: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", @@ -456,7 +465,7 @@ func TestSkipAutoSync(t *testing.T) { app := newFakeApp() now := metav1.Now() app.DeletionTimestamp = &now - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) syncStatus := v1alpha1.SyncStatus{ Status: v1alpha1.SyncStatusCodeOutOfSync, Revision: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", @@ -482,7 +491,7 @@ func TestSkipAutoSync(t *testing.T) { Source: *app.Spec.Source.DeepCopy(), }, } - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) syncStatus := v1alpha1.SyncStatus{ Status: v1alpha1.SyncStatusCodeOutOfSync, Revision: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", @@ -496,7 +505,7 @@ func TestSkipAutoSync(t *testing.T) { t.Run("NeedsToPruneResourcesOnlyButAutomatedPruneDisabled", func(t *testing.T) { app := newFakeApp() - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) syncStatus := v1alpha1.SyncStatus{ Status: v1alpha1.SyncStatusCodeOutOfSync, Revision: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", @@ -522,7 +531,7 @@ func TestAutoSyncIndicateError(t *testing.T) { }, }, } - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) syncStatus := v1alpha1.SyncStatus{ Status: v1alpha1.SyncStatusCodeOutOfSync, Revision: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", @@ -557,7 +566,7 @@ func TestAutoSyncParameterOverrides(t *testing.T) { }, }, } - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) syncStatus := v1alpha1.SyncStatus{ Status: v1alpha1.SyncStatusCodeOutOfSync, Revision: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", @@ -614,7 +623,7 @@ func TestFinalizeAppDeletion(t *testing.T) { appObj := kube.MustToUnstructured(&app) ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, &defaultProj}, managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{ kube.GetResourceKey(appObj): appObj, - }}) + }}, nil) patched := false fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) @@ -664,7 +673,7 @@ func TestFinalizeAppDeletion(t *testing.T) { kube.GetResourceKey(appObj): appObj, kube.GetResourceKey(strayObj): strayObj, }, - }) + }, nil) patched := false fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) @@ -699,7 +708,7 @@ func TestFinalizeAppDeletion(t *testing.T) { appObj := kube.MustToUnstructured(&app) ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, &defaultProj}, managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{ kube.GetResourceKey(appObj): appObj, - }}) + }}, nil) patched := false fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) defaultReactor := fakeAppCs.ReactionChain[0] @@ -728,7 +737,7 @@ func TestFinalizeAppDeletion(t *testing.T) { appObj := kube.MustToUnstructured(&app) ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, &defaultProj}, managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{ kube.GetResourceKey(appObj): appObj, - }}) + }}, nil) fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) defaultReactor := fakeAppCs.ReactionChain[0] @@ -792,7 +801,7 @@ func TestNormalizeApplication(t *testing.T) { { // Verify we normalize the app because project is missing - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) key, _ := cache.MetaNamespaceKeyFunc(app) ctrl.appRefreshQueue.AddRateLimited(key) fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) @@ -814,7 +823,7 @@ func TestNormalizeApplication(t *testing.T) { // Verify we don't unnecessarily normalize app when project is set app.Spec.Project = "default" data.apps[0] = app - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) key, _ := cache.MetaNamespaceKeyFunc(app) ctrl.appRefreshQueue.AddRateLimited(key) fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) @@ -839,7 +848,7 @@ func TestHandleAppUpdated(t *testing.T) { app.Spec.Destination.Server = v1alpha1.KubernetesInternalAPIServerAddr proj := defaultProj.DeepCopy() proj.Spec.SourceNamespaces = []string{test.FakeArgoCDNamespace} - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, proj}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, proj}}, nil) ctrl.handleObjectUpdated(map[string]bool{app.InstanceName(ctrl.namespace): true}, kube.GetObjectRef(kube.MustToUnstructured(app))) isRequested, level := ctrl.isRefreshRequested(app.QualifiedName()) @@ -866,7 +875,7 @@ func TestHandleOrphanedResourceUpdated(t *testing.T) { proj := defaultProj.DeepCopy() proj.Spec.OrphanedResources = &v1alpha1.OrphanedResourcesMonitorSettings{} - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app1, app2, proj}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app1, app2, proj}}, nil) ctrl.handleObjectUpdated(map[string]bool{}, corev1.ObjectReference{UID: "test", Kind: kube.DeploymentKind, Name: "test", Namespace: test.FakeArgoCDNamespace}) @@ -901,7 +910,7 @@ func TestGetResourceTree_HasOrphanedResources(t *testing.T) { kube.NewResourceKey("apps", "Deployment", "default", "deploy1"): {ResourceNode: orphanedDeploy1}, kube.NewResourceKey("apps", "Deployment", "default", "deploy2"): {ResourceNode: orphanedDeploy2}, }, - }) + }, nil) tree, err := ctrl.getResourceTree(app, []*v1alpha1.ResourceDiff{{ Namespace: "default", Name: "nginx-deployment", @@ -917,7 +926,7 @@ func TestGetResourceTree_HasOrphanedResources(t *testing.T) { } func TestSetOperationStateOnDeletedApp(t *testing.T) { - ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}, nil) fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) fakeAppCs.ReactionChain = nil patched := false @@ -948,7 +957,7 @@ func TestSetOperationStateLogRetries(t *testing.T) { t.Cleanup(func() { logrus.StandardLogger().ReplaceHooks(logrus.LevelHooks{}) }) - ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}, nil) fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) fakeAppCs.ReactionChain = nil patched := false @@ -999,7 +1008,7 @@ func TestNeedRefreshAppStatus(t *testing.T) { app.Status.Sync.ComparedTo.Source = app.Spec.GetSource() } - ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}, nil) t.Run("no need to refresh just reconciled application", func(t *testing.T) { needRefresh, _, _ := ctrl.needRefreshAppStatus(app, 1*time.Hour, 2*time.Hour) @@ -1011,7 +1020,7 @@ func TestNeedRefreshAppStatus(t *testing.T) { assert.False(t, needRefresh) // use a one-off controller so other tests don't have a manual refresh request - ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}, nil) // refresh app using the 'deepest' requested comparison level ctrl.requestAppRefresh(app.Name, CompareWithRecent.Pointer(), nil) @@ -1039,7 +1048,7 @@ func TestNeedRefreshAppStatus(t *testing.T) { app := app.DeepCopy() // use a one-off controller so other tests don't have a manual refresh request - ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}, nil) needRefresh, _, _ := ctrl.needRefreshAppStatus(app, 1*time.Hour, 2*time.Hour) assert.False(t, needRefresh) @@ -1069,7 +1078,7 @@ func TestNeedRefreshAppStatus(t *testing.T) { } // use a one-off controller so other tests don't have a manual refresh request - ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}, nil) needRefresh, _, _ := ctrl.needRefreshAppStatus(app, 1*time.Hour, 2*time.Hour) assert.False(t, needRefresh) @@ -1149,7 +1158,7 @@ func TestNeedRefreshAppStatus(t *testing.T) { } func TestUpdatedManagedNamespaceMetadata(t *testing.T) { - ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}, nil) app := newFakeApp() app.Spec.SyncPolicy.ManagedNamespaceMetadata = &v1alpha1.ManagedNamespaceMetadata{ Labels: map[string]string{ @@ -1173,7 +1182,7 @@ func TestUpdatedManagedNamespaceMetadata(t *testing.T) { } func TestUnchangedManagedNamespaceMetadata(t *testing.T) { - ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{}}, nil) app := newFakeApp() app.Spec.SyncPolicy.ManagedNamespaceMetadata = &v1alpha1.ManagedNamespaceMetadata{ Labels: map[string]string{ @@ -1216,7 +1225,7 @@ func TestRefreshAppConditions(t *testing.T) { t.Run("NoErrorConditions", func(t *testing.T) { app := newFakeApp() - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, &defaultProj}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, &defaultProj}}, nil) _, hasErrors := ctrl.refreshAppConditions(app) assert.False(t, hasErrors) @@ -1227,7 +1236,7 @@ func TestRefreshAppConditions(t *testing.T) { app := newFakeApp() app.Status.SetConditions([]v1alpha1.ApplicationCondition{{Type: v1alpha1.ApplicationConditionExcludedResourceWarning}}, nil) - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, &defaultProj}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, &defaultProj}}, nil) _, hasErrors := ctrl.refreshAppConditions(app) assert.False(t, hasErrors) @@ -1240,7 +1249,7 @@ func TestRefreshAppConditions(t *testing.T) { app.Spec.Project = "wrong project" app.Status.SetConditions([]v1alpha1.ApplicationCondition{{Type: v1alpha1.ApplicationConditionInvalidSpecError, Message: "old message"}}, nil) - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, &defaultProj}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, &defaultProj}}, nil) _, hasErrors := ctrl.refreshAppConditions(app) assert.True(t, hasErrors) @@ -1264,7 +1273,7 @@ func TestUpdateReconciledAt(t *testing.T) { Revision: "abc123", }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), - }) + }, nil) key, _ := cache.MetaNamespaceKeyFunc(app) fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) fakeAppCs.ReactionChain = nil @@ -1322,7 +1331,7 @@ func TestProjectErrorToCondition(t *testing.T) { Revision: "abc123", }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), - }) + }, nil) key, _ := cache.MetaNamespaceKeyFunc(app) ctrl.appRefreshQueue.AddRateLimited(key) ctrl.requestAppRefresh(app.Name, CompareWithRecent.Pointer(), nil) @@ -1341,7 +1350,7 @@ func TestProjectErrorToCondition(t *testing.T) { func TestFinalizeProjectDeletion_HasApplications(t *testing.T) { app := newFakeApp() proj := &v1alpha1.AppProject{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: test.FakeArgoCDNamespace}} - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, proj}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, proj}}, nil) fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) patched := false @@ -1357,7 +1366,7 @@ func TestFinalizeProjectDeletion_HasApplications(t *testing.T) { func TestFinalizeProjectDeletion_DoesNotHaveApplications(t *testing.T) { proj := &v1alpha1.AppProject{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: test.FakeArgoCDNamespace}} - ctrl := newFakeController(&fakeData{apps: []runtime.Object{&defaultProj}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{&defaultProj}}, nil) fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) receivedPatch := map[string]interface{}{} @@ -1383,7 +1392,7 @@ func TestProcessRequestedAppOperation_FailedNoRetries(t *testing.T) { app.Operation = &v1alpha1.Operation{ Sync: &v1alpha1.SyncOperation{}, } - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) receivedPatch := map[string]interface{}{} fakeAppCs.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { @@ -1408,7 +1417,7 @@ func TestProcessRequestedAppOperation_InvalidDestination(t *testing.T) { proj := defaultProj proj.Name = "test-project" proj.Spec.SourceNamespaces = []string{test.FakeArgoCDNamespace} - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, &proj}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app, &proj}}, nil) fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) receivedPatch := map[string]interface{}{} func() { @@ -1437,7 +1446,7 @@ func TestProcessRequestedAppOperation_FailedHasRetries(t *testing.T) { Sync: &v1alpha1.SyncOperation{}, Retry: v1alpha1.RetryStrategy{Limit: 1}, } - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) receivedPatch := map[string]interface{}{} fakeAppCs.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { @@ -1480,7 +1489,7 @@ func TestProcessRequestedAppOperation_RunningPreviouslyFailed(t *testing.T) { Revision: "abc123", }, } - ctrl := newFakeController(data) + ctrl := newFakeController(data, nil) fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) receivedPatch := map[string]interface{}{} fakeAppCs.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { @@ -1513,7 +1522,7 @@ func TestProcessRequestedAppOperation_HasRetriesTerminated(t *testing.T) { Revision: "abc123", }, } - ctrl := newFakeController(data) + ctrl := newFakeController(data, nil) fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) receivedPatch := map[string]interface{}{} fakeAppCs.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { @@ -1540,7 +1549,7 @@ func TestGetAppHosts(t *testing.T) { Revision: "abc123", }, } - ctrl := newFakeController(data) + ctrl := newFakeController(data, nil) mockStateCache := &mockstatecache.LiveStateCache{} mockStateCache.On("IterateResources", mock.Anything, mock.MatchedBy(func(callback func(res *clustercache.Resource, info *statecache.ResourceInfo)) bool { // node resource @@ -1590,15 +1599,15 @@ func TestGetAppHosts(t *testing.T) { func TestMetricsExpiration(t *testing.T) { app := newFakeApp() // Check expiration is disabled by default - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) assert.False(t, ctrl.metricsServer.HasExpiration()) // Check expiration is enabled if set - ctrl = newFakeController(&fakeData{apps: []runtime.Object{app}, metricsCacheExpiration: 10 * time.Second}) + ctrl = newFakeController(&fakeData{apps: []runtime.Object{app}, metricsCacheExpiration: 10 * time.Second}, nil) assert.True(t, ctrl.metricsServer.HasExpiration()) } func TestToAppKey(t *testing.T) { - ctrl := newFakeController(&fakeData{}) + ctrl := newFakeController(&fakeData{}, nil) tests := []struct { name string input string @@ -1618,7 +1627,7 @@ func TestToAppKey(t *testing.T) { func Test_canProcessApp(t *testing.T) { app := newFakeApp() - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) ctrl.applicationNamespaces = []string{"good"} t.Run("without cluster filter, good namespace", func(t *testing.T) { app.Namespace = "good" @@ -1651,7 +1660,7 @@ func Test_canProcessAppSkipReconcileAnnotation(t *testing.T) { appSkipReconcileFalse.Annotations = map[string]string{common.AnnotationKeyAppSkipReconcile: "false"} appSkipReconcileTrue := newFakeApp() appSkipReconcileTrue.Annotations = map[string]string{common.AnnotationKeyAppSkipReconcile: "true"} - ctrl := newFakeController(&fakeData{}) + ctrl := newFakeController(&fakeData{}, nil) tests := []struct { name string input interface{} @@ -1672,7 +1681,7 @@ func Test_canProcessAppSkipReconcileAnnotation(t *testing.T) { func Test_syncDeleteOption(t *testing.T) { app := newFakeApp() - ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) cm := newFakeCM() t.Run("without delete option object is deleted", func(t *testing.T) { cmObj := kube.MustToUnstructured(&cm) @@ -1699,7 +1708,7 @@ func TestAddControllerNamespace(t *testing.T) { ctrl := newFakeController(&fakeData{ apps: []runtime.Object{app, &defaultProj}, manifestResponse: &apiclient.ManifestResponse{}, - }) + }, nil) ctrl.processAppRefreshQueueItem() @@ -1718,7 +1727,7 @@ func TestAddControllerNamespace(t *testing.T) { apps: []runtime.Object{app, &proj}, manifestResponse: &apiclient.ManifestResponse{}, applicationNamespaces: []string{appNamespace}, - }) + }, nil) ctrl.processAppRefreshQueueItem() diff --git a/controller/state.go b/controller/state.go index 2704545add445f..15da3d2e624ed9 100644 --- a/controller/state.go +++ b/controller/state.go @@ -3,12 +3,15 @@ package controller import ( "context" "encoding/json" + "errors" "fmt" - v1 "k8s.io/api/core/v1" "reflect" "strings" + goSync "sync" "time" + v1 "k8s.io/api/core/v1" + "github.com/argoproj/gitops-engine/pkg/diff" "github.com/argoproj/gitops-engine/pkg/health" "github.com/argoproj/gitops-engine/pkg/sync" @@ -40,6 +43,10 @@ import ( "github.com/argoproj/argo-cd/v2/util/stats" ) +var ( + CompareStateRepoError = errors.New("failed to get repo objects") +) + type resourceInfoProviderStub struct { } @@ -62,7 +69,7 @@ type managedResource struct { // AppStateManager defines methods which allow to compare application spec and actual application state. type AppStateManager interface { - CompareAppState(app *v1alpha1.Application, project *v1alpha1.AppProject, revisions []string, sources []v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localObjects []string, hasMultipleSources bool) *comparisonResult + CompareAppState(app *v1alpha1.Application, project *v1alpha1.AppProject, revisions []string, sources []v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localObjects []string, hasMultipleSources bool) (*comparisonResult, error) SyncAppState(app *v1alpha1.Application, state *v1alpha1.OperationState) } @@ -105,10 +112,11 @@ type appStateManager struct { statusRefreshTimeout time.Duration resourceTracking argo.ResourceTracking persistResourceHealth bool + repoErrorCache goSync.Map + repoErrorGracePeriod time.Duration } func (m *appStateManager) getRepoObjs(app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, error) { - ts := stats.NewTimingStats() helmRepos, err := m.db.ListHelmRepositories(context.Background()) if err != nil { @@ -345,7 +353,7 @@ func isManagedNamespace(ns *unstructured.Unstructured, app *v1alpha1.Application // CompareAppState compares application git state to the live app state, using the specified // revision and supplied source. If revision or overrides are empty, then compares against // revision and overrides in the app spec. -func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1alpha1.AppProject, revisions []string, sources []v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localManifests []string, hasMultipleSources bool) *comparisonResult { +func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1alpha1.AppProject, revisions []string, sources []v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localManifests []string, hasMultipleSources bool) (*comparisonResult, error) { ts := stats.NewTimingStats() appLabelKey, resourceOverrides, resFilter, err := m.getComparisonSettings() @@ -361,7 +369,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 Revisions: revisions, }, healthStatus: &v1alpha1.HealthStatus{Status: health.HealthStatusUnknown}, - } + }, nil } else { return &comparisonResult{ syncStatus: &v1alpha1.SyncStatus{ @@ -370,7 +378,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 Revision: revisions[0], }, healthStatus: &v1alpha1.HealthStatus{Status: health.HealthStatusUnknown}, - } + }, nil } } @@ -408,7 +416,21 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 targetObjs = make([]*unstructured.Unstructured, 0) msg := fmt.Sprintf("Failed to load target state: %s", err.Error()) conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: msg, LastTransitionTime: &now}) + if firstSeen, ok := m.repoErrorCache.Load(app.Name); ok { + if time.Since(firstSeen.(time.Time)) <= m.repoErrorGracePeriod && !noRevisionCache { + // if first seen is less than grace period and it's not a Level 3 comparison, + // ignore error and short circuit + logCtx.Debugf("Ignoring repo error %v, already encountered error in grace period", err.Error()) + return nil, CompareStateRepoError + } + } else if !noRevisionCache { + logCtx.Debugf("Ignoring repo error %v, new occurrence", err.Error()) + m.repoErrorCache.Store(app.Name, time.Now()) + return nil, CompareStateRepoError + } failedToLoadObjs = true + } else { + m.repoErrorCache.Delete(app.Name) } } else { // Prevent applying local manifests for now when signature verification is enabled @@ -776,7 +798,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 }) ts.AddCheckpoint("health_ms") compRes.timings = ts.Timings() - return &compRes + return &compRes, nil } func (m *appStateManager) persistRevisionHistory(app *v1alpha1.Application, revision string, source v1alpha1.ApplicationSource, revisions []string, sources []v1alpha1.ApplicationSource, hasMultipleSources bool, startedAt metav1.Time) error { @@ -832,6 +854,7 @@ func NewAppStateManager( statusRefreshTimeout time.Duration, resourceTracking argo.ResourceTracking, persistResourceHealth bool, + repoErrorGracePeriod time.Duration, ) AppStateManager { return &appStateManager{ liveStateCache: liveStateCache, @@ -847,6 +870,7 @@ func NewAppStateManager( statusRefreshTimeout: statusRefreshTimeout, resourceTracking: resourceTracking, persistResourceHealth: persistResourceHealth, + repoErrorGracePeriod: repoErrorGracePeriod, } } diff --git a/controller/state_test.go b/controller/state_test.go index dcb48e87fce9b5..9d2df1f2391858 100644 --- a/controller/state_test.go +++ b/controller/state_test.go @@ -2,6 +2,7 @@ package controller import ( "encoding/json" + "fmt" "os" "testing" "time" @@ -37,12 +38,13 @@ func TestCompareAppStateEmpty(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -51,6 +53,31 @@ func TestCompareAppStateEmpty(t *testing.T) { assert.Len(t, app.Status.Conditions, 0) } +// TestCompareAppStateRepoError tests the case when CompareAppState notices a repo error +func TestCompareAppStateRepoError(t *testing.T) { + app := newFakeApp() + ctrl := newFakeController(&fakeData{manifestResponses: make([]*apiclient.ManifestResponse, 3)}, fmt.Errorf("test repo error")) + sources := make([]argoappv1.ApplicationSource, 0) + sources = append(sources, app.Spec.GetSource()) + revisions := make([]string, 0) + revisions = append(revisions, "") + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, compRes) + assert.EqualError(t, err, CompareStateRepoError.Error()) + + // expect to still get compare state error to as inside grace period + compRes, err = ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, compRes) + assert.EqualError(t, err, CompareStateRepoError.Error()) + + time.Sleep(10 * time.Second) + // expect to not get error as outside of grace period, but status should be unknown + compRes, err = ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.NotNil(t, compRes) + assert.Nil(t, err) + assert.Equal(t, compRes.syncStatus.Status, argoappv1.SyncStatusCodeUnknown) +} + // TestCompareAppStateNamespaceMetadataDiffers tests comparison when managed namespace metadata differs func TestCompareAppStateNamespaceMetadataDiffers(t *testing.T) { app := newFakeApp() @@ -75,12 +102,13 @@ func TestCompareAppStateNamespaceMetadataDiffers(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeOutOfSync, compRes.syncStatus.Status) @@ -122,12 +150,13 @@ func TestCompareAppStateNamespaceMetadataIsTheSame(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -149,12 +178,13 @@ func TestCompareAppStateMissing(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeOutOfSync, compRes.syncStatus.Status) @@ -180,12 +210,13 @@ func TestCompareAppStateExtra(t *testing.T) { key: pod, }, } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeOutOfSync, compRes.syncStatus.Status) assert.Equal(t, 1, len(compRes.resources)) @@ -210,12 +241,13 @@ func TestCompareAppStateHook(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) assert.Equal(t, 0, len(compRes.resources)) @@ -241,12 +273,13 @@ func TestCompareAppStateSkipHook(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) assert.Equal(t, 1, len(compRes.resources)) @@ -270,13 +303,14 @@ func TestCompareAppStateCompareOptionIgnoreExtraneous(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -303,12 +337,13 @@ func TestCompareAppStateExtraHook(t *testing.T) { key: pod, }, } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -331,12 +366,13 @@ func TestAppRevisionsSingleSource(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) app := newFakeApp() revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources()) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources()) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.NotEmpty(t, compRes.syncStatus.Revision) @@ -370,12 +406,13 @@ func TestAppRevisionsMultiSource(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) app := newFakeMultiSourceApp() revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources()) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources()) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Empty(t, compRes.syncStatus.Revision) @@ -417,12 +454,13 @@ func TestCompareAppStateDuplicatedNamespacedResources(t *testing.T) { kube.GetResourceKey(obj3): obj3, }, } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.Equal(t, 1, len(app.Status.Conditions)) @@ -457,8 +495,9 @@ func TestCompareAppStateManagedNamespaceMetadataWithLiveNsDoesNotGetPruned(t *te kube.GetResourceKey(ns): ns, }, } - ctrl := newFakeController(&data) - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, []string{}, app.Spec.Sources, false, false, nil, false) + ctrl := newFakeController(&data, nil) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, []string{}, app.Spec.Sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.Equal(t, 0, len(app.Status.Conditions)) @@ -512,13 +551,14 @@ func TestSetHealth(t *testing.T) { managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{ kube.GetResourceKey(deployment): deployment, }, - }) + }, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status) } @@ -548,13 +588,14 @@ func TestSetHealthSelfReferencedApp(t *testing.T) { kube.GetResourceKey(deployment): deployment, kube.GetResourceKey(unstructuredApp): unstructuredApp, }, - }) + }, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status) } @@ -574,7 +615,7 @@ func TestSetManagedResourcesWithOrphanedResources(t *testing.T) { AppName: "", }, }, - }) + }, nil) tree, err := ctrl.setAppManagedResources(app, &comparisonResult{managedResources: make([]managedResource, 0)}) @@ -603,7 +644,7 @@ func TestSetManagedResourcesWithResourcesOfAnotherApp(t *testing.T) { AppName: "app2", }, }, - }) + }, nil) tree, err := ctrl.setAppManagedResources(app1, &comparisonResult{managedResources: make([]managedResource, 0)}) @@ -622,13 +663,14 @@ func TestReturnUnknownComparisonStateOnSettingLoadError(t *testing.T) { configMapData: map[string]string{ "resource.customizations": "invalid setting", }, - }) + }, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.Equal(t, health.HealthStatusUnknown, compRes.healthStatus.Status) assert.Equal(t, argoappv1.SyncStatusCodeUnknown, compRes.syncStatus.Status) @@ -655,7 +697,7 @@ func TestSetManagedResourcesKnownOrphanedResourceExceptions(t *testing.T) { ResourceNode: argoappv1.ResourceNode{ResourceRef: argoappv1.ResourceRef{Kind: kube.ServiceAccountKind, Name: "kubernetes", Namespace: app.Namespace}}, }, }, - }) + }, nil) tree, err := ctrl.setAppManagedResources(app, &comparisonResult{managedResources: make([]managedResource, 0)}) @@ -668,7 +710,7 @@ func Test_appStateManager_persistRevisionHistory(t *testing.T) { app := newFakeApp() ctrl := newFakeController(&fakeData{ apps: []runtime.Object{app}, - }) + }, nil) manager := ctrl.appStateManager.(*appStateManager) setRevisionHistoryLimit := func(value int) { i := int64(value) @@ -763,12 +805,13 @@ func TestSignedResponseNoSignatureRequired(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -789,12 +832,13 @@ func TestSignedResponseNoSignatureRequired(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -820,12 +864,13 @@ func TestSignedResponseSignatureRequired(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -846,12 +891,13 @@ func TestSignedResponseSignatureRequired(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -872,12 +918,13 @@ func TestSignedResponseSignatureRequired(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -898,12 +945,13 @@ func TestSignedResponseSignatureRequired(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -925,14 +973,15 @@ func TestSignedResponseSignatureRequired(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) testProj := signedProj testProj.Spec.SignatureKeys[0].KeyID = "4AEE18F83AFDEB24" sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes := ctrl.appStateManager.CompareAppState(app, &testProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &testProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -956,12 +1005,13 @@ func TestSignedResponseSignatureRequired(t *testing.T) { } // it doesn't matter for our test whether local manifests are valid localManifests := []string{"foobar"} - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, localManifests, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, localManifests, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeUnknown, compRes.syncStatus.Status) @@ -985,12 +1035,13 @@ func TestSignedResponseSignatureRequired(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -1014,12 +1065,13 @@ func TestSignedResponseSignatureRequired(t *testing.T) { } // it doesn't matter for our test whether local manifests are valid localManifests := []string{""} - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) sources := make([]argoappv1.ApplicationSource, 0) sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, localManifests, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, localManifests, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -1154,7 +1206,7 @@ func TestIsLiveResourceManaged(t *testing.T) { kube.GetResourceKey(unmanagedObjWrongGroup): unmanagedObjWrongGroup, kube.GetResourceKey(unmanagedObjWrongNamespace): unmanagedObjWrongNamespace, }, - }) + }, nil) manager := ctrl.appStateManager.(*appStateManager) appName := "guestbook" diff --git a/controller/sync.go b/controller/sync.go index 783183c17fc7c2..2b925b7782b9eb 100644 --- a/controller/sync.go +++ b/controller/sync.go @@ -3,6 +3,7 @@ package controller import ( "context" "encoding/json" + goerrors "errors" "fmt" "os" "strconv" @@ -152,7 +153,13 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha revisions = []string{revision} } - compareResult := m.CompareAppState(app, proj, revisions, sources, false, true, syncOp.Manifests, app.Spec.HasMultipleSources()) + // ignore error if CompareStateRepoError, this shouldn't happen as noRevisionCache is true + compareResult, err := m.CompareAppState(app, proj, revisions, sources, false, true, syncOp.Manifests, app.Spec.HasMultipleSources()) + if err != nil && !goerrors.Is(err, CompareStateRepoError) { + state.Phase = common.OperationError + state.Message = err.Error() + return + } // We now have a concrete commit SHA. Save this in the sync result revision so that we remember // what we should be syncing to when resuming operations. diff --git a/controller/sync_test.go b/controller/sync_test.go index da68e5d9a3dfe8..309f846ca64609 100644 --- a/controller/sync_test.go +++ b/controller/sync_test.go @@ -41,7 +41,7 @@ func TestPersistRevisionHistory(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) // Sync with source unspecified opState := &v1alpha1.OperationState{Operation: v1alpha1.Operation{ @@ -87,7 +87,7 @@ func TestPersistManagedNamespaceMetadataState(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) // Sync with source unspecified opState := &v1alpha1.OperationState{Operation: v1alpha1.Operation{ @@ -118,7 +118,7 @@ func TestPersistRevisionHistoryRollback(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) // Sync with source specified source := v1alpha1.ApplicationSource{ @@ -172,7 +172,7 @@ func TestSyncComparisonError(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) // Sync with source unspecified opState := &v1alpha1.OperationState{Operation: v1alpha1.Operation{ @@ -217,7 +217,7 @@ func TestAppStateManager_SyncAppState(t *testing.T) { }, managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured), } - ctrl := newFakeController(&data) + ctrl := newFakeController(&data, nil) return &fixture{ project: project, diff --git a/docs/operator-manual/argocd-cmd-params-cm.yaml b/docs/operator-manual/argocd-cmd-params-cm.yaml index f9f8b3fc14d52f..11661232f4ab5d 100644 --- a/docs/operator-manual/argocd-cmd-params-cm.yaml +++ b/docs/operator-manual/argocd-cmd-params-cm.yaml @@ -58,6 +58,8 @@ data: controller.sharding.algorithm: legacy # Number of allowed concurrent kubectl fork/execs. Any value less than 1 means no limit. controller.kubectl.parallelism.limit: "20" + # Grace period in seconds for ignoring consecutive errors while communicating with repo server. + controller.repo.error.grace.period.seconds: "180" ## Server properties # Listen on given address for incoming connections (default "0.0.0.0") diff --git a/docs/operator-manual/server-commands/argocd-application-controller.md b/docs/operator-manual/server-commands/argocd-application-controller.md index 99e4c42df28dbd..e03cf7fc515367 100644 --- a/docs/operator-manual/server-commands/argocd-application-controller.md +++ b/docs/operator-manual/server-commands/argocd-application-controller.md @@ -55,6 +55,7 @@ argocd-application-controller [flags] --redis-insecure-skip-tls-verify Skip Redis server certificate validation. --redis-use-tls Use TLS when connecting to Redis. --redisdb int Redis database. + --repo-error-grace-period-seconds int Grace period in seconds for ignoring consecutive errors while communicating with repo server. (default 180) --repo-server string Repo server address. (default "argocd-repo-server:8081") --repo-server-plaintext Disable TLS on connections to repo server --repo-server-strict-tls Whether to use strict validation of the TLS cert presented by the repo server diff --git a/manifests/base/application-controller/argocd-application-controller-statefulset.yaml b/manifests/base/application-controller/argocd-application-controller-statefulset.yaml index 33f02100a947ad..0b7a230881c8ed 100644 --- a/manifests/base/application-controller/argocd-application-controller-statefulset.yaml +++ b/manifests/base/application-controller/argocd-application-controller-statefulset.yaml @@ -35,6 +35,12 @@ spec: name: argocd-cm key: timeout.hard.reconciliation optional: true + - name: ARGOCD_REPO_ERROR_GRACE_PERIOD_SECONDS + valueFrom: + configMapKeyRef: + name: argocd-cmd-params-cm + key: controller.repo.error.grace.period.seconds + optional: true - name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER valueFrom: configMapKeyRef: diff --git a/manifests/core-install.yaml b/manifests/core-install.yaml index 69514971c03e9e..bd196b10ca374b 100644 --- a/manifests/core-install.yaml +++ b/manifests/core-install.yaml @@ -21211,6 +21211,12 @@ spec: key: timeout.hard.reconciliation name: argocd-cm optional: true + - name: ARGOCD_REPO_ERROR_GRACE_PERIOD_SECONDS + valueFrom: + configMapKeyRef: + key: controller.repo.error.grace.period.seconds + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER valueFrom: configMapKeyRef: diff --git a/manifests/ha/install.yaml b/manifests/ha/install.yaml index 8576a7d651f67a..522347b1854ea7 100644 --- a/manifests/ha/install.yaml +++ b/manifests/ha/install.yaml @@ -23014,6 +23014,12 @@ spec: key: timeout.hard.reconciliation name: argocd-cm optional: true + - name: ARGOCD_REPO_ERROR_GRACE_PERIOD_SECONDS + valueFrom: + configMapKeyRef: + key: controller.repo.error.grace.period.seconds + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER valueFrom: configMapKeyRef: diff --git a/manifests/ha/namespace-install.yaml b/manifests/ha/namespace-install.yaml index 4b33a7ceb867eb..471aac5912c046 100644 --- a/manifests/ha/namespace-install.yaml +++ b/manifests/ha/namespace-install.yaml @@ -2669,6 +2669,12 @@ spec: key: timeout.hard.reconciliation name: argocd-cm optional: true + - name: ARGOCD_REPO_ERROR_GRACE_PERIOD_SECONDS + valueFrom: + configMapKeyRef: + key: controller.repo.error.grace.period.seconds + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER valueFrom: configMapKeyRef: diff --git a/manifests/install.yaml b/manifests/install.yaml index 45d1864c556914..86f45d765f90a0 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -22058,6 +22058,12 @@ spec: key: timeout.hard.reconciliation name: argocd-cm optional: true + - name: ARGOCD_REPO_ERROR_GRACE_PERIOD_SECONDS + valueFrom: + configMapKeyRef: + key: controller.repo.error.grace.period.seconds + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER valueFrom: configMapKeyRef: diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml index 89cb5ed4ff5eda..5641d06dbe56b0 100644 --- a/manifests/namespace-install.yaml +++ b/manifests/namespace-install.yaml @@ -1713,6 +1713,12 @@ spec: key: timeout.hard.reconciliation name: argocd-cm optional: true + - name: ARGOCD_REPO_ERROR_GRACE_PERIOD_SECONDS + valueFrom: + configMapKeyRef: + key: controller.repo.error.grace.period.seconds + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_APPLICATION_CONTROLLER_REPO_SERVER valueFrom: configMapKeyRef: