fix: GCE Provider attributes which don't rely on MAC should always be published

pkg/cloudprovider/gce/gce.go

@@ -113,29 +113,47 @@ func GetInstance(ctx context.Context) (*cloudprovider.CloudInstance, error) {
 	return instance, nil
 }
 
-// GetGCEAttributes fetches all attributes related to the provided GCP network.
-func GetGCEAttributes(network, topology string) map[resourceapi.QualifiedName]resourceapi.DeviceAttribute {
+// GetGCEAttributes fetches all attributes related to the provided device,
+// identified by it's MAC.
+func GetGCEAttributes(mac string, instance *cloudprovider.CloudInstance) map[resourceapi.QualifiedName]resourceapi.DeviceAttribute {
 	attributes := make(map[resourceapi.QualifiedName]resourceapi.DeviceAttribute)
-	var projectNumber int64
-	var name string
-	// Use custom parsing because the network path is
-	// different from the format expected by k8s-cloud-provider
-	_, err := fmt.Sscanf(network, "projects/%d/networks/%s", &projectNumber, &name)
-	if err != nil {
-		klog.Warningf("Error parsing network %q : %v", network, err)
-		return nil
-	}
-	topologyParts := strings.SplitN(strings.TrimPrefix(topology, "/"), "/", 3)
+
+	topologyParts := strings.SplitN(strings.TrimPrefix(instance.Topology, "/"), "/", 3)
 	// topology may not be always available
 	if len(topologyParts) == 3 {
 		attributes["gce.dra.net/block"] = resourceapi.DeviceAttribute{StringValue: &topologyParts[0]}
 		attributes["gce.dra.net/subblock"] = resourceapi.DeviceAttribute{StringValue: &topologyParts[1]}
 		attributes["gce.dra.net/host"] = resourceapi.DeviceAttribute{StringValue: &topologyParts[2]}
 	} else {
-		klog.Warningf("Error parsing host topology, may be unsupported on VM %q : %v", topology, err)
+		klog.Warningf("Error parsing host topology %q; it may be unsupported for the VM", instance.Topology)
+	}
+
+	// Determine properties specific to the device identified by this mac
+
+	interfaceForMacFound := false
+	var interfaceForMac cloudprovider.NetworkInterface
+	for _, cloudInterface := range instance.Interfaces {
+		if cloudInterface.Mac == mac {
+			interfaceForMacFound = true
+			interfaceForMac = cloudInterface
+			break
+		}
+	}
+	if interfaceForMacFound {
+		var projectNumber int64
+		var name string
+		// Use custom parsing because the network path is
+		// different from the format expected by k8s-cloud-provider
+		_, err := fmt.Sscanf(interfaceForMac.Network, "projects/%d/networks/%s", &projectNumber, &name)
+		if err != nil {
+			klog.Warningf("Error parsing network %q : %v", interfaceForMac.Network, err)
+			return nil
+		}
+		attributes["gce.dra.net/networkName"] = resourceapi.DeviceAttribute{StringValue: &name}
+		attributes["gce.dra.net/networkProjectNumber"] = resourceapi.DeviceAttribute{IntValue: &projectNumber}
+	} else {
+		klog.V(4).Infof("No cloud metadata found for device with mac %q; it is possible this device has no associated cloud provider metadata", mac)
 	}
-	attributes["gce.dra.net/networkName"] = resourceapi.DeviceAttribute{StringValue: &name}
-	attributes["gce.dra.net/networkProjectNumber"] = resourceapi.DeviceAttribute{IntValue: &projectNumber}
-	klog.Info(attributes)
+
 	return attributes
 }

pkg/inventory/cloud.go

@@ -53,16 +53,12 @@ func getProviderAttributes(mac string, instance *cloudprovider.CloudInstance) ma
 		klog.Warningf("instance metadata is nil, cannot get provider attributes.")
 		return nil
 	}
-	if instance.Provider != cloudprovider.CloudProviderGCE {
+	switch instance.Provider {
+	case cloudprovider.CloudProviderGCE:
+		return gce.GetGCEAttributes(mac, instance)
+	default:
 		klog.Warningf("cloud provider %q is not supported", instance.Provider)
-		return nil
-	}
-	for _, cloudInterface := range instance.Interfaces {
-		if cloudInterface.Mac == mac {
-			return gce.GetGCEAttributes(cloudInterface.Network, instance.Topology)
-		}
 	}
-	klog.Warningf("no matching cloud interface found for mac %s", mac)
 	return nil
 }
 

pkg/inventory/cloud_test.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/google/dranet/pkg/cloudprovider"
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	resourceapi "k8s.io/api/resource/v1"
 	"k8s.io/utils/ptr"
 )
@@ -48,7 +49,7 @@ func TestGetProviderAttributes(t *testing.T) {
 			want: nil,
 		},
 		{
-			name: "MAC not found in instance interfaces",
+			name: "MAC not found in instance interfaces, no topology",
 			mac:  "00:11:22:33:44:FF", // MAC that won't be found
 			instance: &cloudprovider.CloudInstance{
 				Provider: cloudprovider.CloudProviderGCE,
@@ -58,6 +59,22 @@ func TestGetProviderAttributes(t *testing.T) {
 			},
 			want: nil,
 		},
+		{
+			name: "MAC not found in instance interfaces, has topology",
+			mac:  "00:11:22:33:44:FF", // MAC that won't be found
+			instance: &cloudprovider.CloudInstance{
+				Provider: cloudprovider.CloudProviderGCE,
+				Interfaces: []cloudprovider.NetworkInterface{
+					{Mac: "00:11:22:33:44:55", Network: "projects/12345/networks/test-network"},
+				},
+				Topology: "/block/subblock/host",
+			},
+			want: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{
+				"gce.dra.net/block":    {StringValue: ptr.To("block")},
+				"gce.dra.net/subblock": {StringValue: ptr.To("subblock")},
+				"gce.dra.net/host":     {StringValue: ptr.To("host")},
+			},
+		},
 		{
 			name: "GCE provider, MAC found, valid network",
 			mac:  "00:11:22:33:44:55",
@@ -120,8 +137,8 @@ func TestGetProviderAttributes(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got := getProviderAttributes(tt.mac, tt.instance)
-			if diff := cmp.Diff(tt.want, got); diff != "" {
-				t.Errorf("getProviderAttributes() got = %v, want %v", got, tt.want)
+			if diff := cmp.Diff(tt.want, got, cmpopts.EquateEmpty()); diff != "" {
+				t.Errorf("getProviderAttributes() returned unexpected diff (-want, +got):\n%s", diff)
 			}
 		})
 	}