From 20547431d49a6fc973a6904cd123b9108079d05d Mon Sep 17 00:00:00 2001 From: Rahul Date: Thu, 13 Jun 2024 16:40:59 +0530 Subject: [PATCH] chore: merge 24.05.2 content (#2984) * fix: fixing non-exported flexgroup instances error * fix: Harvest should log errors when grafana import fails (#2962) Thanks to Chris Gautcher (@gautcher) for raising. * ci: bump go (#2965) * chore: merge 24.05.2 content * fix: Rest templates should not have hyphon (#2943) * fix: Rest templates should not have hyphon * fix: address review comments * chore: fix ci --------- Co-authored-by: hardikl Co-authored-by: Chris Grindstaff --- .harvest.env | 2 +- .../restperf/plugins/volume/volume.go | 18 +++--------------- .../zapiperf/plugins/volume/volume.go | 19 +++---------------- cmd/tools/grafana/grafana.go | 15 +++++++++------ cmd/tools/template/template_test.go | 16 ++++++++++++++++ conf/rest/9.12.0/volume.yaml | 2 +- integration/test/dashboard_json_test.go | 5 +++++ integration/test/dashboard_test.go | 2 +- integration/test/grafana/grafana_mgr.go | 8 ++------ integration/test/utils/utils.go | 2 +- 10 files changed, 42 insertions(+), 47 deletions(-) diff --git a/.harvest.env b/.harvest.env index f410f80d8..f02398a86 100644 --- a/.harvest.env +++ b/.harvest.env @@ -1 +1 @@ -GO_VERSION=1.22.3 \ No newline at end of file +GO_VERSION=1.22.4 \ No newline at end of file diff --git a/cmd/collectors/restperf/plugins/volume/volume.go b/cmd/collectors/restperf/plugins/volume/volume.go index 0da357585..b23c94cd5 100644 --- a/cmd/collectors/restperf/plugins/volume/volume.go +++ b/cmd/collectors/restperf/plugins/volume/volume.go @@ -52,7 +52,6 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, *util fgAggrMap := make(map[string]*set.Set) flexgroupAggrsMap := make(map[string]*set.Set) - nonExportedInstanceMap := make(map[string]bool) // volume_aggr_labels metric is deprecated now and will be removed later. metricName := "labels" @@ -69,11 +68,7 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, *util cache.UUID += ".Volume" // create flexgroup instance cache - for iKey, i := range data.GetInstances() { - if !i.IsExportable() { - nonExportedInstanceMap[iKey] = true - continue - } + for _, i := range data.GetInstances() { if match := re.FindStringSubmatch(i.GetLabel("volume")); len(match) == 3 { // instance key is svm.flexgroup-volume key := i.GetLabel("svm") + "." + match[1] @@ -102,10 +97,7 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, *util fgAggrMap[key].Add(i.GetLabel("aggr")) flexgroupAggrsMap[key].Add(i.GetLabel("aggr")) i.SetLabel(style, "flexgroup_constituent") - if !v.includeConstituents { - i.SetExportable(false) - nonExportedInstanceMap[iKey] = true - } + i.SetExportable(v.includeConstituents) } else { i.SetLabel(style, "flexvol") key := i.GetLabel("svm") + "." + i.GetLabel("volume") @@ -127,11 +119,7 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, *util // cache.Reset() // create summary - for iKey, i := range data.GetInstances() { - if nonExportedInstanceMap[iKey] { - continue - } - + for _, i := range data.GetInstances() { match := re.FindStringSubmatch(i.GetLabel("volume")) if len(match) != 3 { continue diff --git a/cmd/collectors/zapiperf/plugins/volume/volume.go b/cmd/collectors/zapiperf/plugins/volume/volume.go index 1acebf80d..acc3ff7e3 100644 --- a/cmd/collectors/zapiperf/plugins/volume/volume.go +++ b/cmd/collectors/zapiperf/plugins/volume/volume.go @@ -60,7 +60,6 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, *util fgAggrMap := make(map[string]*set.Set) flexgroupAggrsMap := make(map[string]*set.Set) - nonExportedInstanceMap := make(map[string]bool) // volume_aggr_labels metric is deprecated now and will be removed later. metricName := "labels" @@ -77,11 +76,7 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, *util cache.UUID += ".Volume" // create flexgroup instance cache - for iKey, i := range data.GetInstances() { - if !i.IsExportable() { - nonExportedInstanceMap[iKey] = true - continue - } + for _, i := range data.GetInstances() { if match := re.FindStringSubmatch(i.GetLabel("volume")); len(match) == 3 { // instance key is svm.flexgroup-volume key := i.GetLabel("svm") + "." + match[1] @@ -110,10 +105,7 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, *util fgAggrMap[key].Add(i.GetLabel("aggr")) flexgroupAggrsMap[key].Add(i.GetLabel("aggr")) i.SetLabel(style, "flexgroup_constituent") - if !v.includeConstituents { - i.SetExportable(false) - nonExportedInstanceMap[iKey] = true - } + i.SetExportable(v.includeConstituents) } else { i.SetLabel(style, "flexvol") key := i.GetLabel("svm") + "." + i.GetLabel("volume") @@ -128,7 +120,6 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, *util v.Logger.Error().Err(err).Str("metric", metricName).Msg("Unable to set value on metric") } } - } v.Logger.Debug().Msgf("extracted %d flexgroup volumes", len(cache.GetInstances())) @@ -136,11 +127,7 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, *util // cache.Reset() // create summary - for iKey, i := range data.GetInstances() { - if nonExportedInstanceMap[iKey] { - continue - } - + for _, i := range data.GetInstances() { match := re.FindStringSubmatch(i.GetLabel("volume")) if len(match) != 3 { continue diff --git a/cmd/tools/grafana/grafana.go b/cmd/tools/grafana/grafana.go index d2744ff7d..02306dc00 100644 --- a/cmd/tools/grafana/grafana.go +++ b/cmd/tools/grafana/grafana.go @@ -364,7 +364,7 @@ func doImport(_ *cobra.Command, _ []string) { opts.command = "import" _, err := conf.LoadHarvestConfig(opts.config) if err != nil { - return + printErrorAndExit(err) } adjustOptions() @@ -376,6 +376,11 @@ func doImport(_ *cobra.Command, _ []string) { importDashboards(opts) } +func printErrorAndExit(err error) { + fmt.Println(err) + os.Exit(1) +} + func validateImport() { // default case if opts.dir == "" && opts.serverfolder.name == "" { @@ -401,7 +406,7 @@ func initImportVars() { // default behaviour if opts.dir == "grafana/dashboards" && opts.serverfolder.name == "" { m[filepath.Join(opts.dir, "cmode")] = &Folder{name: "Harvest-" + harvestRelease + "-cDOT"} - m[filepath.Join(opts.dir, "cmode", "details")] = &Folder{name: "Harvest-" + harvestRelease + "-cDOT Details"} + m[filepath.Join(opts.dir, "cmode-details")] = &Folder{name: "Harvest-" + harvestRelease + "-cDOT Details"} m[filepath.Join(opts.dir, "7mode")] = &Folder{name: "Harvest-" + harvestRelease + "-7mode"} m[filepath.Join(opts.dir, "storagegrid")] = &Folder{name: "Harvest-" + harvestRelease + "-StorageGrid"} } else if opts.dir != "" && opts.serverfolder.name != "" { @@ -412,8 +417,7 @@ func initImportVars() { if opts.customizeDir == "" { err := checkAndCreateServerFolder(v) if err != nil { - fmt.Print(err) - os.Exit(1) + printErrorAndExit(err) } } opts.dirGrafanaFolderMap[k] = v @@ -469,8 +473,7 @@ func importFiles(dir string, folder *Folder) { return } if files, err = os.ReadDir(dir); err != nil { - // TODO check for not exist - return + printErrorAndExit(err) } for _, file := range files { diff --git a/cmd/tools/template/template_test.go b/cmd/tools/template/template_test.go index 193d4a166..46bba578c 100644 --- a/cmd/tools/template/template_test.go +++ b/cmd/tools/template/template_test.go @@ -345,6 +345,22 @@ func TestOverrideMetricsExist(t *testing.T) { }, allTemplatesButEms...) } +func TestNoHyphenInMetrics(t *testing.T) { + visitTemplates(t, func(path string, model Model) { + isRest := strings.Contains(path, "rest") + + if !isRest { + return + } + + for _, m := range model.metrics { + if strings.Contains(m.left, "-") || strings.Contains(m.right, "-") { + t.Errorf("Metric fields 'left' and 'right' should not contain hyphens. Found in path=%s, left=%s, right=%s", shortPath(path), m.left, m.right) + } + } + }, allTemplatesButEms...) +} + type sorted struct { got string want string diff --git a/conf/rest/9.12.0/volume.yaml b/conf/rest/9.12.0/volume.yaml index 30ffc39d5..e9ecc3df1 100644 --- a/conf/rest/9.12.0/volume.yaml +++ b/conf/rest/9.12.0/volume.yaml @@ -30,7 +30,7 @@ counters: - files => inode_files_total - files_used => inode_files_used - filesystem_size => filesystem_size - - logical-available => space_logical_available + - logical_available => space_logical_available - logical_used => space_logical_used - logical_used_by_afs => space_logical_used_by_afs - logical_used_by_snapshots => space_logical_used_by_snapshots diff --git a/integration/test/dashboard_json_test.go b/integration/test/dashboard_json_test.go index a1987c5e0..ba3fe9095 100644 --- a/integration/test/dashboard_json_test.go +++ b/integration/test/dashboard_json_test.go @@ -39,6 +39,11 @@ var zapiCounterMap = map[string]struct{}{ "volume_arw_status": {}, "volume_num_compress_fail": {}, "volume_num_compress_attempts": {}, + // sar is experiencing high api time for ZapiPerf. The u2 cluster does not have fabricpool added for the collection of these counters. Remove the following once sar is capable of running ZapiPerf. + "volume_performance_tier_footprint": {}, + "volume_capacity_tier_footprint": {}, + "volume_capacity_tier_footprint_percent": {}, + "volume_performance_tier_footprint_percent": {}, } // restCounterMap are additional counters, above and beyond the ones from counterMap, which should be excluded from Rest diff --git a/integration/test/dashboard_test.go b/integration/test/dashboard_test.go index cf9b304cc..4ccba0f17 100644 --- a/integration/test/dashboard_test.go +++ b/integration/test/dashboard_test.go @@ -40,7 +40,7 @@ func TestGrafanaAndPrometheusAreConfigured(t *testing.T) { cDotFolder = "Harvest-" + version.VERSION + "-cDOT" sevenModeFolder = "Harvest-" + version.VERSION + "-7mode" log.Info().Str("cMode", cDotFolder).Str("7mode", sevenModeFolder).Msg("Folder name details") - status, out := new(grafana.Mgr).Import("") // send empty so that it will import all dashboards + status, out := new(grafana.Mgr).Import() if !status { t.Errorf("Grafana import operation failed out=%s", out) } diff --git a/integration/test/grafana/grafana_mgr.go b/integration/test/grafana/grafana_mgr.go index 2ce0dfaff..de379f780 100644 --- a/integration/test/grafana/grafana_mgr.go +++ b/integration/test/grafana/grafana_mgr.go @@ -12,7 +12,7 @@ import ( type Mgr struct { } -func (g *Mgr) Import(jsonDir string) (bool, string) { +func (g *Mgr) Import() (bool, string) { var ( importOutput string status bool @@ -31,15 +31,11 @@ func (g *Mgr) Import(jsonDir string) (bool, string) { if err != nil { panic(err) } - directoryOption := "" - if jsonDir != "" { - directoryOption = "--directory" - } grafanaURL := utils.GetGrafanaURL() if docker.IsDockerBasedPoller() { grafanaURL = "grafana:3000" } - importCmds := []string{"grafana", "import", "--overwrite", "--addr", grafanaURL, directoryOption, jsonDir} + importCmds := []string{"grafana", "import", "--overwrite", "--addr", grafanaURL} if docker.IsDockerBasedPoller() { params := []string{"exec", containerIDs[0].ID, "bin/harvest"} params = append(params, importCmds...) diff --git a/integration/test/utils/utils.go b/integration/test/utils/utils.go index 609d52851..3fe265fbf 100644 --- a/integration/test/utils/utils.go +++ b/integration/test/utils/utils.go @@ -184,7 +184,7 @@ func IsURLReachable(url string) bool { return false } defer response.Body.Close() - return response.StatusCode == 200 + return response.StatusCode == http.StatusOK } func AddPrometheusToGrafana() {