Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Git server controller test does not include built-in controllers #37

Closed
nabuskey opened this issue Oct 18, 2023 · 3 comments · Fixed by #53
Closed

Git server controller test does not include built-in controllers #37

nabuskey opened this issue Oct 18, 2023 · 3 comments · Fixed by #53
Labels
bug Something isn't working

Comments

@nabuskey
Copy link
Collaborator

nabuskey commented Oct 18, 2023

env test is set up here

testEnv := &envtest.Environment{

UseExistingCluster is set to false (zero value)

https://github.com/kubernetes-sigs/controller-runtime/blob/7db81c7b27468b517d70ce3f7da49b134ec51fec/pkg/envtest/server.go#L152

Per this doc, no built-in controllers run https://book.kubebuilder.io/reference/envtest.html#testing-considerations
So no deployment is created and the test fails.

Not sure if there is a easy way around this.

@nabuskey nabuskey added the bug Something isn't working label Oct 18, 2023
@nabuskey nabuskey mentioned this issue Oct 18, 2023
@cmoulliard
Copy link
Contributor

So, if I understand correctly the problem, it is then needed that:

  • We install a kind cluster and local registry first
  • Next we deploy the needed controllers
    before to execute the tests: make test or run a test file using ide tool ?
    @nabuskey

Were the test cases working before ?

@nabuskey
Copy link
Collaborator Author

nabuskey commented Oct 18, 2023

Not sure if it was working before. @greghaynes ?

To me looks like it was written before writing the code being tested, which is good. It's just that the envtest didn't provide a good explanation in code.

We could use kind cluster to test them all but not sure if it's necessary. The advantage of envtest is the lightweight nature to test your controller fast. To me, waiting for gotResource.Status.DeploymentAvailable may not be necessary. We are testing our controller logic, not the deployment controller. We could look at the deployment object and ensure it's what we expect.

As for running the test, looks like you need to run make envtest at minimum. This is executed as part of make test as well. It pretty much just downloads minimum binaries to run a K8s control plane.

~/idpbuilder$ tree bin/
bin/
├── controller-gen
├── k8s
│   └── 1.27.1-linux-amd64
│       ├── etcd
│       ├── kube-apiserver
│       └── kubectl
└── setup-envtest

@cmoulliard
Copy link
Contributor

We could use kind cluster to test them all but not sure if it's necessary.

Correct. We just need to run as testing environment kubebuilder where the idp controllers are registered for the tests.
There is nevertheless one issue as the image_test.go requires an existing container registry where image is pushed/removed. Do we have to perform such a test ? Does it make sense - not sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants