Add additional information when reporting platform as other

Author: bjee19

internal/mode/static/telemetry/collector.go

@@ -309,7 +309,7 @@ func collectClusterInformation(ctx context.Context, k8sClient client.Reader) (cl
 		return clusterInformation{}, fmt.Errorf("failed to collect cluster information: %w", err)
 	}
 
-	clusterInfo.Platform = collectK8sPlatform(node, namespaces)
+	clusterInfo.Platform = getPlatform(node, namespaces)
 
 	return clusterInfo, nil
 }

internal/mode/static/telemetry/collector_test.go

@@ -537,7 +537,60 @@ var _ = Describe("Collector", Ordered, func() {
 			})
 
 			When("platform is none of the above", func() {
-				It("marks the platform as 'other'", func() {
+				It("marks the platform as 'other: ' with whatever was in the providerID's providerName", func() {
+					nodes := &v1.NodeList{
+						Items: []v1.Node{
+							{
+								ObjectMeta: metav1.ObjectMeta{
+									Name: "node1",
+								},
+								Spec: v1.NodeSpec{
+									ProviderID: "other-cloud-provider://test-here",
+								},
+								Status: v1.NodeStatus{
+									NodeInfo: v1.NodeSystemInfo{
+										KubeletVersion: "v1.29.2",
+									},
+								},
+							},
+						},
+					}
+
+					k8sClientReader.ListCalls(createListCallsFunc(nodes))
+					expData.K8sVersion = "1.29.2"
+					expData.K8sPlatform = "other: other-cloud-provider"
+
+					data, err := dataCollector.Collect(ctx)
+
+					Expect(err).To(BeNil())
+					Expect(expData).To(Equal(data))
+				})
+				It("marks the platform as 'other: ' when providerID is empty", func() {
+					nodes := &v1.NodeList{
+						Items: []v1.Node{
+							{
+								ObjectMeta: metav1.ObjectMeta{
+									Name: "node1",
+								},
+								Status: v1.NodeStatus{
+									NodeInfo: v1.NodeSystemInfo{
+										KubeletVersion: "v1.29.2",
+									},
+								},
+							},
+						},
+					}
+
+					k8sClientReader.ListCalls(createListCallsFunc(nodes))
+					expData.K8sVersion = "1.29.2"
+					expData.K8sPlatform = "other: "
+
+					data, err := dataCollector.Collect(ctx)
+
+					Expect(err).To(BeNil())
+					Expect(expData).To(Equal(data))
+				})
+				It("marks the platform as 'other: ' when providerID is missing '://' separator", func() {
 					nodes := &v1.NodeList{
 						Items: []v1.Node{
 							{
@@ -558,7 +611,7 @@ var _ = Describe("Collector", Ordered, func() {
 
 					k8sClientReader.ListCalls(createListCallsFunc(nodes))
 					expData.K8sVersion = "1.29.2"
-					expData.K8sPlatform = "other"
+					expData.K8sPlatform = "other: "
 
 					data, err := dataCollector.Collect(ctx)
 

internal/mode/static/telemetry/platform.go

@@ -6,13 +6,29 @@ import (
 	v1 "k8s.io/api/core/v1"
 )
 
+type k8sState struct {
+	node       v1.Node
+	namespaces v1.NamespaceList
+}
+
+type platformExtractor func(k8sState) (string, bool)
+
+func buildProviderIDExtractor(id string, platform string) platformExtractor {
+	return func(state k8sState) (string, bool) {
+		if strings.HasPrefix(state.node.Spec.ProviderID, id) {
+			return platform, true
+		}
+		return "", false
+	}
+}
+
 const (
-	openshiftIdentifier = "node.openshift.io/os_id"
-	k3sIdentifier       = "k3s"
-	awsIdentifier       = "aws"
 	gkeIdentifier       = "gce"
+	awsIdentifier       = "aws"
 	azureIdentifier     = "azure"
 	kindIdentifier      = "kind"
+	k3sIdentifier       = "k3s"
+	openshiftIdentifier = "node.openshift.io/os_id"
 	rancherIdentifier   = "cattle-system"
 
 	platformGKE       = "gke"
@@ -22,87 +38,66 @@ const (
 	platformK3S       = "k3s"
 	platformOpenShift = "openshift"
 	platformRancher   = "rancher"
-	platformOther     = "other"
 )
 
-func collectK8sPlatform(node v1.Node, namespaces v1.NamespaceList) string {
-	if result := isMultiplePlatforms(node, namespaces); result != "" {
-		return result
-	}
-
-	if isAWSPlatform(node) {
-		return platformAWS
-	}
-	if isGKEPlatform(node) {
-		return platformGKE
-	}
-	if isAzurePlatform(node) {
-		return platformAzure
-	}
-	if isKindPlatform(node) {
-		return platformKind
-	}
-	if isK3SPlatform(node) {
-		return platformK3S
-	}
+var multiDistributionPlatformExtractors = []platformExtractor{
+	rancherExtractor,
+	openShiftExtractor,
+}
 
-	return platformOther
+var platformExtractors = []platformExtractor{
+	buildProviderIDExtractor(gkeIdentifier, platformGKE),
+	buildProviderIDExtractor(awsIdentifier, platformAWS),
+	buildProviderIDExtractor(azureIdentifier, platformAzure),
+	buildProviderIDExtractor(kindIdentifier, platformKind),
+	buildProviderIDExtractor(k3sIdentifier, platformK3S),
 }
 
-// isMultiplePlatforms checks for platforms that run on other platforms. e.g. Rancher on K3s.
-func isMultiplePlatforms(node v1.Node, namespaces v1.NamespaceList) string {
-	if isRancherPlatform(namespaces) {
-		return platformRancher
+func getPlatform(node v1.Node, namespaces v1.NamespaceList) string {
+	state := k8sState{
+		node:       node,
+		namespaces: namespaces,
 	}
 
-	if isOpenshiftPlatform(node) {
-		return platformOpenShift
+	// must be run before providerIDPlatformExtractors as these platforms
+	// may have multiple platforms e.g. Rancher on K3S, and we want to record the
+	// higher level platform.
+	for _, extractor := range multiDistributionPlatformExtractors {
+		if platform, ok := extractor(state); ok {
+			return platform
+		}
 	}
 
-	return ""
-}
-
-// For each of these, if we want to we can do both check the providerID AND check labels/annotations,
-// I'm not too sure why we would want to do BOTH.
-//
-// I think doing both would add a greater certainty of a specific platform, however will potentially add to upkeep
-// where if either the label/annotation or providerID changes it will mess this up and may group more clusters in
-// the "Other" platform if they messed with any of the node labels/annotations.
-
-func isOpenshiftPlatform(node v1.Node) bool {
-	// openshift platform won't show up in node's ProviderID
-	value, ok := node.Labels[openshiftIdentifier]
-
-	return ok && value != ""
-}
-
-func isK3SPlatform(node v1.Node) bool {
-	return strings.HasPrefix(node.Spec.ProviderID, k3sIdentifier)
-}
+	for _, extractor := range platformExtractors {
+		if platform, ok := extractor(state); ok {
+			return platform
+		}
+	}
 
-func isAWSPlatform(node v1.Node) bool {
-	return strings.HasPrefix(node.Spec.ProviderID, awsIdentifier)
-}
+	var providerName string
+	if prefix, _, found := strings.Cut(node.Spec.ProviderID, "://"); found {
+		providerName = prefix
+	}
 
-func isGKEPlatform(node v1.Node) bool {
-	return strings.HasPrefix(node.Spec.ProviderID, gkeIdentifier)
+	return "other: " + providerName
 }
 
-func isAzurePlatform(node v1.Node) bool {
-	return strings.HasPrefix(node.Spec.ProviderID, azureIdentifier)
-}
+func openShiftExtractor(state k8sState) (string, bool) {
+	// openshift platform won't show up in node's ProviderID
+	if value, ok := state.node.Labels[openshiftIdentifier]; ok && value != "" {
+		return platformOpenShift, true
+	}
 
-func isKindPlatform(node v1.Node) bool {
-	return strings.HasPrefix(node.Spec.ProviderID, kindIdentifier)
+	return "", false
 }
 
-func isRancherPlatform(namespaces v1.NamespaceList) bool {
+func rancherExtractor(state k8sState) (string, bool) {
 	// rancher platform won't show up in the node's ProviderID
-	for _, ns := range namespaces.Items {
+	for _, ns := range state.namespaces.Items {
 		if ns.Name == rancherIdentifier {
-			return true
+			return platformRancher, true
 		}
 	}
 
-	return false
+	return "", false
 }