diff --git a/cmd/collectors/ontap.go b/cmd/collectors/ontap.go index 05c313f61..7d592d942 100644 --- a/cmd/collectors/ontap.go +++ b/cmd/collectors/ontap.go @@ -13,23 +13,26 @@ import ( func GatherClusterInfo(pollerName string, cred *auth.Credentials) (conf.Remote, error) { - var ( - err error - ) + remoteZapi, errZapi := checkZapi(pollerName, cred) + remoteRest, errRest := checkRest(pollerName, cred) - remoteZapi, err := checkZapi(pollerName, cred) - if err != nil { - return conf.Remote{}, err + return MergeRemotes(remoteZapi, remoteRest, errZapi, errRest) +} + +func MergeRemotes(remoteZapi conf.Remote, remoteRest conf.Remote, errZapi error, errRest error) (conf.Remote, error) { + err := errors.Join(errZapi, errRest) + + remoteRest.ZAPIsExist = remoteZapi.ZAPIsExist + + if errZapi != nil { + return remoteRest, err } - remoteRest, err := checkRest(pollerName, cred) - if err != nil { + if errRest != nil { return remoteZapi, err } - remoteRest.ZAPIsExist = remoteZapi.ZAPIsExist - - return remoteRest, nil + return remoteRest, err } func checkRest(pollerName string, cred *auth.Credentials) (conf.Remote, error) { @@ -92,7 +95,8 @@ func checkZapi(pollerName string, cred *auth.Credentials) (conf.Remote, error) { } if returnErr { - return conf.Remote{}, err + // Assume that ZAPIs exist so we don't upgrade ZAPI to REST when there is an error + return conf.Remote{ZAPIsExist: true}, err } } diff --git a/cmd/poller/poller.go b/cmd/poller/poller.go index a4ec90903..5d3731db5 100644 --- a/cmd/poller/poller.go +++ b/cmd/poller/poller.go @@ -301,7 +301,7 @@ func (p *Poller) Init() error { return errs.New(errs.ErrNoCollector, "no collectors") } - p.negotiateONTAPAPI(filteredCollectors, collectors.GatherClusterInfo) + p.negotiateONTAPAPI(filteredCollectors) objectsToCollectors := make(map[string][]objectCollector) for _, c := range filteredCollectors { @@ -711,7 +711,11 @@ func (p *Poller) readObjects(c conf.Collector) ([]objectCollector, error) { template, subTemplate *node.Node ) - c = p.upgradeCollector(c, p.remote) + newC := p.upgradeCollector(c, p.remote) + if newC.Name != c.Name { + logger.Info("upgraded collector", slog.String("old", c.Name), slog.String("new", newC.Name)) + } + c = newC class = c.Name // throw warning for deprecated collectors if r, d := deprecatedCollectors[strings.ToLower(class)]; d { @@ -1491,7 +1495,7 @@ func (p *Poller) sendHarvestVersion() error { return nil } -func (p *Poller) negotiateONTAPAPI(cols []conf.Collector, gatherClusterInfo func(pollerName string, cred *auth.Credentials) (conf.Remote, error)) { +func (p *Poller) negotiateONTAPAPI(cols []conf.Collector) { anyONTAP := false for _, c := range cols { if _, ok := util.IsONTAPCollector[c.Name]; ok { @@ -1504,9 +1508,9 @@ func (p *Poller) negotiateONTAPAPI(cols []conf.Collector, gatherClusterInfo func return } - remote, err := gatherClusterInfo(opts.Poller, p.auth) + remote, err := collectors.GatherClusterInfo(opts.Poller, p.auth) if err != nil { - logger.Error("Failed to gather cluster info", slogx.Err(err)) + logger.Warn("gather cluster info", slog.Any("remote", remote), slog.Any("remoteErr", err)) } p.remote = remote diff --git a/cmd/poller/poller_test.go b/cmd/poller/poller_test.go index a531b31ac..999e312ad 100644 --- a/cmd/poller/poller_test.go +++ b/cmd/poller/poller_test.go @@ -4,7 +4,7 @@ import ( "errors" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "github.com/netapp/harvest/v2/pkg/auth" + "github.com/netapp/harvest/v2/cmd/collectors" "github.com/netapp/harvest/v2/pkg/conf" "github.com/netapp/harvest/v2/pkg/tree/node" "strings" @@ -212,11 +212,11 @@ func Test_nonOverlappingCollectors(t *testing.T) { } func ocs(names ...string) []objectCollector { - collectors := make([]objectCollector, 0, len(names)) + objectCollectors := make([]objectCollector, 0, len(names)) for _, n := range names { - collectors = append(collectors, objectCollector{class: n}) + objectCollectors = append(objectCollectors, objectCollector{class: n}) } - return collectors + return objectCollectors } func Test_uniquifyObjectCollectors(t *testing.T) { @@ -272,55 +272,74 @@ func objectCollectorMap(constructors ...string) map[string][]objectCollector { return objectsToCollectors } -func TestNegotiateONTAPAPI(t *testing.T) { +func TestMergeRemotes(t *testing.T) { - tests := []struct { - name string - collectors []conf.Collector - mockReturn conf.Remote - mockError error - expectedRemote conf.Remote - }{ + type test struct { + name string + remoteZapi conf.Remote + remoteRest conf.Remote + errZapi error + errRest error + want conf.Remote + wantErr bool + } + + tests := []test{ { - name: "No ONTAP Collector", - collectors: []conf.Collector{ - {Name: "StorageGrid"}, - }, - mockReturn: conf.Remote{}, - mockError: nil, - expectedRemote: conf.Remote{}, + name: "No ONTAP", + remoteZapi: conf.Remote{}, + remoteRest: conf.Remote{}, + errZapi: errors.New("no ZAPI"), + errRest: errors.New("no REST"), + want: conf.Remote{}, + wantErr: true, }, { - name: "ONTAP Collector with Success", - collectors: []conf.Collector{ - {Name: "Zapi"}, - }, - mockReturn: conf.Remote{Version: "9.11.1"}, - mockError: nil, - expectedRemote: conf.Remote{Version: "9.11.1"}, + name: "No REST", + remoteZapi: conf.Remote{UUID: "abc", Version: "9.11.1", ZAPIsExist: true}, + remoteRest: conf.Remote{}, + errZapi: nil, + errRest: errors.New("no REST"), + want: conf.Remote{UUID: "abc", Version: "9.11.1", ZAPIsExist: true}, + wantErr: true, }, { - name: "ONTAP Collector with Error", - collectors: []conf.Collector{ - {Name: "Zapi"}, - }, - mockReturn: conf.Remote{Version: "9.11.1"}, - mockError: errors.New("failed to gather cluster info"), - expectedRemote: conf.Remote{Version: "9.11.1"}, + name: "No ZAPIs", + remoteZapi: conf.Remote{}, + remoteRest: conf.Remote{UUID: "abc", Version: "9.17.1", HasREST: true}, + errZapi: errors.New("no ZAPI"), + errRest: nil, + want: conf.Remote{UUID: "abc", Version: "9.17.1", HasREST: true}, + wantErr: true, + }, + { + name: "Both", + remoteZapi: conf.Remote{UUID: "abc", Version: "9.17.1", ZAPIsExist: true, HasREST: true}, + remoteRest: conf.Remote{UUID: "abc", Version: "9.17.1", ZAPIsExist: true, HasREST: true}, + errZapi: nil, + errRest: nil, + want: conf.Remote{UUID: "abc", Version: "9.17.1", ZAPIsExist: true, HasREST: true}, + wantErr: false, + }, + { + name: "Both Fail", + remoteZapi: conf.Remote{ZAPIsExist: true}, + remoteRest: conf.Remote{}, + errZapi: errors.New("auth error"), + errRest: errors.New("auth error"), + want: conf.Remote{ZAPIsExist: true}, + wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mockGatherClusterInfo := func(_ string, _ *auth.Credentials) (conf.Remote, error) { - return tt.mockReturn, tt.mockError + gotRemote, err := collectors.MergeRemotes(tt.remoteZapi, tt.remoteRest, tt.errZapi, tt.errRest) + if (err != nil) != tt.wantErr { + t.Errorf("MergeRemotes() error = %v, wantErr %v", err, tt.wantErr) } - poller := Poller{} - - poller.negotiateONTAPAPI(tt.collectors, mockGatherClusterInfo) - - if diff := cmp.Diff(poller.remote, tt.expectedRemote); diff != "" { - t.Errorf("negotiateONTAPAPI() mismatch (-got +want):\n%s", diff) + if diff := cmp.Diff(gotRemote, tt.want); diff != "" { + t.Errorf("MergeRemotes() mismatch (-gotRemote +want):\n%s", diff) } }) }