wip (#128)

finishes webhook, tests, and cli error checking

chase test issue

chase test issue

clean up tests

cmd/ketch/job_deploy.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"strings"
 
 	"github.com/spf13/cobra"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -56,13 +57,19 @@ func jobDeploy(ctx context.Context, cfg config, filename string, out io.Writer)
 	}
 
 	job := &ketchv1.Job{ObjectMeta: metav1.ObjectMeta{Name: spec.Name}}
-	_, err = controllerutil.CreateOrUpdate(ctx, cfg.Client(), job, func() error {
+	res, err := controllerutil.CreateOrUpdate(ctx, cfg.Client(), job, func() error {
 		job.Spec = spec
 		return nil
 	})
 	if err != nil {
+		if strings.Contains(err.Error(), ketchv1.ErrJobExists.Error()) {
+			return ketchv1.ErrJobExists
+		}
 		return err
 	}
+	if res == controllerutil.OperationResultNone {
+		return fmt.Errorf("job \"%s\" already exists and is unchanged", job.Spec.Name)
+	}
 
 	fmt.Fprintln(out, "Successfully added!")
 	return nil

cmd/manager/main.go

@@ -135,6 +135,10 @@ func main() {
 			setupLog.Error(err, "unable to create webhook", "webhook", "Framework")
 			os.Exit(1)
 		}
+		if err = (&ketchv1.Job{}).SetupWebhookWithManager(mgr); err != nil {
+			setupLog.Error(err, "unable to create webhook", "webhook", "Job")
+			os.Exit(1)
+		}
 	}
 	// +kubebuilder:scaffold:builder
 

config/webhook/manifests.yaml

@@ -55,3 +55,24 @@ webhooks:
     resources:
     - frameworks
   sideEffects: None
+- admissionReviewVersions:
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-theketch-io-v1beta1-job
+  failurePolicy: Fail
+  name: vjob.kb.io
+  rules:
+  - apiGroups:
+    - theketch.io
+    apiVersions:
+    - v1beta1
+    operations:
+    - CREATE
+    - UPDATE
+    - DELETE
+    resources:
+    - jobs
+  sideEffects: None

internal/api/v1beta1/errors.go

@@ -23,4 +23,7 @@ const (
 
 	// ErrDecreaseQuota is returned when a new quota is too small.
 	ErrDecreaseQuota Error = "failed to decrease quota because the framework has more running apps than the new quota permits"
+
+	// ErrJobExists
+	ErrJobExists Error = "failed to create job because the job already exists"
 )

internal/api/v1beta1/job_webhook.go

@@ -0,0 +1,84 @@
+/*
+
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1beta1
+
+import (
+	"context"
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+)
+
+// joblog is for logging in this package.
+var joblog = logf.Log.WithName("job-resource")
+
+var jobmgr manager = nil
+
+func (r *Job) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	jobmgr = mgr
+	return ctrl.NewWebhookManagedBy(mgr).
+		For(r).
+		Complete()
+}
+
+// +kubebuilder:webhook:verbs=create;update;delete,path=/validate-theketch-io-v1beta1-job,mutating=false,failurePolicy=fail,groups=theketch.io,resources=jobs,versions=v1beta1,name=vjob.kb.io,sideEffects=none,admissionReviewVersions=v1beta1
+
+var _ webhook.Validator = &Job{}
+
+// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
+func (r *Job) ValidateCreate() error {
+	joblog.Info("validate create", "name", r.Name)
+	client := jobmgr.GetClient()
+	jobs := JobList{}
+	if err := client.List(context.Background(), &jobs); err != nil {
+		return err
+	}
+	for _, job := range jobs.Items {
+		if job.Spec.Name == r.Spec.Name {
+			return ErrJobExists
+		}
+	}
+	return nil
+}
+
+// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
+func (r *Job) ValidateUpdate(old runtime.Object) error {
+	joblog.Info("validate update", "name", r.Name)
+	oldJob, ok := old.(*Job)
+	if !ok {
+		return fmt.Errorf("can't validate job update")
+	}
+	client := jobmgr.GetClient()
+	jobs := JobList{}
+	if err := client.List(context.Background(), &jobs); err != nil {
+		return err
+	}
+	for _, job := range jobs.Items {
+		if job.Spec.Name == oldJob.Spec.Name {
+			return ErrJobExists
+		}
+	}
+	return nil
+}
+
+// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
+func (r *Job) ValidateDelete() error {
+	return nil
+}

internal/api/v1beta1/job_webhook_test.go

@@ -0,0 +1,174 @@
+package v1beta1
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/shipa-corp/ketch/internal/api/v1beta1/mocks"
+)
+
+func TestJob_ValidateDelete(t *testing.T) {
+	tests := []struct {
+		name string
+		job  Job
+	}{
+		{
+			name: "success",
+			job: Job{
+				Status: JobStatus{},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := tt.job.ValidateDelete()
+			require.Nil(t, err)
+		})
+	}
+}
+
+func TestJob_ValidateCreate(t *testing.T) {
+
+	const listError Error = "error"
+
+	tests := []struct {
+		name    string
+		job     Job
+		client  *mocks.MockClient
+		wantErr error
+	}{
+		{
+			name: "error getting a list of jobs",
+			client: &mocks.MockClient{
+				OnList: func(ctx context.Context, list runtime.Object, opts ...client.ListOption) error {
+					return listError
+				},
+			},
+			wantErr: listError,
+		},
+		{
+			name: "job already exists",
+			client: &mocks.MockClient{
+				OnList: func(ctx context.Context, list runtime.Object, opts ...client.ListOption) error {
+					jobs := list.(*JobList)
+					jobs.Items = []Job{
+						{Spec: JobSpec{Name: "test-job"}},
+					}
+					return nil
+				},
+			},
+			job: Job{
+				Spec: JobSpec{
+					Name: "test-job",
+				},
+			},
+			wantErr: ErrJobExists,
+		},
+		{
+			name: "success",
+			client: &mocks.MockClient{
+				OnList: func(ctx context.Context, list runtime.Object, opts ...client.ListOption) error {
+					jobs := list.(*JobList)
+					jobs.Items = []Job{
+						{Spec: JobSpec{Name: "test-job"}},
+					}
+					return nil
+				},
+			},
+			job: Job{
+				Spec: JobSpec{
+					Name: "another-test-job",
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			jobmgr = &mockManager{client: tt.client}
+			if err := tt.job.ValidateCreate(); err != tt.wantErr {
+				t.Errorf("ValidateCreate() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestJob_ValidateUpdate(t *testing.T) {
+
+	const listError Error = "error"
+
+	tests := []struct {
+		name    string
+		job     Job
+		old     runtime.Object
+		client  *mocks.MockClient
+		wantErr error
+	}{
+		{
+			name: "error getting a list of jobs",
+			job: Job{
+				Spec: JobSpec{Name: "test-job"},
+			},
+			client: &mocks.MockClient{
+				OnList: func(ctx context.Context, list runtime.Object, opts ...client.ListOption) error {
+					return listError
+				},
+			},
+			old: &Job{
+				Spec: JobSpec{Name: "test-job"},
+			},
+			wantErr: listError,
+		},
+		{
+			name: "job already exists",
+			job: Job{
+				ObjectMeta: metav1.ObjectMeta{Name: "job-1"},
+				Spec:       JobSpec{Name: "test-job"},
+			},
+			client: &mocks.MockClient{
+				OnList: func(ctx context.Context, list runtime.Object, opts ...client.ListOption) error {
+					jobs := list.(*JobList)
+					jobs.Items = []Job{
+						{ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, Spec: JobSpec{Name: "test-job"}},
+					}
+					return nil
+				},
+			},
+			old: &Job{
+				Spec: JobSpec{Name: "test-job"},
+			},
+			wantErr: ErrJobExists,
+		},
+		{
+			name: "everything is ok",
+			job: Job{
+				ObjectMeta: metav1.ObjectMeta{Name: "job-1"},
+				Spec:       JobSpec{Name: "another-job"},
+			},
+			client: &mocks.MockClient{
+				OnList: func(ctx context.Context, list runtime.Object, opts ...client.ListOption) error {
+					jobs := list.(*JobList)
+					jobs.Items = []Job{
+						{ObjectMeta: metav1.ObjectMeta{Name: "job-1"}, Spec: JobSpec{Name: "another-job"}},
+					}
+					return nil
+				},
+			},
+			old: &Job{
+				Spec: JobSpec{Name: "test-job"},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			jobmgr = &mockManager{client: tt.client}
+			if err := tt.job.ValidateUpdate(tt.old); err != tt.wantErr {
+				t.Errorf("ValidateUpdate() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}