From fea931728fa8abbf4b1054184a8c5fbdb007f7d3 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Wed, 14 Feb 2024 14:48:27 -0800 Subject: [PATCH 01/14] feat: add clusterID to telemetry objects --- internal/mode/static/telemetry/collector.go | 22 +++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/internal/mode/static/telemetry/collector.go b/internal/mode/static/telemetry/collector.go index 6842b5b621..167a926234 100644 --- a/internal/mode/static/telemetry/collector.go +++ b/internal/mode/static/telemetry/collector.go @@ -14,6 +14,9 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" ) +// kubeSystem indicates the name of kube-system namespace +const kubeSystem = "kube-system" + //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . GraphGetter // GraphGetter gets the latest Graph. @@ -49,6 +52,7 @@ type ProjectMetadata struct { // Note: this type might change once https://github.com/nginxinc/nginx-gateway-fabric/issues/1318 is implemented. type Data struct { ProjectMetadata ProjectMetadata + ClusterID string NodeCount int NGFResourceCounts NGFResourceCounts NGFReplicaCount int @@ -99,6 +103,11 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) { return Data{}, fmt.Errorf("failed to collect NGF replica count: %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{ NodeCount: nodeCount, NGFResourceCounts: graphResourceCount, @@ -107,6 +116,7 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) { Version: c.cfg.Version, }, NGFReplicaCount: ngfReplicaCount, + ClusterID: clusterID, } return data, nil @@ -193,3 +203,15 @@ func collectNGFReplicaCount(ctx context.Context, k8sClient client.Reader, podNSN return int(*replicaSet.Spec.Replicas), nil } + +func collectClusterID(ctx context.Context, k8sClient client.Reader) (string, error) { + key := types.NamespacedName{ + Name: kubeSystem, + } + var kubeNamespace v1.Namespace + err := k8sClient.Get(ctx, key, &kubeNamespace) + if err != nil { + return "", fmt.Errorf("failed to get namespace :%w", err) + } + return string(kubeNamespace.GetUID()), nil +} From d7860c5b036f6f924df8a76817e53d9eba5a7c7d Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Thu, 15 Feb 2024 08:50:01 -0800 Subject: [PATCH 02/14] update unit tests --- .../mode/static/telemetry/collector_test.go | 40 ++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go index a2afb16ff9..357ca9c9af 100644 --- a/internal/mode/static/telemetry/collector_test.go +++ b/internal/mode/static/telemetry/collector_test.go @@ -74,6 +74,7 @@ var _ = Describe("Collector", Ordered, func() { podNSName types.NamespacedName ngfPod *v1.Pod ngfReplicaSet *appsv1.ReplicaSet + kubeNamespace *v1.Namespace ) BeforeAll(func() { @@ -103,6 +104,13 @@ var _ = Describe("Collector", Ordered, func() { Namespace: "nginx-gateway", Name: "ngf-pod", } + + kubeNamespace = &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kube-system", + UID: "test-uid", + }, + } }) BeforeEach(func() { @@ -111,6 +119,7 @@ var _ = Describe("Collector", Ordered, func() { NodeCount: 0, NGFResourceCounts: telemetry.NGFResourceCounts{}, NGFReplicaCount: 1, + ClusterID: string(kubeNamespace.GetUID()), } k8sClientReader = &eventsfakes.FakeReader{} @@ -127,7 +136,7 @@ var _ = Describe("Collector", Ordered, func() { Version: version, PodNSName: podNSName, }) - k8sClientReader.GetCalls(createGetCallsFunc(ngfPod, ngfReplicaSet)) + k8sClientReader.GetCalls(createGetCallsFunc(ngfPod, ngfReplicaSet, kubeNamespace)) }) Describe("Normal case", func() { @@ -135,13 +144,19 @@ var _ = Describe("Collector", Ordered, func() { It("collects all fields", func() { nodes := []v1.Node{ { - ObjectMeta: metav1.ObjectMeta{Name: "node1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + }, }, { - ObjectMeta: metav1.ObjectMeta{Name: "node2"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "node2", + }, }, { - ObjectMeta: metav1.ObjectMeta{Name: "node3"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "node3", + }, }, } @@ -150,7 +165,6 @@ var _ = Describe("Collector", Ordered, func() { secret1 := &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "secret1"}} secret2 := &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "secret2"}} nilsecret := &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "nilsecret"}} - svc1 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "svc1"}} svc2 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "svc2"}} nilsvc := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "nilsvc"}} @@ -239,6 +253,18 @@ var _ = Describe("Collector", Ordered, func() { }) }) + Describe("clusterID collector", func() { + When("collecting clusterID", func() { + It("throws an error when collecting clusterID", func() { + expectedError := errors.New("there was an error getting clusterID") + k8sClientReader.GetReturns(expectedError) + + _, err := dataCollector.Collect(ctx) + Expect(err).To(MatchError(expectedError)) + }) + }) + }) + Describe("node count collector", func() { When("collecting node count data", func() { It("collects correct data for no nodes", func() { @@ -451,6 +477,7 @@ var _ = Describe("Collector", Ordered, func() { OwnerReferences: nil, }, }, + kubeNamespace, )) _, err := dataCollector.Collect(ctx) @@ -475,6 +502,7 @@ var _ = Describe("Collector", Ordered, func() { }, }, }, + kubeNamespace, )) _, err := dataCollector.Collect(ctx) @@ -495,6 +523,7 @@ var _ = Describe("Collector", Ordered, func() { }, }, }, + kubeNamespace, )) _, err := dataCollector.Collect(ctx) @@ -520,6 +549,7 @@ var _ = Describe("Collector", Ordered, func() { Replicas: nil, }, }, + kubeNamespace, )) _, err := dataCollector.Collect(ctx) From f64b7badc5c678d4e74495ca23756381b64fd788 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Thu, 15 Feb 2024 10:41:03 -0800 Subject: [PATCH 03/14] Update internal/mode/static/telemetry/collector.go Co-authored-by: Saylor Berman --- internal/mode/static/telemetry/collector.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/mode/static/telemetry/collector.go b/internal/mode/static/telemetry/collector.go index 167a926234..e661d6c836 100644 --- a/internal/mode/static/telemetry/collector.go +++ b/internal/mode/static/telemetry/collector.go @@ -209,8 +209,7 @@ func collectClusterID(ctx context.Context, k8sClient client.Reader) (string, err Name: kubeSystem, } var kubeNamespace v1.Namespace - err := k8sClient.Get(ctx, key, &kubeNamespace) - if err != nil { + if err := k8sClient.Get(ctx, key, &kubeNamespace); err != nil { return "", fmt.Errorf("failed to get namespace :%w", err) } return string(kubeNamespace.GetUID()), nil From 5ecbc88303c2ca380f46e4594ce8685845480244 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Thu, 15 Feb 2024 10:53:05 -0800 Subject: [PATCH 04/14] Update internal/mode/static/telemetry/collector.go Co-authored-by: bjee19 <139261241+bjee19@users.noreply.github.com> --- internal/mode/static/telemetry/collector.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/telemetry/collector.go b/internal/mode/static/telemetry/collector.go index e661d6c836..ad3f206849 100644 --- a/internal/mode/static/telemetry/collector.go +++ b/internal/mode/static/telemetry/collector.go @@ -14,7 +14,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" ) -// kubeSystem indicates the name of kube-system namespace +// kubeSystem indicates the name of kube-system namespace. const kubeSystem = "kube-system" //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . GraphGetter From d919d91fc998032cd78a037398aee96767f2bc74 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Thu, 15 Feb 2024 10:53:54 -0800 Subject: [PATCH 05/14] Update internal/mode/static/telemetry/collector_test.go Co-authored-by: bjee19 <139261241+bjee19@users.noreply.github.com> --- internal/mode/static/telemetry/collector_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go index 357ca9c9af..d1a4bcd0fb 100644 --- a/internal/mode/static/telemetry/collector_test.go +++ b/internal/mode/static/telemetry/collector_test.go @@ -255,7 +255,7 @@ var _ = Describe("Collector", Ordered, func() { Describe("clusterID collector", func() { When("collecting clusterID", func() { - It("throws an error when collecting clusterID", func() { + It("should error if the kubernetes client errored when getting the namespace", func() { expectedError := errors.New("there was an error getting clusterID") k8sClientReader.GetReturns(expectedError) From 2654957bf4aa6c61b4baf27df09ed3632cdc58b8 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Thu, 15 Feb 2024 11:36:52 -0800 Subject: [PATCH 06/14] update unit tests stub --- .../mode/static/telemetry/collector_test.go | 97 +++++++++---------- 1 file changed, 46 insertions(+), 51 deletions(-) diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go index d1a4bcd0fb..ac369af785 100644 --- a/internal/mode/static/telemetry/collector_test.go +++ b/internal/mode/static/telemetry/collector_test.go @@ -41,12 +41,14 @@ func createListCallsFunc(nodes []v1.Node) func( } } -func createGetCallsFunc(objects ...client.Object) func( +type getCallsFunc = func( context.Context, types.NamespacedName, client.Object, ...client.GetOption, -) error { +) error + +func createGetCallsFunc(objects ...client.Object) getCallsFunc { return func(_ context.Context, _ types.NamespacedName, object client.Object, option ...client.GetOption) error { Expect(option).To(BeEmpty()) @@ -57,7 +59,6 @@ func createGetCallsFunc(objects ...client.Object) func( } } - Fail(fmt.Sprintf("unknown type: %T", object)) return nil } } @@ -75,6 +76,8 @@ var _ = Describe("Collector", Ordered, func() { ngfPod *v1.Pod ngfReplicaSet *appsv1.ReplicaSet kubeNamespace *v1.Namespace + + baseGetCalls getCallsFunc ) BeforeAll(func() { @@ -136,9 +139,22 @@ var _ = Describe("Collector", Ordered, func() { Version: version, PodNSName: podNSName, }) - k8sClientReader.GetCalls(createGetCallsFunc(ngfPod, ngfReplicaSet, kubeNamespace)) + + baseGetCalls = createGetCallsFunc(ngfPod, ngfReplicaSet, kubeNamespace) + k8sClientReader.GetCalls(baseGetCalls) }) + mergeGetCallsWithBase := func(f getCallsFunc) getCallsFunc { + return func(ctx context.Context, nsName types.NamespacedName, + object client.Object, option ...client.GetOption, + ) error { + err := baseGetCalls(ctx, nsName, object, option...) + Expect(err).ToNot(HaveOccurred()) + + return f(ctx, nsName, object, option...) + } + } + Describe("Normal case", func() { When("collecting telemetry data", func() { It("collects all fields", func() { @@ -255,9 +271,17 @@ var _ = Describe("Collector", Ordered, func() { Describe("clusterID collector", func() { When("collecting clusterID", func() { - It("should error if the kubernetes client errored when getting the namespace", func() { + It("throws an error when collecting clusterID", func() { expectedError := errors.New("there was an error getting clusterID") - k8sClientReader.GetReturns(expectedError) + k8sClientReader.GetCalls(mergeGetCallsWithBase( + func(_ context.Context, _ types.NamespacedName, object client.Object, option ...client.GetOption) error { + Expect(option).To(BeEmpty()) + switch object.(type) { + case *v1.Namespace: + return expectedError + } + return nil + })) _, err := dataCollector.Collect(ctx) Expect(err).To(MatchError(expectedError)) @@ -451,18 +475,17 @@ var _ = Describe("Collector", Ordered, func() { When("it encounters an error while collecting data", func() { It("should error if the kubernetes client errored when getting the Pod", func() { expectedErr := errors.New("there was an error getting the Pod") - k8sClientReader.GetCalls( + + k8sClientReader.GetCalls(mergeGetCallsWithBase( func(_ context.Context, _ client.ObjectKey, object client.Object, option ...client.GetOption) error { Expect(option).To(BeEmpty()) - - switch typedObj := object.(type) { + switch object.(type) { case *v1.Pod: return expectedErr - default: - Fail(fmt.Sprintf("unknown type: %T", typedObj)) } return nil - }) + }, + )) _, err := dataCollector.Collect(ctx) Expect(err).To(MatchError(expectedErr)) @@ -470,15 +493,14 @@ var _ = Describe("Collector", Ordered, func() { It("should error if the Pod's owner reference is nil", func() { expectedErr := errors.New("expected one owner reference of the NGF Pod, got 0") - k8sClientReader.GetCalls(createGetCallsFunc( + k8sClientReader.GetCalls(mergeGetCallsWithBase(createGetCallsFunc( &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "pod1", OwnerReferences: nil, }, }, - kubeNamespace, - )) + ))) _, err := dataCollector.Collect(ctx) Expect(err).To(MatchError(expectedErr)) @@ -486,7 +508,7 @@ var _ = Describe("Collector", Ordered, func() { It("should error if the Pod has multiple owner references", func() { expectedErr := errors.New("expected one owner reference of the NGF Pod, got 2") - k8sClientReader.GetCalls(createGetCallsFunc( + k8sClientReader.GetCalls(mergeGetCallsWithBase(createGetCallsFunc( &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "pod1", @@ -502,8 +524,7 @@ var _ = Describe("Collector", Ordered, func() { }, }, }, - kubeNamespace, - )) + ))) _, err := dataCollector.Collect(ctx) Expect(err).To(MatchError(expectedErr)) @@ -511,7 +532,7 @@ var _ = Describe("Collector", Ordered, func() { It("should error if the Pod's owner reference is not a ReplicaSet", func() { expectedErr := errors.New("expected pod owner reference to be ReplicaSet, got Deployment") - k8sClientReader.GetCalls(createGetCallsFunc( + k8sClientReader.GetCalls(mergeGetCallsWithBase(createGetCallsFunc( &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "pod1", @@ -523,8 +544,7 @@ var _ = Describe("Collector", Ordered, func() { }, }, }, - kubeNamespace, - )) + ))) _, err := dataCollector.Collect(ctx) Expect(err).To(MatchError(expectedErr)) @@ -532,25 +552,13 @@ var _ = Describe("Collector", Ordered, func() { It("should error if the replica set's replicas is nil", func() { expectedErr := errors.New("replica set replicas was nil") - k8sClientReader.GetCalls(createGetCallsFunc( - &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod1", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "ReplicaSet", - Name: "replicaset1", - }, - }, - }, - }, + k8sClientReader.GetCalls(mergeGetCallsWithBase(createGetCallsFunc( &appsv1.ReplicaSet{ Spec: appsv1.ReplicaSetSpec{ Replicas: nil, }, }, - kubeNamespace, - )) + ))) _, err := dataCollector.Collect(ctx) Expect(err).To(MatchError(expectedErr)) @@ -558,28 +566,15 @@ var _ = Describe("Collector", Ordered, func() { It("should error if the kubernetes client errored when getting the ReplicaSet", func() { expectedErr := errors.New("there was an error getting the ReplicaSet") - k8sClientReader.GetCalls( + k8sClientReader.GetCalls(mergeGetCallsWithBase( func(_ context.Context, _ client.ObjectKey, object client.Object, option ...client.GetOption) error { Expect(option).To(BeEmpty()) - - switch typedObj := object.(type) { - case *v1.Pod: - typedObj.ObjectMeta = metav1.ObjectMeta{ - Name: "pod1", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "ReplicaSet", - Name: "replicaset1", - }, - }, - } + switch object.(type) { case *appsv1.ReplicaSet: return expectedErr - default: - Fail(fmt.Sprintf("unknown type: %T", typedObj)) } return nil - }) + })) _, err := dataCollector.Collect(ctx) Expect(err).To(MatchError(expectedErr)) From 814afd0c17a489ee9da4ffa2c79741dc804b08b1 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Thu, 15 Feb 2024 11:58:05 -0800 Subject: [PATCH 07/14] Update internal/mode/static/telemetry/collector.go Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> --- internal/mode/static/telemetry/collector.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/telemetry/collector.go b/internal/mode/static/telemetry/collector.go index ad3f206849..9a99e3dc9d 100644 --- a/internal/mode/static/telemetry/collector.go +++ b/internal/mode/static/telemetry/collector.go @@ -210,7 +210,7 @@ func collectClusterID(ctx context.Context, k8sClient client.Reader) (string, err } var kubeNamespace v1.Namespace if err := k8sClient.Get(ctx, key, &kubeNamespace); err != nil { - return "", fmt.Errorf("failed to get namespace :%w", err) + return "", fmt.Errorf("failed to get kube-system namespace: %w", err) } return string(kubeNamespace.GetUID()), nil } From edf2888a582d1efd6bc352a6ccd188dcba1d20b5 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Thu, 15 Feb 2024 11:58:19 -0800 Subject: [PATCH 08/14] Update internal/mode/static/telemetry/collector_test.go Co-authored-by: bjee19 <139261241+bjee19@users.noreply.github.com> --- internal/mode/static/telemetry/collector_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go index ac369af785..e85aef5fb6 100644 --- a/internal/mode/static/telemetry/collector_test.go +++ b/internal/mode/static/telemetry/collector_test.go @@ -271,7 +271,7 @@ var _ = Describe("Collector", Ordered, func() { Describe("clusterID collector", func() { When("collecting clusterID", func() { - It("throws an error when collecting clusterID", func() { + It("should error if the kubernetes client errored when getting the namespace", func() { expectedError := errors.New("there was an error getting clusterID") k8sClientReader.GetCalls(mergeGetCallsWithBase( func(_ context.Context, _ types.NamespacedName, object client.Object, option ...client.GetOption) error { From 1838dc8a5f0a7b49d819476fefdd2a48c9cf79aa Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Thu, 15 Feb 2024 12:01:55 -0800 Subject: [PATCH 09/14] update based on reviews --- internal/mode/static/telemetry/collector_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go index e85aef5fb6..ae2e5bbd2e 100644 --- a/internal/mode/static/telemetry/collector_test.go +++ b/internal/mode/static/telemetry/collector_test.go @@ -145,7 +145,9 @@ var _ = Describe("Collector", Ordered, func() { }) mergeGetCallsWithBase := func(f getCallsFunc) getCallsFunc { - return func(ctx context.Context, nsName types.NamespacedName, + return func( + ctx context.Context, + nsName types.NamespacedName, object client.Object, option ...client.GetOption, ) error { err := baseGetCalls(ctx, nsName, object, option...) @@ -184,7 +186,6 @@ var _ = Describe("Collector", Ordered, func() { svc1 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "svc1"}} svc2 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "svc2"}} nilsvc := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "nilsvc"}} - graph := &graph.Graph{ GatewayClass: &graph.GatewayClass{}, Gateway: &graph.Gateway{}, From 074f7f6586f9c1a45bfdc71021394409bc22a05d Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Thu, 15 Feb 2024 12:07:43 -0800 Subject: [PATCH 10/14] fix line spacing --- internal/mode/static/telemetry/collector_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go index ae2e5bbd2e..fb660de01d 100644 --- a/internal/mode/static/telemetry/collector_test.go +++ b/internal/mode/static/telemetry/collector_test.go @@ -183,9 +183,11 @@ var _ = Describe("Collector", Ordered, func() { secret1 := &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "secret1"}} secret2 := &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "secret2"}} nilsecret := &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "nilsecret"}} + svc1 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "svc1"}} svc2 := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "svc2"}} nilsvc := &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "nilsvc"}} + graph := &graph.Graph{ GatewayClass: &graph.GatewayClass{}, Gateway: &graph.Gateway{}, From 2c164f17bb5fab18e8be95aa162f11e67e089fb9 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Thu, 15 Feb 2024 12:10:54 -0800 Subject: [PATCH 11/14] update kube system constant --- internal/mode/static/telemetry/collector.go | 6 ++---- internal/mode/static/telemetry/collector_test.go | 3 ++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/internal/mode/static/telemetry/collector.go b/internal/mode/static/telemetry/collector.go index 9a99e3dc9d..4aa0b0cbae 100644 --- a/internal/mode/static/telemetry/collector.go +++ b/internal/mode/static/telemetry/collector.go @@ -7,6 +7,7 @@ import ( appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" + meta "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -14,9 +15,6 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" ) -// kubeSystem indicates the name of kube-system namespace. -const kubeSystem = "kube-system" - //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . GraphGetter // GraphGetter gets the latest Graph. @@ -206,7 +204,7 @@ func collectNGFReplicaCount(ctx context.Context, k8sClient client.Reader, podNSN func collectClusterID(ctx context.Context, k8sClient client.Reader) (string, error) { key := types.NamespacedName{ - Name: kubeSystem, + Name: meta.NamespaceSystem, } var kubeNamespace v1.Namespace if err := k8sClient.Get(ctx, key, &kubeNamespace); err != nil { diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go index fb660de01d..21ef86c3dc 100644 --- a/internal/mode/static/telemetry/collector_test.go +++ b/internal/mode/static/telemetry/collector_test.go @@ -10,6 +10,7 @@ import ( . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" + meta "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -110,7 +111,7 @@ var _ = Describe("Collector", Ordered, func() { kubeNamespace = &v1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - Name: "kube-system", + Name: meta.NamespaceSystem, UID: "test-uid", }, } From 88b9b0cdd983df43f66c31203bff5e46985f0728 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Thu, 15 Feb 2024 12:14:19 -0800 Subject: [PATCH 12/14] remove redundant code --- internal/mode/static/telemetry/collector_test.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go index 21ef86c3dc..8563aea5f0 100644 --- a/internal/mode/static/telemetry/collector_test.go +++ b/internal/mode/static/telemetry/collector_test.go @@ -278,8 +278,7 @@ var _ = Describe("Collector", Ordered, func() { It("should error if the kubernetes client errored when getting the namespace", func() { expectedError := errors.New("there was an error getting clusterID") k8sClientReader.GetCalls(mergeGetCallsWithBase( - func(_ context.Context, _ types.NamespacedName, object client.Object, option ...client.GetOption) error { - Expect(option).To(BeEmpty()) + func(_ context.Context, _ types.NamespacedName, object client.Object, _ ...client.GetOption) error { switch object.(type) { case *v1.Namespace: return expectedError @@ -479,10 +478,8 @@ var _ = Describe("Collector", Ordered, func() { When("it encounters an error while collecting data", func() { It("should error if the kubernetes client errored when getting the Pod", func() { expectedErr := errors.New("there was an error getting the Pod") - k8sClientReader.GetCalls(mergeGetCallsWithBase( - func(_ context.Context, _ client.ObjectKey, object client.Object, option ...client.GetOption) error { - Expect(option).To(BeEmpty()) + func(_ context.Context, _ client.ObjectKey, object client.Object, _ ...client.GetOption) error { switch object.(type) { case *v1.Pod: return expectedErr @@ -571,8 +568,7 @@ var _ = Describe("Collector", Ordered, func() { It("should error if the kubernetes client errored when getting the ReplicaSet", func() { expectedErr := errors.New("there was an error getting the ReplicaSet") k8sClientReader.GetCalls(mergeGetCallsWithBase( - func(_ context.Context, _ client.ObjectKey, object client.Object, option ...client.GetOption) error { - Expect(option).To(BeEmpty()) + func(_ context.Context, _ client.ObjectKey, object client.Object, _ ...client.GetOption) error { switch object.(type) { case *appsv1.ReplicaSet: return expectedErr From d3c7d7565eac01ac573abfb657390365a9cc5c45 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Thu, 15 Feb 2024 13:08:02 -0800 Subject: [PATCH 13/14] Update internal/mode/static/telemetry/collector_test.go Co-authored-by: bjee19 <139261241+bjee19@users.noreply.github.com> --- internal/mode/static/telemetry/collector_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go index 8563aea5f0..d2ed9d2afa 100644 --- a/internal/mode/static/telemetry/collector_test.go +++ b/internal/mode/static/telemetry/collector_test.go @@ -149,7 +149,8 @@ var _ = Describe("Collector", Ordered, func() { return func( ctx context.Context, nsName types.NamespacedName, - object client.Object, option ...client.GetOption, + object client.Object, + option ...client.GetOption, ) error { err := baseGetCalls(ctx, nsName, object, option...) Expect(err).ToNot(HaveOccurred()) From 502f58515380587716baedb32bc9dc40c9184f1f Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Thu, 15 Feb 2024 13:21:28 -0800 Subject: [PATCH 14/14] fix unit test --- .../mode/static/telemetry/collector_test.go | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go index d2ed9d2afa..56f5fc3df6 100644 --- a/internal/mode/static/telemetry/collector_test.go +++ b/internal/mode/static/telemetry/collector_test.go @@ -276,19 +276,21 @@ var _ = Describe("Collector", Ordered, func() { Describe("clusterID collector", func() { When("collecting clusterID", func() { - It("should error if the kubernetes client errored when getting the namespace", func() { - expectedError := errors.New("there was an error getting clusterID") - k8sClientReader.GetCalls(mergeGetCallsWithBase( - func(_ context.Context, _ types.NamespacedName, object client.Object, _ ...client.GetOption) error { - switch object.(type) { - case *v1.Namespace: - return expectedError - } - return nil - })) + 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") + k8sClientReader.GetCalls(mergeGetCallsWithBase( + func(_ context.Context, _ types.NamespacedName, object client.Object, _ ...client.GetOption) error { + switch object.(type) { + case *v1.Namespace: + return expectedError + } + return nil + })) - _, err := dataCollector.Collect(ctx) - Expect(err).To(MatchError(expectedError)) + _, err := dataCollector.Collect(ctx) + Expect(err).To(MatchError(expectedError)) + }) }) }) })