Add controller tests and CI #97
Conversation
codablock
force-pushed
the
feat-tests
branch
2 times, most recently
from
ecc4f3b to
968ff88
Compare
codablock
changed the title
codablock
force-pushed
the
feat-tests
branch
2 times, most recently
from
8892bb4 to
e0f2c03
Compare
codablock
changed the title
codablock
changed the title
There was a problem hiding this comment.
Very nice @codablock! This is a huge improvement! I've added a few comments/suggestions, but mostly in the nits category.
.github/workflows/tests.yml
Outdated
| fetch-depth: 0 | ||
| - uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: '1.22' |
There was a problem hiding this comment.
| go-version: '1.22' | |
| go-version-file: go.mod |
One less thing to maintain.
.github/workflows/tests.yml
Outdated
| uses: actions/checkout@v4 | ||
| - uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: '1.22' |
There was a problem hiding this comment.
| go-version: '1.22' | |
| go-version-file: go.mod |
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| ) | ||
|
|
||
| func buildTestConfigMap(name string, namespace string, data map[string]string) *unstructured.Unstructured { |
There was a problem hiding this comment.
I would move all constructor helpers to the end of the file to keep focus on the tests. Or to a separate Go file.
| "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" | ||
| "k8s.io/apimachinery/pkg/runtime" | ||
| "math/rand/v2" | ||
| client2 "sigs.k8s.io/controller-runtime/pkg/client" |
There was a problem hiding this comment.
| client2 "sigs.k8s.io/controller-runtime/pkg/client" | |
| "sigs.k8s.io/controller-runtime/pkg/client" |
Is this import alias really needed? If yes, I think you should find a better name. 😉
There was a problem hiding this comment.
Was not needed actually. For some reason gingko caused naming conflicts in the past due to the local import. But I really can't remember the details.
controllers/suite_test.go
Outdated
|
|
||
| func TestAPIs(t *testing.T) { | ||
| kubeconfigName = "template-controller-tests.kubeconfig" | ||
| kubeconfigPath string |
There was a problem hiding this comment.
What is the kubeconfig used for? Do we need it?
There was a problem hiding this comment.
I used it to debug RBAC issues, but I can actually remove it again
| for k, v := range data { | ||
| m[k] = v | ||
| } | ||
| return &unstructured.Unstructured{ |
There was a problem hiding this comment.
Why Unstructured? I prefer type-safety.
There was a problem hiding this comment.
Good point. Using typed objects now.
There was a problem hiding this comment.
Ok, turns out the reason was simple: ObjectTemplate's template entries work with unstructured objects and there is no way around this. I'll at least use typed objects now and then convert them to unstructured.
| for k, v := range data { | ||
| m[k] = v | ||
| } | ||
| return &unstructured.Unstructured{ |
There was a problem hiding this comment.
Why Unstructured? I prefer type-safety.
|
Thanks @erikgb :) I fixed the issues you found and pushed it. |
This PR does multiple things: