Skip to content

Adds more logging and klog logging again #2279

New issue

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

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

Already on GitHub? # to your account

Closed
wants to merge 14 commits into from
Closed

Adds more logging and klog logging again #2279

wants to merge 14 commits into from

Conversation

foot
Copy link
Collaborator

@foot foot commented Jan 25, 2023

Uses weaveworks/weave-gitops#3308

This branch tries to provide a nice env for debugging OIDC token passthrough issues

  1. Try and disable all background caches that use the cluster-service SA (CRDService / HelmWatcher) to reduce noise in cluster-service logs and in logs of k8s api-server which might be handing out auth rejections
  2. Adds klog support back in
  3. Switch to weave-gitops branch with more debugging info

Recommended setup for debugging WEAVE_GITOPS_FEATURE_USE_USER_CLIENT_FOR_NAMESPACES

Assumptions

  • kubeconfig secrets will not have any credentials so the cluster-service service account won't be able to query those clusters.
# turn on debug logging so we can see principal info and what namespaces we think are available to a user
config:
  logLevel: debug

extraEnvVars:
  # Gives you every request and response (200/403) that client-go makes
  - name: KLOG_VERBOSITY
    value: "6"
  # Switch off the helm repo watcher
  - name: DEBUG_DISABLE_HELM_REPO_WATCHER
    value: "false"

The system should not be making any more requests to leaf clusters using the cluster-service SA.

Full HelmRelease / HelmRepo from recent head on this branch

apiVersion: source.toolkit.fluxcd.io/v1beta2
kind: HelmRepository
metadata:
  name: weave-gitops-enterprise-charts
  namespace: flux-system
spec:
  interval: 60m
  url: https://s3.us-east-1.amazonaws.com/weaveworks-wkp/dev/branches/oidc-debugging
---
apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  name: weave-gitops-enterprise
  namespace: flux-system
spec:
  chart:
    spec:
      interval: 65m
      chart: mccp
      sourceRef:
        kind: HelmRepository
        name: weave-gitops-enterprise-charts
        namespace: flux-system
      version: 0.15.1-1674718113-g80088767
  install:
    crds: CreateReplace
  upgrade:
    crds: CreateReplace
  interval: 50m
  values:
    tls:
      enabled: false
    config:
      logLevel: debug
      auth:
        userAccount:
          enabled: false
        tokenPassthrough:
          enabled: true
      oidc:
        enabled: true
        issuerURL: "https://dev-46517780.okta.com/oauth2/default"
        redirectURL: "http://localhost:9001/oauth2/callback"
        cookieDuration: 1h
        claimUsername: "email"
        clientCredentialsSecret: "oidc-auth"
    extraEnvVars:
      - name: KLOG_VERBOSITY
        value: "6"
      - name: WEAVE_GITOPS_FEATURE_USE_USER_CLIENT_FOR_NAMESPACES
        value: "true"
      - name: WEAVE_GITOPS_FEATURE_OIDC_AUTH_PASSTHROUGH
        value: "true"
      - name: DEBUG_DISABLE_HELM_REPO_WATCHER
        value: "true"

foot and others added 11 commits January 25, 2023 17:49
* Support enabling verbose client-go logging

- Can set via extraEnvVars in helm chart
- Allows debugging of k8s client things

* Fix linting error
- Disable background cache processes that use the cluster-service SA to
  query leaf clusters when only OIDC have access (token-passthrough)
@foot foot added the exclude from release notes Use this label to exclude a PR from the release notes label Jan 26, 2023
@foot
Copy link
Collaborator Author

foot commented Jan 26, 2023

@bigkevmcd
Copy link
Contributor

I wonder if we shouldn't use https://github.com/kubernetes/klog/tree/main/klogr as our "logr.Logger" implementation so that we have similar log-levels etc?

// v=5 - log CRD cache things?
// v=6 - log requests (e.g. GET url)
// v=7 - log req/res headers
// v=8 - log res body
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@foot
Copy link
Collaborator Author

foot commented Jan 26, 2023

I wonder if we shouldn't use https://github.com/kubernetes/klog/tree/main/klogr as our "logr.Logger" implementation so that we have similar log-levels etc?

Probably! I find 1-9 much less intuitive than debug/info/trace but thats probably a familiarity thing. But if most tooling in the ecosystem uses it then it makes more sense. Though our mapping seems to be the inverse of klog internally too https://github.com/weaveworks/weave-gitops/blob/62860b066700b92f95a57bd8de6fc7dd4d7beb9f/core/logger/logger.go#L19-L28 (smaller means more verbose here)..

We could probably still keep client-go's logging level vs cluster-service logging level at different levels?

@bigkevmcd
Copy link
Contributor

The simple way to think about "verbosity" is -vvvvvv, the more -v in there, the more verbose (you can also express this as -v=2)

The subtlety of "trace" and "debug" is hard to figure out, for example, "trace" is meant to be more granular than "debug".

@lasomethingsomething
Copy link

Is this still needed?

@foot
Copy link
Collaborator Author

foot commented Sep 1, 2023

This is pretty cool for debugging stuff.. I will try and reincarnate it.

@foot
Copy link
Collaborator Author

foot commented Oct 12, 2023

Closing for now

@foot foot closed this Oct 12, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
exclude from release notes Use this label to exclude a PR from the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants