Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed k8s client unit test panic #678

Merged
merged 2 commits into from
Sep 4, 2024
Merged

Fixed k8s client unit test panic #678

merged 2 commits into from
Sep 4, 2024

Conversation

alexemc
Copy link
Contributor

@alexemc alexemc commented Sep 3, 2024

Description

Fixed k8s client unit test panic when running on a machine without kube config. The client GetKubeAPIServerVersion() function did not allow setting fake client, instead it always tried to initialize a real k8s client and crashed if there were no kube config present on the machine.

Example of the failure:

[root@lglw1113 csm-operator]#
[root@lglw1113 csm-operator]# make test
test -s /root/csm/csm-operator/bin/controller-gen && /root/csm/csm-operator/bin/controller-gen --version | grep -q v0.15.0 || \
GOBIN=/root/csm/csm-operator/bin go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.15.0
/root/csm/csm-operator/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
/root/csm/csm-operator/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
(cd core; rm -f core_generated.go; go generate)
go run core/semver/semver.go -f mk > semver.mk
go fmt ./...
core/core_generated.go
go vet ./...
KUBEBUILDER_ASSETS="/root/.local/share/kubebuilder-envtest/k8s/1.25.0-linux-amd64" go test ./... -coverprofile cover.out
        github.com/dell/csm-operator/api/v1             coverage: 0.0% of statements
        github.com/dell/csm-operator            coverage: 0.0% of statements
        github.com/dell/csm-operator/core               coverage: 0.0% of statements
        github.com/dell/csm-operator/core/semver                coverage: 0.0% of statements
?       github.com/dell/csm-operator/pkg/constants      [no test files]
        github.com/dell/csm-operator/pkg/logger         coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/resources/configmap            coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/resources/csidriver            coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/resources/daemonset            coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/resources/deployment           coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/resources/rbac         coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/resources/serviceaccount               coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/resources/statefulset          coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/utils          coverage: 0.0% of statements
        github.com/dell/csm-operator/tests/shared               coverage: 0.0% of statements
        github.com/dell/csm-operator/tests/shared/clientgoclient                coverage: 0.0% of statements
        github.com/dell/csm-operator/tests/shared/crclient              coverage: 0.0% of statements
ok      github.com/dell/csm-operator/controllers        54.806s coverage: 67.9% of statements
--- FAIL: Test_GetVersion (1.00s)
    --- FAIL: Test_GetVersion/success_ (1.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x12e503d]

goroutine 28 [running]:
testing.tRunner.func1.2({0x17bdc20, 0x28be9d0})
        /usr/local/go/src/testing/testing.go:1631 +0x24a
testing.tRunner.func1()
        /usr/local/go/src/testing/testing.go:1634 +0x377
panic({0x17bdc20?, 0x28be9d0?})
        /usr/local/go/src/runtime/panic.go:770 +0x132
k8s.io/client-go/discovery.NewDiscoveryClientForConfig(0x0?)
        /root/go/pkg/mod/k8s.io/client-go@v0.27.2/discovery/discovery_client.go:711 +0x1d
github.com/dell/csm-operator/k8s.GetKubeAPIServerVersion()
        /root/csm/csm-operator/k8s/client.go:49 +0x4a
github.com/dell/csm-operator/k8s.Test_GetVersion.func3(0xc0002b5d40)
        /root/csm/csm-operator/k8s/client_test.go:169 +0xec
testing.tRunner(0xc0002b5d40, 0xc0000566d0)
        /usr/local/go/src/testing/testing.go:1689 +0xfb
created by testing.(*T).Run in goroutine 27
        /usr/local/go/src/testing/testing.go:1742 +0x390
FAIL    github.com/dell/csm-operator/k8s        1.037s
ok      github.com/dell/csm-operator/pkg/drivers        0.168s  coverage: 41.4% of statements
ok      github.com/dell/csm-operator/pkg/modules        1.442s  coverage: 60.1% of statements
FAIL
make: *** [Makefile:78: test] Error 1
[root@lglw1113 csm-operator]#

GitHub Issues

None.

Checklist:

  • I have performed a self-review of my own code to ensure there are no formatting, vetting, linting, or security issues
  • I have verified that new and existing unit tests pass locally with my changes
  • I have not allowed coverage numbers to degenerate
  • I have maintained backward compatibility

How Has This Been Tested?

Unit tests passed with the fix:

[root@lglw1113 csm-operator]#
[root@lglw1113 csm-operator]# make test
test -s /root/csm/csm-operator/bin/controller-gen && /root/csm/csm-operator/bin/controller-gen --version | grep -q v0.15.0 || \
GOBIN=/root/csm/csm-operator/bin go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.15.0
/root/csm/csm-operator/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
/root/csm/csm-operator/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
(cd core; rm -f core_generated.go; go generate)
go run core/semver/semver.go -f mk > semver.mk
go fmt ./...
core/core_generated.go
go vet ./...
KUBEBUILDER_ASSETS="/root/.local/share/kubebuilder-envtest/k8s/1.25.0-linux-amd64" go test ./... -coverprofile cover.out
        github.com/dell/csm-operator/api/v1             coverage: 0.0% of statements
        github.com/dell/csm-operator            coverage: 0.0% of statements
        github.com/dell/csm-operator/core               coverage: 0.0% of statements
        github.com/dell/csm-operator/core/semver                coverage: 0.0% of statements
?       github.com/dell/csm-operator/pkg/constants      [no test files]
        github.com/dell/csm-operator/pkg/logger         coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/resources/configmap            coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/resources/csidriver            coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/resources/daemonset            coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/resources/deployment           coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/resources/rbac         coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/resources/serviceaccount               coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/resources/statefulset          coverage: 0.0% of statements
        github.com/dell/csm-operator/pkg/utils          coverage: 0.0% of statements
        github.com/dell/csm-operator/tests/shared               coverage: 0.0% of statements
        github.com/dell/csm-operator/tests/shared/clientgoclient                coverage: 0.0% of statements
        github.com/dell/csm-operator/tests/shared/crclient              coverage: 0.0% of statements
ok      github.com/dell/csm-operator/controllers        54.667s coverage: 67.9% of statements
ok      github.com/dell/csm-operator/k8s        0.061s  coverage: 83.9% of statements
ok      github.com/dell/csm-operator/pkg/drivers        0.170s  coverage: 41.4% of statements
ok      github.com/dell/csm-operator/pkg/modules        1.416s  coverage: 60.1% of statements
[root@lglw1113 csm-operator]#

abhi16394
abhi16394 previously approved these changes Sep 3, 2024
bpjain2004
bpjain2004 previously approved these changes Sep 4, 2024
@alexemc alexemc merged commit bf1e037 into main Sep 4, 2024
7 checks passed
@alexemc alexemc deleted the fix-k8s-client-test branch September 4, 2024 12:19
@chimanjain
Copy link
Contributor

Can we also update the k8s version to the latest supported version from k8s/1.25.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants