From 640d97faf16aae343d21b64789bafa81a37a5348 Mon Sep 17 00:00:00 2001 From: Andres Date: Thu, 22 Mar 2018 18:01:07 +0100 Subject: [PATCH] Fix tests --- pkg/controller/cronjob_trigger_controller.go | 2 +- .../cronjob_trigger_controller_test.go | 8 +- pkg/utils/k8sutil.go | 4 +- pkg/utils/k8sutil_test.go | 160 ++++-------------- 4 files changed, 41 insertions(+), 133 deletions(-) diff --git a/pkg/controller/cronjob_trigger_controller.go b/pkg/controller/cronjob_trigger_controller.go index 03d850549..daca4bb82 100644 --- a/pkg/controller/cronjob_trigger_controller.go +++ b/pkg/controller/cronjob_trigger_controller.go @@ -239,7 +239,7 @@ func (c *CronJobTriggerController) syncCronJobTrigger(key string) error { c.logger.Errorf("Unable to find the function %s in the namespace %s. Received %s: ", cronJobtriggerObj.Spec.FunctionName, ns, err) return err } - err = utils.EnsureCronJob(c.clientset, functionObj, cronJobtriggerObj, or) + err = utils.EnsureCronJob(c.clientset, functionObj, cronJobtriggerObj.Spec.Schedule, or) if err != nil { return err } diff --git a/pkg/controller/cronjob_trigger_controller_test.go b/pkg/controller/cronjob_trigger_controller_test.go index 2cf1bf60c..a8c4ade82 100644 --- a/pkg/controller/cronjob_trigger_controller_test.go +++ b/pkg/controller/cronjob_trigger_controller_test.go @@ -109,22 +109,22 @@ func TestCronJobTriggerObjChanged(t *testing.T) { Time: time.Now(), } testObjs := []testObj{ - testObj{ + { old: &kubelessApi.CronJobTrigger{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, new: &kubelessApi.CronJobTrigger{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, expectedChanged: false, }, - testObj{ + { old: &kubelessApi.CronJobTrigger{ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &t1}}, new: &kubelessApi.CronJobTrigger{ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &t2}}, expectedChanged: true, }, - testObj{ + { old: &kubelessApi.CronJobTrigger{Spec: kubelessApi.CronJobTriggerSpec{Schedule: "* * * * *"}}, new: &kubelessApi.CronJobTrigger{Spec: kubelessApi.CronJobTriggerSpec{Schedule: "* * * * *"}}, expectedChanged: false, }, - testObj{ + { old: &kubelessApi.CronJobTrigger{Spec: kubelessApi.CronJobTriggerSpec{Schedule: "*/10 * * * *"}}, new: &kubelessApi.CronJobTrigger{Spec: kubelessApi.CronJobTriggerSpec{Schedule: "* * * * *"}}, expectedChanged: true, diff --git a/pkg/utils/k8sutil.go b/pkg/utils/k8sutil.go index 5e0939d7c..20890eb86 100644 --- a/pkg/utils/k8sutil.go +++ b/pkg/utils/k8sutil.go @@ -960,7 +960,7 @@ func doRESTReq(restIface rest.Interface, groupVersion, verb, resource, elem, nam } // EnsureCronJob creates/updates a function cron job -func EnsureCronJob(client kubernetes.Interface, funcObj *kubelessApi.Function, cronJobObj *kubelessApi.CronJobTrigger, or []metav1.OwnerReference) error { +func EnsureCronJob(client kubernetes.Interface, funcObj *kubelessApi.Function, schedule string, or []metav1.OwnerReference) error { var maxSucccessfulHist, maxFailedHist int32 maxSucccessfulHist = 3 maxFailedHist = 1 @@ -994,7 +994,7 @@ func EnsureCronJob(client kubernetes.Interface, funcObj *kubelessApi.Function, c OwnerReferences: or, }, Spec: batchv1beta1.CronJobSpec{ - Schedule: cronJobObj.Spec.Schedule, + Schedule: schedule, SuccessfulJobsHistoryLimit: &maxSucccessfulHist, FailedJobsHistoryLimit: &maxFailedHist, JobTemplate: batchv1beta1.JobTemplateSpec{ diff --git a/pkg/utils/k8sutil_test.go b/pkg/utils/k8sutil_test.go index 7b0e5bcb2..4916474f6 100644 --- a/pkg/utils/k8sutil_test.go +++ b/pkg/utils/k8sutil_test.go @@ -16,7 +16,6 @@ import ( "github.com/kubeless/kubeless/pkg/langruntime" v2beta1 "k8s.io/api/autoscaling/v2beta1" - batchv2alpha1 "k8s.io/api/batch/v2alpha1" "k8s.io/api/core/v1" "k8s.io/api/extensions/v1beta1" xv1beta1 "k8s.io/api/extensions/v1beta1" @@ -26,7 +25,6 @@ import ( "k8s.io/apimachinery/pkg/apimachinery" "k8s.io/apimachinery/pkg/apimachinery/registered" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/fake" @@ -560,145 +558,55 @@ func TestEnsureCronJob(t *testing.T) { Timeout: "120", }, } - c := &kubelessApi.CronJobTrigger{ - ObjectMeta: metav1.ObjectMeta{ - Name: f1Name, - Namespace: ns, - }, - Spec: kubelessApi.CronJobTriggerSpec{ - Schedule: "*/10 * * * *", - FunctionName: f1Name, - }, - } expectedMeta := metav1.ObjectMeta{ Name: "trigger-" + f1Name, Namespace: ns, OwnerReferences: or, } - client := fakeRESTClient(func(req *http.Request) (*http.Response, error) { - header := http.Header{} - header.Set("Content-Type", runtime.ContentTypeJSON) - listObj := batchv2alpha1.CronJobList{} - if req.Method == "POST" { - reqCronJobBytes, err := ioutil.ReadAll(req.Body) - if err != nil { - t.Fatal(err) - } - cronJob := batchv2alpha1.CronJob{} - err = json.Unmarshal(reqCronJobBytes, &cronJob) - if err != nil { - t.Fatal(err) - } - if !reflect.DeepEqual(expectedMeta, cronJob.ObjectMeta) { - t.Errorf("Unexpected metadata metadata. Expecting\n%+v \nReceived:\n%+v", expectedMeta, cronJob.ObjectMeta) - } - if *cronJob.Spec.SuccessfulJobsHistoryLimit != int32(3) { - t.Errorf("Unexpected SuccessfulJobsHistoryLimit: %d", *cronJob.Spec.SuccessfulJobsHistoryLimit) - } - if *cronJob.Spec.FailedJobsHistoryLimit != int32(1) { - t.Errorf("Unexpected FailedJobsHistoryLimit: %d", *cronJob.Spec.FailedJobsHistoryLimit) - } - if *cronJob.Spec.JobTemplate.Spec.ActiveDeadlineSeconds != int64(120) { - t.Errorf("Unexpected ActiveDeadlineSeconds: %d", *cronJob.Spec.JobTemplate.Spec.ActiveDeadlineSeconds) - } - expectedCommand := []string{"curl", "-Lv", fmt.Sprintf("http://%s.%s.svc.cluster.local:8080", f1Name, ns)} - args := cronJob.Spec.JobTemplate.Spec.Template.Spec.Containers[0].Args - // skip event headers data (i.e -H "event-id: cronjob-controller-2018-03-05T05:55:41.990784027Z" etc) - foundCommand := []string{args[0], args[1], args[len(args)-1]} - if !reflect.DeepEqual(foundCommand, expectedCommand) { - t.Errorf("Unexpected command %s expexted %s", foundCommand, expectedCommand) - } - } else { - t.Fatalf("unexpected verb %s", req.Method) - } - switch req.URL.Path { - case "/apis/batch/v2alpha1/namespaces/default/cronjobs": - return &http.Response{ - StatusCode: 200, - Header: header, - Body: objBody(&listObj), - }, nil - default: - t.Fatalf("unexpected request: %#v\n%#v", req.URL, req) - return nil, nil - } - }) - err := EnsureCronJob(client, f1, c, or, "batch/v2alpha1") + clientset := fake.NewSimpleClientset() + + err := EnsureCronJob(clientset, f1, "* * * * *", or) if err != nil { t.Errorf("Unexpected error: %s", err) } + cronJob, err := clientset.BatchV1beta1().CronJobs(ns).Get(fmt.Sprintf("trigger-%s", f1.Name), metav1.GetOptions{}) + if err != nil { + t.Errorf("Unexpected error: %s", err) + } + if !reflect.DeepEqual(expectedMeta, cronJob.ObjectMeta) { + t.Errorf("Unexpected metadata metadata. Expecting\n%+v \nReceived:\n%+v", expectedMeta, cronJob.ObjectMeta) + } + if *cronJob.Spec.SuccessfulJobsHistoryLimit != int32(3) { + t.Errorf("Unexpected SuccessfulJobsHistoryLimit: %d", *cronJob.Spec.SuccessfulJobsHistoryLimit) + } + if *cronJob.Spec.FailedJobsHistoryLimit != int32(1) { + t.Errorf("Unexpected FailedJobsHistoryLimit: %d", *cronJob.Spec.FailedJobsHistoryLimit) + } + if *cronJob.Spec.JobTemplate.Spec.ActiveDeadlineSeconds != int64(120) { + t.Errorf("Unexpected ActiveDeadlineSeconds: %d", *cronJob.Spec.JobTemplate.Spec.ActiveDeadlineSeconds) + } + expectedCommand := []string{"curl", "-Lv", fmt.Sprintf("http://%s.%s.svc.cluster.local:8080", f1Name, ns)} + args := cronJob.Spec.JobTemplate.Spec.Template.Spec.Containers[0].Args + // skip event headers data (i.e -H "event-id: cronjob-controller-2018-03-05T05:55:41.990784027Z" etc) + foundCommand := []string{args[0], args[1], args[len(args)-1]} + if !reflect.DeepEqual(foundCommand, expectedCommand) { + t.Errorf("Unexpected command %s expexted %s", foundCommand, expectedCommand) + } // It should update the existing cronJob if it is already created - updateCalled := false - client = fakeRESTClient(func(req *http.Request) (*http.Response, error) { - header := http.Header{} - header.Set("Content-Type", runtime.ContentTypeJSON) - switch req.Method { - case "POST": - return &http.Response{ - StatusCode: http.StatusConflict, - Header: header, - Body: objBody(nil), - }, nil - case "GET": - previousCronJob := batchv2alpha1.CronJob{ - ObjectMeta: metav1.ObjectMeta{ - ResourceVersion: "123456", - }, - } - return &http.Response{ - StatusCode: 200, - Header: header, - Body: objBody(&previousCronJob), - }, nil - case "PUT": - updateCalled = true - reqCronJobBytes, err := ioutil.ReadAll(req.Body) - if err != nil { - t.Fatal(err) - } - cronJob := batchv2alpha1.CronJob{} - err = json.Unmarshal(reqCronJobBytes, &cronJob) - if err != nil { - t.Fatal(err) - } - if cronJob.ObjectMeta.ResourceVersion != "123456" { - t.Error("Expecting that the object to update contains the previous information") - } - listObj := batchv2alpha1.CronJobList{} - return &http.Response{ - StatusCode: 200, - Header: header, - Body: objBody(&listObj), - }, nil - default: - t.Fatalf("unexpected request: %#v\n%#v", req.URL, req) - return nil, nil - } - }) - err = EnsureCronJob(client, f1, c, or, "batch/v2alpha1") + newSchedule := "*/10 * * * *" + err = EnsureCronJob(clientset, f1, newSchedule, or) if err != nil { t.Errorf("Unexpected error: %s", err) } - if !updateCalled { - t.Errorf("Expect the update method to be called") + updatedCronJob, err := clientset.BatchV1beta1().CronJobs(ns).Get(fmt.Sprintf("trigger-%s", f1.Name), metav1.GetOptions{}) + if err != nil { + t.Errorf("Unexpected error: %s", err) + } + if updatedCronJob.Spec.Schedule != newSchedule { + t.Errorf("Unexpected schedule %s expecting %s", updatedCronJob.Spec.Schedule, newSchedule) } - - // IT should change the endpoint - client = fakeRESTClient(func(req *http.Request) (*http.Response, error) { - header := http.Header{} - header.Set("Content-Type", runtime.ContentTypeJSON) - if req.URL.Path != "/apis/batch/v1beta1/namespaces/default/cronjobs" { - t.Errorf("Unexpected URL %s", req.URL.Path) - } - return &http.Response{ - StatusCode: 200, - Header: header, - Body: objBody(nil), - }, nil - }) - err = EnsureCronJob(client, f1, c, or, "batch/v1beta1") } func doesNotContain(envs []v1.EnvVar, env v1.EnvVar) bool {