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

Make gitops-must-gather compliant to standard must-gather structure #21

Closed
wants to merge 0 commits into from

Conversation

gmeghnag
Copy link
Contributor

@gmeghnag gmeghnag commented Apr 4, 2024

What type of PR is this?

/kind code-refactoring

What does this PR do / why we need it:
It makes gitops-must-gather compliant with the structure of the standard one to be able to parse it using omc, i.e.:

# omc use must-gather.local.7868235141633891323 
# omc get argocd,application -A 
NAMESPACE          NAME                      AGE
openshift-gitops   argocd/openshift-gitops   4h
pippo              argocd/example            2h

NAMESPACE          NAME                               SYNC STATUS   HEALTH STATUS   REVISION
openshift-gitops   application/app-spring-petclinic   Synced        Healthy         c294d23a3bb98062f96ad9e19426632f433adf40
# omc get argocd example -n pippo -o json | jq
{
  "controller": {
    "processors": {},
    "sharding": {}
  },
  "grafana": {
    "enabled": false,
    "ingress": {
      "enabled": false
    },
    "route": {
      "enabled": false
    }
  },
  "ha": {
    "enabled": false
  },
  "initialSSHKnownHosts": {},
  "monitoring": {
    "enabled": false
  },
  "notifications": {
    "enabled": false
  },
  "prometheus": {
    "enabled": false,
    "ingress": {
      "enabled": false
    },
    "route": {
      "enabled": false
    }
  },
  "rbac": {},
  "redis": {},
  "repo": {},
  "server": {
    "autoscale": {
      "enabled": false
    },
    "grpc": {
      "ingress": {
        "enabled": false
      }
    },
    "ingress": {
      "enabled": false
    },
    "route": {
      "enabled": true
    },
    "service": {
      "type": ""
    }
  },
  "tls": {
    "ca": {}
  }
}
# omc get po -n openshift-gitops
NAME                                                          READY   STATUS    RESTARTS   AGE
cluster-b6c4d4dbf-b4sz2                                       1/1     Running   0          4h
kam-8657fd448d-f4hc8                                          1/1     Running   0          4h
openshift-gitops-application-controller-0                     1/1     Running   0          4h
openshift-gitops-applicationset-controller-69fc864c68-5bwcx   1/1     Running   0          4h
openshift-gitops-dex-server-df476794-49kmb                    1/1     Running   0          4h
openshift-gitops-redis-6b4b4dc49b-ptr6r                       1/1     Running   0          4h
openshift-gitops-repo-server-5c85f7479-tfm65                  1/1     Running   0          4h
openshift-gitops-server-77df6b9f69-twjqn                      1/1     Running   0          4h

Have you updated the necessary documentation?

  • Documentation update is required by this PR.
  • Documentation has been updated.

Which issue(s) this PR fixes:
It "fixes" #20.

Test acceptance criteria:

  • Unit Test
  • E2E Test

How to test changes / Special notes to the reviewer:
You can test it by running:

oc adm must-gather --image=quay.io/gmeghnag/gitops-must-gather:latest

@gmeghnag
Copy link
Contributor Author

@reginapizza @iam-veeramalla any news about this PR merge?

gather_gitops.sh Outdated
Comment on lines 32 to 35
# Inspecting namespace reported in ARGOCD_CLUSTER_CONFIG_NAMESPACES, openshift-gitops and openshift-gitops-operator, and namespaces containing ArgoCD instances
echo "gather_gitops:$LINENO] inspecting \$ARGOCD_CLUSTER_CONFIG_NAMESPACES, openshift-gitops and openshift-gitops-operator namespaces and namespaces containing ArgoCD instances .." | tee -a ${LOGS_DIR}/gather_gitops.log
oc get ns --ignore-not-found $(oc get subs -A --ignore-not-found -o json | jq '.items[] | select(.metadata.name=="openshift-gitops-operator") | .spec.config.env[]?|select(.name=="ARGOCD_CLUSTER_CONFIG_NAMESPACES")| " " + .value | sub(","; " ")' -rj) $(oc get argocd -A -o json | jq '.items[] | " " + .metadata.namespace' -rj) openshift-gitops openshift-gitops-operator -o json \
| jq '.items | unique |.[] | .metadata.name' -r |
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more thing to add: in addition to namespaces containing ArgoCD CR, we should also include namespaces containing RolloutManager CR, and namespaces containing Rollout CR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

oc get ns --ignore-not-found $(oc get subs -A --ignore-not-found -o json | jq '.items[] | select(.metadata.name=="openshift-gitops-operator") | .spec.config.env[]?|select(.name=="ARGOCD_CLUSTER_CONFIG_NAMESPACES")| " " + .value | sub(","; " ")' -rj) $(oc get ArgoCD,Rollout,RolloutManager -A -o json | jq '.items[] | " " + .metadata.namespace' -rj) openshift-gitops openshift-gitops-operator -o json \

@reginapizza
Copy link
Contributor

@gmeghnag could you please sign off on your PR so it passes the DCO check that's failing?

@gmeghnag
Copy link
Contributor Author

gmeghnag commented May 21, 2024

I found an issue in the script, please don't merge yet. fixed, you can merge.

@reginapizza
Copy link
Contributor

closed in favor of #23

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants