From 535b1c17048f7691b79913bb3ee02038140a3437 Mon Sep 17 00:00:00 2001 From: Ciara Stacke Date: Wed, 21 Feb 2024 12:02:28 +0000 Subject: [PATCH 1/2] Add image source --- Makefile | 2 +- build/Dockerfile | 2 + internal/mode/static/telemetry/collector.go | 18 ++++++- .../mode/static/telemetry/collector_test.go | 48 +++++++++++++++++++ 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index cac2bf24e0..87303395e1 100644 --- a/Makefile +++ b/Makefile @@ -45,7 +45,7 @@ build-images-with-plus: build-ngf-image build-nginx-plus-image ## Build the NGF .PHONY: build-ngf-image build-ngf-image: check-for-docker build ## Build the NGF docker image - docker build --platform linux/$(GOARCH) --target $(strip $(TARGET)) -f build/Dockerfile -t $(strip $(PREFIX)):$(strip $(TAG)) . + docker build --platform linux/$(GOARCH) --build-arg BUILD_AGENT=$(BUILD_AGENT) --target $(strip $(TARGET)) -f build/Dockerfile -t $(strip $(PREFIX)):$(strip $(TAG)) . .PHONY: build-nginx-image build-nginx-image: check-for-docker ## Build the custom nginx image diff --git a/build/Dockerfile b/build/Dockerfile index 24026ba3b0..4411500348 100644 --- a/build/Dockerfile +++ b/build/Dockerfile @@ -27,6 +27,8 @@ RUN setcap 'cap_kill=+ep' /usr/bin/gateway FROM scratch as common USER 102:1001 +ARG BUILD_AGENT +ENV BUILD_AGENT=${BUILD_AGENT} ENTRYPOINT [ "/usr/bin/gateway" ] FROM common as container diff --git a/internal/mode/static/telemetry/collector.go b/internal/mode/static/telemetry/collector.go index e7dc692a7b..fc707d802b 100644 --- a/internal/mode/static/telemetry/collector.go +++ b/internal/mode/static/telemetry/collector.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "os" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" @@ -51,8 +52,9 @@ type ProjectMetadata struct { type Data struct { ProjectMetadata ProjectMetadata ClusterID string - NodeCount int + ImageSource string NGFResourceCounts NGFResourceCounts + NodeCount int NGFReplicaCount int } @@ -106,6 +108,8 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) { return Data{}, fmt.Errorf("failed to collect clusterID: %w", err) } + imageSource := CollectImageSource() + data := Data{ NodeCount: nodeCount, NGFResourceCounts: graphResourceCount, @@ -115,6 +119,7 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) { }, NGFReplicaCount: ngfReplicaCount, ClusterID: clusterID, + ImageSource: imageSource, } return data, nil @@ -214,3 +219,14 @@ func CollectClusterID(ctx context.Context, k8sClient client.Reader) (string, err } return string(kubeNamespace.GetUID()), nil } + +// CollectImageSource gets the source of the NGF image. This is done by inspecting the BUILD_AGENT environment variable. +// Valid sources are: "gha" for images built using GitHub Actions in the pipeline, or "local". +// If the environment variable is not set to one of these, the source is "unknown" +func CollectImageSource() string { + buildAgent := os.Getenv("BUILD_AGENT") + if buildAgent != "gha" && buildAgent != "local" { + buildAgent = "unknown" + } + return buildAgent +} diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go index 8b9827c90a..6c9bb42588 100644 --- a/internal/mode/static/telemetry/collector_test.go +++ b/internal/mode/static/telemetry/collector_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "os" "reflect" . "github.com/onsi/ginkgo/v2" @@ -63,6 +64,10 @@ func createGetCallsFunc(objects ...client.Object) getCallsFunc { } } +func setEnvVar(key, value string) { + os.Setenv(key, value) +} + var _ = Describe("Collector", Ordered, func() { var ( k8sClientReader *eventsfakes.FakeReader @@ -123,6 +128,7 @@ var _ = Describe("Collector", Ordered, func() { NGFResourceCounts: telemetry.NGFResourceCounts{}, NGFReplicaCount: 1, ClusterID: string(kubeNamespace.GetUID()), + ImageSource: "unknown", } k8sClientReader = &eventsfakes.FakeReader{} @@ -475,6 +481,48 @@ var _ = Describe("Collector", Ordered, func() { }) }) + Describe("Image source collector", func() { + When("collecting image source", func() { + When("the NGF pod's build agent env var is set to gha", func() { + It("collects the image source as gha", func() { + setEnvVar("BUILD_AGENT", "gha") + expData.ImageSource = "gha" + + data, err := dataCollector.Collect(ctx) + + Expect(err).To(BeNil()) + Expect(expData).To(Equal(data)) + }) + }) + }) + When("collecting image source", func() { + When("the NGF pod's build agent env var is set to local", func() { + It("collects the image source as local", func() { + setEnvVar("BUILD_AGENT", "local") + expData.ImageSource = "local" + + data, err := dataCollector.Collect(ctx) + + Expect(err).To(BeNil()) + Expect(expData).To(Equal(data)) + }) + }) + }) + When("collecting image source", func() { + When("the NGF pod's build agent env var is set to anything else", func() { + It("collects the image source as unknown", func() { + setEnvVar("BUILD_AGENT", "something-else") + expData.ImageSource = "unknown" + + data, err := dataCollector.Collect(ctx) + + Expect(err).To(BeNil()) + Expect(expData).To(Equal(data)) + }) + }) + }) + }) + Describe("NGF replica count collector", func() { When("collecting NGF replica count", func() { When("it encounters an error while collecting data", func() { From ce5ac71cda58171d5a84db230f5407626526fc2e Mon Sep 17 00:00:00 2001 From: Ciara Stacke Date: Wed, 21 Feb 2024 16:17:22 +0000 Subject: [PATCH 2/2] Move env var to join the others --- cmd/gateway/commands.go | 6 +++ internal/mode/static/config/config.go | 2 + internal/mode/static/manager.go | 1 + internal/mode/static/telemetry/collector.go | 18 ++----- .../mode/static/telemetry/collector_test.go | 50 +------------------ 5 files changed, 14 insertions(+), 63 deletions(-) diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index 3cd42ca3a1..e721e697a7 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -146,6 +146,11 @@ func createStaticModeCommand() *cobra.Command { return errors.New("POD_NAME environment variable must be set") } + imageSource := os.Getenv("BUILD_AGENT") + if imageSource != "gha" && imageSource != "local" { + imageSource = "unknown" + } + period, err := time.ParseDuration(telemetryReportPeriod) if err != nil { return fmt.Errorf("error parsing telemetry report period: %w", err) @@ -203,6 +208,7 @@ func createStaticModeCommand() *cobra.Command { TelemetryReportPeriod: period, Version: version, ExperimentalFeatures: gwExperimentalFeatures, + ImageSource: imageSource, } if err := static.StartManager(conf); err != nil { diff --git a/internal/mode/static/config/config.go b/internal/mode/static/config/config.go index 5ab8c811b4..3a21055fdf 100644 --- a/internal/mode/static/config/config.go +++ b/internal/mode/static/config/config.go @@ -11,6 +11,8 @@ import ( type Config struct { // Version is the running NGF version. Version string + // ImageSource is the source of the NGINX Gateway image. + ImageSource string // AtomicLevel is an atomically changeable, dynamic logging level. AtomicLevel zap.AtomicLevel // GatewayNsName is the namespaced name of a Gateway resource that the Gateway will use. diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 99f97b8ebf..2652b72d73 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -251,6 +251,7 @@ func StartManager(cfg config.Config) error { Namespace: cfg.GatewayPodConfig.Namespace, Name: cfg.GatewayPodConfig.Name, }, + ImageSource: cfg.ImageSource, }) if err = mgr.Add(createTelemetryJob(cfg, dataCollector, nginxChecker.getReadyCh())); err != nil { return fmt.Errorf("cannot register telemetry job: %w", err) diff --git a/internal/mode/static/telemetry/collector.go b/internal/mode/static/telemetry/collector.go index fc707d802b..8bc4fdeb2c 100644 --- a/internal/mode/static/telemetry/collector.go +++ b/internal/mode/static/telemetry/collector.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "os" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" @@ -70,6 +69,8 @@ type DataCollectorConfig struct { Version string // PodNSName is the NamespacedName of the NGF Pod. PodNSName types.NamespacedName + // ImageSource is the source of the NGF image. + ImageSource string } // DataCollectorImpl is am implementation of DataCollector. @@ -108,8 +109,6 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) { return Data{}, fmt.Errorf("failed to collect clusterID: %w", err) } - imageSource := CollectImageSource() - data := Data{ NodeCount: nodeCount, NGFResourceCounts: graphResourceCount, @@ -119,7 +118,7 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) { }, NGFReplicaCount: ngfReplicaCount, ClusterID: clusterID, - ImageSource: imageSource, + ImageSource: c.cfg.ImageSource, } return data, nil @@ -219,14 +218,3 @@ func CollectClusterID(ctx context.Context, k8sClient client.Reader) (string, err } return string(kubeNamespace.GetUID()), nil } - -// CollectImageSource gets the source of the NGF image. This is done by inspecting the BUILD_AGENT environment variable. -// Valid sources are: "gha" for images built using GitHub Actions in the pipeline, or "local". -// If the environment variable is not set to one of these, the source is "unknown" -func CollectImageSource() string { - buildAgent := os.Getenv("BUILD_AGENT") - if buildAgent != "gha" && buildAgent != "local" { - buildAgent = "unknown" - } - return buildAgent -} diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go index 6c9bb42588..3fe8287b02 100644 --- a/internal/mode/static/telemetry/collector_test.go +++ b/internal/mode/static/telemetry/collector_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "os" "reflect" . "github.com/onsi/ginkgo/v2" @@ -64,10 +63,6 @@ func createGetCallsFunc(objects ...client.Object) getCallsFunc { } } -func setEnvVar(key, value string) { - os.Setenv(key, value) -} - var _ = Describe("Collector", Ordered, func() { var ( k8sClientReader *eventsfakes.FakeReader @@ -128,7 +123,7 @@ var _ = Describe("Collector", Ordered, func() { NGFResourceCounts: telemetry.NGFResourceCounts{}, NGFReplicaCount: 1, ClusterID: string(kubeNamespace.GetUID()), - ImageSource: "unknown", + ImageSource: "local", } k8sClientReader = &eventsfakes.FakeReader{} @@ -144,6 +139,7 @@ var _ = Describe("Collector", Ordered, func() { ConfigurationGetter: fakeConfigurationGetter, Version: version, PodNSName: podNSName, + ImageSource: "local", }) baseGetCalls = createGetCallsFunc(ngfPod, ngfReplicaSet, kubeNamespace) @@ -481,48 +477,6 @@ var _ = Describe("Collector", Ordered, func() { }) }) - Describe("Image source collector", func() { - When("collecting image source", func() { - When("the NGF pod's build agent env var is set to gha", func() { - It("collects the image source as gha", func() { - setEnvVar("BUILD_AGENT", "gha") - expData.ImageSource = "gha" - - data, err := dataCollector.Collect(ctx) - - Expect(err).To(BeNil()) - Expect(expData).To(Equal(data)) - }) - }) - }) - When("collecting image source", func() { - When("the NGF pod's build agent env var is set to local", func() { - It("collects the image source as local", func() { - setEnvVar("BUILD_AGENT", "local") - expData.ImageSource = "local" - - data, err := dataCollector.Collect(ctx) - - Expect(err).To(BeNil()) - Expect(expData).To(Equal(data)) - }) - }) - }) - When("collecting image source", func() { - When("the NGF pod's build agent env var is set to anything else", func() { - It("collects the image source as unknown", func() { - setEnvVar("BUILD_AGENT", "something-else") - expData.ImageSource = "unknown" - - data, err := dataCollector.Collect(ctx) - - Expect(err).To(BeNil()) - Expect(expData).To(Equal(data)) - }) - }) - }) - }) - Describe("NGF replica count collector", func() { When("collecting NGF replica count", func() { When("it encounters an error while collecting data", func() {