Add functional test for product telemetry #1659

pleshakov · 2024-03-08T23:06:26Z

Proposed changes

Problem:
Ensure product telemetry feature is tested with a functional test

Solution:

Add a functional test.
Because it requires a NGF with a custom built, it only runs when ginkgo runs with its label.

Testing:
Ran successfully:

make test TAG=$(whoami) GINKGO_LABEL=telemetry PLUS_ENABLED=true
make test TAG=$(whoami) GINKGO_LABEL=telemetry
make test TAG=$(whoami) # here telemetry test was skipped, but all the rest ran successfully

ClOSES - #1640

Checklist

Before creating a PR, run through this checklist and mark each as complete.

I have read the CONTRIBUTING doc
I have added tests that prove my fix is effective or that my feature works
I have checked that all unit tests pass after adding my changes
I have updated necessary documentation
I have rebased my branch onto main
I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.

NONE

salonichf5

lgtm

tests/Makefile

tests/suite/telemetry_test.go

pleshakov · 2024-03-11T19:39:33Z

@sjberman
had to squash feedback-related changes together with the rest before resolving the conflicts with the main branch

I also removed the Skip statements, to follow the updated code base in 15bab00

tests/Makefile

tests/suite/telemetry_test.go

sjberman · 2024-03-11T20:31:59Z

Depending on if this or #1634 goes in first, we'll need to make an update to ensure that this is run in the pipeline.

pleshakov · 2024-03-12T20:49:10Z

now that #1634 is merged, I'll rebase and ensure that the added test is run in the pipeline

Problem: Ensure product telemetry feature is tested with a functional test Solution: - Add a functional test. - Because it requires a NGF with a custom built, it needs to run with telemetry label. Testing: Ran successfully: - make test TAG=$(whoami) GINKGO_LABEL=telemetry - make test TAG=$(whoami) # telemetry test didn't run as expected, the functional test succeeded ClOSES - nginxinc#1640

Co-authored-by: Saylor Berman <[email protected]>

ciarams87

LGTM 👍🏼

kate-osborn · 2024-03-13T14:34:03Z

@pleshakov can we use ginkgo to run the functional tests instead of go test?

pleshakov · 2024-03-13T14:46:42Z

@kate-osborn

@pleshakov can we use ginkgo to run the functional tests instead of go test?

yep, probably

but changing to ginko would require re-testing not only telemetry but nfrs and longevity (to make sure that will not break them), so I'm a bit hesitant to do it as part of this PR. Would a separate issue work?

nginx-gateway-fabric/tests/Makefile

Lines 87 to 109 in 746abad

    
           .PHONY: start-longevity-test 
        
           start-longevity-test: ## Start the longevity test to run for 4 days in GKE 
        
           	START_LONGEVITY=true $(MAKE) nfr-test 
        
           .PHONY: stop-longevity-test 
        
           stop-longevity-test: ## Stops the longevity test and collects results 
        
           	STOP_LONGEVITY=true $(MAKE) nfr-test 
        
           .PHONY: .vm-nfr-test 
        
           .vm-nfr-test: ## Runs the NFR tests on the GCP VM (called by `nfr-test`) 
        
           	go test -v ./suite -ginkgo.label-filter "nfr" $(GINKGO_FLAGS) -ginkgo.v -args --gateway-api-version=$(GW_API_VERSION) \ 
        
           		--gateway-api-prev-version=$(GW_API_PREV_VERSION) --image-tag=$(TAG) --version-under-test=$(NGF_VERSION) \ 
        
           		--plus-enabled=$(PLUS_ENABLED) --ngf-image-repo=$(PREFIX) --nginx-image-repo=$(NGINX_PREFIX) \ 
        
           		--pull-policy=$(PULL_POLICY) --k8s-version=$(K8S_VERSION) --service-type=$(GW_SERVICE_TYPE) \ 
        
           		--is-gke-internal-lb=$(GW_SVC_GKE_INTERNAL) 
        
           .PHONY: test 
        
           test: ## Runs the functional tests on your default k8s cluster 
        
           	go test -v ./suite -ginkgo.label-filter "functional" $(GINKGO_FLAGS) -args --gateway-api-version=$(GW_API_VERSION) \ 
        
           		--gateway-api-prev-version=$(GW_API_PREV_VERSION) --image-tag=$(TAG) --version-under-test=$(NGF_VERSION) \ 
        
           		--plus-enabled=$(PLUS_ENABLED) --ngf-image-repo=$(PREFIX) --nginx-image-repo=$(NGINX_PREFIX) \ 
        
           		--pull-policy=$(PULL_POLICY) --k8s-version=$(K8S_VERSION) --service-type=$(GW_SERVICE_TYPE) \ 
        
           		--is-gke-internal-lb=$(GW_SVC_GKE_INTERNAL)

kate-osborn · 2024-03-13T14:48:32Z

@kate-osborn

@pleshakov can we use ginkgo to run the functional tests instead of go test?

yep, probably

but changing to ginko would require re-testing not only telemetry but nfrs and longevity (to make sure that will not break them), so I'm a bit hesitant to do it as part of this PR. Would a separate issue work?

nginx-gateway-fabric/tests/Makefile

Lines 87 to 109 in 746abad

.PHONY: start-longevity-test

start-longevity-test: ## Start the longevity test to run for 4 days in GKE

START_LONGEVITY=true $(MAKE) nfr-test

.PHONY: stop-longevity-test

stop-longevity-test: ## Stops the longevity test and collects results

STOP_LONGEVITY=true $(MAKE) nfr-test

.PHONY: .vm-nfr-test

.vm-nfr-test: ## Runs the NFR tests on the GCP VM (called by `nfr-test`)

go test -v ./suite -ginkgo.label-filter "nfr" $(GINKGO_FLAGS) -ginkgo.v -args --gateway-api-version=$(GW_API_VERSION) \

--gateway-api-prev-version=$(GW_API_PREV_VERSION) --image-tag=$(TAG) --version-under-test=$(NGF_VERSION) \

--plus-enabled=$(PLUS_ENABLED) --ngf-image-repo=$(PREFIX) --nginx-image-repo=$(NGINX_PREFIX) \

--pull-policy=$(PULL_POLICY) --k8s-version=$(K8S_VERSION) --service-type=$(GW_SERVICE_TYPE) \

--is-gke-internal-lb=$(GW_SVC_GKE_INTERNAL)

.PHONY: test

test: ## Runs the functional tests on your default k8s cluster

go test -v ./suite -ginkgo.label-filter "functional" $(GINKGO_FLAGS) -args --gateway-api-version=$(GW_API_VERSION) \

--gateway-api-prev-version=$(GW_API_PREV_VERSION) --image-tag=$(TAG) --version-under-test=$(NGF_VERSION) \

--plus-enabled=$(PLUS_ENABLED) --ngf-image-repo=$(PREFIX) --nginx-image-repo=$(NGINX_PREFIX) \

--pull-policy=$(PULL_POLICY) --k8s-version=$(K8S_VERSION) --service-type=$(GW_SERVICE_TYPE) \

--is-gke-internal-lb=$(GW_SVC_GKE_INTERNAL)

yep, separate issue makes sense here.

pleshakov · 2024-03-13T15:04:08Z

@kate-osborn
#1677

* Add functional test for product telemetry Problem: Ensure product telemetry feature is tested with a functional test Solution: - Add a functional test. - Because it requires a NGF with a custom built, it needs to run with telemetry label. - Enable telemetry test in the pipeline Testing: Ran successfully: - make test TAG=$(whoami) GINKGO_LABEL=telemetry - make test TAG=$(whoami) # telemetry test didn't run as expected, the functional test succeeded Also, the pipeline runs telemetry tests successfully ClOSES - nginxinc#1640 Co-authored-by: Saylor Berman <[email protected]>

pleshakov requested a review from a team as a code owner March 8, 2024 23:06

github-actions bot added documentation Improvements or additions to documentation tests Pull requests that update tests labels Mar 8, 2024

salonichf5 approved these changes Mar 9, 2024

View reviewed changes

sjberman reviewed Mar 11, 2024

View reviewed changes

pleshakov mentioned this pull request Mar 11, 2024

Keep OTel collector version updated in product telemetry functional test #1665

Closed

pleshakov force-pushed the test/telemetry-functional branch from 04b783b to fd959f3 Compare March 11, 2024 19:37

pleshakov requested a review from sjberman March 11, 2024 19:39

sjberman reviewed Mar 11, 2024

View reviewed changes

tests/Makefile Show resolved Hide resolved

sjberman reviewed Mar 11, 2024

View reviewed changes

tests/suite/telemetry_test.go Outdated Show resolved Hide resolved

sjberman approved these changes Mar 11, 2024

View reviewed changes

ciarams87 approved these changes Mar 12, 2024

View reviewed changes

pleshakov and others added 3 commits March 13, 2024 09:40

Update tests/suite/telemetry_test.go

d43eedb

Co-authored-by: Saylor Berman <[email protected]>

Enable telemetry tests in pipeline

c6aa8ae

pleshakov force-pushed the test/telemetry-functional branch from b746a06 to c6aa8ae Compare March 13, 2024 14:08

Fix TELEMETRY_ENDPOINT_INSECURE

746abad

pleshakov requested review from ciarams87 and lucacome March 13, 2024 14:25

ciarams87 approved these changes Mar 13, 2024

View reviewed changes

kate-osborn approved these changes Mar 13, 2024

View reviewed changes

lucacome approved these changes Mar 13, 2024

View reviewed changes

pleshakov merged commit f059cf0 into nginxinc:main Mar 13, 2024
40 checks passed

pleshakov deleted the test/telemetry-functional branch March 13, 2024 16:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add functional test for product telemetry #1659

Add functional test for product telemetry #1659

pleshakov commented Mar 8, 2024

salonichf5 left a comment

pleshakov commented Mar 11, 2024

sjberman commented Mar 11, 2024

pleshakov commented Mar 12, 2024

ciarams87 left a comment

kate-osborn commented Mar 13, 2024

pleshakov commented Mar 13, 2024

kate-osborn commented Mar 13, 2024

pleshakov commented Mar 13, 2024

Add functional test for product telemetry #1659

Add functional test for product telemetry #1659

Conversation

pleshakov commented Mar 8, 2024

Proposed changes

Checklist

Release notes

salonichf5 left a comment

Choose a reason for hiding this comment

pleshakov commented Mar 11, 2024

sjberman commented Mar 11, 2024

pleshakov commented Mar 12, 2024

ciarams87 left a comment

Choose a reason for hiding this comment

kate-osborn commented Mar 13, 2024

pleshakov commented Mar 13, 2024

kate-osborn commented Mar 13, 2024

pleshakov commented Mar 13, 2024