From c60120f54b04a205fd5d0053fdef3d6ac302fa2b Mon Sep 17 00:00:00 2001 From: Paulo Dias Date: Wed, 29 Jan 2025 01:05:33 +0000 Subject: [PATCH 1/4] feat: add multiple client and feature flag support Signed-off-by: Paulo Dias --- discovery/openstack/instance.go | 87 +++++++++++++++++++++++++++++---- 1 file changed, 77 insertions(+), 10 deletions(-) diff --git a/discovery/openstack/instance.go b/discovery/openstack/instance.go index dea327afe3..ddf670d484 100644 --- a/discovery/openstack/instance.go +++ b/discovery/openstack/instance.go @@ -25,6 +25,7 @@ import ( "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers" "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/layer3/floatingips" "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/ports" + "github.com/gophercloud/gophercloud/v2/openstack/utils" "github.com/gophercloud/gophercloud/v2/pagination" "github.com/prometheus/common/model" "github.com/prometheus/common/promslog" @@ -72,6 +73,27 @@ func newInstanceDiscovery(provider *gophercloud.ProviderClient, opts *gopherclou } } +type SupportedFeatures struct { + flavorOriginalName bool // Available from 2.47 +} + +func getSupportedFeatures(minMinor, minMajor, maxMinor, maxMajor int) SupportedFeatures { + features := SupportedFeatures{} + + // Feature: 'flavor.original_name' is supported from microversion 2.47 onwards + if maxMajor > 2 || (maxMajor == 2 && maxMinor >= 47) { + features.flavorOriginalName = true + } + + return features +} + +func setClientMicroversion(sc *gophercloud.ServiceClient, microversion string) *gophercloud.ServiceClient { + microversioned := *sc + microversioned.Microversion = microversion + return µversioned +} + type floatingIPKey struct { deviceID string fixed string @@ -90,6 +112,20 @@ func (i *InstanceDiscovery) refresh(ctx context.Context) ([]*targetgroup.Group, return nil, fmt.Errorf("could not create OpenStack compute session: %w", err) } + supportedVersions, err := utils.GetSupportedMicroversions(ctx, client) + if err != nil { + return nil, fmt.Errorf("failed to fetch supported microversions: %w", err) + } + + // Determine available features + features := getSupportedFeatures(supportedVersions.MinMinor, supportedVersions.MinMajor, supportedVersions.MaxMinor, supportedVersions.MaxMajor) + + minMicroversion := fmt.Sprintf("%d.%d", supportedVersions.MaxMajor, supportedVersions.MaxMinor) + client = setClientMicroversion(client, minMicroversion) + + maxMicroversion := fmt.Sprintf("%d.%d", supportedVersions.MaxMajor, supportedVersions.MaxMinor) + clientLatest := setClientMicroversion(client, maxMicroversion) + networkClient, err := openstack.NewNetworkV2(i.provider, gophercloud.EndpointOpts{ Region: i.region, Availability: i.availability, }) @@ -157,6 +193,27 @@ func (i *InstanceDiscovery) refresh(ctx context.Context) ([]*targetgroup.Group, AllTenants: i.allTenants, } pager := servers.List(client, opts) + + var latestServers []servers.Server + if features.flavorOriginalName { + // Fetch the server list using the client with the latest microversion + allPages, err := servers.List(clientLatest, nil).AllPages(ctx) + if err != nil { + i.logger.Warn("Failed to fetch server list with latest microversion, falling back to flavor.id:", err) + } else { + latestServers, err = servers.ExtractServers(allPages) + if err != nil { + i.logger.Warn("Failed to extract servers with latest microversion, falling back to flavor.id:", err) + } + } + } + latestServerMap := make(map[string]servers.Server) + if len(latestServers) > 0 { + for _, server := range latestServers { + latestServerMap[server.ID] = server + } + } + tg := &targetgroup.Group{ Source: "OS_" + i.region, } @@ -183,19 +240,29 @@ func (i *InstanceDiscovery) refresh(ctx context.Context) ([]*targetgroup.Group, openstackLabelUserID: model.LabelValue(s.UserID), } - flavorName, nameOk := s.Flavor["original_name"].(string) - // "original_name" is only available for microversion >= 2.47. It was added in favor of "id". - if !nameOk { - flavorID, idOk := s.Flavor["id"].(string) - if !idOk { - i.logger.Warn("Invalid type for both flavor original_name and flavor id, expected string") - continue + // Check if we have enriched data for this server in the latestServerMap + if features.flavorOriginalName { + if latestServer, ok := latestServerMap[s.ID]; ok { + flavorName, nameOk := latestServer.Flavor["original_name"].(string) + if nameOk { + labels[openstackLabelInstanceFlavor] = model.LabelValue(flavorName) + } else { + i.logger.Warn("flavor.original_name not found for server, falling back to flavor.id", "instance", s.ID) + } } - labels[openstackLabelInstanceFlavor] = model.LabelValue(flavorID) - } else { - labels[openstackLabelInstanceFlavor] = model.LabelValue(flavorName) } + // Fallback to using flavor.id from the original server list if flavor.original_name wasn't set + if _, exists := labels[openstackLabelInstanceFlavor]; !exists { + flavorID, idOk := s.Flavor["id"].(string) + if idOk { + labels[openstackLabelInstanceFlavor] = model.LabelValue(flavorID) + } else { + i.logger.Warn("Invalid type for both flavor.original_name and flavor.id in server, expected string", "instance", s.ID) + } + } + + imageID, ok := s.Image["id"].(string) if ok { labels[openstackLabelInstanceImage] = model.LabelValue(imageID) From 3853513ea3a790fd26d9f8189e27f7bfdd03ad9e Mon Sep 17 00:00:00 2001 From: Paulo Dias Date: Wed, 29 Jan 2025 10:14:32 +0000 Subject: [PATCH 2/4] feat: add missing client opts and gofmt Signed-off-by: Paulo Dias --- discovery/openstack/instance.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/discovery/openstack/instance.go b/discovery/openstack/instance.go index ddf670d484..3492c8d3d8 100644 --- a/discovery/openstack/instance.go +++ b/discovery/openstack/instance.go @@ -74,18 +74,18 @@ func newInstanceDiscovery(provider *gophercloud.ProviderClient, opts *gopherclou } type SupportedFeatures struct { - flavorOriginalName bool // Available from 2.47 + flavorOriginalName bool // Available from 2.47 } func getSupportedFeatures(minMinor, minMajor, maxMinor, maxMajor int) SupportedFeatures { - features := SupportedFeatures{} + features := SupportedFeatures{} - // Feature: 'flavor.original_name' is supported from microversion 2.47 onwards - if maxMajor > 2 || (maxMajor == 2 && maxMinor >= 47) { - features.flavorOriginalName = true - } + // Feature: 'flavor.original_name' is supported from microversion 2.47 onwards + if maxMajor > 2 || (maxMajor == 2 && maxMinor >= 47) { + features.flavorOriginalName = true + } - return features + return features } func setClientMicroversion(sc *gophercloud.ServiceClient, microversion string) *gophercloud.ServiceClient { @@ -197,13 +197,13 @@ func (i *InstanceDiscovery) refresh(ctx context.Context) ([]*targetgroup.Group, var latestServers []servers.Server if features.flavorOriginalName { // Fetch the server list using the client with the latest microversion - allPages, err := servers.List(clientLatest, nil).AllPages(ctx) + allPages, err := servers.List(clientLatest, opts).AllPages(ctx) if err != nil { - i.logger.Warn("Failed to fetch server list with latest microversion, falling back to flavor.id:", err) + i.logger.Warn("Failed to fetch server list with latest microversion, falling back to flavor.id") } else { latestServers, err = servers.ExtractServers(allPages) if err != nil { - i.logger.Warn("Failed to extract servers with latest microversion, falling back to flavor.id:", err) + i.logger.Warn("Failed to extract servers with latest microversion, falling back to flavor.id") } } } @@ -262,7 +262,6 @@ func (i *InstanceDiscovery) refresh(ctx context.Context) ([]*targetgroup.Group, } } - imageID, ok := s.Image["id"].(string) if ok { labels[openstackLabelInstanceImage] = model.LabelValue(imageID) From 5a09c71b643a24f3eb5cfa99684ea6f93f254f12 Mon Sep 17 00:00:00 2001 From: Paulo Dias Date: Wed, 29 Jan 2025 14:22:09 +0000 Subject: [PATCH 3/4] feat: improve testing for GetSupportedMicroversions Signed-off-by: Paulo Dias --- discovery/openstack/mock_test.go | 38 +++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/discovery/openstack/mock_test.go b/discovery/openstack/mock_test.go index 36620defeb..3bb4c46e32 100644 --- a/discovery/openstack/mock_test.go +++ b/discovery/openstack/mock_test.go @@ -86,6 +86,38 @@ func (m *SDMock) HandleVersionsSuccessfully() { } `, m.Endpoint()+"v3/", m.Endpoint()+"v2.0/") }) + + // Compute API + m.Mux.HandleFunc("/compute/", func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, ` + { + "version": { + "id": "v2.1", + "status": "CURRENT", + "version": "2.96", + "min_version": "2.1", + "updated": "2013-07-23T11:33:21Z", + "links": [ + { + "rel": "self", + "href": "%s" + }, + { + "rel": "describedby", + "type": "text/html", + "href": "http://docs.openstack.org/" + } + ], + "media-types": [ + { + "base": "application/json", + "type": "application/vnd.openstack.compute+json;version=2.1" + } + ] + } + } + `, m.Endpoint()+"v2.1/") + }) } // HandleAuthSuccessfully mocks auth call. @@ -131,7 +163,7 @@ func (m *SDMock) HandleAuthSuccessfully() { "interface": "public", "region": "RegionOne", "region_id": "RegionOne", - "url": "%s" + "url": "%s/compute/v2.1/" } ], "id": "b7f2a5b1a019459cb956e43a8cb41e31", @@ -263,7 +295,7 @@ const hypervisorListBody = ` // HandleHypervisorListSuccessfully mocks os-hypervisors detail call. func (m *SDMock) HandleHypervisorListSuccessfully() { - m.Mux.HandleFunc("/os-hypervisors/detail", func(w http.ResponseWriter, r *http.Request) { + m.Mux.HandleFunc("/compute/v2.1/os-hypervisors/detail", func(w http.ResponseWriter, r *http.Request) { testMethod(m.t, r, http.MethodGet) testHeader(m.t, r, "X-Auth-Token", tokenID) @@ -645,7 +677,7 @@ const serverListBody = ` // HandleServerListSuccessfully mocks server detail call. func (m *SDMock) HandleServerListSuccessfully() { - m.Mux.HandleFunc("/servers/detail", func(w http.ResponseWriter, r *http.Request) { + m.Mux.HandleFunc("/compute/v2.1/servers/detail", func(w http.ResponseWriter, r *http.Request) { testMethod(m.t, r, http.MethodGet) testHeader(m.t, r, "X-Auth-Token", tokenID) From 0b8f59f8f937be13825407e2a635544ddf872e53 Mon Sep 17 00:00:00 2001 From: Paulo Dias Date: Wed, 29 Jan 2025 14:34:45 +0000 Subject: [PATCH 4/4] fix: revert the logic to the previous one Signed-off-by: Paulo Dias --- discovery/openstack/instance.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/discovery/openstack/instance.go b/discovery/openstack/instance.go index 3492c8d3d8..74eeba8b66 100644 --- a/discovery/openstack/instance.go +++ b/discovery/openstack/instance.go @@ -255,11 +255,11 @@ func (i *InstanceDiscovery) refresh(ctx context.Context) ([]*targetgroup.Group, // Fallback to using flavor.id from the original server list if flavor.original_name wasn't set if _, exists := labels[openstackLabelInstanceFlavor]; !exists { flavorID, idOk := s.Flavor["id"].(string) - if idOk { - labels[openstackLabelInstanceFlavor] = model.LabelValue(flavorID) - } else { + if !idOk { i.logger.Warn("Invalid type for both flavor.original_name and flavor.id in server, expected string", "instance", s.ID) + continue } + labels[openstackLabelInstanceFlavor] = model.LabelValue(flavorID) } imageID, ok := s.Image["id"].(string)