feat: add lazy loading kubeconfigs for Test Steps #540

kumar-mallikarjuna · 2024-07-03T12:47:18Z

What this PR does / why we need it:
This PR adds an attribute LazyLoadKubeconfig to the TestStep type which can be used to configure whether the Kubeconfig specified in the TestStep needs to be lazy-loaded, i.e., if the Kubeconfig doesn't need to be loaded at the start of the TestCase eval.

Fixes #539

Tests

.
├── eager
│   └── loading
│       └── 00-teststep.yaml
├── kubeconfig
├── kuttl-test.yaml
└── lazy
    └── loading
        └── 00-teststep.yaml

eager/loading/00-teststep.yaml:

apiVersion: kuttl.dev/v1beta1
kind: TestStep
commands:
- script: echo hi
kubeconfig: test.kubeconfig
kubeconfigLoading: Eager

lazy/loading/00-teststep.yaml:

apiVersion: kuttl.dev/v1beta1
kind: TestStep
commands:
- script: echo hi
kubeconfig: test.kubeconfig
kubeconfigLoading: Lazy

Output on main:

➜  lazy_test git:(main) ✗ go run ../cmd/kubectl-kuttl/main.go test eager
2024/07/04 11:43:22 kutt-test config testdirs is overridden with args: [ eager ]
=== RUN   kuttl
    harness.go:464: starting setup
    harness.go:255: running tests using configured kubeconfig.
    harness.go:278: Successful connection to cluster at: https://127.0.0.1:49639
    harness.go:363: running tests
    harness.go:75: going to run test suite with timeout of 2400 seconds for each step
    harness.go:375: testsuite: eager has 1 tests
=== RUN   kuttl/harness
=== RUN   kuttl/harness/loading
=== PAUSE kuttl/harness/loading
=== CONT  kuttl/harness/loading
    case.go:344: stat /Users/kumarmallikarjuna/Workspace/oss/kuttl/lazy_test/eager/loading/test.kubeconfig: no such file or directory
=== NAME  kuttl
    harness.go:407: run tests finished
    harness.go:515: cleaning up
    harness.go:572: removing temp folder: ""
--- FAIL: kuttl (0.01s)
    --- FAIL: kuttl/harness (0.00s)
        --- FAIL: kuttl/harness/loading (0.00s)
FAIL
exit status 1
➜  lazy_test git:(main) ✗ go run ../cmd/kubectl-kuttl/main.go test lazy
2024/07/04 11:43:27 kutt-test config testdirs is overridden with args: [ lazy ]
=== RUN   kuttl
    harness.go:464: starting setup
    harness.go:255: running tests using configured kubeconfig.
    harness.go:278: Successful connection to cluster at: https://127.0.0.1:49639
    harness.go:363: running tests
    harness.go:75: going to run test suite with timeout of 2400 seconds for each step
    harness.go:375: testsuite: lazy has 1 tests
=== RUN   kuttl/harness
=== RUN   kuttl/harness/loading
=== PAUSE kuttl/harness/loading
=== CONT  kuttl/harness/loading
    case.go:344: stat /Users/kumarmallikarjuna/Workspace/oss/kuttl/lazy_test/lazy/loading/test.kubeconfig: no such file or directory
=== NAME  kuttl
    harness.go:407: run tests finished
    harness.go:515: cleaning up
    harness.go:572: removing temp folder: ""
--- FAIL: kuttl (0.02s)
    --- FAIL: kuttl/harness (0.00s)
        --- FAIL: kuttl/harness/loading (0.00s)
FAIL
exit status 1

Output after these changes:

➜  lazy_test git:(tmp-kubeconfig-lazy-loading) ✗ go run ../cmd/kubectl-kuttl/main.go test eager
2024/07/04 11:41:15 kutt-test config testdirs is overridden with args: [ eager ]
=== RUN   kuttl
    harness.go:464: starting setup
    harness.go:255: running tests using configured kubeconfig.
    harness.go:278: Successful connection to cluster at: https://127.0.0.1:6443
    harness.go:363: running tests
    harness.go:75: going to run test suite with timeout of 2400 seconds for each step
    harness.go:375: testsuite: eager has 1 tests
=== RUN   kuttl/harness
=== RUN   kuttl/harness/loading
=== PAUSE kuttl/harness/loading
=== CONT  kuttl/harness/loading
    case.go:346: stat /Users/kumarmallikarjuna/Workspace/oss/kuttl/lazy_test/eager/loading/test.kubeconfig: no such file or directory
=== NAME  kuttl
    harness.go:407: run tests finished
    harness.go:515: cleaning up
    harness.go:572: removing temp folder: ""
--- FAIL: kuttl (0.01s)
    --- FAIL: kuttl/harness (0.00s)
        --- FAIL: kuttl/harness/loading (0.00s)
FAIL
exit status 1
➜  lazy_test git:(tmp-kubeconfig-lazy-loading) ✗ go run ../cmd/kubectl-kuttl/main.go test lazy
2024/07/04 11:41:18 kutt-test config testdirs is overridden with args: [ lazy ]
=== RUN   kuttl
    harness.go:464: starting setup
    harness.go:255: running tests using configured kubeconfig.
    harness.go:278: Successful connection to cluster at: https://127.0.0.1:6443
    harness.go:363: running tests
    harness.go:75: going to run test suite with timeout of 2400 seconds for each step
    harness.go:375: testsuite: lazy has 1 tests
=== RUN   kuttl/harness
=== RUN   kuttl/harness/loading
=== PAUSE kuttl/harness/loading
=== CONT  kuttl/harness/loading
    logger.go:42: 11:41:18 | loading | Creating namespace: kuttl-test-credible-crow
    case.go:400: failed in step 0-teststep
    case.go:402: failed to lazy-load kubeconfig: stat /Users/kumarmallikarjuna/Workspace/oss/kuttl/lazy_test/lazy/loading/test.kubeconfig: no such file or directory
    logger.go:42: 11:41:18 | loading | loading events from ns kuttl-test-credible-crow:
    logger.go:42: 11:41:18 | loading | Deleting namespace: kuttl-test-credible-crow
=== NAME  kuttl
    harness.go:407: run tests finished
    harness.go:515: cleaning up
    harness.go:572: removing temp folder: ""
--- FAIL: kuttl (5.14s)
    --- FAIL: kuttl/harness (0.00s)
        --- FAIL: kuttl/harness/loading (5.12s)
FAIL
exit status 1

Lazy-loading when the Kubeconfig exists and is same as the global Kubeconfig:

➜  lazy_test git:(main) ✗ go run ../cmd/kubectl-kuttl/main.go test eager
2024/07/04 11:37:44 kutt-test config testdirs is overridden with args: [ eager ]
=== RUN   kuttl
    harness.go:464: starting setup
    harness.go:255: running tests using configured kubeconfig.
    harness.go:278: Successful connection to cluster at: https://127.0.0.1:49639
    harness.go:363: running tests
    harness.go:75: going to run test suite with timeout of 2400 seconds for each step
    harness.go:375: testsuite: eager has 1 tests
=== RUN   kuttl/harness
=== RUN   kuttl/harness/loading
=== PAUSE kuttl/harness/loading
=== CONT  kuttl/harness/loading
    case.go:344: stat /Users/kumarmallikarjuna/Workspace/oss/kuttl/lazy_test/eager/loading/test.kubeconfig: no such file or directory
=== NAME  kuttl
    harness.go:407: run tests finished
    harness.go:515: cleaning up
    harness.go:572: removing temp folder: ""
--- FAIL: kuttl (0.02s)
    --- FAIL: kuttl/harness (0.00s)
        --- FAIL: kuttl/harness/loading (0.00s)
FAIL
exit status 1
➜  lazy_test git:(main) ✗ go run ../cmd/kubectl-kuttl/main.go test lazy
2024/07/04 11:37:50 kutt-test config testdirs is overridden with args: [ lazy ]
=== RUN   kuttl
    harness.go:464: starting setup
    harness.go:255: running tests using configured kubeconfig.
    harness.go:278: Successful connection to cluster at: https://127.0.0.1:49639
    harness.go:363: running tests
    harness.go:75: going to run test suite with timeout of 2400 seconds for each step
    harness.go:375: testsuite: lazy has 1 tests
=== RUN   kuttl/harness
=== RUN   kuttl/harness/loading
=== PAUSE kuttl/harness/loading
=== CONT  kuttl/harness/loading
    logger.go:42: 11:37:50 | loading | Ignoring test.kubeconfig as it does not match file name regexp: ^(\d+)-(?:[^\.]+)(?:\.yaml)?$
    logger.go:42: 11:37:50 | loading | Creating namespace: kuttl-test-needed-ox
    logger.go:42: 11:37:50 | loading | Creating namespace: kuttl-test-needed-ox
    logger.go:42: 11:37:50 | loading/0-teststep | starting test step 0-teststep
    logger.go:42: 11:37:50 | loading/0-teststep | running command: [sh -c echo hi]
    logger.go:42: 11:37:50 | loading/0-teststep | hi
    logger.go:42: 11:37:50 | loading/0-teststep | test step completed 0-teststep
    logger.go:42: 11:37:50 | loading | loading events from ns kuttl-test-needed-ox:
    logger.go:42: 11:37:50 | loading | Deleting namespace: kuttl-test-needed-ox
    logger.go:42: 11:37:55 | loading | Deleting namespace: kuttl-test-needed-ox

porridge

Thanks for the contribution!
Please see the comments inline.

Also, could you please either add an automated test, or if (likely) this will be difficult, then paste how the kuttl output looks like in case the lazy loading fails (perhaps in comparison to how it fails eagerly now).

pkg/test/case.go

pkg/apis/testharness/v1beta1/test_types.go

kumar-mallikarjuna · 2024-07-03T14:19:04Z

Hi @porridge , thanks for the quick review. I've incorporated your comments. I found it a little difficult to add tests for this. For now, I've updated the PR description with the outputs comparing the failures to load lazily on main and with these changes. Can you please take a look again?

kumar-mallikarjuna · 2024-07-03T14:20:07Z

I see the CRDs are in crds/. But I'm unable to re-generate them with make generate. Is this expected?

porridge

Nice!

I don't see the failed to lazy-load kubeconfig string in the ouput you pasted? Was the code changed after you copied? Can you please update them?

As for the CRDs, they seem to have gone stale. I didn't even know they were there 😅

If you could file a bug to remind us to fix generation and check their freshness in CI, that would be great.

This is almost good to go, just a few more nitpicks.

pkg/test/case.go

pkg/test/step.go

docs/testing/reference.md

kumar-mallikarjuna · 2024-07-03T17:48:22Z

I don't see the failed to lazy-load kubeconfig string in the ouput you pasted? Was the code changed after you copied? Can you please update them?

Apologies, I seem to have mixed up the outputs. I've updated them now.

As for the CRDs, they seem to have gone stale. I didn't even know they were there 😅

Oh, okay.

If you could file a bug to remind us to fix generation and check their freshness in CI, that would be great.

Opened #541.

porridge · 2024-07-04T04:52:12Z

pkg/test/case.go

+			}
+		}
+
+		errs = append(errs, testStep.Run(test, ns.Name)...)


I noticed in the output that we're getting the error twice now:

case.go:392: failed to lazy-load kubeconfig "/Users/kumarmallikarjuna/Workspace/kuttl/lazy_test/lazy/loading/test.kubeconfig": stat /Users/kumarmallikarjuna/Workspace/kuttl/lazy_test/lazy/loading/test.kubeconfig: no such file or directory case.go:392: stat /Users/kumarmallikarjuna/Workspace/kuttl/lazy_test/lazy/loading/test.kubeconfig: no such file or directory

I guess this is because we Run the step on this line regardless of whether the lazy loading succeeded or not 😟

However looking deeper into this I realized a few more problems that I didn't see earlier, sorry:

the conditional creation of a client on line 377 is mostly redundant since we already prepare one on line 364,

more importantly, we're failing to create a namespace for lazy-loaded clients, I think we should replace the block 377-382 with one that creates a namespace if it hasn't been done eagerly (not sure how to do this elegantly and robustly, but I'm sure you can come up with something 😉 )

I noticed in the output that we're getting the error twice now:

case.go:392: failed to lazy-load kubeconfig "/Users/kumarmallikarjuna/Workspace/kuttl/lazy_test/lazy/loading/test.kubeconfig": stat /Users/kumarmallikarjuna/Workspace/kuttl/lazy_test/lazy/loading/test.kubeconfig: no such file or directory case.go:392: stat /Users/kumarmallikarjuna/Workspace/kuttl/lazy_test/lazy/loading/test.kubeconfig: no such file or directory

I guess this is because we Run the step on this line regardless of whether the lazy loading succeeded or not 😟

Fixed here: 75eb6a9. I've skipped the test setup from running in-case the kubeconfig setup fails.

However looking deeper into this I realized a few more problems that I didn't see earlier, sorry:

the conditional creation of a client on line 377 is mostly redundant since we already prepare one on line 364,

more importantly, we're failing to create a namespace for lazy-loaded clients, I think we should replace the block 377-382 with one that creates a namespace if it hasn't been done eagerly (not sure how to do this elegantly and robustly, but I'm sure you can come up with something 😉 )

Totally missed that. Since, the namespace is created for each eager client, I've replicated that and placed the logic right after the lazy-client is created:
82fe683.

Possible edge case here: the Kubeconfigs are common b/w steps or the same as the global one. In that case, the namespace would attempt to be re-created and its failure would keep the test from evaluating. I'll add an additional check.

Added it here: ab07d9a.

pkg/test/case.go

porridge · 2024-07-04T06:21:53Z

Also please take a look at the failing checks.

Signed-off-by: Kumar Mallikarjuna <[email protected]>

The transformation from "" -> "Eager" is confusing. Removing it since both trigger "Eager" evaluation. Signed-off-by: Kumar Mallikarjuna <[email protected]>

Signed-off-by: Kumar Mallikarjuna <[email protected]>

kumar-mallikarjuna requested review from kensipe and porridge as code owners July 3, 2024 12:47

porridge requested changes Jul 3, 2024

View reviewed changes

pkg/test/case.go Outdated Show resolved Hide resolved

pkg/apis/testharness/v1beta1/test_types.go Outdated Show resolved Hide resolved

kumar-mallikarjuna force-pushed the tmp-kubeconfig-lazy-loading branch from 4d07acc to 37c107a Compare July 3, 2024 14:17

kumar-mallikarjuna requested a review from porridge July 3, 2024 14:19

porridge requested changes Jul 3, 2024

View reviewed changes

pkg/test/case.go Outdated Show resolved Hide resolved

pkg/test/step.go Outdated Show resolved Hide resolved

docs/testing/reference.md Outdated Show resolved Hide resolved

kumar-mallikarjuna requested a review from porridge July 3, 2024 17:48

porridge requested changes Jul 4, 2024

View reviewed changes

kumar-mallikarjuna requested a review from porridge July 4, 2024 05:33

porridge reviewed Jul 4, 2024

View reviewed changes

pkg/test/case.go Show resolved Hide resolved

kumar-mallikarjuna added 10 commits July 5, 2024 07:15

feat: add lazy loading kubeconfigs for Test Steps

1451de9

Signed-off-by: Kumar Mallikarjuna <[email protected]>

chore: incorporate comments

434d143

Signed-off-by: Kumar Mallikarjuna <[email protected]>

docs: add info for kubeconfigLoading

ced11f0

Signed-off-by: Kumar Mallikarjuna <[email protected]>

fix: update condition

cfbad97

Signed-off-by: Kumar Mallikarjuna <[email protected]>

chore: incorporate comments

af67558

Signed-off-by: Kumar Mallikarjuna <[email protected]>

chore: disable case run on setup errors

4a4c09d

Signed-off-by: Kumar Mallikarjuna <[email protected]>

fix: add namespace creation for lazy-loaded kubeconfigs

c827c1d

Signed-off-by: Kumar Mallikarjuna <[email protected]>

chore: add namespace already-exists check

340e611

Signed-off-by: Kumar Mallikarjuna <[email protected]>

chore: remove vacuous transformation "Eager"

0a66a34

The transformation from "" -> "Eager" is confusing. Removing it since both trigger "Eager" evaluation. Signed-off-by: Kumar Mallikarjuna <[email protected]>

chore: remove redundant import

e7f1165

Signed-off-by: Kumar Mallikarjuna <[email protected]>

porridge force-pushed the tmp-kubeconfig-lazy-loading branch from f600a92 to e7f1165 Compare July 5, 2024 05:15

porridge added enhancement New feature or request release/highlight labels Jul 5, 2024

porridge approved these changes Jul 5, 2024

View reviewed changes

porridge merged commit 7160f43 into kudobuilder:main Jul 5, 2024
5 checks passed

kumar-mallikarjuna deleted the tmp-kubeconfig-lazy-loading branch July 5, 2024 06:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add lazy loading kubeconfigs for Test Steps #540

feat: add lazy loading kubeconfigs for Test Steps #540

kumar-mallikarjuna commented Jul 3, 2024 •

edited

Loading

porridge left a comment

kumar-mallikarjuna commented Jul 3, 2024

kumar-mallikarjuna commented Jul 3, 2024 •

edited

Loading

porridge left a comment

kumar-mallikarjuna commented Jul 3, 2024 •

edited

Loading

porridge Jul 4, 2024

kumar-mallikarjuna Jul 4, 2024

kumar-mallikarjuna Jul 4, 2024 •

edited

Loading

porridge commented Jul 4, 2024

feat: add lazy loading kubeconfigs for Test Steps #540

feat: add lazy loading kubeconfigs for Test Steps #540

Conversation

kumar-mallikarjuna commented Jul 3, 2024 • edited Loading

porridge left a comment

Choose a reason for hiding this comment

kumar-mallikarjuna commented Jul 3, 2024

kumar-mallikarjuna commented Jul 3, 2024 • edited Loading

porridge left a comment

Choose a reason for hiding this comment

kumar-mallikarjuna commented Jul 3, 2024 • edited Loading

porridge Jul 4, 2024

Choose a reason for hiding this comment

kumar-mallikarjuna Jul 4, 2024

Choose a reason for hiding this comment

kumar-mallikarjuna Jul 4, 2024 • edited Loading

Choose a reason for hiding this comment

porridge commented Jul 4, 2024

kumar-mallikarjuna commented Jul 3, 2024 •

edited

Loading

kumar-mallikarjuna commented Jul 3, 2024 •

edited

Loading

kumar-mallikarjuna commented Jul 3, 2024 •

edited

Loading

kumar-mallikarjuna Jul 4, 2024 •

edited

Loading