zapier / kubechecks

Check your Kubernetes changes before they hit the cluster
https://kubechecks.readthedocs.io/en/latest/
Mozilla Public License 2.0
179 stars 16 forks source link

Diffing applications doesn't work when generated from an app-of-app-sets #78

Closed audrey-mux closed 10 months ago

audrey-mux commented 11 months ago

We're starting to use Helm directly within ArgoCD more often vs rendering them via a CMP. Are Helm rendering diffs supported?

For example

argocd app diff <cluster>-grafana-agent-operator --revision fix-name

===== /ConfigMap <cluster>-monitoring/grafana-agent-traces ======
13c13
<                 value: <cluster>1
---
>                 value: <cluster>2

Produces no changes in the kubechecks output

I tried with both the argocd matcher and the best effort matcher

djeebus commented 11 months ago

This should work; in fact it's how almost all of our apps are rendered. We delegate the manifest generation to argocd's code, then pull the live resources from argocd and run argocd's diff code ourselves.

Would you mind pasting one of your Application CRDs? You might have things configured in a way that we don't expect.

One thing to note is that we diff the live resources against the resources in the diff, meaning that if you use something like a sync window to perform the changes live, and the branch is simply recording the changes you've already made, you won't see a diff.

audrey-mux commented 11 months ago

Yep, heres the one in question. It was newly installed in this cluster and i screwed up some values in the cluster-specific file we pass.

No sync window for this one, the root app is set for auto-sync

apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: grafana-agent-operator
  namespace: argocd
spec:
  generators:
    - list:
        elements:
          - name: <cluster>
            values:
              environment: staging
  template:
    metadata:
      name: "{{name}}-grafana-agent-operator"
      labels:
      annotations:
        argocd.argoproj.io/manifest-generate-paths: .;/tools/argo/cd/apps/thirdparty/grafana-agent
    spec:
      syncPolicy:
        syncOptions:
          - RespectIgnoreDifferences=true
      destination:
        name: "{{name}}"
        namespace: "{{name}}-monitoring"
      project: "{{name}}"
      source:
        helm:
          releaseName: grafana-agent-operator
          valueFiles:
            - values.yaml
            - "../../{{name}}.yaml"
            - "../../traces-tail-sampling-policies/{{name}}.yaml"
        path: ./tools/argo/cd/apps/thirdparty/grafana-agent/helm/v0.2.15
        repoURL: <path-to-repo>
        targetRevision: main
      ignoreDifferences:
        - group: apps
          kind: Deployment
          name: otel-collector
          namespace: "{{name}}-monitoring"
          jsonPointers:
            - /spec/replicas
        - group: apps
          kind: Deployment
          name: grafana-agent-traces
          namespace: "{{name}}-monitoring"
          jsonPointers:
            - /spec/replicas
djeebus commented 11 months ago

oooh, ApplicationSet which generates Applications. in theory we support this, but in practice I'm not sure if it works correctly. This might prove that it does not =/

audrey-mux commented 11 months ago

ahh, ok. yeah, we use appsets for everything except the root apps - so we have the app-of-appsets pattern.

audrey-mux commented 11 months ago

More detail:

I had changed this instance of Kubechecks to look for a label.

When i created that grafana appset PR i forgot to include the label, so i added it after the PR was created and then i made a new commit to the values file. This did not trigger the app diff.

I made another change to the cluster values file and this time remember to add the label before creating the PR and I got the diff I was expecting. So the appset diff does work, but label order matters.

djeebus commented 10 months ago

@audrey-mux if you're still looking at this, mind checking out the latest release, v1.2.0? I think we've sorted out a bunch of these appset/branch related bugs.

audrey-mux commented 10 months ago

@audrey-mux if you're still looking at this, mind checking out the latest release, v1.2.0? I think we've sorted out a bunch of these appset/branch related bugs.

fantastic! i'll give it a shot and let you know.

audrey-mux commented 10 months ago

Rolled v1.2.0 out to my testing cluster and i see some great diff action happening! however, adding the label after creating the PR still fails with 8:11PM INF ignoring Github pull request event due to non commit based action action=labeled

one other thing that seems new, when the log is in debug mode its printing the github access token in the log

djeebus commented 10 months ago

The first issue makes sense, we try to only act when new commits are pushed to the repo. Toggling a label does seem like an easy way to trigger a new build, although we're considering adding some kind of a chat command to retrigger it as well. Closing and opening it would work as well.

The second issue is a problem though :( I'll get that sorted. Would you mind making an issue for it?

audrey-mux commented 10 months ago

done, issue #96

djeebus commented 10 months ago

Thanks! I'll close this one out.