File injection test using fakekubeclient

[jellonek]

This change is 

[ivan4th]

Some thoughts... We have vmconfig loading from fakekubeclient already tested. So we can for sure keep this test case, but what about also adding a test case here

virtlet/pkg/libvirttools/virtualization_test.go

Line 466 in ffad578

// TODO: add test cases for rootfs / persistent rootfs file injection

(it would signal file injection breakage that's not due to a problem with kubeclient), and also, in the same place, a test case for persistent rootfs file injection?

Reviewable status: 0 of 1 approvals obtained

[jellonek]

I'm not fully sure how you would add that. Are you suggesting to add to struct in

virtlet/pkg/libvirttools/virtualization_test.go

Line 343 in ffad578

for _, tc := range []struct {

additional field for parameters needed by fake kube client as in https://github.com/Mirantis/virtlet/pull/861/files#diff-1fd8fde46fcffd8abd9ee1c4e043e6beR296 and basically rest of the lines from this test case before

virtlet/pkg/libvirttools/virtualization_test.go

Line 476 in ffad578

ct.setPodSandbox(sandbox)

line for testcases with non nil values for fake kube client parameters?
That would do the job, but would imo unnecessary enlarge already long body of

virtlet/pkg/libvirttools/virtualization_test.go

Line 468 in ffad578

t.Run(tc.name, func(t *testing.T) {

loop.
Hmmm... I can refactor that a bit to make it more readable adding also handling this extension to tc struct.

[jellonek]

PR updated according to suggestion.

[ivan4th]

Well my idea was to have TestDomainDefinitions cases w/o using the defaultExternalDataLoader and passing in a pre-filled VMConfig instead. What you did is probably even better, but doesn't it make TestFileInjectionHandlingDuringContainerCreation redundant, as it does basically the same thing?

Reviewable status: 0 of 1 approvals obtained

[jellonek]

In fact it does. Ok, removed.

[ivan4th]

Reviewed 3 of 3 files at r2.
Reviewable status: complete! 1 of 1 approvals obtained