Sample test code breaks when a second test is added

[mogsie]

What broke? What's expected?

The generated test suite contains the following code (taken from the test data) abbreviated slightly:

    Context("Memcached controller test", func() {
        namespace := &corev1.Namespace{
            ObjectMeta: metav1.ObjectMeta{
                Name:      MemcachedName,
                Namespace: MemcachedName,
            },
        }

       BeforeEach(func() {
            By("Creating the Namespace to perform the tests")
            err := k8sClient.Create(ctx, namespace)
            Expect(err).To(Not(HaveOccurred()))
            ...
        })

        AfterEach(func() {
            By("Deleting the Namespace to perform the tests")
            _ = k8sClient.Delete(ctx, namespace)
            ...
        })

Then a single It() which is a test of some sort.

Addina second (empty) It() the above code will break, because the namespace object contains metadata not suitable for creating a namespace.

Adding a second It() yields the following error (make test)

    Unexpected error:
        <*errors.StatusError | 0xc0004abae0>: {
            ErrStatus: {
                TypeMeta: {Kind: "", APIVersion: ""},
                ListMeta: {
                    SelfLink: "",
                    ResourceVersion: "",
                    Continue: "",
                    RemainingItemCount: nil,
                },
                Status: "Failure",
                Message: "resourceVersion should not be set on objects to be created",
                Reason: "",
                Details: nil,
                Code: 500,
            },
        }
        resourceVersion should not be set on objects to be created
    occurred

Note that fixing this by resetting the Namespace object, only yields another problem, possibly in kubebuilder, that the old namespace isn't really being deleted.

Reproducing this issue

1. Check out master
2. make the following change to testdata/project-v4-with-deploy-image/controllers/memcached_controller_test.go:

   @@ -112,5 +112,9 @@ var _ = Describe("Memcached controller", func() {
                               return k8sClient.Get(ctx, typeNamespaceName, found)
                       }, time.Minute, time.Second).Should(Succeed())
               })
   +
   +               It("is funny", func() {
   +                       By("doing nothing")
   +               })
       })
   })

3. make test

Expected result: The new test passes

Actual results: The test suite fails to create the namespace

KubeBuilder (CLI) Version

master

PROJECT version

No response

Plugin versions

No response

Other versions

No response

Extra Labels

No response

[mogsie]

On a related note, this controller-runtime issue mentions that deletion of namespaces is not supported by envtest, and that the suggestion (if namespace isolation is important) is to use an external cluster like k3s or kind. Perhaps the issue of namespace isolation should be documented, and the whole "delete / create namespace" code path be removed?

[camilamacedo86]

Hi @mogsie,

The example scaffold works well and fine. Kubebuilder uses controller runtime but it is not responsible to implement ENV TEST. Therefore, I understand what you are looking for is an RFE to make ENV TEST allow this scenario.

In this way, could you please raise an issue against controller-runtime? Here, the only thing that we can do is to document this scenario to help others.

We have desired to have/add a FAQ section and it seems a very good fit for there. Would you like to contribute with it?

[mogsie]

Yes, I'd like to help write a FAQ entry. But the source code I mention is in this repository — the tests work, because there's only one test in them. It's not useful for continuing, regardless of envtest issues.

[camilamacedo86]

Hi @mogsie,

Yes, I'd like to help write a FAQ entry. But the source code I mention is in this repository — the tests work, because there's only one test in them. It's not useful for continuing, regardless of envtest issues.

Regards the code IMO: You are completed right about the need we call out and highlight the limitation However, not all controller tests would need to have more than one context test. For example in the scaffold done for the deploy image plugin, I do not see a hall to add another one. So, I think we should address it by adding a comment on the code, i.e.:

        AfterEach(func() {
                 // TODO(user): Attention if you improve this code by adding other context test you MUST
                 // be aware of the current delete namespace limitations. More info: https://book.kubebuilder.io/reference/envtest.html#testing-considerations 
            By("Deleting the Namespace to perform the tests")
            _ = k8sClient.Delete(ctx, namespace)

            By("Removing the Image ENV VAR which stores the Operand image")
            _ = os.Unsetenv("MEMCACHED_IMAGE")
        })

WDYT? Could you please help us do these changes? It would need to be done here: https://github.com/kubernetes-sigs/kubebuilder/blob/master/pkg/plugins/golang/deploy-image/v1alpha1/scaffolds/internal/templates/controllers/controller-test.go#L109-L115