From a26917e997e718b9f7efb249a4f841f92e3d4e9b Mon Sep 17 00:00:00 2001 From: Jakub Jarosz Date: Thu, 7 Mar 2024 18:51:31 +0000 Subject: [PATCH 1/8] Add Platform name to telemetry data --- internal/telemetry/cluster.go | 41 +++++++ internal/telemetry/cluster_test.go | 174 +++++++++++++++++++++++++++++ 2 files changed, 215 insertions(+) diff --git a/internal/telemetry/cluster.go b/internal/telemetry/cluster.go index 076c0be4e..28bd9d327 100644 --- a/internal/telemetry/cluster.go +++ b/internal/telemetry/cluster.go @@ -2,6 +2,8 @@ package telemetry import ( "context" + "errors" + "strings" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -35,3 +37,42 @@ func (c *Collector) K8sVersion() (string, error) { } return sv.String(), nil } + +// Platform returns a string representing a platform name +// where the K8s cluster and NIC runs. +func (c *Collector) Platform(ctx context.Context) (string, error) { + nodes, err := c.Config.K8sClientReader.CoreV1().Nodes().List(ctx, metaV1.ListOptions{}) + if err != nil { + return "", err + } + if len(nodes.Items) == 0 { + return "", errors.New("no nodes in the cluster, cannot determine platform name") + } + return lookupPlatform(nodes.Items[0].Spec.ProviderID), nil +} + +// lookupPlatform takes a string representing a K8s PlatformID +// retrieved from a cluster node and returns a string +// representing the platform name. +func lookupPlatform(platformID string) string { + platform := strings.ToLower(platformID) + if strings.HasPrefix(platform, "aws") { + return "aws" + } + if strings.HasPrefix(platform, "azure") { + return "azure" + } + if strings.HasPrefix(platform, "gce") { + return "gke" + } + if strings.HasPrefix(platform, "kind") { + return "kind" + } + if strings.HasPrefix(platform, "vsphere") { + return "vsphere" + } + if strings.HasPrefix(platform, "k3s") { + return "k3s" + } + return "unknown" +} diff --git a/internal/telemetry/cluster_test.go b/internal/telemetry/cluster_test.go index 667153645..b4f61f41c 100644 --- a/internal/telemetry/cluster_test.go +++ b/internal/telemetry/cluster_test.go @@ -80,6 +80,96 @@ func TestK8sVersionRetrievesClusterVersion(t *testing.T) { } } +func TestAwsPlatformDeterminesOwnName(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeAWS) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "aws" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestAzurePlatformDeterminesOwnName(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeAzure) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "azure" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestGcpPlatformDeterminesOwnName(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeGCP) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "gke" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestKindPlatformDeterminesOwnName(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeKind) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "kind" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestK3sPlatformDeterminesOwnName(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeK3S) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "k3s" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestVSpherePlatformDeterminesOwnName(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeVSphere) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "vsphere" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + // newTestCollectorForClusterWithNodes returns a telemetry collector configured // to simulate collecting data on a cluser with provided nodes. func newTestCollectorForClusterWithNodes(t *testing.T, nodes ...runtime.Object) *telemetry.Collector { @@ -132,6 +222,90 @@ var ( Spec: apiCoreV1.NodeSpec{}, } + nodeKind = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "kind://docker/local/local-control-plane", + }, + } + + nodeAWS = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "aws:///eu-central-1a/i-088b4f07708408cc0", + }, + } + + nodeAzure = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "azure:///subscriptions/ba96ef31-4a42-40f5-8740-03f7e3c439eb/resourceGroups/mc_hibrid-weu_be3rr5ovr8ulf_westeurope/providers/Microsoft.Compute/virtualMachines/aks-pool1-27255451-0", + }, + } + + nodeGCP = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "gce://gcp-banzaidevgcp-nprd-38306/europe-north1-a/gke-vzf3z1vvleco9-pool1-7e48d363-8qz1", + }, + } + + nodeK3S = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "k3s://ip-1.2.3.4", + }, + } + + nodeVSphere = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "vsphere://4232e3c7-d83c-d72b-758c-71d07a3d9310", + }, + } + kubeNS = &apiCoreV1.Namespace{ TypeMeta: metaV1.TypeMeta{ Kind: "Namespace", From 79790c71631301b53a421216e12b0a08e62550e0 Mon Sep 17 00:00:00 2001 From: Jakub Jarosz Date: Thu, 7 Mar 2024 19:27:18 +0000 Subject: [PATCH 2/8] Add Platform to collected telemetry data --- internal/telemetry/collector.go | 4 ++++ internal/telemetry/collector_test.go | 32 ++++++++++++++++++++++++++-- internal/telemetry/exporter.go | 1 + 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/internal/telemetry/collector.go b/internal/telemetry/collector.go index 6c1222342..925b438cd 100644 --- a/internal/telemetry/collector.go +++ b/internal/telemetry/collector.go @@ -124,5 +124,9 @@ func (c *Collector) BuildReport(ctx context.Context) (Data, error) { if d.K8sVersion, err = c.K8sVersion(); err != nil { glog.Errorf("Error collecting telemetry data: K8s Version: %v", err) } + + if d.Platform, err = c.Platform(ctx); err != nil { + glog.Errorf("Error collecting telemetry data: Platform: %v", err) + } return d, err } diff --git a/internal/telemetry/collector_test.go b/internal/telemetry/collector_test.go index 0c0bb2d5b..dfae7541d 100644 --- a/internal/telemetry/collector_test.go +++ b/internal/telemetry/collector_test.go @@ -106,6 +106,7 @@ func TestCollectNodeCountInClusterWithOneNode(t *testing.T) { NodeCount: 1, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + Platform: "unknown", } want := fmt.Sprintf("%+v", td) got := buf.String() @@ -144,6 +145,7 @@ func TestCollectNodeCountInClusterWithThreeNodes(t *testing.T) { NodeCount: 3, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + Platform: "unknown", } want := fmt.Sprintf("%+v", td) got := buf.String() @@ -183,6 +185,7 @@ func TestCollectClusterIDInClusterWithOneNode(t *testing.T) { ClusterID: "329766ff-5d78-4c9e-8736-7faad1f2e937", K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + Platform: "unknown", } want := fmt.Sprintf("%+v", td) got := buf.String() @@ -222,6 +225,7 @@ func TestCollectK8sVersion(t *testing.T) { ClusterID: "329766ff-5d78-4c9e-8736-7faad1f2e937", K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + Platform: "unknown", } want := fmt.Sprintf("%+v", td) got := buf.String() @@ -252,6 +256,8 @@ func TestCountVirtualServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + Platform: "unknown", + NodeCount: 1, }, expectedTraceDataOnDelete: telemetry.Data{ ProjectMeta: telemetry.ProjectMeta{ @@ -263,6 +269,8 @@ func TestCountVirtualServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + Platform: "unknown", + NodeCount: 1, }, virtualServers: []*configs.VirtualServerEx{ { @@ -289,6 +297,8 @@ func TestCountVirtualServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + Platform: "unknown", + NodeCount: 1, }, expectedTraceDataOnDelete: telemetry.Data{ ProjectMeta: telemetry.ProjectMeta{ @@ -300,6 +310,8 @@ func TestCountVirtualServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + Platform: "unknown", + NodeCount: 1, }, virtualServers: []*configs.VirtualServerEx{ { @@ -335,6 +347,8 @@ func TestCountVirtualServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + Platform: "unknown", + NodeCount: 1, }, expectedTraceDataOnDelete: telemetry.Data{ ProjectMeta: telemetry.ProjectMeta{ @@ -346,6 +360,8 @@ func TestCountVirtualServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + Platform: "unknown", + NodeCount: 1, }, virtualServers: []*configs.VirtualServerEx{ { @@ -375,7 +391,7 @@ func TestCountVirtualServers(t *testing.T) { configurator := newConfigurator(t) c, err := telemetry.NewCollector(telemetry.CollectorConfig{ - K8sClientReader: newTestClientset(dummyKubeNS), + K8sClientReader: newTestClientset(dummyKubeNS, node1), Configurator: configurator, Version: "3.5.0", }) @@ -441,6 +457,8 @@ func TestCountTransportServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + NodeCount: 1, + Platform: "unknown", }, expectedTraceDataOnDelete: telemetry.Data{ ProjectMeta: telemetry.ProjectMeta{ @@ -452,6 +470,8 @@ func TestCountTransportServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + NodeCount: 1, + Platform: "unknown", }, transportServers: []*configs.TransportServerEx{ { @@ -482,6 +502,8 @@ func TestCountTransportServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + NodeCount: 1, + Platform: "unknown", }, expectedTraceDataOnDelete: telemetry.Data{ ProjectMeta: telemetry.ProjectMeta{ @@ -493,6 +515,8 @@ func TestCountTransportServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + NodeCount: 1, + Platform: "unknown", }, transportServers: []*configs.TransportServerEx{ { @@ -536,6 +560,8 @@ func TestCountTransportServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + NodeCount: 1, + Platform: "unknown", }, expectedTraceDataOnDelete: telemetry.Data{ ProjectMeta: telemetry.ProjectMeta{ @@ -547,6 +573,8 @@ func TestCountTransportServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, + NodeCount: 1, + Platform: "unknown", }, transportServers: []*configs.TransportServerEx{ { @@ -584,7 +612,7 @@ func TestCountTransportServers(t *testing.T) { configurator := newConfigurator(t) c, err := telemetry.NewCollector(telemetry.CollectorConfig{ - K8sClientReader: newTestClientset(dummyKubeNS), + K8sClientReader: newTestClientset(dummyKubeNS, node1), Configurator: configurator, Version: "3.5.0", }) diff --git a/internal/telemetry/exporter.go b/internal/telemetry/exporter.go index 19793d22e..4cf618900 100644 --- a/internal/telemetry/exporter.go +++ b/internal/telemetry/exporter.go @@ -33,6 +33,7 @@ type Data struct { ClusterID string K8sVersion string Arch string + Platform string } // ProjectMeta holds metadata for the project. From 09b7facf4f8447a4bb4fc429fe21123721386d7e Mon Sep 17 00:00:00 2001 From: Jakub Jarosz Date: Thu, 7 Mar 2024 20:07:38 +0000 Subject: [PATCH 3/8] Match naming conventions with NGF --- internal/telemetry/cluster.go | 5 ++--- internal/telemetry/collector_test.go | 32 ++++++++++++++-------------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/internal/telemetry/cluster.go b/internal/telemetry/cluster.go index 28bd9d327..fc442b493 100644 --- a/internal/telemetry/cluster.go +++ b/internal/telemetry/cluster.go @@ -38,8 +38,7 @@ func (c *Collector) K8sVersion() (string, error) { return sv.String(), nil } -// Platform returns a string representing a platform name -// where the K8s cluster and NIC runs. +// Platform returns a string representing platform name. func (c *Collector) Platform(ctx context.Context) (string, error) { nodes, err := c.Config.K8sClientReader.CoreV1().Nodes().List(ctx, metaV1.ListOptions{}) if err != nil { @@ -74,5 +73,5 @@ func lookupPlatform(platformID string) string { if strings.HasPrefix(platform, "k3s") { return "k3s" } - return "unknown" + return "other" } diff --git a/internal/telemetry/collector_test.go b/internal/telemetry/collector_test.go index dfae7541d..43cad5e05 100644 --- a/internal/telemetry/collector_test.go +++ b/internal/telemetry/collector_test.go @@ -106,7 +106,7 @@ func TestCollectNodeCountInClusterWithOneNode(t *testing.T) { NodeCount: 1, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, - Platform: "unknown", + Platform: "other", } want := fmt.Sprintf("%+v", td) got := buf.String() @@ -145,7 +145,7 @@ func TestCollectNodeCountInClusterWithThreeNodes(t *testing.T) { NodeCount: 3, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, - Platform: "unknown", + Platform: "other", } want := fmt.Sprintf("%+v", td) got := buf.String() @@ -185,7 +185,7 @@ func TestCollectClusterIDInClusterWithOneNode(t *testing.T) { ClusterID: "329766ff-5d78-4c9e-8736-7faad1f2e937", K8sVersion: "v1.29.2", Arch: runtime.GOARCH, - Platform: "unknown", + Platform: "other", } want := fmt.Sprintf("%+v", td) got := buf.String() @@ -225,7 +225,7 @@ func TestCollectK8sVersion(t *testing.T) { ClusterID: "329766ff-5d78-4c9e-8736-7faad1f2e937", K8sVersion: "v1.29.2", Arch: runtime.GOARCH, - Platform: "unknown", + Platform: "other", } want := fmt.Sprintf("%+v", td) got := buf.String() @@ -256,7 +256,7 @@ func TestCountVirtualServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, - Platform: "unknown", + Platform: "other", NodeCount: 1, }, expectedTraceDataOnDelete: telemetry.Data{ @@ -269,7 +269,7 @@ func TestCountVirtualServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, - Platform: "unknown", + Platform: "other", NodeCount: 1, }, virtualServers: []*configs.VirtualServerEx{ @@ -297,7 +297,7 @@ func TestCountVirtualServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, - Platform: "unknown", + Platform: "other", NodeCount: 1, }, expectedTraceDataOnDelete: telemetry.Data{ @@ -310,7 +310,7 @@ func TestCountVirtualServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, - Platform: "unknown", + Platform: "other", NodeCount: 1, }, virtualServers: []*configs.VirtualServerEx{ @@ -347,7 +347,7 @@ func TestCountVirtualServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, - Platform: "unknown", + Platform: "other", NodeCount: 1, }, expectedTraceDataOnDelete: telemetry.Data{ @@ -360,7 +360,7 @@ func TestCountVirtualServers(t *testing.T) { }, K8sVersion: "v1.29.2", Arch: runtime.GOARCH, - Platform: "unknown", + Platform: "other", NodeCount: 1, }, virtualServers: []*configs.VirtualServerEx{ @@ -458,7 +458,7 @@ func TestCountTransportServers(t *testing.T) { K8sVersion: "v1.29.2", Arch: runtime.GOARCH, NodeCount: 1, - Platform: "unknown", + Platform: "other", }, expectedTraceDataOnDelete: telemetry.Data{ ProjectMeta: telemetry.ProjectMeta{ @@ -471,7 +471,7 @@ func TestCountTransportServers(t *testing.T) { K8sVersion: "v1.29.2", Arch: runtime.GOARCH, NodeCount: 1, - Platform: "unknown", + Platform: "other", }, transportServers: []*configs.TransportServerEx{ { @@ -503,7 +503,7 @@ func TestCountTransportServers(t *testing.T) { K8sVersion: "v1.29.2", Arch: runtime.GOARCH, NodeCount: 1, - Platform: "unknown", + Platform: "other", }, expectedTraceDataOnDelete: telemetry.Data{ ProjectMeta: telemetry.ProjectMeta{ @@ -516,7 +516,7 @@ func TestCountTransportServers(t *testing.T) { K8sVersion: "v1.29.2", Arch: runtime.GOARCH, NodeCount: 1, - Platform: "unknown", + Platform: "other", }, transportServers: []*configs.TransportServerEx{ { @@ -561,7 +561,7 @@ func TestCountTransportServers(t *testing.T) { K8sVersion: "v1.29.2", Arch: runtime.GOARCH, NodeCount: 1, - Platform: "unknown", + Platform: "other", }, expectedTraceDataOnDelete: telemetry.Data{ ProjectMeta: telemetry.ProjectMeta{ @@ -574,7 +574,7 @@ func TestCountTransportServers(t *testing.T) { K8sVersion: "v1.29.2", Arch: runtime.GOARCH, NodeCount: 1, - Platform: "unknown", + Platform: "other", }, transportServers: []*configs.TransportServerEx{ { From 54ea837f3be96823d1ae5349ee01619221b3da7b Mon Sep 17 00:00:00 2001 From: Jakub Jarosz Date: Fri, 8 Mar 2024 05:24:55 +0000 Subject: [PATCH 4/8] Add cloud providers listed in K8s Cloud Providers SIG --- internal/telemetry/cluster.go | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/internal/telemetry/cluster.go b/internal/telemetry/cluster.go index fc442b493..e87e704e2 100644 --- a/internal/telemetry/cluster.go +++ b/internal/telemetry/cluster.go @@ -53,7 +53,13 @@ func (c *Collector) Platform(ctx context.Context) (string, error) { // lookupPlatform takes a string representing a K8s PlatformID // retrieved from a cluster node and returns a string // representing the platform name. +// +// Cloud providers identified by PlatformID (in K8s SIGs): +// https://github.com/orgs/kubernetes-sigs/repositories?q=cluster-api-provider func lookupPlatform(platformID string) string { + if platformID == "" { // PlatformID field not used by a provider. + return "other" + } platform := strings.ToLower(platformID) if strings.HasPrefix(platform, "aws") { return "aws" @@ -73,5 +79,28 @@ func lookupPlatform(platformID string) string { if strings.HasPrefix(platform, "k3s") { return "k3s" } - return "other" + if strings.HasPrefix(platform, "ibmcloud") { + return "ibmcloud" + } + if strings.HasPrefix(platform, "cloudstack") { + return "cloudstack" + } + if strings.HasPrefix(platform, "openstack") { + return "openstack" + } + if strings.HasPrefix(platform, "digitalocean") { + return "digitalocean" + } + if strings.HasPrefix(platform, "equinixmetal") { // former "packet" provider + return "equinixmetal" + } + + p := strings.Split(platform, ":") + if len(p) == 0 { + return platform + } + if p[0] == "" { + return platform + } + return p[0] } From f5072f76dc8509af2a775b1ff1eb66a15300916e Mon Sep 17 00:00:00 2001 From: Jakub Jarosz Date: Fri, 8 Mar 2024 07:12:16 +0000 Subject: [PATCH 5/8] Add cloud providers --- internal/telemetry/cluster.go | 42 ++-- internal/telemetry/cluster_test.go | 359 +++++++++++++++++++++++++++-- 2 files changed, 360 insertions(+), 41 deletions(-) diff --git a/internal/telemetry/cluster.go b/internal/telemetry/cluster.go index e87e704e2..5019660e5 100644 --- a/internal/telemetry/cluster.go +++ b/internal/telemetry/cluster.go @@ -56,51 +56,59 @@ func (c *Collector) Platform(ctx context.Context) (string, error) { // // Cloud providers identified by PlatformID (in K8s SIGs): // https://github.com/orgs/kubernetes-sigs/repositories?q=cluster-api-provider -func lookupPlatform(platformID string) string { - if platformID == "" { // PlatformID field not used by a provider. +// +//gocyclo:ignore +func lookupPlatform(providerID string) string { + provider := strings.TrimSpace(providerID) + // The case when the ProviderID field not used by the cloud provider. + if provider == "" { return "other" } - platform := strings.ToLower(platformID) - if strings.HasPrefix(platform, "aws") { + + provider = strings.ToLower(providerID) + if strings.HasPrefix(provider, "aws") { return "aws" } - if strings.HasPrefix(platform, "azure") { + if strings.HasPrefix(provider, "azure") { return "azure" } - if strings.HasPrefix(platform, "gce") { + if strings.HasPrefix(provider, "gce") { return "gke" } - if strings.HasPrefix(platform, "kind") { + if strings.HasPrefix(provider, "kind") { return "kind" } - if strings.HasPrefix(platform, "vsphere") { + if strings.HasPrefix(provider, "vsphere") { return "vsphere" } - if strings.HasPrefix(platform, "k3s") { + if strings.HasPrefix(provider, "k3s") { return "k3s" } - if strings.HasPrefix(platform, "ibmcloud") { + if strings.HasPrefix(provider, "ibmcloud") { return "ibmcloud" } - if strings.HasPrefix(platform, "cloudstack") { + if strings.HasPrefix(provider, "ibmpowervs") { + return "ibmpowervs" + } + if strings.HasPrefix(provider, "cloudstack") { return "cloudstack" } - if strings.HasPrefix(platform, "openstack") { + if strings.HasPrefix(provider, "openstack") { return "openstack" } - if strings.HasPrefix(platform, "digitalocean") { + if strings.HasPrefix(provider, "digitalocean") { return "digitalocean" } - if strings.HasPrefix(platform, "equinixmetal") { // former "packet" provider + if strings.HasPrefix(provider, "equinixmetal") { return "equinixmetal" } - p := strings.Split(platform, ":") + p := strings.Split(provider, ":") if len(p) == 0 { - return platform + return "other" } if p[0] == "" { - return platform + return "other" } return p[0] } diff --git a/internal/telemetry/cluster_test.go b/internal/telemetry/cluster_test.go index b4f61f41c..d5a30c72c 100644 --- a/internal/telemetry/cluster_test.go +++ b/internal/telemetry/cluster_test.go @@ -80,7 +80,7 @@ func TestK8sVersionRetrievesClusterVersion(t *testing.T) { } } -func TestAwsPlatformDeterminesOwnName(t *testing.T) { +func TestAWSPlatformDeterminesOwnName(t *testing.T) { t.Parallel() c := newTestCollectorForClusterWithNodes(t, nodeAWS) @@ -110,7 +110,7 @@ func TestAzurePlatformDeterminesOwnName(t *testing.T) { } } -func TestGcpPlatformDeterminesOwnName(t *testing.T) { +func TestGCPPlatformDeterminesOwnName(t *testing.T) { t.Parallel() c := newTestCollectorForClusterWithNodes(t, nodeGCP) @@ -140,7 +140,22 @@ func TestKindPlatformDeterminesOwnName(t *testing.T) { } } -func TestK3sPlatformDeterminesOwnName(t *testing.T) { +func TestVSpherePlatformDeterminesOwnName(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeVSphere) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "vsphere" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestK3SPlatformDeterminesOwnName(t *testing.T) { t.Parallel() c := newTestCollectorForClusterWithNodes(t, nodeK3S) @@ -155,16 +170,166 @@ func TestK3sPlatformDeterminesOwnName(t *testing.T) { } } -func TestVSpherePlatformDeterminesOwnName(t *testing.T) { +func TestIBMCloudPlatformDeterminesOwnName(t *testing.T) { t.Parallel() - c := newTestCollectorForClusterWithNodes(t, nodeVSphere) + c := newTestCollectorForClusterWithNodes(t, nodeIBMCloud) got, err := c.Platform(context.Background()) if err != nil { t.Fatal(err) } - want := "vsphere" + want := "ibmcloud" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestIBMPowerPlatformDeterminesOwnName(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeIBMPowerVS) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "ibmpowervs" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestCloudStackPlatformDeterminesOwnName(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeCloudStack) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "cloudstack" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestOpenStackPlatformDeterminesOwnName(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeOpenStack) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "openstack" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestDigitalOceanPlatformDeterminesOwnName(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeDigitalOcean) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "digitalocean" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestEquinixMetallPlatformDeterminesOwnName(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeEquinixMetal) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "equinixmetal" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestPlatformLookupOnMissingPlatformIDField(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, node1) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "other" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestPlatformLookupOnMalformedPlatformIDField(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeMalformedPlatformID) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "//4232e3c7-d83c-d72b-758c-71d07a3d9310" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestPlatformLookupOnMalformedBlankPlatformIDField(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeMalformedBlankPlatformID) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "other" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestPlatformLookupOnMalformedEmptyPlatformIDField(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeMalformedEmptyPlatformID) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "other" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + +func TestPlatformLookupOnMalformedPartialPlatformIDField(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeMalformedPartialPlatformID) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "other" if want != got { t.Errorf("want %s, got %s", want, got) } @@ -222,20 +387,33 @@ var ( Spec: apiCoreV1.NodeSpec{}, } - nodeKind = &apiCoreV1.Node{ + kubeNS = &apiCoreV1.Namespace{ TypeMeta: metaV1.TypeMeta{ - Kind: "Node", + Kind: "Namespace", APIVersion: "v1", }, ObjectMeta: metaV1.ObjectMeta{ - Name: "node", - Namespace: "default", + Name: "kube-system", + UID: "329766ff-5d78-4c9e-8736-7faad1f2e937", }, - Spec: apiCoreV1.NodeSpec{ - ProviderID: "kind://docker/local/local-control-plane", + Spec: apiCoreV1.NamespaceSpec{}, + } + + dummyKubeNS = &apiCoreV1.Namespace{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Namespace", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "kube-system", + UID: "", }, + Spec: apiCoreV1.NamespaceSpec{}, } +) +// Cloud providers' nodes for testing ProviderID lookups. +var ( nodeAWS = &apiCoreV1.Node{ TypeMeta: metaV1.TypeMeta{ Kind: "Node", @@ -278,7 +456,7 @@ var ( }, } - nodeK3S = &apiCoreV1.Node{ + nodeKind = &apiCoreV1.Node{ TypeMeta: metaV1.TypeMeta{ Kind: "Node", APIVersion: "v1", @@ -288,7 +466,7 @@ var ( Namespace: "default", }, Spec: apiCoreV1.NodeSpec{ - ProviderID: "k3s://ip-1.2.3.4", + ProviderID: "kind://docker/local/local-control-plane", }, } @@ -306,27 +484,160 @@ var ( }, } - kubeNS = &apiCoreV1.Namespace{ + nodeK3S = &apiCoreV1.Node{ TypeMeta: metaV1.TypeMeta{ - Kind: "Namespace", + Kind: "Node", APIVersion: "v1", }, ObjectMeta: metaV1.ObjectMeta{ - Name: "kube-system", - UID: "329766ff-5d78-4c9e-8736-7faad1f2e937", + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "k3s://ip-1.2.3.4", }, - Spec: apiCoreV1.NamespaceSpec{}, } - dummyKubeNS = &apiCoreV1.Namespace{ + nodeIBMCloud = &apiCoreV1.Node{ TypeMeta: metaV1.TypeMeta{ - Kind: "Namespace", + Kind: "Node", APIVersion: "v1", }, ObjectMeta: metaV1.ObjectMeta{ - Name: "kube-system", - UID: "", + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "ibmcloud://4232e3c7-d83c-d72b-758c-71d07a3d9310", + }, + } + + nodeIBMPowerVS = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "ibmpowervs://4232e3c7-d83c-d72b-758c-71d07a3d9310", + }, + } + + nodeCloudStack = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "cloudstack://4232e3c7-d83c-d72b-758c-71d07a3d9310", + }, + } + + nodeOpenStack = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "openstack://4232e3c7-d83c-d72b-758c-71d07a3d9310", + }, + } + + nodeDigitalOcean = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "digitalocean://4232e3c7-d83c-d72b-758c-71d07a3d9310", + }, + } + + nodeEquinixMetal = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "equinixmetal://4232e3c7-d83c-d72b-758c-71d07a3d9310", + }, + } +) + +// Nodes with missing or malformed PorviderID. +var ( + nodeMalformedPlatformID = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "//4232e3c7-d83c-d72b-758c-71d07a3d9310", + }, + } + + nodeMalformedPartialPlatformID = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "://4232e3c7-d83c-d72b-758c-71d07a3d9310", + }, + } + + nodeMalformedEmptyPlatformID = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "", + }, + } + + nodeMalformedBlankPlatformID = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: " ", }, - Spec: apiCoreV1.NamespaceSpec{}, } ) From 4c22b166f34294904563fd363fe0aa6efdc0313d Mon Sep 17 00:00:00 2001 From: Jakub Jarosz Date: Fri, 8 Mar 2024 08:01:27 +0000 Subject: [PATCH 6/8] Add Alibaba cloud provider --- internal/telemetry/cluster.go | 3 +++ internal/telemetry/cluster_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/internal/telemetry/cluster.go b/internal/telemetry/cluster.go index 5019660e5..774312975 100644 --- a/internal/telemetry/cluster.go +++ b/internal/telemetry/cluster.go @@ -102,6 +102,9 @@ func lookupPlatform(providerID string) string { if strings.HasPrefix(provider, "equinixmetal") { return "equinixmetal" } + if strings.HasPrefix(provider, "alicloud") { + return "alicloud" + } p := strings.Split(provider, ":") if len(p) == 0 { diff --git a/internal/telemetry/cluster_test.go b/internal/telemetry/cluster_test.go index d5a30c72c..c3421c21b 100644 --- a/internal/telemetry/cluster_test.go +++ b/internal/telemetry/cluster_test.go @@ -260,6 +260,21 @@ func TestEquinixMetallPlatformDeterminesOwnName(t *testing.T) { } } +func TestAlibabaPlatformDeterminesOwnName(t *testing.T) { + t.Parallel() + + c := newTestCollectorForClusterWithNodes(t, nodeAlibaba) + got, err := c.Platform(context.Background()) + if err != nil { + t.Fatal(err) + } + + want := "alicloud" + if want != got { + t.Errorf("want %s, got %s", want, got) + } +} + func TestPlatformLookupOnMissingPlatformIDField(t *testing.T) { t.Parallel() @@ -581,6 +596,20 @@ var ( ProviderID: "equinixmetal://4232e3c7-d83c-d72b-758c-71d07a3d9310", }, } + + nodeAlibaba = &apiCoreV1.Node{ + TypeMeta: metaV1.TypeMeta{ + Kind: "Node", + APIVersion: "v1", + }, + ObjectMeta: metaV1.ObjectMeta{ + Name: "node", + Namespace: "default", + }, + Spec: apiCoreV1.NodeSpec{ + ProviderID: "alicloud://4232e3c7-d83c-d72b-758c-71d07a3d9310", + }, + } ) // Nodes with missing or malformed PorviderID. From eea64a80bab17c001c144def86f0277a8c79ecf1 Mon Sep 17 00:00:00 2001 From: Jakub Jarosz Date: Fri, 8 Mar 2024 10:33:44 +0000 Subject: [PATCH 7/8] Use ProviderID originally assigned by Cloud or Platform provider --- internal/telemetry/cluster.go | 45 ------------------------------ internal/telemetry/cluster_test.go | 2 +- 2 files changed, 1 insertion(+), 46 deletions(-) diff --git a/internal/telemetry/cluster.go b/internal/telemetry/cluster.go index 774312975..06b5a9921 100644 --- a/internal/telemetry/cluster.go +++ b/internal/telemetry/cluster.go @@ -53,58 +53,13 @@ func (c *Collector) Platform(ctx context.Context) (string, error) { // lookupPlatform takes a string representing a K8s PlatformID // retrieved from a cluster node and returns a string // representing the platform name. -// -// Cloud providers identified by PlatformID (in K8s SIGs): -// https://github.com/orgs/kubernetes-sigs/repositories?q=cluster-api-provider -// -//gocyclo:ignore func lookupPlatform(providerID string) string { provider := strings.TrimSpace(providerID) - // The case when the ProviderID field not used by the cloud provider. if provider == "" { return "other" } provider = strings.ToLower(providerID) - if strings.HasPrefix(provider, "aws") { - return "aws" - } - if strings.HasPrefix(provider, "azure") { - return "azure" - } - if strings.HasPrefix(provider, "gce") { - return "gke" - } - if strings.HasPrefix(provider, "kind") { - return "kind" - } - if strings.HasPrefix(provider, "vsphere") { - return "vsphere" - } - if strings.HasPrefix(provider, "k3s") { - return "k3s" - } - if strings.HasPrefix(provider, "ibmcloud") { - return "ibmcloud" - } - if strings.HasPrefix(provider, "ibmpowervs") { - return "ibmpowervs" - } - if strings.HasPrefix(provider, "cloudstack") { - return "cloudstack" - } - if strings.HasPrefix(provider, "openstack") { - return "openstack" - } - if strings.HasPrefix(provider, "digitalocean") { - return "digitalocean" - } - if strings.HasPrefix(provider, "equinixmetal") { - return "equinixmetal" - } - if strings.HasPrefix(provider, "alicloud") { - return "alicloud" - } p := strings.Split(provider, ":") if len(p) == 0 { diff --git a/internal/telemetry/cluster_test.go b/internal/telemetry/cluster_test.go index c3421c21b..0d5c21747 100644 --- a/internal/telemetry/cluster_test.go +++ b/internal/telemetry/cluster_test.go @@ -119,7 +119,7 @@ func TestGCPPlatformDeterminesOwnName(t *testing.T) { t.Fatal(err) } - want := "gke" + want := "gce" if want != got { t.Errorf("want %s, got %s", want, got) } From 23ab904b1ca0fcfd386e2c2b5cb7a4f9d240012a Mon Sep 17 00:00:00 2001 From: Jakub Jarosz Date: Mon, 11 Mar 2024 17:20:36 +0000 Subject: [PATCH 8/8] Fix merge conflicts This PR adds support for Exporter telemetry data; decouples NIC internal data from imported Exporter data --- internal/configs/configurator.go | 5 +- internal/telemetry/cluster.go | 4 +- internal/telemetry/cluster_test.go | 4 +- internal/telemetry/collector.go | 86 ++++++-- internal/telemetry/collector_test.go | 302 ++++++++++----------------- internal/telemetry/exporter.go | 19 +- 6 files changed, 186 insertions(+), 234 deletions(-) diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go index 8f057e134..6ccd1e2b2 100644 --- a/internal/configs/configurator.go +++ b/internal/configs/configurator.go @@ -1479,8 +1479,9 @@ func (cnf *Configurator) GetIngressCounts() map[string]int { // GetVirtualServerCounts returns the total count of // VirtualServer and VirtualServerRoute resources that are handled by the Ingress Controller -func (cnf *Configurator) GetVirtualServerCounts() (vsCount int, vsrCount int) { - vsCount = len(cnf.virtualServers) +func (cnf *Configurator) GetVirtualServerCounts() (int, int) { + vsCount := len(cnf.virtualServers) + vsrCount := 0 for _, vs := range cnf.virtualServers { vsrCount += len(vs.VirtualServerRoutes) } diff --git a/internal/telemetry/cluster.go b/internal/telemetry/cluster.go index a55da297c..05e88edae 100644 --- a/internal/telemetry/cluster.go +++ b/internal/telemetry/cluster.go @@ -10,12 +10,12 @@ import ( // NodeCount returns the total number of nodes in the cluster. // It returns an error if the underlying k8s API client errors. -func (c *Collector) NodeCount(ctx context.Context) (int64, error) { +func (c *Collector) NodeCount(ctx context.Context) (int, error) { nodes, err := c.Config.K8sClientReader.CoreV1().Nodes().List(ctx, metaV1.ListOptions{}) if err != nil { return 0, err } - return int64(len(nodes.Items)), nil + return len(nodes.Items), nil } // ClusterID returns the UID of the kube-system namespace representing cluster id. diff --git a/internal/telemetry/cluster_test.go b/internal/telemetry/cluster_test.go index 7479708a0..c37d78022 100644 --- a/internal/telemetry/cluster_test.go +++ b/internal/telemetry/cluster_test.go @@ -19,7 +19,7 @@ func TestNodeCountInAClusterWithThreeNodes(t *testing.T) { if err != nil { t.Fatal(err) } - var want int64 = 3 + want := 3 if want != got { t.Errorf("want %v, got %v", want, got) } @@ -33,7 +33,7 @@ func TestNodeCountInAClusterWithOneNode(t *testing.T) { if err != nil { t.Fatal(err) } - var want int64 = 1 + want := 1 if want != got { t.Errorf("want %v, got %v", want, got) } diff --git a/internal/telemetry/collector.go b/internal/telemetry/collector.go index 816e24fe4..583e9f424 100644 --- a/internal/telemetry/collector.go +++ b/internal/telemetry/collector.go @@ -7,7 +7,7 @@ import ( "runtime" "time" - telemetry "github.com/nginxinc/telemetry-exporter/pkg/telemetry" + tel "github.com/nginxinc/telemetry-exporter/pkg/telemetry" "github.com/nginxinc/kubernetes-ingress/internal/configs" @@ -85,50 +85,92 @@ func (c *Collector) Start(ctx context.Context) { // It exports data using provided exporter. func (c *Collector) Collect(ctx context.Context) { glog.V(3).Info("Collecting telemetry data") - data, err := c.BuildReport(ctx) + report, err := c.BuildReport(ctx) if err != nil { glog.Errorf("Error collecting telemetry data: %v", err) } - err = c.Exporter.Export(ctx, data) - if err != nil { - glog.Errorf("Error exporting telemetry data: %v", err) - } - glog.V(3).Infof("Exported telemetry data: %+v", data) -} -// BuildReport takes context and builds report from gathered telemetry data. -func (c *Collector) BuildReport(ctx context.Context) (telemetry.Exportable, error) { - d := Data{ - Data: telemetry.Data{ - ProjectName: "NIC", + nicData := Data{ + tel.Data{ + ProjectName: report.Name, ProjectVersion: c.Config.Version, ProjectArchitecture: runtime.GOARCH, + ClusterID: report.ClusterID, + ClusterVersion: report.ClusterVersion, + ClusterPlatform: report.ClusterPlatform, + ClusterNodeCount: int64(report.ClusterNodeCount), }, + NICResourceCounts{ + VirtualServers: int64(report.VirtualServers), + VirtualServerRoutes: int64(report.VirtualServerRoutes), + TransportServers: int64(report.TransportServers), + }, + } + + err = c.Exporter.Export(ctx, &nicData) + if err != nil { + glog.Errorf("Error exporting telemetry data: %v", err) } + glog.V(3).Infof("Exported telemetry data: %+v", nicData) +} + +// Report holds collected NIC telemetry data. It is the package internal +// data structure used for decoupling types between the NIC `telemetry` +// package and the imported `telemetry` exporter. +type Report struct { + Name string + Version string + Architecture string + ClusterID string + ClusterVersion string + ClusterPlatform string + ClusterNodeCount int + VirtualServers int + VirtualServerRoutes int + TransportServers int +} - var err error +// BuildReport takes context, collects telemetry data and builds the report. +func (c *Collector) BuildReport(ctx context.Context) (Report, error) { + vsCount := 0 + vsrCount := 0 + tsCount := 0 if c.Config.Configurator != nil { - vsCount, vsrCount := c.Config.Configurator.GetVirtualServerCounts() - d.VirtualServers, d.VirtualServerRoutes = int64(vsCount), int64(vsrCount) - d.TransportServers = int64(c.Config.Configurator.GetTransportServerCounts()) + vsCount, vsrCount = c.Config.Configurator.GetVirtualServerCounts() + tsCount = c.Config.Configurator.GetTransportServerCounts() } - if d.ClusterID, err = c.ClusterID(ctx); err != nil { + clusterID, err := c.ClusterID(ctx) + if err != nil { glog.Errorf("Error collecting telemetry data: ClusterID: %v", err) } - if d.ClusterNodeCount, err = c.NodeCount(ctx); err != nil { + nodes, err := c.NodeCount(ctx) + if err != nil { glog.Errorf("Error collecting telemetry data: Nodes: %v", err) } - if d.ClusterVersion, err = c.ClusterVersion(); err != nil { + version, err := c.ClusterVersion() + if err != nil { glog.Errorf("Error collecting telemetry data: K8s Version: %v", err) } - if d.Platform, err = c.Platform(ctx); err != nil { + platform, err := c.Platform(ctx) + if err != nil { glog.Errorf("Error collecting telemetry data: Platform: %v", err) } - return &d, err + return Report{ + Name: "NIC", + Version: c.Config.Version, + Architecture: runtime.GOARCH, + ClusterID: clusterID, + ClusterVersion: version, + ClusterPlatform: platform, + ClusterNodeCount: nodes, + VirtualServers: vsCount, + VirtualServerRoutes: vsrCount, + TransportServers: tsCount, + }, err } diff --git a/internal/telemetry/collector_test.go b/internal/telemetry/collector_test.go index 7dcc57461..dfba4ce8f 100644 --- a/internal/telemetry/collector_test.go +++ b/internal/telemetry/collector_test.go @@ -16,7 +16,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/nginxinc/kubernetes-ingress/internal/telemetry" conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1" - exporter "github.com/nginxinc/telemetry-exporter/pkg/telemetry" + tel "github.com/nginxinc/telemetry-exporter/pkg/telemetry" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8sruntime "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/version" @@ -24,13 +24,6 @@ import ( testClient "k8s.io/client-go/kubernetes/fake" ) -var commonData = exporter.Data{ - ProjectName: "NIC", - ProjectVersion: "3.5.0", - ClusterVersion: "v1.29.2", - ProjectArchitecture: runtime.GOARCH, -} - func TestCreateNewCollectorWithCustomReportingPeriod(t *testing.T) { t.Parallel() @@ -60,7 +53,7 @@ func TestCreateNewCollectorWithCustomExporter(t *testing.T) { cfg := telemetry.CollectorConfig{ K8sClientReader: newTestClientset(), Configurator: newConfigurator(t), - Version: "3.5.0", + Version: telemetryNICData.ProjectVersion, } c, err := telemetry.NewCollector(cfg, telemetry.WithExporter(exp)) if err != nil { @@ -69,10 +62,10 @@ func TestCreateNewCollectorWithCustomExporter(t *testing.T) { c.Collect(context.Background()) td := telemetry.Data{ - Data: exporter.Data{ - ProjectName: "NIC", - ProjectVersion: "3.5.0", - ClusterVersion: "v1.29.2", + Data: tel.Data{ + ProjectName: telemetryNICData.ProjectName, + ProjectVersion: telemetryNICData.ProjectVersion, + ClusterVersion: telemetryNICData.ClusterVersion, ProjectArchitecture: runtime.GOARCH, }, } @@ -91,7 +84,7 @@ func TestCollectNodeCountInClusterWithOneNode(t *testing.T) { cfg := telemetry.CollectorConfig{ Configurator: newConfigurator(t), K8sClientReader: newTestClientset(node1), - Version: "3.5.0", + Version: telemetryNICData.ProjectVersion, } c, err := telemetry.NewCollector(cfg, telemetry.WithExporter(exp)) @@ -101,22 +94,19 @@ func TestCollectNodeCountInClusterWithOneNode(t *testing.T) { c.Collect(context.Background()) td := telemetry.Data{ - Data: exporter.Data{ - ProjectName: "NIC", - ProjectVersion: "3.5.0", - ClusterVersion: "v1.29.2", + Data: tel.Data{ + ProjectName: telemetryNICData.ProjectName, + ProjectVersion: telemetryNICData.ProjectVersion, + ClusterVersion: telemetryNICData.ClusterVersion, ProjectArchitecture: runtime.GOARCH, ClusterNodeCount: 1, + ClusterPlatform: "other", }, NICResourceCounts: telemetry.NICResourceCounts{ VirtualServers: 0, VirtualServerRoutes: 0, TransportServers: 0, }, - NodeCount: 1, - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - Platform: "other", } want := fmt.Sprintf("%+v", &td) @@ -134,7 +124,7 @@ func TestCollectNodeCountInClusterWithThreeNodes(t *testing.T) { cfg := telemetry.CollectorConfig{ Configurator: newConfigurator(t), K8sClientReader: newTestClientset(node1, node2, node3), - Version: "3.5.0", + Version: telemetryNICData.ProjectVersion, } c, err := telemetry.NewCollector(cfg, telemetry.WithExporter(exp)) @@ -143,24 +133,26 @@ func TestCollectNodeCountInClusterWithThreeNodes(t *testing.T) { } c.Collect(context.Background()) + telData := tel.Data{ + ProjectName: telemetryNICData.ProjectName, + ProjectVersion: telemetryNICData.ProjectVersion, + ClusterVersion: telemetryNICData.ClusterVersion, + ClusterPlatform: "other", + ProjectArchitecture: runtime.GOARCH, + ClusterNodeCount: 3, + } + + nicResourceCounts := telemetry.NICResourceCounts{ + VirtualServers: 0, + VirtualServerRoutes: 0, + TransportServers: 0, + } + td := telemetry.Data{ - Data: exporter.Data{ - ProjectName: "NIC", - ProjectVersion: "3.5.0", - ClusterVersion: "v1.29.2", - ProjectArchitecture: runtime.GOARCH, - ClusterNodeCount: 3, - }, - NICResourceCounts: telemetry.NICResourceCounts{ - VirtualServers: 0, - VirtualServerRoutes: 0, - TransportServers: 0, - }, - NodeCount: 3, - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - Platform: "other", + telData, + nicResourceCounts, } + want := fmt.Sprintf("%+v", &td) got := buf.String() if !cmp.Equal(want, got) { @@ -176,7 +168,7 @@ func TestCollectClusterIDInClusterWithOneNode(t *testing.T) { cfg := telemetry.CollectorConfig{ Configurator: newConfigurator(t), K8sClientReader: newTestClientset(node1, kubeNS), - Version: "3.5.0", + Version: telemetryNICData.ProjectVersion, } c, err := telemetry.NewCollector(cfg, telemetry.WithExporter(exp)) @@ -186,33 +178,29 @@ func TestCollectClusterIDInClusterWithOneNode(t *testing.T) { c.Collect(context.Background()) td := telemetry.Data{ - Data: exporter.Data{ - ProjectName: "NIC", - ProjectVersion: "3.5.0", - ClusterVersion: "v1.29.2", + Data: tel.Data{ + ProjectName: telemetryNICData.ProjectName, + ProjectVersion: telemetryNICData.ProjectVersion, + ClusterVersion: telemetryNICData.ClusterVersion, + ClusterPlatform: "other", ProjectArchitecture: runtime.GOARCH, ClusterNodeCount: 1, - ClusterID: "329766ff-5d78-4c9e-8736-7faad1f2e937", + ClusterID: telemetryNICData.ClusterID, }, NICResourceCounts: telemetry.NICResourceCounts{ VirtualServers: 0, VirtualServerRoutes: 0, TransportServers: 0, }, - NodeCount: 1, - ClusterID: "329766ff-5d78-4c9e-8736-7faad1f2e937", - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - Platform: "other", } - want := fmt.Sprintf("%+v", td) + want := fmt.Sprintf("%+v", &td) got := buf.String() if !cmp.Equal(want, got) { t.Error(cmp.Diff(want, got)) } } -func TestCollectK8sVersion(t *testing.T) { +func TestCollectClusterVersion(t *testing.T) { t.Parallel() buf := &bytes.Buffer{} @@ -220,7 +208,7 @@ func TestCollectK8sVersion(t *testing.T) { cfg := telemetry.CollectorConfig{ Configurator: newConfigurator(t), K8sClientReader: newTestClientset(node1, kubeNS), - Version: "3.5.0", + Version: telemetryNICData.ProjectVersion, } c, err := telemetry.NewCollector(cfg, telemetry.WithExporter(exp)) @@ -229,22 +217,27 @@ func TestCollectK8sVersion(t *testing.T) { } c.Collect(context.Background()) + telData := tel.Data{ + ProjectName: telemetryNICData.ProjectName, + ProjectVersion: telemetryNICData.ProjectVersion, + ProjectArchitecture: telemetryNICData.ProjectArchitecture, + ClusterNodeCount: 1, + ClusterID: telemetryNICData.ClusterID, + ClusterVersion: telemetryNICData.ClusterVersion, + ClusterPlatform: "other", + } + + nicResourceCounts := telemetry.NICResourceCounts{ + VirtualServers: 0, + VirtualServerRoutes: 0, + TransportServers: 0, + } + td := telemetry.Data{ - ProjectMeta: telemetry.ProjectMeta{ - Name: "NIC", - Version: "3.5.0", - }, - NICResourceCounts: telemetry.NICResourceCounts{ - VirtualServers: 0, - VirtualServerRoutes: 0, - TransportServers: 0, - }, - NodeCount: 1, - ClusterID: "329766ff-5d78-4c9e-8736-7faad1f2e937", - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - Platform: "other", + telData, + nicResourceCounts, } + want := fmt.Sprintf("%+v", &td) got := buf.String() if !cmp.Equal(want, got) { @@ -257,32 +250,18 @@ func TestCountVirtualServers(t *testing.T) { testCases := []struct { testName string - expectedTraceDataOnAdd *telemetry.Data - expectedTraceDataOnDelete *telemetry.Data + expectedTraceDataOnAdd telemetry.Report + expectedTraceDataOnDelete telemetry.Report virtualServers []*configs.VirtualServerEx deleteCount int }{ { testName: "Create and delete 1 VirtualServer", - expectedTraceDataOnAdd: &telemetry.Data{ - Data: commonData, - NICResourceCounts: telemetry.NICResourceCounts{ - VirtualServers: 1, - }, - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - Platform: "other", - NodeCount: 1, + expectedTraceDataOnAdd: telemetry.Report{ + VirtualServers: 1, }, - expectedTraceDataOnDelete: &telemetry.Data{ - Data: commonData, - NICResourceCounts: telemetry.NICResourceCounts{ - VirtualServers: 0, - }, - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - Platform: "other", - NodeCount: 1, + expectedTraceDataOnDelete: telemetry.Report{ + VirtualServers: 0, }, virtualServers: []*configs.VirtualServerEx{ { @@ -299,25 +278,11 @@ func TestCountVirtualServers(t *testing.T) { }, { testName: "Create 2 VirtualServers and delete 2", - expectedTraceDataOnAdd: &telemetry.Data{ - Data: commonData, - NICResourceCounts: telemetry.NICResourceCounts{ - VirtualServers: 2, - }, - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - Platform: "other", - NodeCount: 1, + expectedTraceDataOnAdd: telemetry.Report{ + VirtualServers: 2, }, - expectedTraceDataOnDelete: &telemetry.Data{ - Data: commonData, - NICResourceCounts: telemetry.NICResourceCounts{ - VirtualServers: 0, - }, - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - Platform: "other", - NodeCount: 1, + expectedTraceDataOnDelete: telemetry.Report{ + VirtualServers: 0, }, virtualServers: []*configs.VirtualServerEx{ { @@ -343,25 +308,11 @@ func TestCountVirtualServers(t *testing.T) { }, { testName: "Create 2 VirtualServers and delete 1", - expectedTraceDataOnAdd: &telemetry.Data{ - Data: commonData, - NICResourceCounts: telemetry.NICResourceCounts{ - VirtualServers: 2, - }, - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - Platform: "other", - NodeCount: 1, + expectedTraceDataOnAdd: telemetry.Report{ + VirtualServers: 2, }, - expectedTraceDataOnDelete: &telemetry.Data{ - Data: commonData, - NICResourceCounts: telemetry.NICResourceCounts{ - VirtualServers: 1, - }, - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - Platform: "other", - NodeCount: 1, + expectedTraceDataOnDelete: telemetry.Report{ + VirtualServers: 1, }, virtualServers: []*configs.VirtualServerEx{ { @@ -391,9 +342,9 @@ func TestCountVirtualServers(t *testing.T) { configurator := newConfigurator(t) c, err := telemetry.NewCollector(telemetry.CollectorConfig{ - K8sClientReader: newTestClientset(dummyKubeNS, node1), + K8sClientReader: newTestClientset(kubeNS, node1), Configurator: configurator, - Version: "3.5.0", + Version: telemetryNICData.ProjectVersion, }) if err != nil { t.Fatal(err) @@ -411,8 +362,8 @@ func TestCountVirtualServers(t *testing.T) { t.Fatal(err) } - if !cmp.Equal(test.expectedTraceDataOnAdd, gotTraceDataOnAdd) { - t.Error(cmp.Diff(test.expectedTraceDataOnAdd, gotTraceDataOnAdd)) + if !cmp.Equal(test.expectedTraceDataOnAdd.VirtualServers, gotTraceDataOnAdd.VirtualServers) { + t.Error(cmp.Diff(test.expectedTraceDataOnAdd.VirtualServers, gotTraceDataOnAdd.VirtualServers)) } for i := 0; i < test.deleteCount; i++ { @@ -429,8 +380,8 @@ func TestCountVirtualServers(t *testing.T) { t.Fatal(err) } - if !cmp.Equal(test.expectedTraceDataOnDelete, gotTraceDataOnDelete) { - t.Error(cmp.Diff(test.expectedTraceDataOnDelete, gotTraceDataOnDelete)) + if !cmp.Equal(test.expectedTraceDataOnDelete.VirtualServers, gotTraceDataOnDelete.VirtualServers) { + t.Error(cmp.Diff(test.expectedTraceDataOnDelete.VirtualServers, gotTraceDataOnDelete.VirtualServers)) } } } @@ -440,32 +391,18 @@ func TestCountTransportServers(t *testing.T) { testCases := []struct { testName string - expectedTraceDataOnAdd *telemetry.Data - expectedTraceDataOnDelete *telemetry.Data + expectedTraceDataOnAdd telemetry.Report + expectedTraceDataOnDelete telemetry.Report transportServers []*configs.TransportServerEx deleteCount int }{ { testName: "Create and delete 1 TransportServer", - expectedTraceDataOnAdd: &telemetry.Data{ - Data: commonData, - NICResourceCounts: telemetry.NICResourceCounts{ - TransportServers: 1, - }, - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - NodeCount: 1, - Platform: "other", + expectedTraceDataOnAdd: telemetry.Report{ + TransportServers: 1, }, - expectedTraceDataOnDelete: &telemetry.Data{ - Data: commonData, - NICResourceCounts: telemetry.NICResourceCounts{ - TransportServers: 0, - }, - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - NodeCount: 1, - Platform: "other", + expectedTraceDataOnDelete: telemetry.Report{ + TransportServers: 0, }, transportServers: []*configs.TransportServerEx{ { @@ -486,25 +423,11 @@ func TestCountTransportServers(t *testing.T) { }, { testName: "Create 2 and delete 2 TransportServer", - expectedTraceDataOnAdd: &telemetry.Data{ - Data: commonData, - NICResourceCounts: telemetry.NICResourceCounts{ - TransportServers: 2, - }, - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - NodeCount: 1, - Platform: "other", + expectedTraceDataOnAdd: telemetry.Report{ + TransportServers: 2, }, - expectedTraceDataOnDelete: &telemetry.Data{ - Data: commonData, - NICResourceCounts: telemetry.NICResourceCounts{ - TransportServers: 0, - }, - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - NodeCount: 1, - Platform: "other", + expectedTraceDataOnDelete: telemetry.Report{ + TransportServers: 0, }, transportServers: []*configs.TransportServerEx{ { @@ -538,25 +461,11 @@ func TestCountTransportServers(t *testing.T) { }, { testName: "Create 2 and delete 1 TransportServer", - expectedTraceDataOnAdd: &telemetry.Data{ - Data: commonData, - NICResourceCounts: telemetry.NICResourceCounts{ - TransportServers: 2, - }, - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - NodeCount: 1, - Platform: "other", + expectedTraceDataOnAdd: telemetry.Report{ + TransportServers: 2, }, - expectedTraceDataOnDelete: &telemetry.Data{ - Data: commonData, - NICResourceCounts: telemetry.NICResourceCounts{ - TransportServers: 1, - }, - K8sVersion: "v1.29.2", - Arch: runtime.GOARCH, - NodeCount: 1, - Platform: "other", + expectedTraceDataOnDelete: telemetry.Report{ + TransportServers: 1, }, transportServers: []*configs.TransportServerEx{ { @@ -594,9 +503,9 @@ func TestCountTransportServers(t *testing.T) { configurator := newConfigurator(t) c, err := telemetry.NewCollector(telemetry.CollectorConfig{ - K8sClientReader: newTestClientset(dummyKubeNS, node1), + K8sClientReader: newTestClientset(kubeNS, node1), Configurator: configurator, - Version: "3.5.0", + Version: telemetryNICData.ProjectVersion, }) if err != nil { t.Fatal(err) @@ -614,8 +523,8 @@ func TestCountTransportServers(t *testing.T) { t.Fatal(err) } - if !cmp.Equal(test.expectedTraceDataOnAdd, gotTraceDataOnAdd) { - t.Error(cmp.Diff(test.expectedTraceDataOnAdd, gotTraceDataOnAdd)) + if !cmp.Equal(test.expectedTraceDataOnAdd.TransportServers, gotTraceDataOnAdd.TransportServers) { + t.Error(cmp.Diff(test.expectedTraceDataOnAdd.TransportServers, gotTraceDataOnAdd.TransportServers)) } for i := 0; i < test.deleteCount; i++ { @@ -632,8 +541,8 @@ func TestCountTransportServers(t *testing.T) { t.Fatal(err) } - if !cmp.Equal(test.expectedTraceDataOnDelete, gotTraceDataOnDelete) { - t.Error(cmp.Diff(test.expectedTraceDataOnDelete, gotTraceDataOnDelete)) + if !cmp.Equal(test.expectedTraceDataOnDelete.TransportServers, gotTraceDataOnDelete.TransportServers) { + t.Error(cmp.Diff(test.expectedTraceDataOnDelete.TransportServers, gotTraceDataOnDelete.TransportServers)) } } } @@ -711,3 +620,14 @@ const ( virtualServerTemplatePath = "../configs/version2/nginx-plus.virtualserver.tmpl" transportServerTemplatePath = "../configs/version2/nginx-plus.transportserver.tmpl" ) + +// telemetryNICData holds static test data for telemetry tests. +var telemetryNICData = tel.Data{ + ProjectName: "NIC", + ProjectVersion: "3.5.0", + ClusterVersion: "v1.29.2", + ProjectArchitecture: runtime.GOARCH, + ClusterID: "329766ff-5d78-4c9e-8736-7faad1f2e937", + ClusterNodeCount: 1, + ClusterPlatform: "other", +} diff --git a/internal/telemetry/exporter.go b/internal/telemetry/exporter.go index fd5080fbf..3969fa385 100644 --- a/internal/telemetry/exporter.go +++ b/internal/telemetry/exporter.go @@ -5,12 +5,12 @@ import ( "fmt" "io" - "github.com/nginxinc/telemetry-exporter/pkg/telemetry" + tel "github.com/nginxinc/telemetry-exporter/pkg/telemetry" ) // Exporter interface for exporters. type Exporter interface { - Export(ctx context.Context, data telemetry.Exportable) error + Export(ctx context.Context, data tel.Exportable) error } // StdoutExporter represents a temporary telemetry data exporter. @@ -19,7 +19,7 @@ type StdoutExporter struct { } // Export takes context and trace data and writes to the endpoint. -func (e *StdoutExporter) Export(_ context.Context, data telemetry.Exportable) error { +func (e *StdoutExporter) Export(_ context.Context, data tel.Exportable) error { fmt.Fprintf(e.Endpoint, "%+v", data) return nil } @@ -28,19 +28,8 @@ func (e *StdoutExporter) Export(_ context.Context, data telemetry.Exportable) er // //go:generate go run -tags=generator github.com/nginxinc/telemetry-exporter/cmd/generator -type Data -scheme -scheme-protocol=NICProductTelemetry -scheme-df-datatype=nic-product-telemetry -scheme-namespace=ingress.nginx.com type Data struct { - telemetry.Data + tel.Data NICResourceCounts - NodeCount int64 - ClusterID string - K8sVersion string - Arch string - Platform string -} - -// ProjectMeta holds metadata for the project. -type ProjectMeta struct { - Name string - Version string } // NICResourceCounts holds a count of NIC specific resource.