From 806b2fbb4195e753cd1d2f6152a47ba0bf0f1245 Mon Sep 17 00:00:00 2001 From: Marc Campbell Date: Wed, 18 Dec 2019 17:30:16 -0800 Subject: [PATCH] Revert "Regex analyzer" --- cmd/preflight/cli/run.go | 2 +- .../preflight/postgres-connection-valid.yaml | 24 --- pkg/analyze/analyzer.go | 6 - pkg/analyze/regex.go | 166 ------------------ pkg/analyze/regex_test.go | 57 ------ .../troubleshoot/v1beta1/analyzer_shared.go | 8 - .../troubleshoot/v1beta1/collector_shared.go | 3 + .../v1beta1/zz_generated.deepcopy.go | 32 ---- pkg/collect/copy.go | 5 +- pkg/collect/exec.go | 5 +- pkg/collect/run.go | 2 +- 11 files changed, 13 insertions(+), 297 deletions(-) delete mode 100644 examples/preflight/postgres-connection-valid.yaml delete mode 100644 pkg/analyze/regex.go delete mode 100644 pkg/analyze/regex_test.go diff --git a/cmd/preflight/cli/run.go b/cmd/preflight/cli/run.go index a653d7beb..a5f9f89ac 100644 --- a/cmd/preflight/cli/run.go +++ b/cmd/preflight/cli/run.go @@ -60,7 +60,7 @@ func runPreflights(v *viper.Viper, arg string) error { preflight := troubleshootv1beta1.Preflight{} if err := yaml.Unmarshal([]byte(preflightContent), &preflight); err != nil { - return fmt.Errorf("unable to parse %s as a preflight, error: %s", arg, err.Error()) + return fmt.Errorf("unable to parse %s as a preflight", arg) } s := spin.New() diff --git a/examples/preflight/postgres-connection-valid.yaml b/examples/preflight/postgres-connection-valid.yaml deleted file mode 100644 index 5a95796c7..000000000 --- a/examples/preflight/postgres-connection-valid.yaml +++ /dev/null @@ -1,24 +0,0 @@ -apiVersion: troubleshoot.replicated.com/v1beta1 -kind: Preflight -metadata: - name: postgres-connection-valid -spec: - collectors: - - run: - collectorName: "run-ping" - image: busybox:1 - namespace: default - command: [ping] - args: [-w, "5", www.google.com] - imagePullPolicy: IfNotPresent - analyzers: - - regex: - collectorName: "run-ping" - checkName: "Ping google with low packet loss?" - expression: "(?P\\d+) packets? transmitted, (?P\\d+) packets? received, (?P\\d+\\.?\\d+)% packet loss" - outcomes: - - pass: - when: "Loss < 20" - message: "Solid connection to google.com" - - fail: - message: "Really high packet loss" diff --git a/pkg/analyze/analyzer.go b/pkg/analyze/analyzer.go index 95503c3c9..bb71c6607 100644 --- a/pkg/analyze/analyzer.go +++ b/pkg/analyze/analyzer.go @@ -79,12 +79,6 @@ func Analyze(analyzer *troubleshootv1beta1.Analyze, getFile getCollectedFileCont } return analyzeDistribution(analyzer.Distribution, getFile) } - if analyzer.RegEx != nil { - if analyzer.RegEx.Exclude { - return nil, nil - } - return analyzeRegex(analyzer.RegEx, getFile) - } return nil, errors.New("invalid analyzer") } diff --git a/pkg/analyze/regex.go b/pkg/analyze/regex.go deleted file mode 100644 index 50d6c81e0..000000000 --- a/pkg/analyze/regex.go +++ /dev/null @@ -1,166 +0,0 @@ -package analyzer - -import ( - "fmt" - "path" - "regexp" - "strconv" - "strings" - - "github.com/pkg/errors" - troubleshootv1beta1 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta1" -) - -func analyzeRegex(analyzer *troubleshootv1beta1.RegEx, getCollectedFileContents func(string) ([]byte, error)) (*AnalyzeResult, error) { - contents, err := getCollectedFileContents(path.Join(analyzer.CollectorName, analyzer.CollectorName+".txt")) - if err != nil { - return nil, errors.Wrap(err, "failed to get file contents for regex") - } - - expression := regexp.MustCompile(analyzer.Expression) - match := expression.FindStringSubmatch(string(contents)) - - result := &AnalyzeResult{ - Title: analyzer.CheckName, - } - // to avoid empty strings in the UI..,. - if result.Title == "" { - result.Title = analyzer.CollectorName - } - - foundMatches := map[string]string{} - for i, name := range expression.SubexpNames() { - if i != 0 && name != "" { - foundMatches[name] = match[i] - } - } - - // allow fallthrough - for _, outcome := range analyzer.Outcomes { - if outcome.Fail != nil { - if outcome.Fail.When == "" { - result.IsFail = true - result.Message = outcome.Fail.Message - result.URI = outcome.Fail.URI - - return result, nil - } - - isMatch, err := compareRegex(outcome.Fail.When, foundMatches) - if err != nil { - return result, errors.Wrap(err, "failed to compare regex fail conditional") - } - - if isMatch { - result.IsFail = true - result.Message = outcome.Fail.Message - result.URI = outcome.Fail.URI - - return result, nil - } - } else if outcome.Warn != nil { - if outcome.Warn.When == "" { - result.IsWarn = true - result.Message = outcome.Warn.Message - result.URI = outcome.Warn.URI - - return result, nil - } - - isMatch, err := compareRegex(outcome.Warn.When, foundMatches) - if err != nil { - return result, errors.Wrap(err, "failed to compare regex warn conditional") - } - - if isMatch { - result.IsWarn = true - result.Message = outcome.Warn.Message - result.URI = outcome.Warn.URI - - return result, nil - } - } else if outcome.Pass != nil { - if outcome.Pass.When == "" { - result.IsPass = true - result.Message = outcome.Pass.Message - result.URI = outcome.Pass.URI - - return result, nil - } - - isMatch, err := compareRegex(outcome.Pass.When, foundMatches) - if err != nil { - return result, errors.Wrap(err, "failed to compare regex pass conditional") - } - - if isMatch { - result.IsPass = true - result.Message = outcome.Pass.Message - result.URI = outcome.Pass.URI - - return result, nil - } - } - } - - return result, nil -} - -func compareRegex(conditional string, foundMatches map[string]string) (bool, error) { - parts := strings.Split(strings.TrimSpace(conditional), " ") - - if len(parts) != 3 { - return false, errors.New("unable to parse regex conditional") - } - - lookForMatchName := parts[0] - operator := parts[1] - lookForValue := parts[2] - - foundValue, ok := foundMatches[lookForMatchName] - if !ok { - // not an error, just wasn't matched - return false, nil - } - - // if the value side of the conditional is an int, we assume it's an int - lookForValueInt, err := strconv.Atoi(lookForValue) - if err == nil { - fmt.Printf("look for = %s, found = %s\n", lookForValue, foundValue) - foundValueInt, err := strconv.Atoi(foundValue) - if err != nil { - // not an error but maybe it should be... - return false, nil - } - - switch operator { - case "=": - fallthrough - case "==": - fallthrough - case "===": - return lookForValueInt == foundValueInt, nil - - case "<": - return lookForValueInt < foundValueInt, nil - - case ">": - return lookForValueInt > foundValueInt, nil - - case "<=": - return lookForValueInt <= foundValueInt, nil - - case ">=": - return lookForValueInt >= foundValueInt, nil - } - } else { - // all we can support is "=" and "==" and "===" for now - if operator != "=" && operator != "==" && operator != "===" { - return false, errors.New("unexpected operator in regex comparator") - } - - return foundValue == lookForValue, nil - } - - return false, nil -} diff --git a/pkg/analyze/regex_test.go b/pkg/analyze/regex_test.go deleted file mode 100644 index 48cae730b..000000000 --- a/pkg/analyze/regex_test.go +++ /dev/null @@ -1,57 +0,0 @@ -package analyzer - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func Test_compareRegex(t *testing.T) { - tests := []struct { - name string - conditional string - foundMatches map[string]string - expected bool - }{ - { - name: "Loss < 5", - conditional: "Loss < 5", - foundMatches: map[string]string{ - "Transmitted": "5", - "Received": "4", - "Loss": "20", - }, - expected: true, - }, - { - name: "Hostname = icecream", - conditional: "Hostname = icecream", - foundMatches: map[string]string{ - "Something": "5", - "Hostname": "icecream", - }, - expected: true, - }, - { - name: "Day >= 23", - conditional: "Day >= 23", - foundMatches: map[string]string{ - "day": "5", - "Day": "24", - }, - expected: false, - }, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - req := require.New(t) - - actual, err := compareRegex(test.conditional, test.foundMatches) - req.NoError(err) - - assert.Equal(t, test.expected, actual) - }) - } - -} diff --git a/pkg/apis/troubleshoot/v1beta1/analyzer_shared.go b/pkg/apis/troubleshoot/v1beta1/analyzer_shared.go index 3389bc039..29b6b9552 100644 --- a/pkg/apis/troubleshoot/v1beta1/analyzer_shared.go +++ b/pkg/apis/troubleshoot/v1beta1/analyzer_shared.go @@ -74,13 +74,6 @@ type Distribution struct { Outcomes []*Outcome `json:"outcomes" yaml:"outcomes"` } -type RegEx struct { - AnalyzeMeta `json:",inline" yaml:",inline"` - CollectorName string `json:"collectorName" yaml:"collectorName"` - Expression string `json:"expression" yaml:"expression"` - Outcomes []*Outcome `json:"outcomes" yaml:"outcomes"` -} - type AnalyzeMeta struct { CheckName string `json:"checkName,omitempty" yaml:"checkName,omitempty"` Exclude bool `json:"exclude,omitempty" yaml:"exclude,omitempty"` @@ -97,5 +90,4 @@ type Analyze struct { StatefulsetStatus *StatefulsetStatus `json:"statefulsetStatus,omitempty" yaml:"statefulsetStatus,omitempty"` ContainerRuntime *ContainerRuntime `json:"containerRuntime,omitempty" yaml:"containerRuntime,omitempty"` Distribution *Distribution `json:"distribution,omitempty" yaml:"distribution,omitempty"` - RegEx *RegEx `json:"regex,omitempty" yaml:"regex,omitempty"` } diff --git a/pkg/apis/troubleshoot/v1beta1/collector_shared.go b/pkg/apis/troubleshoot/v1beta1/collector_shared.go index 568c3b5f7..e437a0566 100644 --- a/pkg/apis/troubleshoot/v1beta1/collector_shared.go +++ b/pkg/apis/troubleshoot/v1beta1/collector_shared.go @@ -43,6 +43,7 @@ type Data struct { type Run struct { CollectorMeta `json:",inline" yaml:",inline"` + Name string `json:"name,omitempty" yaml:"name,omitempty"` Namespace string `json:"namespace" yaml:"namespace"` Image string `json:"image" yaml:"image"` Command []string `json:"command,omitempty" yaml:"command,omitempty"` @@ -53,6 +54,7 @@ type Run struct { type Exec struct { CollectorMeta `json:",inline" yaml:",inline"` + Name string `json:"name,omitempty" yaml:"name,omitempty"` Selector []string `json:"selector" yaml:"selector"` Namespace string `json:"namespace" yaml:"namespace"` ContainerName string `json:"containerName,omitempty" yaml:"containerName,omitempty"` @@ -63,6 +65,7 @@ type Exec struct { type Copy struct { CollectorMeta `json:",inline" yaml:",inline"` + Name string `json:"name,omitempty" yaml:"name,omitempty"` Selector []string `json:"selector" yaml:"selector"` Namespace string `json:"namespace" yaml:"namespace"` ContainerPath string `json:"containerPath" yaml:"containerPath"` diff --git a/pkg/apis/troubleshoot/v1beta1/zz_generated.deepcopy.go b/pkg/apis/troubleshoot/v1beta1/zz_generated.deepcopy.go index 04c281da9..6d6455ceb 100644 --- a/pkg/apis/troubleshoot/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/troubleshoot/v1beta1/zz_generated.deepcopy.go @@ -102,11 +102,6 @@ func (in *Analyze) DeepCopyInto(out *Analyze) { *out = new(Distribution) (*in).DeepCopyInto(*out) } - if in.RegEx != nil { - in, out := &in.RegEx, &out.RegEx - *out = new(RegEx) - (*in).DeepCopyInto(*out) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Analyze. @@ -1364,33 +1359,6 @@ func (in *Put) DeepCopy() *Put { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *RegEx) DeepCopyInto(out *RegEx) { - *out = *in - out.AnalyzeMeta = in.AnalyzeMeta - if in.Outcomes != nil { - in, out := &in.Outcomes, &out.Outcomes - *out = make([]*Outcome, len(*in)) - for i := range *in { - if (*in)[i] != nil { - in, out := &(*in)[i], &(*out)[i] - *out = new(Outcome) - (*in).DeepCopyInto(*out) - } - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RegEx. -func (in *RegEx) DeepCopy() *RegEx { - if in == nil { - return nil - } - out := new(RegEx) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResultRequest) DeepCopyInto(out *ResultRequest) { *out = *in diff --git a/pkg/collect/copy.go b/pkg/collect/copy.go index f5734d376..f39678992 100644 --- a/pkg/collect/copy.go +++ b/pkg/collect/copy.go @@ -34,7 +34,7 @@ func Copy(ctx *Context, copyCollector *troubleshootv1beta1.Copy) ([]byte, error) if len(pods) > 0 { for _, pod := range pods { - bundlePath := filepath.Join(copyCollector.CollectorName, pod.Namespace, pod.Name) + bundlePath := filepath.Join(copyCollector.Name, pod.Namespace, pod.Name) files, copyErrors := copyFiles(ctx, client, pod, copyCollector) if len(copyErrors) > 0 { @@ -136,6 +136,9 @@ func (c CopyOutput) Redact() (CopyOutput, error) { } func getCopyErrosFileName(copyCollector *troubleshootv1beta1.Copy) string { + if len(copyCollector.Name) > 0 { + return fmt.Sprintf("%s-errors.json", copyCollector.Name) + } if len(copyCollector.CollectorName) > 0 { return fmt.Sprintf("%s-errors.json", copyCollector.CollectorName) } diff --git a/pkg/collect/exec.go b/pkg/collect/exec.go index 275653837..a375973a6 100644 --- a/pkg/collect/exec.go +++ b/pkg/collect/exec.go @@ -70,7 +70,7 @@ func execWithoutTimeout(ctx *Context, execCollector *troubleshootv1beta1.Exec) ( for _, pod := range pods { stdout, stderr, execErrors := getExecOutputs(ctx, client, pod, execCollector) - bundlePath := filepath.Join(execCollector.CollectorName, pod.Namespace, pod.Name) + bundlePath := filepath.Join(execCollector.Name, pod.Namespace, pod.Name) if len(stdout) > 0 { execOutput[filepath.Join(bundlePath, execCollector.CollectorName+"-stdout.txt")] = stdout } @@ -158,6 +158,9 @@ func (r ExecOutput) Redact() (ExecOutput, error) { } func getExecErrosFileName(execCollector *troubleshootv1beta1.Exec) string { + if len(execCollector.Name) > 0 { + return fmt.Sprintf("%s-errors.json", execCollector.Name) + } if len(execCollector.CollectorName) > 0 { return fmt.Sprintf("%s-errors.json", execCollector.CollectorName) } diff --git a/pkg/collect/run.go b/pkg/collect/run.go index ecc781029..6c341fb06 100644 --- a/pkg/collect/run.go +++ b/pkg/collect/run.go @@ -85,7 +85,7 @@ func runWithoutTimeout(ctx *Context, pod *corev1.Pod, runCollector *troubleshoot limits := troubleshootv1beta1.LogLimits{ MaxLines: 10000, } - podLogs, err := getPodLogs(client, *pod, runCollector.CollectorName, "", &limits, true) + podLogs, err := getPodLogs(client, *pod, runCollector.Name, "", &limits, true) for k, v := range podLogs { runOutput[k] = v