From 6319d2a264b4ab94d739c10930da8c88d432787b Mon Sep 17 00:00:00 2001 From: Dan Jones Date: Tue, 2 Aug 2022 18:11:29 +0100 Subject: [PATCH 1/5] Allow custom messages to be passed if a deployment/statefulset is absent Signed-off-by: Dan Jones --- pkg/analyze/common_status.go | 61 +++++++++++++++++++++++++++---- pkg/analyze/deployment_status.go | 23 ++++++------ pkg/analyze/statefulset_status.go | 19 +++++----- 3 files changed, 75 insertions(+), 28 deletions(-) diff --git a/pkg/analyze/common_status.go b/pkg/analyze/common_status.go index 7c1e2adf..d46ea974 100644 --- a/pkg/analyze/common_status.go +++ b/pkg/analyze/common_status.go @@ -3,21 +3,31 @@ package analyzer import ( "strconv" "strings" - + "fmt" "github.com/pkg/errors" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" ) -func commonStatus(outcomes []*troubleshootv1beta2.Outcome, title, iconKey string, iconURI string, readyReplicas int) (*AnalyzeResult, error) { +func commonStatus(outcomes []*troubleshootv1beta2.Outcome, name string, iconKey string, iconURI string, readyReplicas int, exists bool, resourceType string) (*AnalyzeResult, error) { result := &AnalyzeResult{ - Title: title, + Title: fmt.Sprintf("%s Status", name), IconKey: iconKey, IconURI: iconURI, } // ordering from the spec is important, the first one that matches returns for _, outcome := range outcomes { + if outcome.Fail != nil { + + // if we're not checking that something is absent but it is, we should throw a default but meaningful error. + if exists == false && outcome.Fail.When != "absent" { + result.IsFail = true + result.Message = fmt.Sprintf("The %s %s was not found", resourceType, name) + result.URI = outcome.Fail.URI + return result, nil + } + if outcome.Fail.When == "" { result.IsFail = true result.Message = outcome.Fail.Message @@ -26,7 +36,14 @@ func commonStatus(outcomes []*troubleshootv1beta2.Outcome, title, iconKey string return result, nil } - match, err := compareActualToWhen(outcome.Fail.When, readyReplicas) + if outcome.Fail.When == "absent" && exists == false { + result.IsFail = true + result.Message = outcome.Fail.Message + result.URI = outcome.Fail.URI + return result, nil + } + + match, err := compareActualToWhen(outcome.Fail.When, readyReplicas, exists) if err != nil { return nil, errors.Wrap(err, "failed to parse fail range") } @@ -39,6 +56,14 @@ func commonStatus(outcomes []*troubleshootv1beta2.Outcome, title, iconKey string return result, nil } } else if outcome.Warn != nil { + + if exists == false && outcome.Warn.When != "absent" { + result.IsFail = true + result.Message = fmt.Sprintf("The %s %s was not found", resourceType, name) + result.URI = outcome.Fail.URI + return result, nil + } + if outcome.Warn.When == "" { result.IsWarn = true result.Message = outcome.Warn.Message @@ -47,7 +72,14 @@ func commonStatus(outcomes []*troubleshootv1beta2.Outcome, title, iconKey string return result, nil } - match, err := compareActualToWhen(outcome.Warn.When, readyReplicas) + if outcome.Warn.When == "absent" && exists == false { + result.IsWarn = true + result.Message = outcome.Warn.Message + result.URI = outcome.Warn.URI + return result, nil + } + + match, err := compareActualToWhen(outcome.Warn.When, readyReplicas, exists) if err != nil { return nil, errors.Wrap(err, "failed to parse warn range") } @@ -60,6 +92,14 @@ func commonStatus(outcomes []*troubleshootv1beta2.Outcome, title, iconKey string return result, nil } } else if outcome.Pass != nil { + + if exists == false && outcome.Pass.When != "absent" { + result.IsFail = true + result.Message = fmt.Sprintf("The %s %s was not found", resourceType, name) + result.URI = outcome.Fail.URI + return result, nil + } + if outcome.Pass.When == "" { result.IsPass = true result.Message = outcome.Pass.Message @@ -68,7 +108,14 @@ func commonStatus(outcomes []*troubleshootv1beta2.Outcome, title, iconKey string return result, nil } - match, err := compareActualToWhen(outcome.Pass.When, readyReplicas) + if outcome.Pass.When == "absent" && exists == false { + result.IsPass = true + result.Message = outcome.Pass.Message + result.URI = outcome.Pass.URI + return result, nil + } + + match, err := compareActualToWhen(outcome.Pass.When, readyReplicas, exists) if err != nil { return nil, errors.Wrap(err, "failed to parse pass range") } @@ -86,7 +133,7 @@ func commonStatus(outcomes []*troubleshootv1beta2.Outcome, title, iconKey string return result, nil } -func compareActualToWhen(when string, actual int) (bool, error) { +func compareActualToWhen(when string, actual int, exists bool) (bool, error) { parts := strings.Split(strings.TrimSpace(when), " ") // we can make this a lot more flexible diff --git a/pkg/analyze/deployment_status.go b/pkg/analyze/deployment_status.go index 9ad74c1e..e55fb9e7 100644 --- a/pkg/analyze/deployment_status.go +++ b/pkg/analyze/deployment_status.go @@ -26,6 +26,9 @@ func analyzeOneDeploymentStatus(analyzer *troubleshootv1beta2.DeploymentStatus, var result *AnalyzeResult for _, collected := range files { // only 1 file here + var exists bool = true + var readyReplicas int + var deployments appsv1.DeploymentList if err := json.Unmarshal(collected, &deployments); err != nil { return nil, errors.Wrap(err, "failed to unmarshal deployment list") @@ -39,19 +42,15 @@ func analyzeOneDeploymentStatus(analyzer *troubleshootv1beta2.DeploymentStatus, } if status == nil { - // there's not an error, but maybe the requested deployment is not even deployed - result = &AnalyzeResult{ - Title: fmt.Sprintf("%s Deployment Status", analyzer.Name), - IconKey: "kubernetes_deployment_status", - IconURI: "https://troubleshoot.sh/images/analyzer-icons/deployment-status.svg?w=17&h=17", - IsFail: true, - Message: fmt.Sprintf("The deployment %q was not found", analyzer.Name), - } + exists = false + readyReplicas = 0 } else { - result, err = commonStatus(analyzer.Outcomes, fmt.Sprintf("%s Status", analyzer.Name), "kubernetes_deployment_status", "https://troubleshoot.sh/images/analyzer-icons/deployment-status.svg?w=17&h=17", int(status.ReadyReplicas)) - if err != nil { - return nil, errors.Wrap(err, "failed to process status") - } + readyReplicas = int(status.ReadyReplicas) + } + + result, err = commonStatus(analyzer.Outcomes, analyzer.Name, "kubernetes_deployment_status", "https://troubleshoot.sh/images/analyzer-icons/deployment-status.svg?w=17&h=17", readyReplicas, exists, "deployment") + if err != nil { + return nil, errors.Wrap(err, "failed to process status") } } diff --git a/pkg/analyze/statefulset_status.go b/pkg/analyze/statefulset_status.go index 84e7600e..d8c5391f 100644 --- a/pkg/analyze/statefulset_status.go +++ b/pkg/analyze/statefulset_status.go @@ -26,6 +26,9 @@ func analyzeOneStatefulsetStatus(analyzer *troubleshootv1beta2.StatefulsetStatus var result *AnalyzeResult for _, collected := range files { // only 1 file here + var exists bool = true + var readyReplicas int + var statefulsets appsv1.StatefulSetList if err := json.Unmarshal(collected, &statefulsets); err != nil { return nil, errors.Wrap(err, "failed to unmarshal statefulset list") @@ -40,15 +43,13 @@ func analyzeOneStatefulsetStatus(analyzer *troubleshootv1beta2.StatefulsetStatus } if statefulset == nil { - result = &AnalyzeResult{ - Title: fmt.Sprintf("%s Statefulset Status", analyzer.Name), - IconKey: "kubernetes_statefulset_status", - IconURI: "https://troubleshoot.sh/images/analyzer-icons/statefulset-status.svg?w=23&h=14", - IsFail: true, - Message: fmt.Sprintf("The statefulset %q was not found", analyzer.Name), - } - } else if len(analyzer.Outcomes) > 0 { - result, err = commonStatus(analyzer.Outcomes, fmt.Sprintf("%s Status", analyzer.Name), "kubernetes_statefulset_status", "https://troubleshoot.sh/images/analyzer-icons/statefulset-status.svg?w=23&h=14", int(statefulset.Status.ReadyReplicas)) + exists = false + readyReplicas = 0 + } else { + readyReplicas = int(statefulset.Status.ReadyReplicas) + } + if len(analyzer.Outcomes) > 0 { + result, err = commonStatus(analyzer.Outcomes, analyzer.Name, "kubernetes_statefulset_status", "https://troubleshoot.sh/images/analyzer-icons/statefulset-status.svg?w=23&h=14", readyReplicas, exists, "statefulset") if err != nil { return nil, errors.Wrap(err, "failed to process status") } From 3a5b65dec44a532d28c3a0b94df9c0a139cd5ba8 Mon Sep 17 00:00:00 2001 From: Dan Jones Date: Thu, 11 Aug 2022 17:11:58 +0100 Subject: [PATCH 2/5] Add tests for absent deployments/statefulsets Signed-off-by: Dan Jones --- pkg/analyze/deployment_status_test.go | 31 ++++++++++++++++++++++++++ pkg/analyze/statefulset_status_test.go | 29 ++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/pkg/analyze/deployment_status_test.go b/pkg/analyze/deployment_status_test.go index 09bb90f2..03f5c939 100644 --- a/pkg/analyze/deployment_status_test.go +++ b/pkg/analyze/deployment_status_test.go @@ -15,6 +15,37 @@ func Test_deploymentStatus(t *testing.T) { expectResult []*AnalyzeResult files map[string][]byte }{ + { + name: "1/1, fail when absent", + analyzer: troubleshootv1beta2.DeploymentStatus{ + Outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + When: "absent", + Message: "fail", + }, + }, + }, + Namespace: "default", + Name: "nonexistant-deployment", + }, + expectResult: []*AnalyzeResult{ + { + IsPass: false, + IsWarn: false, + IsFail: true, + Title: "nonexistant-deployment Status", + Message: "fail", + IconKey: "kubernetes_deployment_status", + IconURI: "https://troubleshoot.sh/images/analyzer-icons/deployment-status.svg?w=17&h=17", + }, + }, + files: map[string][]byte{ + "cluster-resources/deployments/default.json": []byte(defaultDeployments), + "cluster-resources/deployments/monitoring.json": []byte(monitoringDeployments), + "cluster-resources/deployments/kube-system.json": []byte(kubeSystemDeployments), + }, + }, { name: "1/1, pass when = 1", analyzer: troubleshootv1beta2.DeploymentStatus{ diff --git a/pkg/analyze/statefulset_status_test.go b/pkg/analyze/statefulset_status_test.go index 78f5a00c..62886e70 100644 --- a/pkg/analyze/statefulset_status_test.go +++ b/pkg/analyze/statefulset_status_test.go @@ -15,6 +15,35 @@ func Test_analyzeStatefulsetStatus(t *testing.T) { expectResult []*AnalyzeResult files map[string][]byte }{ + { + name: "fail when absent", + analyzer: troubleshootv1beta2.StatefulsetStatus{ + Outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + When: "absent", + Message: "fail", + }, + }, + }, + Namespace: "default", + Name: "nonexistant", + }, + expectResult: []*AnalyzeResult{ + { + IsPass: false, + IsWarn: false, + IsFail: true, + Title: "nonexistant Status", + Message: "fail", + IconKey: "kubernetes_statefulset_status", + IconURI: "https://troubleshoot.sh/images/analyzer-icons/statefulset-status.svg?w=23&h=14", + }, + }, + files: map[string][]byte{ + "cluster-resources/statefulsets/default.json": []byte(defaultStatefulSets), + }, + }, { name: "analyze all statefulsets", analyzer: troubleshootv1beta2.StatefulsetStatus{}, From fa2daa70b1b1a1b5151cda81f2ddc9587e1bf41e Mon Sep 17 00:00:00 2001 From: Dan Jones Date: Fri, 12 Aug 2022 09:46:18 +0100 Subject: [PATCH 3/5] %s -> %q Signed-off-by: Dan Jones --- pkg/analyze/common_status.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/analyze/common_status.go b/pkg/analyze/common_status.go index d46ea974..bf5b4dc1 100644 --- a/pkg/analyze/common_status.go +++ b/pkg/analyze/common_status.go @@ -23,7 +23,7 @@ func commonStatus(outcomes []*troubleshootv1beta2.Outcome, name string, iconKey // if we're not checking that something is absent but it is, we should throw a default but meaningful error. if exists == false && outcome.Fail.When != "absent" { result.IsFail = true - result.Message = fmt.Sprintf("The %s %s was not found", resourceType, name) + result.Message = fmt.Sprintf("The %s %q was not found", resourceType, name) result.URI = outcome.Fail.URI return result, nil } From c3773b7f265cf8a6a62b3ee0acd29367bc2f923d Mon Sep 17 00:00:00 2001 From: Dan Jones Date: Fri, 12 Aug 2022 09:47:46 +0100 Subject: [PATCH 4/5] %s -> %q Signed-off-by: Dan Jones --- pkg/analyze/common_status.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/analyze/common_status.go b/pkg/analyze/common_status.go index bf5b4dc1..1a5e9ea0 100644 --- a/pkg/analyze/common_status.go +++ b/pkg/analyze/common_status.go @@ -59,7 +59,7 @@ func commonStatus(outcomes []*troubleshootv1beta2.Outcome, name string, iconKey if exists == false && outcome.Warn.When != "absent" { result.IsFail = true - result.Message = fmt.Sprintf("The %s %s was not found", resourceType, name) + result.Message = fmt.Sprintf("The %s %q was not found", resourceType, name) result.URI = outcome.Fail.URI return result, nil } @@ -95,7 +95,7 @@ func commonStatus(outcomes []*troubleshootv1beta2.Outcome, name string, iconKey if exists == false && outcome.Pass.When != "absent" { result.IsFail = true - result.Message = fmt.Sprintf("The %s %s was not found", resourceType, name) + result.Message = fmt.Sprintf("The %s %q was not found", resourceType, name) result.URI = outcome.Fail.URI return result, nil } From b3ee989cfeedd7f39036b1f6d955928e5f3fa7a8 Mon Sep 17 00:00:00 2001 From: Dan Jones Date: Fri, 12 Aug 2022 18:55:05 +0100 Subject: [PATCH 5/5] Add some more tests, and fix and issue where "absent" was falling through to the int comparison Signed-off-by: Dan Jones --- pkg/analyze/common_status.go | 48 ++++++++++++++++---------- pkg/analyze/deployment_status_test.go | 24 +++++++++++++ pkg/analyze/statefulset_status_test.go | 6 ++++ 3 files changed, 60 insertions(+), 18 deletions(-) diff --git a/pkg/analyze/common_status.go b/pkg/analyze/common_status.go index 1a5e9ea0..cce09d2d 100644 --- a/pkg/analyze/common_status.go +++ b/pkg/analyze/common_status.go @@ -21,7 +21,7 @@ func commonStatus(outcomes []*troubleshootv1beta2.Outcome, name string, iconKey if outcome.Fail != nil { // if we're not checking that something is absent but it is, we should throw a default but meaningful error. - if exists == false && outcome.Fail.When != "absent" { + if exists == false && outcome.Fail.When != "absent" { result.IsFail = true result.Message = fmt.Sprintf("The %s %q was not found", resourceType, name) result.URI = outcome.Fail.URI @@ -36,11 +36,15 @@ func commonStatus(outcomes []*troubleshootv1beta2.Outcome, name string, iconKey return result, nil } - if outcome.Fail.When == "absent" && exists == false { - result.IsFail = true - result.Message = outcome.Fail.Message - result.URI = outcome.Fail.URI - return result, nil + if outcome.Fail.When == "absent" { + if exists == false { + result.IsFail = true + result.Message = outcome.Fail.Message + result.URI = outcome.Fail.URI + return result, nil + } else { + continue + } } match, err := compareActualToWhen(outcome.Fail.When, readyReplicas, exists) @@ -57,7 +61,7 @@ func commonStatus(outcomes []*troubleshootv1beta2.Outcome, name string, iconKey } } else if outcome.Warn != nil { - if exists == false && outcome.Warn.When != "absent" { + if exists == false && outcome.Warn.When != "absent" { result.IsFail = true result.Message = fmt.Sprintf("The %s %q was not found", resourceType, name) result.URI = outcome.Fail.URI @@ -72,11 +76,15 @@ func commonStatus(outcomes []*troubleshootv1beta2.Outcome, name string, iconKey return result, nil } - if outcome.Warn.When == "absent" && exists == false { - result.IsWarn = true - result.Message = outcome.Warn.Message - result.URI = outcome.Warn.URI - return result, nil + if outcome.Warn.When == "absent" { + if exists == false { + result.IsWarn = true + result.Message = outcome.Warn.Message + result.URI = outcome.Warn.URI + return result, nil + } else { + continue + } } match, err := compareActualToWhen(outcome.Warn.When, readyReplicas, exists) @@ -93,7 +101,7 @@ func commonStatus(outcomes []*troubleshootv1beta2.Outcome, name string, iconKey } } else if outcome.Pass != nil { - if exists == false && outcome.Pass.When != "absent" { + if exists == false && outcome.Pass.When != "absent" { result.IsFail = true result.Message = fmt.Sprintf("The %s %q was not found", resourceType, name) result.URI = outcome.Fail.URI @@ -108,11 +116,15 @@ func commonStatus(outcomes []*troubleshootv1beta2.Outcome, name string, iconKey return result, nil } - if outcome.Pass.When == "absent" && exists == false { - result.IsPass = true - result.Message = outcome.Pass.Message - result.URI = outcome.Pass.URI - return result, nil + if outcome.Pass.When == "absent" { + if exists == false { + result.IsPass = true + result.Message = outcome.Pass.Message + result.URI = outcome.Pass.URI + return result, nil + } else { + continue + } } match, err := compareActualToWhen(outcome.Pass.When, readyReplicas, exists) diff --git a/pkg/analyze/deployment_status_test.go b/pkg/analyze/deployment_status_test.go index 03f5c939..66382082 100644 --- a/pkg/analyze/deployment_status_test.go +++ b/pkg/analyze/deployment_status_test.go @@ -25,6 +25,12 @@ func Test_deploymentStatus(t *testing.T) { Message: "fail", }, }, + { + Pass: &troubleshootv1beta2.SingleOutcome{ + When: "= 1", + Message: "pass", + }, + }, }, Namespace: "default", Name: "nonexistant-deployment", @@ -50,6 +56,12 @@ func Test_deploymentStatus(t *testing.T) { name: "1/1, pass when = 1", analyzer: troubleshootv1beta2.DeploymentStatus{ Outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + When: "absent", + Message: "fail", + }, + }, { Pass: &troubleshootv1beta2.SingleOutcome{ When: "= 1", @@ -86,6 +98,12 @@ func Test_deploymentStatus(t *testing.T) { name: "1/1, pass when = 2", analyzer: troubleshootv1beta2.DeploymentStatus{ Outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + When: "absent", + Message: "fail", + }, + }, { Pass: &troubleshootv1beta2.SingleOutcome{ When: "= 2", @@ -122,6 +140,12 @@ func Test_deploymentStatus(t *testing.T) { name: "1/1, pass when >= 2, warn when = 1, fail when 0", analyzer: troubleshootv1beta2.DeploymentStatus{ Outcomes: []*troubleshootv1beta2.Outcome{ + { + Fail: &troubleshootv1beta2.SingleOutcome{ + When: "absent", + Message: "fail", + }, + }, { Pass: &troubleshootv1beta2.SingleOutcome{ When: ">= 2", diff --git a/pkg/analyze/statefulset_status_test.go b/pkg/analyze/statefulset_status_test.go index 62886e70..2a088776 100644 --- a/pkg/analyze/statefulset_status_test.go +++ b/pkg/analyze/statefulset_status_test.go @@ -25,6 +25,12 @@ func Test_analyzeStatefulsetStatus(t *testing.T) { Message: "fail", }, }, + { + Pass: &troubleshootv1beta2.SingleOutcome{ + When: "= 1", + Message: "pass", + }, + }, }, Namespace: "default", Name: "nonexistant",