From 3082107acae1e1708d4fafc4bdd70d4bdf4ba1af Mon Sep 17 00:00:00 2001 From: Akash Shrivastava Date: Thu, 14 Dec 2023 14:22:11 +0530 Subject: [PATCH 1/4] feat: [ISSUE-1401]: Added helm get values option in Helm collector Signed-off-by: Akash Shrivastava --- config/crds/troubleshoot.sh_collectors.yaml | 2 ++ config/crds/troubleshoot.sh_preflights.yaml | 2 ++ .../crds/troubleshoot.sh_supportbundles.yaml | 2 ++ .../troubleshoot/v1beta2/collector_shared.go | 1 + pkg/collect/helm.go | 34 ++++++++++++++----- schemas/collector-troubleshoot-v1beta2.json | 3 ++ schemas/preflight-troubleshoot-v1beta2.json | 3 ++ .../supportbundle-troubleshoot-v1beta2.json | 3 ++ 8 files changed, 41 insertions(+), 9 deletions(-) diff --git a/config/crds/troubleshoot.sh_collectors.yaml b/config/crds/troubleshoot.sh_collectors.yaml index db95296be..3bb537752 100644 --- a/config/crds/troubleshoot.sh_collectors.yaml +++ b/config/crds/troubleshoot.sh_collectors.yaml @@ -349,6 +349,8 @@ spec: type: object helm: properties: + collectValues: + type: boolean collectorName: type: string exclude: diff --git a/config/crds/troubleshoot.sh_preflights.yaml b/config/crds/troubleshoot.sh_preflights.yaml index b653b0816..9c0744e26 100644 --- a/config/crds/troubleshoot.sh_preflights.yaml +++ b/config/crds/troubleshoot.sh_preflights.yaml @@ -1910,6 +1910,8 @@ spec: type: object helm: properties: + collectValues: + type: boolean collectorName: type: string exclude: diff --git a/config/crds/troubleshoot.sh_supportbundles.yaml b/config/crds/troubleshoot.sh_supportbundles.yaml index e809b8f6b..a18c0a1af 100644 --- a/config/crds/troubleshoot.sh_supportbundles.yaml +++ b/config/crds/troubleshoot.sh_supportbundles.yaml @@ -1941,6 +1941,8 @@ spec: type: object helm: properties: + collectValues: + type: boolean collectorName: type: string exclude: diff --git a/pkg/apis/troubleshoot/v1beta2/collector_shared.go b/pkg/apis/troubleshoot/v1beta2/collector_shared.go index 4c16c74f1..408bfb44f 100644 --- a/pkg/apis/troubleshoot/v1beta2/collector_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/collector_shared.go @@ -257,6 +257,7 @@ type Helm struct { CollectorMeta `json:",inline" yaml:",inline"` Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"` ReleaseName string `json:"releaseName,omitempty" yaml:"releaseName,omitempty"` + CollectValues bool `json:"collectValues,omitempty" yaml:"collectValues,omitempty"` } type Goldpinger struct { diff --git a/pkg/collect/helm.go b/pkg/collect/helm.go index c886c1120..9f95f8c09 100644 --- a/pkg/collect/helm.go +++ b/pkg/collect/helm.go @@ -39,10 +39,11 @@ type ReleaseInfo struct { // Helm release version information struct type VersionInfo struct { - Revision string `json:"revision"` - Date string `json:"date"` - Status string `json:"status"` - IsPending bool `json:"isPending,omitempty"` + Revision string `json:"revision"` + Date string `json:"date"` + Status string `json:"status"` + IsPending bool `json:"isPending,omitempty"` + Values map[string]interface{} `json:"values,omitempty"` } func (c *CollectHelm) Title() string { @@ -57,7 +58,7 @@ func (c *CollectHelm) Collect(progressChan chan<- interface{}) (CollectorResult, output := NewResult() - releaseInfos, err := helmReleaseHistoryCollector(c.Collector.ReleaseName, c.Collector.Namespace) + releaseInfos, err := helmReleaseHistoryCollector(c.Collector.ReleaseName, c.Collector.Namespace, c.Collector.CollectValues) if err != nil { return nil, errors.Wrap(err, "failed to get Helm release history") } @@ -72,6 +73,9 @@ func (c *CollectHelm) Collect(progressChan chan<- interface{}) (CollectorResult, } filePath := fmt.Sprintf("helm/%s.json", namespace) + if c.Collector.ReleaseName != "" { + filePath = fmt.Sprintf("helm/%s/%s.json", namespace, c.Collector.ReleaseName) + } err := output.SaveResult(c.BundlePath, filePath, bytes.NewBuffer(helmHistoryJson)) if err != nil { @@ -82,7 +86,7 @@ func (c *CollectHelm) Collect(progressChan chan<- interface{}) (CollectorResult, return output, nil } -func helmReleaseHistoryCollector(releaseName string, namespace string) ([]ReleaseInfo, error) { +func helmReleaseHistoryCollector(releaseName string, namespace string, collectValues bool) ([]ReleaseInfo, error) { var results []ReleaseInfo actionConfig := new(action.Configuration) @@ -103,7 +107,7 @@ func helmReleaseHistoryCollector(releaseName string, namespace string) ([]Releas ChartVersion: r.Chart.Metadata.Version, AppVersion: r.Chart.Metadata.AppVersion, Namespace: r.Namespace, - VersionInfo: getVersionInfo(actionConfig, releaseName), + VersionInfo: getVersionInfo(actionConfig, releaseName, collectValues), }) return results, nil } @@ -124,26 +128,31 @@ func helmReleaseHistoryCollector(releaseName string, namespace string) ([]Releas ChartVersion: r.Chart.Metadata.Version, AppVersion: r.Chart.Metadata.AppVersion, Namespace: r.Namespace, - VersionInfo: getVersionInfo(actionConfig, r.Name), + VersionInfo: getVersionInfo(actionConfig, r.Name, collectValues), }) } return results, nil } -func getVersionInfo(actionConfig *action.Configuration, releaseName string) []VersionInfo { +func getVersionInfo(actionConfig *action.Configuration, releaseName string, collectValues bool) []VersionInfo { versionCollect := []VersionInfo{} history, _ := action.NewHistory(actionConfig).Run(releaseName) for _, release := range history { + values := map[string]interface{}{} + if collectValues { + values = getHelmValues(actionConfig, releaseName, release.Version) + } versionCollect = append(versionCollect, VersionInfo{ Revision: strconv.Itoa(release.Version), Date: release.Info.LastDeployed.String(), Status: release.Info.Status.String(), IsPending: release.Info.Status.IsPending(), + Values: values, }) } return versionCollect @@ -158,3 +167,10 @@ func helmReleaseInfoByNamespaces(releaseInfo []ReleaseInfo) map[string][]Release return releaseInfoByNamespace } + +func getHelmValues(actionConfig *action.Configuration, releaseName string, revision int) map[string]interface{} { + getAction := action.NewGetValues(actionConfig) + getAction.Version = revision + helmValues, _ := getAction.Run(releaseName) + return helmValues +} diff --git a/schemas/collector-troubleshoot-v1beta2.json b/schemas/collector-troubleshoot-v1beta2.json index 247496a92..1a571499e 100644 --- a/schemas/collector-troubleshoot-v1beta2.json +++ b/schemas/collector-troubleshoot-v1beta2.json @@ -483,6 +483,9 @@ "helm": { "type": "object", "properties": { + "collectValues": { + "type": "boolean" + }, "collectorName": { "type": "string" }, diff --git a/schemas/preflight-troubleshoot-v1beta2.json b/schemas/preflight-troubleshoot-v1beta2.json index 851a7c625..7b9000d10 100644 --- a/schemas/preflight-troubleshoot-v1beta2.json +++ b/schemas/preflight-troubleshoot-v1beta2.json @@ -2888,6 +2888,9 @@ "helm": { "type": "object", "properties": { + "collectValues": { + "type": "boolean" + }, "collectorName": { "type": "string" }, diff --git a/schemas/supportbundle-troubleshoot-v1beta2.json b/schemas/supportbundle-troubleshoot-v1beta2.json index 43b92b520..5d4f0a13c 100644 --- a/schemas/supportbundle-troubleshoot-v1beta2.json +++ b/schemas/supportbundle-troubleshoot-v1beta2.json @@ -2934,6 +2934,9 @@ "helm": { "type": "object", "properties": { + "collectValues": { + "type": "boolean" + }, "collectorName": { "type": "string" }, From b9e8112abed47b0342ff144555fe86a8c20bb8b1 Mon Sep 17 00:00:00 2001 From: Akash Shrivastava Date: Mon, 8 Jan 2024 18:39:19 +0530 Subject: [PATCH 2/4] Made changes in error handling Signed-off-by: Akash Shrivastava --- pkg/collect/helm.go | 61 +++++++++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 22 deletions(-) diff --git a/pkg/collect/helm.go b/pkg/collect/helm.go index 9f95f8c09..8a9f13142 100644 --- a/pkg/collect/helm.go +++ b/pkg/collect/helm.go @@ -5,12 +5,10 @@ import ( "context" "encoding/json" "fmt" - "log" "strconv" "github.com/pkg/errors" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" - "helm.sh/helm/v3/pkg/action" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -60,7 +58,9 @@ func (c *CollectHelm) Collect(progressChan chan<- interface{}) (CollectorResult, releaseInfos, err := helmReleaseHistoryCollector(c.Collector.ReleaseName, c.Collector.Namespace, c.Collector.CollectValues) if err != nil { - return nil, errors.Wrap(err, "failed to get Helm release history") + output.SaveResult(c.BundlePath, "helm/errors.json", marshalErrors(err)) + klog.Error(err) + return nil, nil } releaseInfoByNamespace := helmReleaseInfoByNamespaces(releaseInfos) @@ -77,21 +77,19 @@ func (c *CollectHelm) Collect(progressChan chan<- interface{}) (CollectorResult, filePath = fmt.Sprintf("helm/%s/%s.json", namespace, c.Collector.ReleaseName) } - err := output.SaveResult(c.BundlePath, filePath, bytes.NewBuffer(helmHistoryJson)) - if err != nil { - return nil, err - } + output.SaveResult(c.BundlePath, filePath, bytes.NewBuffer(helmHistoryJson)) } return output, nil } -func helmReleaseHistoryCollector(releaseName string, namespace string, collectValues bool) ([]ReleaseInfo, error) { +func helmReleaseHistoryCollector(releaseName string, namespace string, collectValues bool) ([]ReleaseInfo, []error) { var results []ReleaseInfo + error_list := []error{} actionConfig := new(action.Configuration) if err := actionConfig.Init(nil, namespace, "", klog.V(2).Infof); err != nil { - return nil, errors.Wrap(err, "failed to initialize Helm action config") + return nil, []error{err} } // If releaseName is specified, get the history of that release @@ -99,17 +97,18 @@ func helmReleaseHistoryCollector(releaseName string, namespace string, collectVa getAction := action.NewGet(actionConfig) r, err := getAction.Run(releaseName) if err != nil { - return nil, errors.Wrapf(err, "failed to get Helm release %s", releaseName) + return nil, []error{err} } + versionInfo, err := getVersionInfo(actionConfig, r.Name, collectValues) results = append(results, ReleaseInfo{ ReleaseName: r.Name, Chart: r.Chart.Metadata.Name, ChartVersion: r.Chart.Metadata.Version, AppVersion: r.Chart.Metadata.AppVersion, Namespace: r.Namespace, - VersionInfo: getVersionInfo(actionConfig, releaseName, collectValues), + VersionInfo: versionInfo, }) - return results, nil + return results, []error{err} } // If releaseName is not specified, get the history of all releases @@ -118,33 +117,46 @@ func helmReleaseHistoryCollector(releaseName string, namespace string, collectVa listAction := action.NewList(actionConfig) releases, err := listAction.Run() if err != nil { - log.Fatalf("Failed to list Helm releases: %v", err) + return nil, []error{err} } for _, r := range releases { + versionInfo, err := getVersionInfo(actionConfig, r.Name, collectValues) + if err != nil { + error_list = append(error_list, err) + } results = append(results, ReleaseInfo{ ReleaseName: r.Name, Chart: r.Chart.Metadata.Name, ChartVersion: r.Chart.Metadata.Version, AppVersion: r.Chart.Metadata.AppVersion, Namespace: r.Namespace, - VersionInfo: getVersionInfo(actionConfig, r.Name, collectValues), + VersionInfo: versionInfo, }) } - + if len(error_list) > 0 { + return nil, error_list + } return results, nil } -func getVersionInfo(actionConfig *action.Configuration, releaseName string, collectValues bool) []VersionInfo { +func getVersionInfo(actionConfig *action.Configuration, releaseName string, collectValues bool) ([]VersionInfo, error) { versionCollect := []VersionInfo{} + error_list := []error{} - history, _ := action.NewHistory(actionConfig).Run(releaseName) + history, err := action.NewHistory(actionConfig).Run(releaseName) + if err != nil { + return nil, err + } for _, release := range history { values := map[string]interface{}{} if collectValues { - values = getHelmValues(actionConfig, releaseName, release.Version) + values, err = getHelmValues(actionConfig, releaseName, release.Version) + if err != nil { + error_list = append(error_list, err) + } } versionCollect = append(versionCollect, VersionInfo{ @@ -155,7 +167,11 @@ func getVersionInfo(actionConfig *action.Configuration, releaseName string, coll Values: values, }) } - return versionCollect + if len(error_list) > 0 { + errs, _ := json.MarshalIndent(error_list, "", " ") + return nil, errors.New(string(errs)) + } + return versionCollect, nil } func helmReleaseInfoByNamespaces(releaseInfo []ReleaseInfo) map[string][]ReleaseInfo { @@ -168,9 +184,10 @@ func helmReleaseInfoByNamespaces(releaseInfo []ReleaseInfo) map[string][]Release return releaseInfoByNamespace } -func getHelmValues(actionConfig *action.Configuration, releaseName string, revision int) map[string]interface{} { +func getHelmValues(actionConfig *action.Configuration, releaseName string, revision int) (map[string]interface{}, error) { getAction := action.NewGetValues(actionConfig) + getAction.AllValues = true getAction.Version = revision - helmValues, _ := getAction.Run(releaseName) - return helmValues + helmValues, err := getAction.Run(releaseName) + return helmValues, err } From cbed2c2a7a570479509212aca9c24d3a6f9c53aa Mon Sep 17 00:00:00 2001 From: Akash Shrivastava Date: Thu, 11 Jan 2024 21:31:57 +0530 Subject: [PATCH 3/4] feat: [ISSUE-1401]: Added test cases and fixes Signed-off-by: Akash Shrivastava --- pkg/collect/helm.go | 22 +++++++++++++------ .../support-bundle/helm_collector_e2e_test.go | 3 ++- test/e2e/support-bundle/spec/helm.yaml | 4 +++- .../support-bundle/testdata/helm-values.yaml | 4 ++++ 4 files changed, 24 insertions(+), 9 deletions(-) create mode 100644 test/e2e/support-bundle/testdata/helm-values.yaml diff --git a/pkg/collect/helm.go b/pkg/collect/helm.go index 8a9f13142..b9a463d30 100644 --- a/pkg/collect/helm.go +++ b/pkg/collect/helm.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "strconv" + "strings" "github.com/pkg/errors" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" @@ -99,7 +100,7 @@ func helmReleaseHistoryCollector(releaseName string, namespace string, collectVa if err != nil { return nil, []error{err} } - versionInfo, err := getVersionInfo(actionConfig, r.Name, collectValues) + versionInfo, err := getVersionInfo(actionConfig, r.Name, r.Namespace, collectValues) results = append(results, ReleaseInfo{ ReleaseName: r.Name, Chart: r.Chart.Metadata.Name, @@ -121,7 +122,7 @@ func helmReleaseHistoryCollector(releaseName string, namespace string, collectVa } for _, r := range releases { - versionInfo, err := getVersionInfo(actionConfig, r.Name, collectValues) + versionInfo, err := getVersionInfo(actionConfig, r.Name, r.Namespace, collectValues) if err != nil { error_list = append(error_list, err) } @@ -140,7 +141,7 @@ func helmReleaseHistoryCollector(releaseName string, namespace string, collectVa return results, nil } -func getVersionInfo(actionConfig *action.Configuration, releaseName string, collectValues bool) ([]VersionInfo, error) { +func getVersionInfo(actionConfig *action.Configuration, releaseName, namespace string, collectValues bool) ([]VersionInfo, error) { versionCollect := []VersionInfo{} error_list := []error{} @@ -153,7 +154,7 @@ func getVersionInfo(actionConfig *action.Configuration, releaseName string, coll for _, release := range history { values := map[string]interface{}{} if collectValues { - values, err = getHelmValues(actionConfig, releaseName, release.Version) + values, err = getHelmValues(releaseName, namespace, release.Version) if err != nil { error_list = append(error_list, err) } @@ -168,8 +169,11 @@ func getVersionInfo(actionConfig *action.Configuration, releaseName string, coll }) } if len(error_list) > 0 { - errs, _ := json.MarshalIndent(error_list, "", " ") - return nil, errors.New(string(errs)) + errs := []string{} + for _, e := range error_list { + errs = append(errs, e.Error()) + } + return nil, errors.New(strings.Join(errs, "\n")) } return versionCollect, nil } @@ -184,7 +188,11 @@ func helmReleaseInfoByNamespaces(releaseInfo []ReleaseInfo) map[string][]Release return releaseInfoByNamespace } -func getHelmValues(actionConfig *action.Configuration, releaseName string, revision int) (map[string]interface{}, error) { +func getHelmValues(releaseName, namespace string, revision int) (map[string]interface{}, error) { + actionConfig := new(action.Configuration) + if err := actionConfig.Init(nil, namespace, "", klog.V(2).Infof); err != nil { + return nil, err + } getAction := action.NewGetValues(actionConfig) getAction.AllValues = true getAction.Version = revision diff --git a/test/e2e/support-bundle/helm_collector_e2e_test.go b/test/e2e/support-bundle/helm_collector_e2e_test.go index 0475a574b..cdf67af9e 100644 --- a/test/e2e/support-bundle/helm_collector_e2e_test.go +++ b/test/e2e/support-bundle/helm_collector_e2e_test.go @@ -27,7 +27,7 @@ func Test_HelmCollector(t *testing.T) { Setup(func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { cluster := getClusterFromContext(t, ctx, ClusterName) manager := helm.New(cluster.GetKubeconfig()) - manager.RunInstall(helm.WithName(releaseName), helm.WithNamespace(c.Namespace()), helm.WithChart(filepath.Join(curDir, "testdata/charts/nginx-15.2.0.tgz")), helm.WithWait(), helm.WithTimeout("1m")) + manager.RunInstall(helm.WithName(releaseName), helm.WithNamespace(c.Namespace()), helm.WithChart(filepath.Join(curDir, "testdata/charts/nginx-15.2.0.tgz")), helm.WithArgs("-f "+filepath.Join(curDir, "testdata/helm-values.yaml")), helm.WithWait(), helm.WithTimeout("1m")) //ignore error to allow test to speed up, helm collector will catch the pending or deployed helm release status return ctx }). @@ -66,6 +66,7 @@ func Test_HelmCollector(t *testing.T) { assert.Equal(t, 1, len(results)) assert.Equal(t, releaseName, results[0].ReleaseName) assert.Equal(t, "nginx", results[0].Chart) + assert.Equal(t, []interface{}([]interface{}{map[string]interface{}{"name": "TEST_ENV_VAR", "value": "test-value"}}), results[0].VersionInfo[0].Values["extraEnvVars"]) return ctx }). Teardown(func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { diff --git a/test/e2e/support-bundle/spec/helm.yaml b/test/e2e/support-bundle/spec/helm.yaml index 3f22529a3..c272fd4e5 100644 --- a/test/e2e/support-bundle/spec/helm.yaml +++ b/test/e2e/support-bundle/spec/helm.yaml @@ -1,7 +1,9 @@ +--- apiVersion: troubleshoot.sh/v1beta2 kind: SupportBundle metadata: name: default spec: collectors: - - helm: {} + - helm: + collectValues: true diff --git a/test/e2e/support-bundle/testdata/helm-values.yaml b/test/e2e/support-bundle/testdata/helm-values.yaml new file mode 100644 index 000000000..c5c62d0d1 --- /dev/null +++ b/test/e2e/support-bundle/testdata/helm-values.yaml @@ -0,0 +1,4 @@ +--- +extraEnvVars: + - name: TEST_ENV_VAR + value: "test-value" From 2e15632a90cb4edc5dca02974f832c8486c70a4b Mon Sep 17 00:00:00 2001 From: Akash Shrivastava Date: Sat, 13 Jan 2024 02:10:26 +0530 Subject: [PATCH 4/4] fixed test case Signed-off-by: Akash Shrivastava --- pkg/collect/helm.go | 10 +++++++--- test/e2e/support-bundle/helm_collector_e2e_test.go | 4 ++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/collect/helm.go b/pkg/collect/helm.go index b9a463d30..4bf03c59f 100644 --- a/pkg/collect/helm.go +++ b/pkg/collect/helm.go @@ -59,9 +59,13 @@ func (c *CollectHelm) Collect(progressChan chan<- interface{}) (CollectorResult, releaseInfos, err := helmReleaseHistoryCollector(c.Collector.ReleaseName, c.Collector.Namespace, c.Collector.CollectValues) if err != nil { - output.SaveResult(c.BundlePath, "helm/errors.json", marshalErrors(err)) - klog.Error(err) - return nil, nil + errsToMarhsal := []string{} + for _, e := range err { + errsToMarhsal = append(errsToMarhsal, e.Error()) + } + output.SaveResult(c.BundlePath, "helm/errors.json", marshalErrors(errsToMarhsal)) + klog.Errorf("error collecting helm release info: %v", err) + return output, nil } releaseInfoByNamespace := helmReleaseInfoByNamespaces(releaseInfos) diff --git a/test/e2e/support-bundle/helm_collector_e2e_test.go b/test/e2e/support-bundle/helm_collector_e2e_test.go index cdf67af9e..acd4b49cc 100644 --- a/test/e2e/support-bundle/helm_collector_e2e_test.go +++ b/test/e2e/support-bundle/helm_collector_e2e_test.go @@ -62,11 +62,11 @@ func Test_HelmCollector(t *testing.T) { if err != nil { t.Fatal(err) } - assert.Equal(t, 1, len(results)) assert.Equal(t, releaseName, results[0].ReleaseName) assert.Equal(t, "nginx", results[0].Chart) - assert.Equal(t, []interface{}([]interface{}{map[string]interface{}{"name": "TEST_ENV_VAR", "value": "test-value"}}), results[0].VersionInfo[0].Values["extraEnvVars"]) + assert.Equal(t, map[string]interface{}{"name": "TEST_ENV_VAR", "value": "test-value"}, results[0].VersionInfo[0].Values["extraEnvVars"].([]interface{})[0]) + assert.Equal(t, "1.25.2-debian-11-r3", results[0].VersionInfo[0].Values["image"].(map[string]interface{})["tag"]) return ctx }). Teardown(func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context {