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

WIP: feat(lint): add troubleshoot lint #1532

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

DexterYan
Copy link
Member

@DexterYan DexterYan commented Apr 26, 2024

Description, Motivation and Context

  • introduce a new troubleshoot command: troubleshoot lint
  • if detect a wrong format troubleshoot yaml like
apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
  name: default
spec:
  collectors:
    - helm:
        releaseName: mysql-1692919203
        namespace: "default"
  analyzers:
    - distribution:
      outcomes:
        - pass:
            when: "== kurl"
            message: kurl is a supported platform
        - pass:
            when: "== gke"
            message: GKE is a supported platform

It will return a error and tips

distribution is empty
outcomes is misaligned in distribution
Error: Warning: Wrong analyzer found

Screenshot:
Screenshot 2024-04-26 at 6 31 10 PM

In a more complicated case like

apiVersion: troubleshoot.sh/v1beta2
kind: SupportBundle
metadata:
  name: default
spec:
  collectors:
    - helm:
        releaseName: mysql-1692919203
        namespace: "default"
  analyzers:
    - distribution:
      outcomes:
        - pass:
            when: "== kurl"
            message: kurl is a supported platform
        - pass:
            when: "== gke"
            message: GKE is a supported platform
    - textAnalyze:
      checkName: Replica Count
      fileName: config/replicas.txt
      regexGroups: '(?P<Replicas>\d+)'
      outcomes:
        - fail:
            when: "Replicas < 5"
            message: That's not enough replicas!
        - pass:
            message: You have at least 5 replicas

it will return

====================
distribution analyzer is empty
--------------------
outcomes is misaligned in distribution
====================
textAnalyze analyzer is empty
--------------------
checkName is misaligned in textAnalyze
fileName is misaligned in textAnalyze
regexGroups is misaligned in textAnalyze
outcomes is misaligned in textAnalyze
Error: Warning: Wrong analyzer found
Screenshot 2024-04-26 at 11 17 49 PM

sc-103245

Checklist

  • New and existing tests pass locally with introduced changes.
  • Tests for the changes have been added (for bug fixes / features)
  • The commit message(s) are informative and highlight any breaking changes
  • Any documentation required has been added/updated. For changes to https://troubleshoot.sh/ create a PR here

Does this PR introduce a breaking change?

  • Yes
  • No

@DexterYan DexterYan requested a review from a team as a code owner April 26, 2024 06:23
@DexterYan DexterYan marked this pull request as draft April 26, 2024 06:23
@DexterYan DexterYan added the type::feature New feature or request label Apr 26, 2024
@@ -38,6 +42,38 @@ import (

func runTroubleshoot(v *viper.Viper, args []string) error {
ctx := context.Background()
if len(args) == 1 && args[0] == "lint" {
Copy link
Member

Choose a reason for hiding this comment

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

I assume you implemented this subcommand this way cause its a WIP PR, right?

I would have expected lint to be a cobra subcommand

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it is a WIP PR for showing what lint could be like. Sure, I will put it into cmd/lint as a cobra subcommand


analyzeFieldsInLowerCamelCase := strings.ToLower(strings.Join(analyzerFields, " "))

for _, n := range node.Content[0].Content { // Traverse the root map
Copy link
Member

Choose a reason for hiding this comment

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

The idea you have with validating and reporting is correct, I'm however not sure we want to implement this feature by parsing yaml and inspecting fields like this. We'll be reinventing the wheel. There are a number of libraries that already exist that utilize yaml/json/openapi spec annotations. I've listed a few in #871 issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

great! I will have a look. In the meanwhile, I am checking the product team to see what their opinions. We can discuss the output of this command together.

@DexterYan DexterYan force-pushed the dx/sc-103245/add-troubleshoot-lint branch from 36f4fd6 to 0cb775a Compare May 2, 2024 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type::feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants