Cleanup after ephemeral volume test #127

alexemc · 2025-01-10T20:28:57Z

Description

Ephemeral volume test never did cleanup, regardless of the --no-cleanup flag. The fix adds code to do auto-cleanup for ephemeral volume test unless --no-cleanup is set. Other functional tests don't have --no-cleanup flag, since it make no sense for them.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
dell/csm#1674

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 at least 90% code coverage
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
I have maintained backward compatibility

How Has This Been Tested?

Tested with a new unit-test that checks that auto-cleanup is done after ephemeral volume tests, unless no-cleanup is set. Results attached.

=== NAME  TestCleanupAfterTest
    functionalcmd_test.go:129: namespace mock-ns-prefix-e559b768 creation called
time="2025-01-14T04:01:32Z" level=info msg="Successfully created namespace mock-ns-prefix-e559b768"
    functionalcmd_test.go:33: mock test suite run
time="2025-01-14T04:01:32Z" level=info msg="Deleting all resources in namespace mock-ns-prefix-e559b768"
    functionalcmd_test.go:142: namespace mock-ns-prefix-e559b768 deletion called
time="2025-01-14T04:01:32Z" level=info msg="Namespace mock-ns-prefix-e559b768 was deleted in 178.083Âµs"
time="2025-01-14T04:01:32Z" level=info msg="SUCCESS: mock in 14.150687ms"
time="2025-01-14T04:01:32Z" level=info msg="Trying to connect to cluster..."
time="2025-01-14T04:01:32Z" level=info msg="Created new KubeClient"
time="2025-01-14T04:01:32Z" level=info msg="During this run 100.0% of suites succeeded"
=== RUN   TestCleanupAfterTest/Functional_test_with_no-cleanup_=_true
time="2025-01-14T04:01:32Z" level=info msg="Using EVENT observer type"
time="2025-01-14T04:01:32Z" level=info msg="Using default config"
time="2025-01-14T04:01:32Z" level=info msg="Successfully loaded config. Host: https://xxxx:6443"
time="2025-01-14T04:01:32Z" level=info msg="Created new KubeClient"
=== NAME  TestCleanupAfterTest
    functionalcmd_test.go:129: namespace mock-ns-prefix-e2845167 creation called
time="2025-01-14T04:01:32Z" level=info msg="Successfully created namespace mock-ns-prefix-e2845167"
    functionalcmd_test.go:33: mock test suite run
time="2025-01-14T04:01:32Z" level=info msg="SUCCESS: mock in 13.119763ms"
time="2025-01-14T04:01:32Z" level=info msg="Trying to connect to cluster..."
time="2025-01-14T04:01:32Z" level=info msg="Created new KubeClient"
time="2025-01-14T04:01:32Z" level=info msg="During this run 100.0% of suites succeeded"
--- PASS: TestCleanupAfterTest (0.17s)
    --- PASS: TestCleanupAfterTest/Functional_test_with_no-cleanup_=_false (0.11s)
    --- PASS: TestCleanupAfterTest/Functional_test_with_no-cleanup_=_true (0.06s)
PASS

Full unit-test log:
test.txt

… it's NA to other tests. Auto-cleanup is now the default for the eph volume test, other tests never clean up.

…ling for ephemeral volume test).

gallacher

Please address the test failure in the failed check:

`=== RUN TestCleanupAfterTest/Functional_test_with_no-cleanup_=false
time="2025-01-14T03:55:30Z" level=info msg="Using EVENT observer type"
time="2025-01-14T03:55:30Z" level=info msg="Using default config"
time="2025-01-14T03:55:30Z" level=error msg="Can't load config at "/github/home/.kube/config", error = stat /github/home/.kube/config: no such file or directory"
time="2025-01-14T03:55:30Z" level=error msg="can't get config from specified file; &{%!e(string=stat) %!e(string=/github/home/.kube/config) %!e(syscall.Errno=2)}"
time="2025-01-14T03:55:30Z" level=error msg="Couldn't create new kubernetes client. Error = config can't be nil"
--- FAIL: TestCleanupAfterTest (0.01s)
--- FAIL: TestCleanupAfterTest/Functional_test_with_no-cleanup=_false (0.01s)
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=0x28f4b40]

goroutine 12 [running]:
testing.tRunner.func1.2({0x2e08040, 0x46e47e0})
/usr/local/go/src/testing/testing.go:1632 +0x230
testing.tRunner.func1()
/usr/local/go/src/testing/testing.go:1635 +0x35e
panic({0x2e08040?, 0x46e47e0?})
/usr/local/go/src/runtime/panic.go:785 +0x132
github.com/dell/cert-csi/pkg/testcore/runner.NewFunctionalSuiteRunner({0x0?, 0x3499320?}, {0x0?, 0x180?}, 0x1?, 0xa8?, 0xb4?, 0x0, 0xc0002485b0)
/github/workspace/pkg/testcore/runner/functional-runner.go:58 +0x60
github.com/dell/cert-csi/pkg/cmd.createFunctionalSuiteRunner(0xc00051bce0, 0x0, 0x0)
/github/workspace/pkg/cmd/functionalcmd.go:138 +0x714
github.com/dell/cert-csi/pkg/cmd.TestCleanupAfterTest.func3(0xc0000fa000)
/github/workspace/pkg/cmd/functionalcmd_test.go:108 +0x85
testing.tRunner(0xc0000fa000, 0xc0005e1860)
/usr/local/go/src/testing/testing.go:1690 +0xf4
created by testing.(*T).Run in goroutine 11
/usr/local/go/src/testing/testing.go:1743 +0x390
FAIL github.com/dell/cert-csi/pkg/cmd 0.047s
? github.com/dell/cert-csi/pkg/helm [no test files]
? github.com/dell/cert-csi/pkg/k8sclient/resources/commonparams [no test files]
? github.com/dell/cert-csi/pkg/k8sclient/resources/metrics [no test files]
? github.com/dell/cert-csi/pkg/k8sclient/resources/node [no test files]
? github.com/dell/cert-csi/pkg/k8sclient/resources/csistoragecapacity [no test files]`

alexemc · 2025-01-14T16:57:28Z

@gallacher GH action failed, since it was run in env without kube config. Fixed that, now test env doesn't need kube config.

=== RUN   TestCleanupAfterTest
    functionalcmd_test.go:197: Created dummy kube config file at: /tmp/TestCleanupAfterTest2185193583/001/kube.config
=== RUN   TestCleanupAfterTest/Functional_test_with_no-cleanup_=_false
time="2025-01-14T16:51:57Z" level=info msg="Using EVENT observer type"
time="2025-01-14T16:51:57Z" level=info msg="Using config from /tmp/TestCleanupAfterTest2185193583/001/kube.config"
time="2025-01-14T16:51:57Z" level=info msg="Successfully loaded config. Host: https://unit.test/"
time="2025-01-14T16:51:57Z" level=info msg="Created new KubeClient"
=== NAME  TestCleanupAfterTest
    functionalcmd_test.go:143: namespace mock-ns-prefix-60306044 creation called
time="2025-01-14T16:51:57Z" level=info msg="Successfully created namespace mock-ns-prefix-60306044"
    functionalcmd_test.go:36: mock test suite run
time="2025-01-14T16:51:57Z" level=info msg="Deleting all resources in namespace mock-ns-prefix-60306044"
    functionalcmd_test.go:156: namespace mock-ns-prefix-60306044 deletion called
time="2025-01-14T16:51:57Z" level=info msg="Namespace mock-ns-prefix-60306044 was deleted in 74.028µs"
time="2025-01-14T16:51:57Z" level=info msg="SUCCESS: mock in 1.46701ms"
time="2025-01-14T16:51:57Z" level=info msg="Trying to connect to cluster..."
time="2025-01-14T16:51:57Z" level=info msg="Created new KubeClient"
time="2025-01-14T16:51:57Z" level=info msg="During this run 100.0% of suites succeeded"
=== RUN   TestCleanupAfterTest/Functional_test_with_no-cleanup_=_true
time="2025-01-14T16:51:57Z" level=info msg="Using EVENT observer type"
time="2025-01-14T16:51:57Z" level=info msg="Using config from /tmp/TestCleanupAfterTest2185[193](https://github.com/dell/cert-csi/actions/runs/12772438821/job/35601970764?pr=127#step:4:194)583/001/kube.config"
time="2025-01-14T16:51:57Z" level=info msg="Successfully loaded config. Host: https://unit.test"
time="2025-01-14T16:51:57Z" level=info msg="Created new KubeClient"
=== NAME  TestCleanupAfterTest
    functionalcmd_test.go:143: namespace mock-ns-prefix-9f1805ed creation called
time="2025-01-14T16:51:57Z" level=info msg="Successfully created namespace mock-ns-prefix-9f1805ed"
    functionalcmd_test.go:36: mock test suite run
time="2025-01-14T16:51:57Z" level=info msg="SUCCESS: mock in 1.334119ms"
time="2025-01-14T16:51:57Z" level=info msg="Trying to connect to cluster..."
time="2025-01-14T16:51:57Z" level=info msg="Created new KubeClient"
time="2025-01-14T16:51:57Z" level=info msg="During this run 100.0% of suites succeeded"
--- PASS: TestCleanupAfterTest (0.02s)
    --- PASS: TestCleanupAfterTest/Functional_test_with_no-cleanup_=_false (0.02s)
    --- PASS: TestCleanupAfterTest/Functional_test_with_no-cleanup_=_true (0.00s)
PASS

sharmilarama

LGTM

AronAtDell · 2025-01-15T20:57:46Z

pkg/cmd/functionalcmd.go

@@ -682,6 +674,14 @@ func getFunctionalEphemeralCreationCommand(globalFlags []cli.Flag) cli.Command {
 					Name:  "pod-name, pname",
 					Usage: "custom name for pod and cloned volume pod to create",
 				},
+				cli.BoolFlag{
+					Name:  "no-cleanup, nc",
+					Usage: "include this flag do disable cleanup after test",


AronAtDell · 2025-01-15T20:57:55Z

pkg/cmd/functionalcmd.go

+				},
+				cli.BoolFlag{
+					Name:  "no-cleanup-on-fail, ncof",
+					Usage: "include this flag do disable cleanup on fail",


alikdell

Approving this fix to the test whereby the cleanup logic was broken, leaving garbage around impacting possible subsequent runs. CSM 1.13 only.

AronAtDell

Approving this fix to the test whereby the cleanup logic was broken, leaving garbage around impacting possible subsequent runs. CSM 1.13 only.

mjsdell

Approving this fix to the test whereby the cleanup logic was broken, leaving garbage around impacting possible subsequent runs. CSM 1.13 only.

anandrajak1

Approving this fix to the test whereby the cleanup logic was broken, leaving garbage around impacting possible subsequent runs. CSM 1.13 only.

alexemc added 3 commits January 10, 2025 15:24

Made no-cleanup flag available only to the eph functional test, since…

2d72b0c

… it's NA to other tests. Auto-cleanup is now the default for the eph volume test, other tests never clean up.

Added a unit-test to cover the fixed scenario (proper no-cleanup hand…

1fdebd5

…ling for ephemeral volume test).

Linter errors fixed.

565d7c5

alexemc marked this pull request as ready for review January 14, 2025 04:06

atye previously approved these changes Jan 14, 2025

View reviewed changes

JacobGros previously approved these changes Jan 14, 2025

View reviewed changes

EvgenyUglov previously approved these changes Jan 14, 2025

View reviewed changes

gallacher requested changes Jan 14, 2025

View reviewed changes

Fix missing kubeconfig in GH action.

a5cef77

alexemc dismissed stale reviews from EvgenyUglov and JacobGros via a5cef77 January 14, 2025 16:34

alexemc dismissed atye’s stale review via a5cef77 January 14, 2025 16:34

Fix missing kubeconfig in GH action.

528337b

alexemc requested a review from gallacher January 14, 2025 16:57

atye previously approved these changes Jan 14, 2025

View reviewed changes

EvgenyUglov previously approved these changes Jan 14, 2025

View reviewed changes

gallacher previously approved these changes Jan 14, 2025

View reviewed changes

sharmilarama previously approved these changes Jan 14, 2025

View reviewed changes

Merge branch 'main' into usr/babija/eph-vol-cleanup

bd3a80e

AronAtDell reviewed Jan 15, 2025

View reviewed changes

Fixed typos.

43b9430

alexemc dismissed stale reviews from gallacher, EvgenyUglov, sharmilarama, and atye via 43b9430 January 15, 2025 21:06

alikdell approved these changes Jan 15, 2025

View reviewed changes

AronAtDell approved these changes Jan 15, 2025

View reviewed changes

Merge branch 'main' into usr/babija/eph-vol-cleanup

a0cf1f3

mjsdell approved these changes Jan 15, 2025

View reviewed changes

anandrajak1 approved these changes Jan 15, 2025

View reviewed changes

mjsdell merged commit dd44b6c into main Jan 15, 2025
5 of 6 checks passed

mjsdell deleted the usr/babija/eph-vol-cleanup branch January 15, 2025 21:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup after ephemeral volume test #127

Cleanup after ephemeral volume test #127

alexemc commented Jan 10, 2025 •

edited

Loading

gallacher left a comment

alexemc commented Jan 14, 2025

sharmilarama left a comment

AronAtDell Jan 15, 2025

AronAtDell Jan 15, 2025

alikdell left a comment •

edited

Loading

AronAtDell left a comment

mjsdell left a comment

anandrajak1 left a comment

Cleanup after ephemeral volume test #127

Cleanup after ephemeral volume test #127

Conversation

alexemc commented Jan 10, 2025 • edited Loading

Description

GitHub Issues

Checklist:

How Has This Been Tested?

gallacher left a comment

Choose a reason for hiding this comment

alexemc commented Jan 14, 2025

sharmilarama left a comment

Choose a reason for hiding this comment

AronAtDell Jan 15, 2025

Choose a reason for hiding this comment

AronAtDell Jan 15, 2025

Choose a reason for hiding this comment

alikdell left a comment • edited Loading

Choose a reason for hiding this comment

AronAtDell left a comment

Choose a reason for hiding this comment

mjsdell left a comment

Choose a reason for hiding this comment

anandrajak1 left a comment

Choose a reason for hiding this comment

alexemc commented Jan 10, 2025 •

edited

Loading

alikdell left a comment •

edited

Loading