Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

test: unit test for individual patch generator #21

Merged
merged 5 commits into from
Apr 5, 2024
Merged

test: unit test for individual patch generator #21

merged 5 commits into from
Apr 5, 2024

Conversation

supershal
Copy link

Enable execution of unit test for each mutator package.
Resolves Issue: nutanix-cloud-native#474

This PR is first iteration of the refactoring to collect feedback.

This PR addresses patches for two sample variables, which covers all type of patch generator tests.

  • AWS Region - Does not require TestEnv
  • HTTPProxy - Requires TestEnv

TestEnv is initialized only if the test that requires is executed.
The existing AWS and Docker MetaMutator tests are not modified or removed yet. Once tests for all variables are moved to their individual packages we should remove MetaMutator tests that runs test cases for all the variables (unless we decided to run the test twice)

AWS region patch

❯ go test -v ./pkg/handlers/aws/mutation/region
=== RUN   TestRegionPatch
Running Suite: AWS Region mutator suite - /Users/shalinpatel/gitrepos/cluster-api-runtime-extensions-nutanix/pkg/handlers/aws/mutation/region
=============================================================================================================================================
Random Seed: 1712030686

Will run 2 of 2 specs
••

Ran 2 of 2 Specs in 0.001 seconds
SUCCESS! -- 2 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestRegionPatch (0.00s)
=== RUN   TestVariableValidation
=== PAUSE TestVariableValidation
=== CONT  TestVariableValidation
=== RUN   TestVariableValidation/specified_region
=== PAUSE TestVariableValidation/specified_region
=== CONT  TestVariableValidation/specified_region
--- PASS: TestVariableValidation (0.00s)
    --- PASS: TestVariableValidation/specified_region (0.00s)
PASS
ok  	github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pkg/handlers/aws/mutation/region	0.377s

HTTPProxy Patch

❯ go test -v  ./pkg/handlers/generic/mutation/httpproxy
=== RUN   TestGenerateNoProxy
=== PAUSE TestGenerateNoProxy
=== RUN   TestHTTPProxyPatch
Running Suite: HTTP Proxy mutator suite - /Users/shalinpatel/gitrepos/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/httpproxy
====================================================================================================================================================
Random Seed: 1712031152

Will run 3 of 3 specs
I0401 21:12:36.447338   20050 internal.go:516] "Stopping and waiting for non leader election runnables"
I0401 21:12:36.447362   20050 internal.go:520] "Stopping and waiting for leader election runnables"
I0401 21:12:36.447367   20050 internal.go:526] "Stopping and waiting for caches"
E0401 21:12:36.447283   20050 inject.go:72] "failed to resolve no proxy value" err="no kind is registered for the type v1beta1.Cluster in scheme \"pkg/runtime/scheme.go:100\"" template="bootstrap.cluster.x-k8s.io/v1beta1/KubeadmConfigTemplate" holder="MachineDeployment" holderRef={"apiVersion":"","kind":"MachineDeployment","namespace":"","name":"","fieldPath":"spec.template.spec.infrastructureRef"}
I0401 21:12:36.447391   20050 internal.go:530] "Stopping and waiting for webhooks"
I0401 21:12:36.447395   20050 internal.go:533] "Stopping and waiting for HTTP servers"
I0401 21:12:36.447397   20050 internal.go:537] "Wait completed, proceeding to shutdown the manager"
I0401 21:12:36.447773   20050 inject.go:119] "adding files to worker node kubeadm config template" template="bootstrap.cluster.x-k8s.io/v1beta1/KubeadmConfigTemplate" holder="MachineDeployment" holderRef={"apiVersion":"","kind":"MachineDeployment","namespace":"","name":"","fieldPath":"spec.template.spec.infrastructureRef"} variableName="clusterConfig" variableFieldPath=["proxy"] variableValue={"http":"http://example.com","https":"https://example.com","additionalNo":["no-proxy.example.com"]} patchedObjectKind="bootstrap.cluster.x-k8s.io/v1beta1, Kind=KubeadmConfigTemplate" patchedObjectName="default/test-kubeadmconfigtemplate"
•E0401 21:12:36.448308   20050 inject.go:72] "failed to resolve no proxy value" err="no kind is registered for the type v1beta1.Cluster in scheme \"pkg/runtime/scheme.go:100\"" template="controlplane.cluster.x-k8s.io/v1beta1/KubeadmControlPlaneTemplate" holder="cluster.x-k8s.io/v1beta1/Cluster/default/test-cluster" holderRef={"apiVersion":"cluster.x-k8s.io/v1beta1","kind":"Cluster","namespace":"default","name":"test-cluster","fieldPath":"spec.controlPlaneRef"}
I0401 21:12:36.448509   20050 inject.go:103] "adding files to control plane kubeadm config spec" template="controlplane.cluster.x-k8s.io/v1beta1/KubeadmControlPlaneTemplate" holder="cluster.x-k8s.io/v1beta1/Cluster/default/test-cluster" holderRef={"apiVersion":"cluster.x-k8s.io/v1beta1","kind":"Cluster","namespace":"default","name":"test-cluster","fieldPath":"spec.controlPlaneRef"} variableName="clusterConfig" variableFieldPath=["proxy"] variableValue={"http":"http://example.com","https":"https://example.com","additionalNo":["no-proxy.example.com"]} patchedObjectKind="controlplane.cluster.x-k8s.io/v1beta1, Kind=KubeadmControlPlaneTemplate" patchedObjectName="default/test-kubeadmcontrolplanetemplate"
••

Ran 3 of 3 Specs in 5.213 seconds
SUCCESS! -- 3 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestHTTPProxyPatch (5.21s)
=== RUN   TestGenerateSystemdFiles
=== PAUSE TestGenerateSystemdFiles
=== RUN   TestVariableValidation
=== PAUSE TestVariableValidation
=== CONT  TestGenerateNoProxy
=== CONT  TestVariableValidation
=== RUN   TestGenerateNoProxy/no_networking_config
=== PAUSE TestGenerateNoProxy/no_networking_config
=== RUN   TestGenerateNoProxy/custom_pod_network
=== PAUSE TestGenerateNoProxy/custom_pod_network
=== CONT  TestGenerateSystemdFiles
=== RUN   TestGenerateNoProxy/Unknown_infrastructure_cluster
=== RUN   TestGenerateSystemdFiles/no_proxy_configuration
=== PAUSE TestGenerateNoProxy/Unknown_infrastructure_cluster
=== PAUSE TestGenerateSystemdFiles/no_proxy_configuration
=== RUN   TestGenerateNoProxy/AWS_cluster
=== RUN   TestGenerateSystemdFiles/all_vars_set
=== PAUSE TestGenerateSystemdFiles/all_vars_set
=== RUN   TestGenerateSystemdFiles/http_only
=== PAUSE TestGenerateNoProxy/AWS_cluster
=== PAUSE TestGenerateSystemdFiles/http_only
=== RUN   TestGenerateNoProxy/AWS_managed_(EKS)_cluster
=== RUN   TestGenerateSystemdFiles/https_only
=== PAUSE TestGenerateSystemdFiles/https_only
=== PAUSE TestGenerateNoProxy/AWS_managed_(EKS)_cluster
=== RUN   TestGenerateNoProxy/Azure_cluster
=== RUN   TestGenerateSystemdFiles/no_proxy_only
=== PAUSE TestGenerateNoProxy/Azure_cluster
=== RUN   TestGenerateNoProxy/Azure_managed_(AKS)_cluster
=== PAUSE TestGenerateSystemdFiles/no_proxy_only
=== PAUSE TestGenerateNoProxy/Azure_managed_(AKS)_cluster
=== RUN   TestGenerateSystemdFiles/multiple_no_proxy_only
=== RUN   TestGenerateNoProxy/GCP_cluster
=== PAUSE TestGenerateSystemdFiles/multiple_no_proxy_only
=== RUN   TestGenerateSystemdFiles/default_no_proxy_values
=== PAUSE TestGenerateNoProxy/GCP_cluster
=== PAUSE TestGenerateSystemdFiles/default_no_proxy_values
=== CONT  TestGenerateSystemdFiles/no_proxy_configuration
=== CONT  TestGenerateSystemdFiles/multiple_no_proxy_only
=== CONT  TestGenerateSystemdFiles/http_only
=== RUN   TestGenerateNoProxy/custom_service_network
=== PAUSE TestGenerateNoProxy/custom_service_network
=== RUN   TestGenerateNoProxy/custom_servicedomain
=== PAUSE TestGenerateNoProxy/custom_servicedomain
=== RUN   TestVariableValidation/valid_proxy_config
=== CONT  TestGenerateSystemdFiles/no_proxy_only
=== PAUSE TestVariableValidation/valid_proxy_config
=== CONT  TestVariableValidation/valid_proxy_config
=== RUN   TestGenerateNoProxy/all_options
=== CONT  TestGenerateSystemdFiles/all_vars_set
=== PAUSE TestGenerateNoProxy/all_options
=== CONT  TestGenerateSystemdFiles/default_no_proxy_values
=== CONT  TestGenerateNoProxy/all_options
=== CONT  TestGenerateNoProxy/custom_servicedomain
=== CONT  TestGenerateNoProxy/Azure_cluster
=== CONT  TestGenerateNoProxy/no_networking_config
=== CONT  TestGenerateNoProxy/AWS_managed_(EKS)_cluster
=== CONT  TestGenerateNoProxy/custom_service_network
=== CONT  TestGenerateSystemdFiles/https_only
=== CONT  TestGenerateNoProxy/AWS_cluster
=== CONT  TestGenerateNoProxy/Unknown_infrastructure_cluster
=== CONT  TestGenerateNoProxy/GCP_cluster
=== CONT  TestGenerateNoProxy/Azure_managed_(AKS)_cluster
--- PASS: TestGenerateSystemdFiles (0.00s)
    --- PASS: TestGenerateSystemdFiles/no_proxy_configuration (0.00s)
    --- PASS: TestGenerateSystemdFiles/multiple_no_proxy_only (0.00s)
    --- PASS: TestGenerateSystemdFiles/http_only (0.00s)
    --- PASS: TestGenerateSystemdFiles/no_proxy_only (0.00s)
    --- PASS: TestGenerateSystemdFiles/all_vars_set (0.00s)
    --- PASS: TestGenerateSystemdFiles/default_no_proxy_values (0.00s)
    --- PASS: TestGenerateSystemdFiles/https_only (0.00s)
=== CONT  TestGenerateNoProxy/custom_pod_network
--- PASS: TestGenerateNoProxy (0.00s)
    --- PASS: TestGenerateNoProxy/all_options (0.00s)
    --- PASS: TestGenerateNoProxy/custom_servicedomain (0.00s)
    --- PASS: TestGenerateNoProxy/Azure_cluster (0.00s)
    --- PASS: TestGenerateNoProxy/no_networking_config (0.00s)
    --- PASS: TestGenerateNoProxy/AWS_managed_(EKS)_cluster (0.00s)
    --- PASS: TestGenerateNoProxy/custom_service_network (0.00s)
    --- PASS: TestGenerateNoProxy/AWS_cluster (0.00s)
    --- PASS: TestGenerateNoProxy/Unknown_infrastructure_cluster (0.00s)
    --- PASS: TestGenerateNoProxy/GCP_cluster (0.00s)
    --- PASS: TestGenerateNoProxy/Azure_managed_(AKS)_cluster (0.00s)
    --- PASS: TestGenerateNoProxy/custom_pod_network (0.00s)
--- PASS: TestVariableValidation (0.00s)
    --- PASS: TestVariableValidation/valid_proxy_config (0.00s)
PASS
ok  	github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/httpproxy	(cached)

Other alternative considered:

  • Duplicate TestEnv initialization to each package wherever k8s client is needed for the patch
    Ginkgo's BeforeSuite and AfterSuite seemed better alternative then duplicating the code.

Future work:

  • Once feedback is finalized, move tests for all the variables
  • remove meta mutator tests
  • Documentation

@supershal supershal self-assigned this Apr 2, 2024
@supershal supershal force-pushed the shalin/refactor_patch_tests branch from 0c95d64 to aa467be Compare April 2, 2024 14:34
@supershal
Copy link
Author

When I run make test multiple times locally, I do get data race almost 50% of the time. looking into it.

=== FAIL: pkg/handlers/generic/mutation/httpproxy TestHTTPProxyPatch (6.03s)
Running Suite: HTTP Proxy mutator suite - /Users/shalinpatel/gitrepos/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/httpproxy
====================================================================================================================================================
Random Seed: 1712077816

Will run 3 of 3 specs
==================
WARNING: DATA RACE
Write at 0x00c0003b9cd0 by goroutine 242:
  github.com/onsi/ginkgo/v2/internal.(*Suite).handleSpecEvent()
      /Users/shalinpatel/go/pkg/mod/github.com/onsi/ginkgo/v2@v2.16.0/internal/suite.go:284 +0x150
  github.com/onsi/ginkgo/v2/internal.(*Suite).By()
      /Users/shalinpatel/go/pkg/mod/github.com/onsi/ginkgo/v2@v2.16.0/internal/suite.go:307 +0x154
  github.com/onsi/ginkgo/v2.By()
      /Users/shalinpatel/go/pkg/mod/github.com/onsi/ginkgo/v2@v2.16.0/core_dsl.go:600 +0x5c
  github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/test/helpers.init.func2.1()
      /Users/shalinpatel/gitrepos/cluster-api-runtime-extensions-nutanix/test/helpers/environment.go:30 +0x5c

Previous read at 0x00c0003b9cd0 by goroutine 53:
  github.com/onsi/ginkgo/v2/internal.(*Suite).processCurrentSpecReport()
      /Users/shalinpatel/go/pkg/mod/github.com/onsi/ginkgo/v2@v2.16.0/internal/suite.go:414 +0x5c
  github.com/onsi/ginkgo/v2/internal.(*Suite).runBeforeSuite()
      /Users/shalinpatel/go/pkg/mod/github.com/onsi/ginkgo/v2@v2.16.0/internal/suite.go:541 +0x434
  github.com/onsi/ginkgo/v2/internal.(*Suite).runSpecs()
      /Users/shalinpatel/go/pkg/mod/github.com/onsi/ginkgo/v2@v2.16.0/internal/suite.go:458 +0x33c
  github.com/onsi/ginkgo/v2/internal.(*Suite).Run()
      /Users/shalinpatel/go/pkg/mod/github.com/onsi/ginkgo/v2@v2.16.0/internal/suite.go:130 +0x420
  github.com/onsi/ginkgo/v2.RunSpecs()
      /Users/shalinpatel/go/pkg/mod/github.com/onsi/ginkgo/v2@v2.16.0/core_dsl.go:300 +0xc80
  github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/httpproxy.TestHTTPProxyPatch()
      /Users/shalinpatel/gitrepos/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/httpproxy/inject_test.go:191 +0x54
  testing.tRunner()
      /nix/store/k9srp8ngvblscg68fdpcyqkydh86429k-go-1.22.1/share/go/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /nix/store/k9srp8ngvblscg68fdpcyqkydh86429k-go-1.22.1/share/go/src/testing/testing.go:1742 +0x40

Goroutine 242 (running) created at:
  github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/test/helpers.init.func2()
      /Users/shalinpatel/gitrepos/cluster-api-runtime-extensions-nutanix/test/helpers/environment.go:29 +0x288
  github.com/onsi/ginkgo/v2/internal.(*Suite).runNode.func3()
      /Users/shalinpatel/go/pkg/mod/github.com/onsi/ginkgo/v2@v2.16.0/internal/suite.go:889 +0xf0

Goroutine 53 (running) created at:
  testing.(*T).Run()
      /nix/store/k9srp8ngvblscg68fdpcyqkydh86429k-go-1.22.1/share/go/src/testing/testing.go:1742 +0x5e4
  testing.runTests.func1()
      /nix/store/k9srp8ngvblscg68fdpcyqkydh86429k-go-1.22.1/share/go/src/testing/testing.go:2161 +0x80
  testing.tRunner()
      /nix/store/k9srp8ngvblscg68fdpcyqkydh86429k-go-1.22.1/share/go/src/testing/testing.go:1689 +0x180
  testing.runTests()
      /nix/store/k9srp8ngvblscg68fdpcyqkydh86429k-go-1.22.1/share/go/src/testing/testing.go:2159 +0x6e0
  testing.(*M).Run()
      /nix/store/k9srp8ngvblscg68fdpcyqkydh86429k-go-1.22.1/share/go/src/testing/testing.go:2027 +0xb6c
  main.main()
      _testmain.go:87 +0x2b4

dkoshkin
dkoshkin reviewed Apr 2, 2024
View reviewed changes
Copy link
Collaborator

@dkoshkin dkoshkin left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes, I like this, lets us run the individual meta handler tests in my IDE.

Just some minor nits and the race condition :(

common/pkg/testutils/capitest/patches.go Show resolved Hide resolved
common/pkg/testutils/capitest/patches.go Show resolved Hide resolved
pkg/handlers/aws/mutation/region/inject_test.go Outdated Show resolved Hide resolved
pkg/handlers/aws/mutation/region/inject_test.go Show resolved Hide resolved
@supershal supershal force-pushed the shalin/refactor_patch_tests branch from 18a6ab0 to 66c2656 Compare April 2, 2024 21:13
@supershal
Copy link
Author

Fixed the race condition. I tested by running 100 iterations of make test locally. The actual issue was By("Starting the manager") line inside go routine. https://github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/pull/21/files#diff-8fcbbd9007f224817ca23bf8307eda8b662da6403b0c869475b706a007e3b3c0R40 This was causing the ginkgo framework's event reporter to write/read from the same object. moving it outside go routine fixed the issue.

@supershal supershal requested a review from dkoshkin April 2, 2024 23:08
@supershal supershal mentioned this pull request Apr 3, 2024
Merged
@supershal supershal force-pushed the shalin/refactor_patch_tests branch from 89a8bb9 to f3883c3 Compare April 5, 2024 03:47
@supershal supershal merged commit f8c47e2 into main Apr 5, 2024
13 of 16 checks passed
@supershal supershal deleted the shalin/refactor_patch_tests branch April 5, 2024 20:37
@github-actions github-actions bot mentioned this pull request Apr 5, 2024
Open
@supershal supershal mentioned this pull request Apr 6, 2024
Closed
jimmidyson pushed a commit that referenced this pull request Apr 11, 2024
@supershal @jimmidyson
test: unit test for individual patch generator (#21)
5e7fe95 
* test: unit test for individual patch generator

* test: package level unit test for HTTPProxy

* test: fix data race between multiple unit test that use envtest

* test: make patchgenerator generic function

* fix: linting errors after rebase from main
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@faiq faiq faiq approved these changes

@dkoshkin dkoshkin dkoshkin approved these changes

@jimmidyson jimmidyson Awaiting requested review from jimmidyson

@dlipovetsky dlipovetsky Awaiting requested review from dlipovetsky

Assignees

@supershal supershal

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@supershal @dkoshkin @faiq