search

hasura / gitkube

Build and deploy docker images to Kubernetes using git push
Apache License 2.0
3.81k stars 207 forks source link

refactor: Utilize kubernetes.Interface instead of *kubernetes.ClientSet #94

Closed princerachit closed 4 years ago

princerachit commented 5 years ago

Using concrete object instead of an interface makes code untestable. This PR addresses the issue by changing funcs/methods to accept interface instead of concrete object of kubernetes client set.

Signed-off-by: princerachit prince.rachit@mayadata.io

tirumaraiselvan commented 5 years ago

Hi @princerachit thanks for the PR. Can you give me an example of why testing this is hard ( i.e. what kind of tests are run on interface)?

Also, we are planning to upgrade the client-go version soon, will this be affected by that change?

princerachit commented 5 years ago

@tirumaraiselvan For example If I want to test createRegistryJson(kubeclientset kubernetes.ClientSet, remote *v1alpha1.Remote) interface{} in pkg/controller/common.go which internally calls methods of ClientSet to get values of secret, then It sould not be possible to inject values to this object. But if I use Interface then I can use fake clientset.

princerachit commented 5 years ago

This pr will not break if you bump client go

tirumaraiselvan commented 5 years ago

@princerachit Makes sense.

princerachit commented 5 years ago

@tirumaraiselvan I have not done any integration testing wrt my changes. Do we have any ci cd setup for gitkube?

tirumaraiselvan commented 5 years ago

No, but there are e2e tests in the e2e folder. Pls run them according to the contributing.md guide and see if alls well.

tirumaraiselvan commented 5 years ago

@princerachit Can you also please add one dummy test to show how the interface can be used in tests?

This would also help in setting up a unit test framework.

princerachit commented 5 years ago

@tirumaraiselvan I am getting this error when I run the test

--> current kubectl context: gke_mayaiodev-attur_us-central1-a_prince-cluster
--> derived from: /home/prince/.kube/config
--> creating gitkube namespace gitkube-64rg2z5g
namespace/gitkube-64rg2z5g created
--> creating gikubed and dependent resources
customresourcedefinition.apiextensions.k8s.io/remotes.gitkube.sh created
serviceaccount/gitkube created
clusterrolebinding.rbac.authorization.k8s.io/gitkube created
configmap/gitkube-ci-conf created
deployment.extensions/gitkubed created
deployment.extensions/gitkube-controller created
--> waiting for gitkube to start running
Waiting for deployment "gitkubed" rollout to finish: 0 of 1 updated replicas are available...
deployment "gitkubed" successfully rolled out
deployment "gitkube-controller" successfully rolled out
service/gitkubed exposed
--> creating test namespace gitkube-test-4b6jz52o
namespace/gitkube-test-4b6jz52o created
Cloning into '/tmp/gitkube-test-xrm2mbci'...
remote: Enumerating objects: 115, done.
remote: Total 115 (delta 0), reused 0 (delta 0), pack-reused 115
Receiving objects: 100% (115/115), 24.37 KiB | 8.12 MiB/s, done.
Resolving deltas: 100% (38/38), done.
--> creating test deployment
cat: /tmp/gitkube-test-xrm2mbci/k8s.yaml: No such file or directory
error: no objects passed to create
FAILED> failed executing: kubectl --kubeconfig /home/prince/.kube/config create -f -

On inspecting the gitkube-example repo I found that the k8s.yaml file is missing. It seems like the corresponding change has not been made in e2e.