diff --git a/.travis.yml b/.travis.yml index 323510fd61e..47a8a6a858e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,6 +15,9 @@ jobs: - go get -u golang.org/x/lint/golint script: - make verify + - stage: UT Tests + script: + - make unit-test - stage: E2E Tests before_script: # Download kubectl diff --git a/pkg/admission/admit_job_test.go b/pkg/admission/admit_job_test.go new file mode 100644 index 00000000000..35bb6451826 --- /dev/null +++ b/pkg/admission/admit_job_test.go @@ -0,0 +1,291 @@ +package admission + +import ( + "strings" + "testing" + + "k8s.io/api/admission/v1beta1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + v1alpha1 "volcano.sh/volcano/pkg/apis/batch/v1alpha1" +) + +func TestValidateExecution(t *testing.T) { + + namespace := "test" + var invTtl int32 = -1 + + testCases := []struct { + Name string + Job v1alpha1.Job + ExpectErr bool + reviewResponse v1beta1.AdmissionResponse + ret string + }{ + { + Name: "validate valid-job", + Job: v1alpha1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid-Job", + Namespace: namespace, + }, + Spec: v1alpha1.JobSpec{ + MinAvailable: 1, + Tasks: []v1alpha1.TaskSpec{ + { + Name: "task-1", + Replicas: 1, + Template: v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"name": "test"}, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "fake-name", + Image: "busybox:1.24", + }, + }, + }, + }, + }, + }, + }, + }, + reviewResponse: v1beta1.AdmissionResponse{Allowed: true}, + ret: "", + ExpectErr: false, + }, + // duplicate task name + { + Name: "duplicate-task-job", + Job: v1alpha1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "duplicate-task-job", + Namespace: namespace, + }, + Spec: v1alpha1.JobSpec{ + MinAvailable: 1, + Tasks: []v1alpha1.TaskSpec{ + { + Name: "duplicated-task-1", + Replicas: 1, + Template: v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"name": "test"}, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "fake-name", + Image: "busybox:1.24", + }, + }, + }, + }, + }, + { + Name: "duplicated-task-1", + Replicas: 1, + Template: v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"name": "test"}, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "fake-name", + Image: "busybox:1.24", + }, + }, + }, + }, + }, + }, + }, + }, + reviewResponse: v1beta1.AdmissionResponse{Allowed: true}, + ret: "duplicated task name duplicated-task-1", + ExpectErr: true, + }, + // Duplicated Policy Event + { + Name: "job-policy-duplicated", + Job: v1alpha1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "job-policy-duplicated", + Namespace: namespace, + }, + Spec: v1alpha1.JobSpec{ + MinAvailable: 1, + Tasks: []v1alpha1.TaskSpec{ + { + Name: "task-1", + Replicas: 1, + Template: v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"name": "test"}, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "fake-name", + Image: "busybox:1.24", + }, + }, + }, + }, + }, + }, + Policies: []v1alpha1.LifecyclePolicy{ + { + Event: v1alpha1.PodFailedEvent, + Action: v1alpha1.AbortJobAction, + }, + { + Event: v1alpha1.PodFailedEvent, + Action: v1alpha1.RestartJobAction, + }, + }, + }, + }, + reviewResponse: v1beta1.AdmissionResponse{Allowed: true}, + ret: "duplicate", + ExpectErr: true, + }, + // Min Available illegal + { + Name: "Min Available illegal", + Job: v1alpha1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "job-min-illegal", + Namespace: namespace, + }, + Spec: v1alpha1.JobSpec{ + MinAvailable: 2, + Tasks: []v1alpha1.TaskSpec{ + { + Name: "task-1", + Replicas: 1, + Template: v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"name": "test"}, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "fake-name", + Image: "busybox:1.24", + }, + }, + }, + }, + }, + }, + }, + }, + reviewResponse: v1beta1.AdmissionResponse{Allowed: true}, + ret: "'minAvailable' should not be greater than total replicas in tasks", + ExpectErr: true, + }, + // Job Plugin illegal + { + Name: "Job Plugin illegal", + Job: v1alpha1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "job-plugin-illegal", + Namespace: namespace, + }, + Spec: v1alpha1.JobSpec{ + MinAvailable: 1, + Tasks: []v1alpha1.TaskSpec{ + { + Name: "task-1", + Replicas: 1, + Template: v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"name": "test"}, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "fake-name", + Image: "busybox:1.24", + }, + }, + }, + }, + }, + }, + Plugins: map[string][]string{ + "big_plugin": {}, + }, + }, + }, + reviewResponse: v1beta1.AdmissionResponse{Allowed: true}, + ret: "unable to find job plugin: big_plugin", + ExpectErr: true, + }, + // ttl-illegal + { + Name: "job-ttl-illegal", + Job: v1alpha1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "job-ttl-illegal", + Namespace: namespace, + }, + Spec: v1alpha1.JobSpec{ + MinAvailable: 1, + Tasks: []v1alpha1.TaskSpec{ + { + Name: "task-1", + Replicas: 1, + Template: v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"name": "test"}, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "fake-name", + Image: "busybox:1.24", + }, + }, + }, + }, + }, + }, + TTLSecondsAfterFinished: &invTtl, + }, + }, + reviewResponse: v1beta1.AdmissionResponse{Allowed: true}, + ret: "'ttlSecondsAfterFinished' cannot be less than zero", + ExpectErr: true, + }, + } + + for _, testCase := range testCases { + + ret := validateJob(testCase.Job, &testCase.reviewResponse) + //fmt.Printf("test-case name:%s, ret:%v testCase.reviewResponse:%v \n", testCase.Name, ret,testCase.reviewResponse) + if testCase.ExpectErr == true && ret == "" { + t.Errorf("%s: test case Expect error msg :%s, but got nil.", testCase.Name, testCase.ret) + } + if testCase.ExpectErr == true && testCase.reviewResponse.Allowed != false { + t.Errorf("%s: test case Expect Allowed as false but got true.", testCase.Name) + } + if testCase.ExpectErr == true && !strings.Contains(ret, testCase.ret) { + t.Errorf("%s: test case Expect error msg :%s, but got diff error %v", testCase.Name, testCase.ret, ret) + } + + if testCase.ExpectErr == false && ret != "" { + t.Errorf("%s: test case Expect no error, but got error %v", testCase.Name, ret) + } + if testCase.ExpectErr == false && testCase.reviewResponse.Allowed != true { + t.Errorf("%s: test case Expect Allowed as true but got false. %v", testCase.Name, testCase.reviewResponse) + } + + } + +} diff --git a/pkg/controllers/queue/queue_controller.go b/pkg/controllers/queue/queue_controller.go index 544dcadbb3c..2a2a828f826 100644 --- a/pkg/controllers/queue/queue_controller.go +++ b/pkg/controllers/queue/queue_controller.go @@ -175,7 +175,7 @@ func (c *Controller) syncQueue(key string) error { queue, err := c.queueLister.Get(key) if err != nil { if errors.IsNotFound(err) { - glog.V(2).Infof("queue %s has been deleted", queue) + glog.V(2).Infof("queue %s has been deleted", key) return nil } return err diff --git a/test/e2e/admission.go b/test/e2e/admission.go index 60cd71a5646..968240ac82e 100644 --- a/test/e2e/admission.go +++ b/test/e2e/admission.go @@ -19,140 +19,10 @@ package e2e import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1" - "volcano.sh/volcano/pkg/apis/batch/v1alpha1" ) var _ = Describe("Job E2E Test: Test Admission service", func() { - It("Duplicated Task Name", func() { - jobName := "job-duplicated" - namespace := "test" - context := initTestContext() - defer cleanupTestContext(context) - rep := clusterSize(context, oneCPU) - - _, err := createJobInner(context, &jobSpec{ - namespace: namespace, - name: jobName, - tasks: []taskSpec{ - { - img: defaultNginxImage, - req: oneCPU, - min: rep, - rep: rep, - name: "duplicated", - }, - { - img: defaultNginxImage, - req: oneCPU, - min: rep, - rep: rep, - name: "duplicated", - }, - }, - }) - Expect(err).To(HaveOccurred()) - stError, ok := err.(*errors.StatusError) - Expect(ok).To(Equal(true)) - Expect(stError.ErrStatus.Code).To(Equal(int32(500))) - Expect(stError.ErrStatus.Message).To(ContainSubstring("duplicated task name")) - }) - - It("Duplicated Policy Event", func() { - jobName := "job-policy-duplicated" - namespace := "test" - context := initTestContext() - defer cleanupTestContext(context) - rep := clusterSize(context, oneCPU) - - _, err := createJobInner(context, &jobSpec{ - namespace: namespace, - name: jobName, - tasks: []taskSpec{ - { - img: defaultNginxImage, - req: oneCPU, - min: rep, - rep: rep, - name: "taskname", - }, - }, - policies: []v1alpha1.LifecyclePolicy{ - { - Event: v1alpha1.PodFailedEvent, - Action: v1alpha1.AbortJobAction, - }, - { - Event: v1alpha1.PodFailedEvent, - Action: v1alpha1.RestartJobAction, - }, - }, - }) - Expect(err).To(HaveOccurred()) - stError, ok := err.(*errors.StatusError) - Expect(ok).To(Equal(true)) - Expect(stError.ErrStatus.Code).To(Equal(int32(500))) - Expect(stError.ErrStatus.Message).To(ContainSubstring("duplicate event PodFailed")) - }) - - It("Min Available illegal", func() { - jobName := "job-min-illegal" - namespace := "test" - context := initTestContext() - defer cleanupTestContext(context) - rep := clusterSize(context, oneCPU) - - _, err := createJobInner(context, &jobSpec{ - min: rep * 2, - namespace: namespace, - name: jobName, - tasks: []taskSpec{ - { - img: defaultNginxImage, - req: oneCPU, - min: rep, - rep: rep, - name: "taskname", - }, - }, - }) - Expect(err).To(HaveOccurred()) - stError, ok := err.(*errors.StatusError) - Expect(ok).To(Equal(true)) - Expect(stError.ErrStatus.Code).To(Equal(int32(500))) - Expect(stError.ErrStatus.Message).To(ContainSubstring("'minAvailable' should not be greater than total replicas in tasks")) - }) - - It("Job Plugin illegal", func() { - jobName := "job-plugin-illegal" - namespace := "test" - context := initTestContext() - defer cleanupTestContext(context) - - _, err := createJobInner(context, &jobSpec{ - min: 1, - namespace: namespace, - name: jobName, - plugins: map[string][]string{ - "big_plugin": {}, - }, - tasks: []taskSpec{ - { - img: defaultNginxImage, - req: oneCPU, - min: 1, - rep: 1, - name: "taskname", - }, - }, - }) - Expect(err).To(HaveOccurred()) - stError, ok := err.(*errors.StatusError) - Expect(ok).To(Equal(true)) - Expect(stError.ErrStatus.Code).To(Equal(int32(500))) - Expect(stError.ErrStatus.Message).To(ContainSubstring("unable to find job plugin: big_plugin")) - }) It("Default queue would be added", func() { jobName := "job-default-queue" @@ -181,33 +51,4 @@ var _ = Describe("Job E2E Test: Test Admission service", func() { "Job queue attribute would default to 'default' ") }) - It("ttl illegal", func() { - jobName := "job-ttl-illegal" - namespace := "test" - var ttl int32 - ttl = -1 - context := initTestContext() - defer cleanupTestContext(context) - - _, err := createJobInner(context, &jobSpec{ - min: 1, - namespace: namespace, - name: jobName, - ttl: &ttl, - tasks: []taskSpec{ - { - img: defaultNginxImage, - req: oneCPU, - min: 1, - rep: 1, - name: "taskname", - }, - }, - }) - Expect(err).To(HaveOccurred()) - stError, ok := err.(*errors.StatusError) - Expect(ok).To(Equal(true)) - Expect(stError.ErrStatus.Code).To(Equal(int32(500))) - Expect(stError.ErrStatus.Message).To(ContainSubstring("'ttlSecondsAfterFinished' cannot be less than zero.")) - }) })