Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Endless Sync Loop when installing Helm Chart via ArgoCD #4114

Closed
jstewart612 opened this issue Jun 18, 2021 · 22 comments
Closed

Endless Sync Loop when installing Helm Chart via ArgoCD #4114

jstewart612 opened this issue Jun 18, 2021 · 22 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@jstewart612
Copy link

jstewart612 commented Jun 18, 2021

Describe the bug:
Helm chart's ValidatingWebhookConfiguration is missing a configuration section which Kubernetes adds in on Azure Kubernetes Service to every ValidatingWebhookConfiguration which causes ArgoCD to place the Helm chart into a constant sync loop and never become healthy.

Expected behaviour:
This ValidatingWebhookConfiguration

Steps to reproduce the bug:
Install this Application CRD on your cluster running ArgoCD 2.0.3 on an Azure Kubernetes Service cluster:

---
apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  finalizers:
    - resources-finalizer.argocd.argoproj.io
  name: cert-manager
spec:
  project: default
  destination:
    namespace: cert-manager
    server: 'https://kubernetes.default.svc'
  syncPolicy:
    automated:
      prune: true
      selfHeal: true
  source:
    chart: cert-manager
    repoURL: 'https://charts.jetstack.io'
    targetRevision: v1.4.0
    helm:
      releaseName: cert-manager
      values: |+
        global:
          logLevel: 2
        ingressShim:
          defaultIssuerName: letsencrypt
          defaultIssuerKind: ClusterIssuer
        installCRDs: true
        prometheus:
          enabled: true
          servicemonitor:
            enabled: true

Once you do, this section will repeatedly want to delete webhooks[0].namespaceSelector.matchExpressions[2]:

- key: control-plane
  operator: DoesNotExist

Anything else we need to know?:
Nope.

Environment details::

  • Kubernetes version: 1.20.7
  • Cloud-provider/provisioner: Azure Kubernetes Service
  • cert-manager version: v1.4.0
  • Install method: e.g. helm/static manifests: ArgoCD 2.0.3 using above Application CRD instance.

/kind bug

@jetstack-bot jetstack-bot added the kind/bug Categorizes issue or PR as related to a bug. label Jun 18, 2021
@jstewart612
Copy link
Author

Untitled

@jetstack-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle stale

@jetstack-bot jetstack-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 16, 2021
@jetstack-bot
Copy link
Contributor

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle rotten
/remove-lifecycle stale

@jetstack-bot jetstack-bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 16, 2021
@jemag
Copy link

jemag commented Oct 29, 2021

Had the same problem after adding it to argocd using kustomize. In my case, I added the following patch:

patchesJson6902:
- target:
    group: admissionregistration.k8s.io
    version: v1
    kind: ValidatingWebhookConfiguration
    name: cert-manager-webhook
  patch: |-
    - op: add
      path: /webhooks/0/namespaceSelector/matchExpressions/-
      value:
        key: control-plane
        operator: DoesNotExist

@jetstack-bot
Copy link
Contributor

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

@jetstack-bot
Copy link
Contributor

@jetstack-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@wallrj
Copy link
Member

wallrj commented Dec 3, 2021

/reopen
/remove-lifecycle rotten

@jetstack-bot jetstack-bot reopened this Dec 3, 2021
@jetstack-bot
Copy link
Contributor

@wallrj: Reopened this issue.

In response to this:

/reopen
/remove-lifecycle rotten

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@jetstack-bot jetstack-bot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Dec 3, 2021
@wallrj
Copy link
Member

wallrj commented Dec 3, 2021

@jstewart612 Thanks for the PR and apologies for not replying sooner.
The cert-manager Helm chart has grown a number of very specific parameters, such as this, over the years
and it is difficult for us to test them all.
If at all possible we're trying to avoid adding more parameters.

Would you be able to work around this some other way?
For example by using the kustomize based work around from @jemag in #4114 (comment) (above).

Or could you use any of the work arounds described in:

@hijmen
Copy link

hijmen commented Jan 8, 2022

I worked around this using ArgoCD IgnoreDifferences. This way you won't have to change the cert-manager resources. See the ArgoCD Docs for more information

ignoreDifferences:
  - group: admissionregistration.k8s.io
    kind: ValidatingWebhookConfiguration
    name: cert-manager-webhook
    jqPathExpressions:
      - .webhooks[].namespaceSelector.matchExpressions[] | select(.key == "control-plane")

edit: On 23 September 2023 it looks like Microsoft Azure (AKS) added an extra matchExpressions item with the key kubernetes.azure.com/managedby. We changed above ignoreDifferences to the following, which resolved the issue for us:

ignoreDifferences:
  - group: admissionregistration.k8s.io
    kind: ValidatingWebhookConfiguration
    name: cert-manager-webhook
    jqPathExpressions:
      - .webhooks[].namespaceSelector.matchExpressions[] | select(.key == "control-plane")
      - .webhooks[].namespaceSelector.matchExpressions[] | select(.key == "kubernetes.azure.com/managedby")

@jsoref
Copy link
Contributor

jsoref commented Feb 3, 2022

It'd be nice if #4114 (comment) was documented in the installation instructions somewhere. -- I'd consider that a "fix' for this.

Alternatively, perhaps we can just get this fixed in ArgoCD:
argoproj/argo-cd#4276 (comment)

@jetstack-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle stale

@jetstack-bot jetstack-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 4, 2022
@tete17
Copy link

tete17 commented Jun 2, 2022

I found another alternative.

In their documentation AKS & microsoft explain you can disable the Admissions Enforcer that is adding that matchExpression with either a label or an annotation.

I simply dropped this into my helm configuration

webhook:
  validatingWebhookConfigurationAnnotations:
    admissions.enforcer/disabled: "true"

And the diff is gone. I know AKS doesn't recommend you doing this since it can affect protected namespaces but given this webhook is configured to only affect cert-manager CRD's and not vanilla k8s resources this is a better solution for people like me who uses ApplicationAets and changing one single app is not feasible.

@jetstack-bot
Copy link
Contributor

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle rotten
/remove-lifecycle stale

@jetstack-bot jetstack-bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 2, 2022
@jetstack-bot
Copy link
Contributor

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

@jetstack-bot
Copy link
Contributor

@jetstack-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ams0 pushed a commit to joaquinrz/capi-gitops that referenced this issue Apr 15, 2023
ams0 pushed a commit to joaquinrz/capi-gitops that referenced this issue Apr 20, 2023
* argocd self-manage

* infra name

* policy

* root-on root

* name

* argocd app and values

* exclude values

* exclude

* multi sources

* root path for values

* ref

* syncOptions

* root-app-infra

* root-app-infra no

* move values

* allow resource in proj

* better excl

* new path

* rm values

* * in exclude

* argocd name

* application.namespaces: "*"

* ingress-nginx-values.yaml

* ext-dns different ns

* - CreateNamespace=true

* ns

* ns

* ext-dns chart

* conf extdns

* parms for ext-dns

* annotation nginx

* no id

* v1.11.1cert-manage

* cluster-issuer

* cert-manager-issuers

* thanos

* monitoring

* objstoreConfig

* healthz

* grafana

* auth

* prom

* - ServerSideApply=true

* cert-manager/cert-manager#4114

* port

* 19291

* multicluster

* Main Org.

* dash1

* no need for app for dash

* exclude dashes from root

* dash name

* dash app

* ns

* remove app

* dash2

* all mixin dashes

* rename

* newline

* 2

* more dah

* fix some dash

* more dasj

* more dash

* pvv dash

* pv

* last dash

* fixes

* kyverno

* https://kyverno.github.io/kyverno/

* kyverno2

* replace true

* policy

* exclude

* appset

* workload

* tabs

* description

* infra proj

* typo

* proj

* (

* argocd ingress

* typo

* ingressClassName

* vcluster helm chart

* path

* sources

* url

* https://charts.loft.sh

* policy vc*

* cluster name

* test

* fixed dahs

* single source for vcluster

* prom on workload

* move to appset

* proper prom

* proj

* ns

* path

* no node-expo

* - ServerSideApply=true

* remove clusters

* vc34

* better prome

* nodeExporter:
  enabled: false

* 5-6

* 123

* use argocd-app

* disable argo self-manage

* re-enable

* replace=true

* - ServerSideApply=true

* annotations:
    argocd.argoproj.io/sync-wave: "5"

* disabled

* move

* mapServices

* prometheus-sample-app

* nane

* pullpolicy

* docker pull ams0/prometheus-sample-app

* capi-operator

* only capi-op

* -

* root-app-infra exclude

* ns

* ns2

* repo

* cluster-api-operator

* operato

* ignore diff

* ingorediff

* InfrastructureProvider

* test

* ''

* 1

* ""

* no infra

* azure

* X

* comment

* no core

* capi machinepool

* wave

* different strategy

* spaces

* disabled capi-operator

* gitops/management/argocd/argocd-values.yaml

* gitignore

* readme

* readme

* k8sis.fun

* k8sis.fun

* Added imdb app

* capz policy

* readme

* policy for capz cluster

* no precondition

* replace branch

* helm req

* capz appset

---------

Co-authored-by: Alessandro Vozza <alessandro.vozza@microsoft.com>
Co-authored-by: Joaquin Rodriguez <rjoaquin@microsoft.com>
joachimprinzbach added a commit to joachimprinzbach/aks-infra-argocd-cluster-apps that referenced this issue Aug 1, 2023
@wallrj
Copy link
Member

wallrj commented Nov 24, 2023

/reopen

We should document the work around for this in:

xref: cert-manager/website#320

@jetstack-bot jetstack-bot reopened this Nov 24, 2023
@jetstack-bot
Copy link
Contributor

@wallrj: Reopened this issue.

In response to this:

/reopen

We should document the work around for this in:

xref: cert-manager/website#320

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@shahkv95
Copy link

/reopen

We should document the work around for this in:

xref: cert-manager/website#320

Hi @wallrj
I would like to take this issue and update the existing document with the workaround for cd with argocd.
Could you pls assign the issue to me?

Thanks

@shahkv95
Copy link

@wallrj
requesting for a review for the PR: cert-manager/website#1350

@jetstack-bot
Copy link
Contributor

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

@jetstack-bot
Copy link
Contributor

@jetstack-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment