diff --git a/internal/mode/static/telemetry/collector.go b/internal/mode/static/telemetry/collector.go index b809df6ea2..e400999070 100644 --- a/internal/mode/static/telemetry/collector.go +++ b/internal/mode/static/telemetry/collector.go @@ -11,6 +11,7 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + k8sversion "k8s.io/apimachinery/pkg/util/version" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" @@ -100,14 +101,11 @@ func NewDataCollectorImpl( } } -// notImplemented is a value for string field, for which collection is not implemented yet. -const notImplemented = "not-implemented" - // Collect collects and returns telemetry Data. func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) { - nodeCount, err := CollectNodeCount(ctx, c.cfg.K8sClientReader) + clusterInfo, err := collectClusterInformation(ctx, c.cfg.K8sClientReader) if err != nil { - return Data{}, fmt.Errorf("failed to collect node count: %w", err) + return Data{}, fmt.Errorf("failed to collect cluster information: %w", err) } graphResourceCount, err := collectGraphResourceCount(c.cfg.GraphGetter, c.cfg.ConfigurationGetter) @@ -130,21 +128,16 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) { return Data{}, fmt.Errorf("failed to get NGF deploymentID: %w", err) } - var clusterID string - if clusterID, err = CollectClusterID(ctx, c.cfg.K8sClientReader); err != nil { - return Data{}, fmt.Errorf("failed to collect clusterID: %w", err) - } - data := Data{ Data: tel.Data{ ProjectName: "NGF", ProjectVersion: c.cfg.Version, ProjectArchitecture: runtime.GOARCH, - ClusterID: clusterID, - ClusterVersion: notImplemented, - ClusterPlatform: notImplemented, + ClusterID: clusterInfo.ClusterID, + ClusterVersion: clusterInfo.Version, + ClusterPlatform: clusterInfo.Platform, InstallationID: deploymentID, - ClusterNodeCount: int64(nodeCount), + ClusterNodeCount: int64(clusterInfo.NodeCount), }, NGFResourceCounts: graphResourceCount, ImageSource: c.cfg.ImageSource, @@ -156,16 +149,6 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) { return data, nil } -// CollectNodeCount returns the number of nodes in the cluster. -func CollectNodeCount(ctx context.Context, k8sClient client.Reader) (int, error) { - var nodes v1.NodeList - if err := k8sClient.List(ctx, &nodes); err != nil { - return 0, fmt.Errorf("failed to get NodeList: %w", err) - } - - return len(nodes.Items), nil -} - func collectGraphResourceCount( graphGetter GraphGetter, configurationGetter ConfigurationGetter, @@ -275,3 +258,51 @@ func CollectClusterID(ctx context.Context, k8sClient client.Reader) (string, err } return string(kubeNamespace.GetUID()), nil } + +type clusterInformation struct { + Platform string + Version string + ClusterID string + NodeCount int +} + +func collectClusterInformation(ctx context.Context, k8sClient client.Reader) (clusterInformation, error) { + var clusterInfo clusterInformation + + var nodes v1.NodeList + if err := k8sClient.List(ctx, &nodes); err != nil { + return clusterInformation{}, fmt.Errorf("failed to get NodeList: %w", err) + } + + nodeCount := len(nodes.Items) + if nodeCount == 0 { + return clusterInformation{}, errors.New("failed to collect cluster information: NodeList length is zero") + } + clusterInfo.NodeCount = nodeCount + + node := nodes.Items[0] + + kubeletVersion := node.Status.NodeInfo.KubeletVersion + version, err := k8sversion.ParseGeneric(kubeletVersion) + if err != nil { + clusterInfo.Version = "unknown" + } else { + clusterInfo.Version = version.String() + } + + var namespaces v1.NamespaceList + if err = k8sClient.List(ctx, &namespaces); err != nil { + return clusterInformation{}, fmt.Errorf("failed to collect cluster information: %w", err) + } + + clusterInfo.Platform = getPlatform(node, namespaces) + + var clusterID string + clusterID, err = CollectClusterID(ctx, k8sClient) + if err != nil { + return clusterInformation{}, fmt.Errorf("failed to collect cluster information: %w", err) + } + clusterInfo.ClusterID = clusterID + + return clusterInfo, nil +} diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go index 65525d0e61..cd5ae9cdaf 100644 --- a/internal/mode/static/telemetry/collector_test.go +++ b/internal/mode/static/telemetry/collector_test.go @@ -3,7 +3,6 @@ package telemetry_test import ( "context" "errors" - "fmt" "reflect" "runtime" @@ -26,20 +25,23 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/telemetry/telemetryfakes" ) -func createListCallsFunc(nodes []v1.Node) func( - ctx context.Context, - list client.ObjectList, - option ...client.ListOption, -) error { - return func(_ context.Context, list client.ObjectList, option ...client.ListOption) error { +type listCallsFunc = func( + context.Context, + client.ObjectList, + ...client.ListOption, +) error + +func createListCallsFunc(objects ...client.ObjectList) listCallsFunc { + return func(_ context.Context, object client.ObjectList, option ...client.ListOption) error { Expect(option).To(BeEmpty()) - switch typedList := list.(type) { - case *v1.NodeList: - typedList.Items = append(typedList.Items, nodes...) - default: - Fail(fmt.Sprintf("unknown type: %T", typedList)) + for _, obj := range objects { + if reflect.TypeOf(obj) == reflect.TypeOf(object) { + reflect.ValueOf(object).Elem().Set(reflect.ValueOf(obj).Elem()) + return nil + } } + return nil } } @@ -80,7 +82,9 @@ var _ = Describe("Collector", Ordered, func() { ngfReplicaSet *appsv1.ReplicaSet kubeNamespace *v1.Namespace baseGetCalls getCallsFunc + baseListCalls listCallsFunc flags config.Flags + nodeList *v1.NodeList ) BeforeAll(func() { @@ -132,6 +136,24 @@ var _ = Describe("Collector", Ordered, func() { Names: []string{"boolFlag", "intFlag", "stringFlag"}, Values: []string{"false", "default", "user-defined"}, } + + nodeList = &v1.NodeList{ + Items: []v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + }, + Spec: v1.NodeSpec{ + ProviderID: "k3s://ip-172-16-0-210", + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{ + KubeletVersion: "v1.28.6+k3s2", + }, + }, + }, + }, + } }) BeforeEach(func() { @@ -141,10 +163,10 @@ var _ = Describe("Collector", Ordered, func() { ProjectVersion: version, ProjectArchitecture: runtime.GOARCH, ClusterID: string(kubeNamespace.GetUID()), - ClusterVersion: "not-implemented", - ClusterPlatform: "not-implemented", + ClusterVersion: "1.28.6", + ClusterPlatform: "k3s", InstallationID: string(ngfReplicaSet.ObjectMeta.OwnerReferences[0].UID), - ClusterNodeCount: 0, + ClusterNodeCount: 1, }, NGFResourceCounts: telemetry.NGFResourceCounts{}, NGFReplicaCount: 1, @@ -172,6 +194,9 @@ var _ = Describe("Collector", Ordered, func() { baseGetCalls = createGetCallsFunc(ngfPod, ngfReplicaSet, kubeNamespace) k8sClientReader.GetCalls(baseGetCalls) + + baseListCalls = createListCallsFunc(nodeList) + k8sClientReader.ListCalls(baseListCalls) }) mergeGetCallsWithBase := func(f getCallsFunc) getCallsFunc { @@ -188,23 +213,46 @@ var _ = Describe("Collector", Ordered, func() { } } + mergeListCallsWithBase := func(f listCallsFunc) listCallsFunc { + return func( + ctx context.Context, + object client.ObjectList, + option ...client.ListOption, + ) error { + err := baseListCalls(ctx, object, option...) + Expect(err).ToNot(HaveOccurred()) + + return f(ctx, object, option...) + } + } + Describe("Normal case", func() { When("collecting telemetry data", func() { It("collects all fields", func() { - nodes := []v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node1", + nodes := &v1.NodeList{ + Items: []v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + }, + Spec: v1.NodeSpec{ + ProviderID: "kind://docker/kind/kind-control-plane", + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{ + KubeletVersion: "v1.29.2", + }, + }, }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node2", + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node2", + }, }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node3", + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node3", + }, }, }, } @@ -294,6 +342,8 @@ var _ = Describe("Collector", Ordered, func() { ServiceCount: 3, EndpointCount: 4, } + expData.ClusterVersion = "1.29.2" + expData.ClusterPlatform = "kind" data, err := dataCollector.Collect(ctx) @@ -303,8 +353,27 @@ var _ = Describe("Collector", Ordered, func() { }) }) - Describe("clusterID collector", func() { - When("collecting clusterID", func() { + Describe("cluster information collector", func() { + When("collecting cluster platform", func() { + When("it encounters an error while collecting data", func() { + It("should error if the kubernetes client errored when getting the NamespaceList", func() { + expectedError := errors.New("failed to get NamespaceList") + k8sClientReader.ListCalls(mergeListCallsWithBase( + func(_ context.Context, object client.ObjectList, _ ...client.ListOption) error { + switch object.(type) { + case *v1.NamespaceList: + return expectedError + } + return nil + })) + + _, err := dataCollector.Collect(ctx) + Expect(err).To(MatchError(expectedError)) + }) + }) + }) + + When("collecting clusterID data", func() { When("it encounters an error while collecting data", func() { It("should error if the kubernetes client errored when getting the namespace", func() { expectedError := errors.New("there was an error getting clusterID") @@ -322,12 +391,42 @@ var _ = Describe("Collector", Ordered, func() { }) }) }) + + When("collecting cluster version data", func() { + When("the kubelet version is missing", func() { + It("should be report 'unknown'", func() { + nodes := &v1.NodeList{ + Items: []v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + }, + Spec: v1.NodeSpec{ + ProviderID: "k3s://ip-172-16-0-210", + }, + }, + }, + } + + k8sClientReader.ListCalls(createListCallsFunc(nodes)) + expData.ClusterVersion = "unknown" + expData.ClusterPlatform = "k3s" + + data, err := dataCollector.Collect(ctx) + + Expect(err).To(BeNil()) + Expect(expData).To(Equal(data)) + }) + }) + }) }) Describe("node count collector", func() { When("collecting node count data", func() { - It("collects correct data for no nodes", func() { - k8sClientReader.ListCalls(createListCallsFunc(nil)) + It("collects correct data for one node", func() { + k8sClientReader.ListCalls(createListCallsFunc(nodeList)) + + expData.Data.ClusterNodeCount = 1 data, err := dataCollector.Collect(ctx) @@ -335,30 +434,30 @@ var _ = Describe("Collector", Ordered, func() { Expect(expData).To(Equal(data)) }) - It("collects correct data for one node", func() { - nodes := []v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{Name: "node1"}, - }, - } - - k8sClientReader.ListCalls(createListCallsFunc(nodes)) + When("it encounters an error while collecting data", func() { + It("should error when there are no nodes", func() { + expectedError := errors.New("failed to collect cluster information: NodeList length is zero") + k8sClientReader.ListCalls(createListCallsFunc(nil)) - expData.ClusterNodeCount = 1 + _, err := dataCollector.Collect(ctx) - data, err := dataCollector.Collect(ctx) + Expect(err).To(MatchError(expectedError)) + }) - Expect(err).To(BeNil()) - Expect(expData).To(Equal(data)) - }) - }) - When("it encounters an error while collecting data", func() { - It("should error on kubernetes client api errors", func() { - expectedError := errors.New("there was an error getting NodeList") - k8sClientReader.ListReturns(expectedError) + It("should error on kubernetes client api errors", func() { + expectedError := errors.New("failed to get NodeList") + k8sClientReader.ListCalls( + func(_ context.Context, object client.ObjectList, _ ...client.ListOption) error { + switch object.(type) { + case *v1.NodeList: + return expectedError + } + return nil + }) - _, err := dataCollector.Collect(ctx) - Expect(err).To(MatchError(expectedError)) + _, err := dataCollector.Collect(ctx) + Expect(err).To(MatchError(expectedError)) + }) }) }) }) diff --git a/internal/mode/static/telemetry/platform.go b/internal/mode/static/telemetry/platform.go new file mode 100644 index 0000000000..ff63117c70 --- /dev/null +++ b/internal/mode/static/telemetry/platform.go @@ -0,0 +1,101 @@ +package telemetry + +import ( + "strings" + + v1 "k8s.io/api/core/v1" +) + +type k8sState struct { + node v1.Node + namespaces v1.NamespaceList +} + +type platformExtractor func(k8sState) string + +func buildProviderIDExtractor(id, platform string) platformExtractor { + return func(state k8sState) string { + if strings.HasPrefix(state.node.Spec.ProviderID, id) { + return platform + } + return "" + } +} + +const ( + gkeIdentifier = "gce" + awsIdentifier = "aws" + azureIdentifier = "azure" + kindIdentifier = "kind" + k3sIdentifier = "k3s" + openshiftIdentifier = "node.openshift.io/os_id" + rancherIdentifier = "cattle-system" + + platformGKE = "gke" + platformAWS = "eks" + platformAzure = "aks" + platformKind = "kind" + platformK3S = "k3s" + platformOpenShift = "openshift" + platformRancher = "rancher" + platformOther = "other" +) + +var platformExtractors = []platformExtractor{ + openShiftExtractor, + rancherExtractor, + // ID provider extractors must run after the rest + buildProviderIDExtractor(gkeIdentifier, platformGKE), + buildProviderIDExtractor(awsIdentifier, platformAWS), + buildProviderIDExtractor(azureIdentifier, platformAzure), + buildProviderIDExtractor(kindIdentifier, platformKind), + buildProviderIDExtractor(k3sIdentifier, platformK3S), +} + +func getPlatform(node v1.Node, namespaces v1.NamespaceList) string { + state := k8sState{ + node: node, + namespaces: namespaces, + } + + for _, extractor := range platformExtractors { + if platform := extractor(state); platform != "" { + return platform + } + } + + return unknownProviderIDExtractor(state) +} + +func openShiftExtractor(state k8sState) string { + // openshift platform won't show up in node's ProviderID + if state.node.Labels[openshiftIdentifier] != "" { + return platformOpenShift + } + + return "" +} + +func rancherExtractor(state k8sState) string { + // rancher platform won't show up in the node's ProviderID + for _, ns := range state.namespaces.Items { + if ns.Name == rancherIdentifier { + return platformRancher + } + } + + return "" +} + +func unknownProviderIDExtractor(state k8sState) string { + var providerName string + if prefix, _, found := strings.Cut(state.node.Spec.ProviderID, "://"); found { + providerName = strings.TrimSpace(prefix) + } + + if providerName == "" { + return platformOther + } + + return platformOther + "_" + providerName +} diff --git a/internal/mode/static/telemetry/platform_test.go b/internal/mode/static/telemetry/platform_test.go new file mode 100644 index 0000000000..0c2939d820 --- /dev/null +++ b/internal/mode/static/telemetry/platform_test.go @@ -0,0 +1,137 @@ +package telemetry + +import ( + "testing" + + . "github.com/onsi/gomega" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestGetPlatform(t *testing.T) { + tests := []struct { + node *v1.Node + namespaces *v1.NamespaceList + expectedPlatform string + name string + }{ + { + node: &v1.Node{ + Spec: v1.NodeSpec{ + ProviderID: "kind://docker/kind/kind-control-plane", + }, + }, + namespaces: &v1.NamespaceList{}, + expectedPlatform: "kind", + name: "kind platform", + }, + { + node: &v1.Node{ + Spec: v1.NodeSpec{ + ProviderID: "k3s://ip-172-16-0-210", + }, + }, + namespaces: &v1.NamespaceList{}, + expectedPlatform: "k3s", + name: "k3s platform", + }, + { + node: &v1.Node{ + Spec: v1.NodeSpec{ + ProviderID: "gce://test-data/us-central1-c/test-data", + }, + }, + namespaces: &v1.NamespaceList{}, + expectedPlatform: "gke", + name: "gke platform", + }, + { + node: &v1.Node{ + Spec: v1.NodeSpec{ + ProviderID: "azure://test-data/us-central1-c/test-data", + }, + }, + namespaces: &v1.NamespaceList{}, + expectedPlatform: "aks", + name: "aks platform", + }, + { + node: &v1.Node{ + Spec: v1.NodeSpec{ + ProviderID: "aws://test-data/us-central1-c/test-data", + }, + }, + namespaces: &v1.NamespaceList{}, + expectedPlatform: "eks", + name: "eks platform", + }, + { + node: &v1.Node{ + Spec: v1.NodeSpec{ + ProviderID: "k3s://ip-172-16-0-210", + }, + }, + namespaces: &v1.NamespaceList{ + Items: []v1.Namespace{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cattle-system", + }, + }, + }, + }, + expectedPlatform: "rancher", + name: "rancher platform on k3s", + }, + { + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"node.openshift.io/os_id": "test"}, + }, + Spec: v1.NodeSpec{ + ProviderID: "k3s://ip-172-16-0-210", + }, + }, + namespaces: &v1.NamespaceList{}, + expectedPlatform: "openshift", + name: "openshift platform on k3s", + }, + { + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"node.openshift.io/os_id": "test"}, + }, + Spec: v1.NodeSpec{ + ProviderID: "aws://test-data/us-central1-c/test-data", + }, + }, + namespaces: &v1.NamespaceList{}, + expectedPlatform: "openshift", + name: "openshift platform on aws", + }, + { + node: &v1.Node{ + Spec: v1.NodeSpec{ + ProviderID: "different-platform://ip-172-16-0-210", + }, + }, + namespaces: &v1.NamespaceList{}, + expectedPlatform: "other_different-platform", + name: "other platform", + }, + { + node: &v1.Node{}, + namespaces: &v1.NamespaceList{}, + expectedPlatform: "other", + name: "missing providerID", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + platform := getPlatform(*test.node, *test.namespaces) + g.Expect(platform).To(Equal(test.expectedPlatform)) + }) + } +} diff --git a/internal/mode/static/usage/job_worker.go b/internal/mode/static/usage/job_worker.go index 3f039181e3..f71fdf64fb 100644 --- a/internal/mode/static/usage/job_worker.go +++ b/internal/mode/static/usage/job_worker.go @@ -6,6 +6,7 @@ import ( "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" + v1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" @@ -19,7 +20,7 @@ func CreateUsageJobWorker( cfg config.Config, ) func(ctx context.Context) { return func(ctx context.Context) { - nodeCount, err := telemetry.CollectNodeCount(ctx, k8sClient) + nodeCount, err := CollectNodeCount(ctx, k8sClient) if err != nil { logger.Error(err, "Failed to collect node count") } @@ -79,3 +80,13 @@ func GetTotalNGFPodCount(ctx context.Context, k8sClient client.Reader) (int, err return count, nil } + +// CollectNodeCount returns the number of nodes in the cluster. +func CollectNodeCount(ctx context.Context, k8sClient client.Reader) (int, error) { + var nodes v1.NodeList + if err := k8sClient.List(ctx, &nodes); err != nil { + return 0, fmt.Errorf("failed to get NodeList: %w", err) + } + + return len(nodes.Items), nil +} diff --git a/internal/mode/static/usage/job_worker_test.go b/internal/mode/static/usage/job_worker_test.go index d7272ae28a..d0c9270a6f 100644 --- a/internal/mode/static/usage/job_worker_test.go +++ b/internal/mode/static/usage/job_worker_test.go @@ -156,3 +156,32 @@ func TestGetTotalNGFPodCount(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) g.Expect(count).To(Equal(expCount)) } + +func TestCollectNodeCount(t *testing.T) { + g := NewWithT(t) + + node1 := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + }, + Spec: v1.NodeSpec{ + ProviderID: "k3s://ip-172-16-0-210", + }, + } + + node2 := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node2", + }, + Spec: v1.NodeSpec{ + ProviderID: "k3s://ip-172-16-0-210", + }, + } + + k8sClient := fake.NewFakeClient(node1, node2) + + expCount := 2 + count, err := usage.CollectNodeCount(context.Background(), k8sClient) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(count).To(Equal(expCount)) +}