Feedback from #552

[maelvls]

This is a follow-up PR to #552. I had forgotten to push these few commits before clicking "Merge"... 😨

• The first commit fixes a few log that should have been logs.Log.
• The second is about the tests: it moves the Go objects to inline YAML manifests for the RBAC and Secrets objects with the aim of making the tests a little more readable as well as making the task of comparing the inline YAML with the Helm chart easier.
• The third commit deduplicates the loadRESTConfig func (suggested in Richard's comment).
• Fourth commit fixes the CI on the master branch. CI is failing with "/bin/sh: git: not found". The issue is that git is missing when the GitHub Action "ssh-agent" runs. Example of failure: https://github.com/jetstack/jetstack-secure/actions/runs/10509465707/job/29115732869

[wallrj]

Thanks @maelvls

There was also the t.Cleanup(envtest.Stop()) suggestion, but that can be done in another PR if you prefer (and if you agree that it is desired and causing orphan etcd , kube-apiserver processes to be left behind)

[maelvls]

the t.Cleanup(envtest.Stop()) suggestion

I will investigate this now: t.Cleanup(envtest.Stop) is already called (#552 (comment)), but it seems like it doesn't work as expected since you have noticed there are leftover forked processes hanging around after the end of the test.

[wallrj]

I will investigate this now: t.Cleanup(envtest.Stop) is already called (#552 (comment)), but it seems like it doesn't work as expected since you have noticed there are leftover forked processes hanging around after the end of the test.

Apologies. I didn't see the envtest.Stop code and I jumped to a conclusion.

I think the problem orphan processes happened when I was fixing the loading of the envtest CRDs.
The following patch might solve it:

diff --git a/pkg/client/client_venconn_test.go b/pkg/client/client_venconn_test.go
index 7e7790a..7177b4a 100644
--- a/pkg/client/client_venconn_test.go
+++ b/pkg/client/client_venconn_test.go
@@ -336,16 +336,15 @@ func fakeTPP(t testing.TB) (*httptest.Server, *x509.Certificate) {
 func startEnvtest(t testing.TB) (_ *envtest.Environment, _ *rest.Config, kclient ctrlruntime.WithWatch) {
        envtest := &envtest.Environment{
                ErrorIfCRDPathMissing: true,
-               CRDDirectoryPaths:     []string{"../../deploy/charts/venafi-kubernetes-agent/crd_bases/jetstack.io_venaficonnections.yaml"},
+               CRDDirectoryPaths:     []string{"../../deploy/charts/venafi-kubernetes-agent/crd_bases/jetstack.io_venaficonnections.yamlX"},
        }
        restconf, err := envtest.Start()
-       require.NoError(t, err)
-
        t.Cleanup(func() {
                t.Log("Waiting for envtest to exit")
                err = envtest.Stop()
                require.NoError(t, err)
        })
+       require.NoError(t, err)

        sch := runtime.NewScheme()
        _ = v1alpha1.AddToScheme(sch)

[maelvls]

You are correct, the left over processes were caused by the test exiting before t.Cleanup was called due to envtest.Start failing. I will create a PR with your patch.