From 8e61906b8693ae6207c17fe78e9ad5abb93f467f Mon Sep 17 00:00:00 2001 From: Qian Deng Date: Thu, 10 Apr 2025 15:58:44 +1000 Subject: [PATCH 1/3] Implement AppDeployment controller logic and validation - Added AppDeployment controller logic including job creation and status checking. - Introduced validation for AppDeployment specifications to enforce constraints. - Created unit tests for job handling and validation logic. - Updated .gitignore to include the bin directory. - Added dependencies for testing and Kubernetes API. --- .gitignore | 2 +- config/manager/kustomization.yaml | 6 + go.mod | 1 + .../controller/appdeployment/conditions.go | 14 + .../appdeployment/conditions_test.go | 53 ++ .../utils/controller/appdeployment/const.go | 17 + .../utils/controller/appdeployment/job.go | 132 +++++ .../controller/appdeployment/job_test.go | 230 ++++++++ .../controller/appdeployment/validate.go | 139 +++++ .../controller/appdeployment/validate_test.go | 496 ++++++++++++++++++ internal/utils/ptr/ptr.go | 13 + internal/utils/ptr/ptr_test.go | 79 +++ internal/utils/reconciler/operations.go | 85 +++ internal/utils/reconciler/operations_test.go | 120 +++++ 14 files changed, 1386 insertions(+), 1 deletion(-) create mode 100644 internal/utils/controller/appdeployment/conditions.go create mode 100644 internal/utils/controller/appdeployment/conditions_test.go create mode 100644 internal/utils/controller/appdeployment/const.go create mode 100644 internal/utils/controller/appdeployment/job.go create mode 100644 internal/utils/controller/appdeployment/job_test.go create mode 100644 internal/utils/controller/appdeployment/validate.go create mode 100644 internal/utils/controller/appdeployment/validate_test.go create mode 100644 internal/utils/ptr/ptr.go create mode 100644 internal/utils/ptr/ptr_test.go create mode 100644 internal/utils/reconciler/operations.go create mode 100644 internal/utils/reconciler/operations_test.go diff --git a/.gitignore b/.gitignore index 5c3de25..1b906dd 100644 --- a/.gitignore +++ b/.gitignore @@ -25,4 +25,4 @@ go.work.sum .env # kubebuilder binaries -# bin/ +bin/ diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 5c5f0b8..c3d1806 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -1,2 +1,8 @@ resources: - manager.yaml +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +images: +- name: controller + newName: example.com/operation-cache-controller + newTag: v0.0.1 diff --git a/go.mod b/go.mod index 673137b..3c2b96f 100644 --- a/go.mod +++ b/go.mod @@ -52,6 +52,7 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.19.1 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.55.0 // indirect diff --git a/internal/utils/controller/appdeployment/conditions.go b/internal/utils/controller/appdeployment/conditions.go new file mode 100644 index 0000000..c026aea --- /dev/null +++ b/internal/utils/controller/appdeployment/conditions.go @@ -0,0 +1,14 @@ +package appdeployment + +import ( + "context" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + appv1 "github.com/Azure/operation-cache-controller/api/v1" +) + +func ClearConditions(ctx context.Context, appdeployment *appv1.AppDeployment) { + // Clear all conditions + appdeployment.Status.Conditions = []metav1.Condition{} +} diff --git a/internal/utils/controller/appdeployment/conditions_test.go b/internal/utils/controller/appdeployment/conditions_test.go new file mode 100644 index 0000000..c732452 --- /dev/null +++ b/internal/utils/controller/appdeployment/conditions_test.go @@ -0,0 +1,53 @@ +package appdeployment_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + appv1 "github.com/Azure/operation-cache-controller/api/v1" + "github.com/Azure/operation-cache-controller/internal/utils/controller/appdeployment" +) + +func TestClearConditions(t *testing.T) { + tests := []struct { + name string + existingConditions []metav1.Condition + }{ + { + name: "No existing conditions", + existingConditions: []metav1.Condition{}, + }, + { + name: "Multiple existing conditions", + existingConditions: []metav1.Condition{ + { + Type: "TestType", + Status: metav1.ConditionTrue, + Reason: "TestReason", + Message: "TestMessage", + }, + { + Type: "AnotherTestType", + Status: metav1.ConditionFalse, + Reason: "AnotherTestReason", + Message: "AnotherTestMessage", + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + appDep := &appv1.AppDeployment{ + Status: appv1.AppDeploymentStatus{ + Conditions: tt.existingConditions, + }, + } + appdeployment.ClearConditions(context.Background(), appDep) + assert.Empty(t, appDep.Status.Conditions, "conditions should be cleared") + }) + } +} diff --git a/internal/utils/controller/appdeployment/const.go b/internal/utils/controller/appdeployment/const.go new file mode 100644 index 0000000..1d7a874 --- /dev/null +++ b/internal/utils/controller/appdeployment/const.go @@ -0,0 +1,17 @@ +package appdeployment + +const ( + FinalizerName = "finalizer.appdeployment.devinfra.goms.io" + + // phase types + PhaseEmpty = "" + PhasePending = "Pending" + PhaseDeploying = "Deploying" + PhaseReady = "Ready" + PhaseDeleting = "Deleting" + PhaseDeleted = "Deleted" + + // log keys + LogKeyJobName = "jobName" + LogKeyAppDeploymentName = "appDeploymentName" +) diff --git a/internal/utils/controller/appdeployment/job.go b/internal/utils/controller/appdeployment/job.go new file mode 100644 index 0000000..8feb7cc --- /dev/null +++ b/internal/utils/controller/appdeployment/job.go @@ -0,0 +1,132 @@ +package appdeployment + +import ( + "context" + + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + appv1 "github.com/Azure/operation-cache-controller/api/v1" +) + +const ( + OperationIDEnvKey = "OPERATION_ID" + + JobTypeProvision = "provision" + JobTypeTeardown = "teardown" +) + +var ( + backOffLimit int32 = 10 + ttlSecondsAfterFinished int32 = 3600 + MaxAppNameLength int = 36 + MaxOpIdLength int = 18 +) + +func validJName(appName, operationId, jobType string) string { + if len(appName) > MaxAppNameLength { + appName = appName[:MaxAppNameLength] + } + if len(operationId) > MaxOpIdLength { + operationId = operationId[:MaxOpIdLength] + } + originName := jobType + "-" + appName + "-" + operationId + return originName +} + +func ProvisionJobFromAppDeploymentSpec(appDeployment *appv1.AppDeployment) *batchv1.Job { + return jobFromAppDeploymentSpec(appDeployment, JobTypeProvision) +} + +func TeardownJobFromAppDeploymentSpec(appDeployment *appv1.AppDeployment) *batchv1.Job { + return jobFromAppDeploymentSpec(appDeployment, JobTypeTeardown) +} + +func GetProvisionJobName(appDeployment *appv1.AppDeployment) string { + return validJName(appDeployment.Name, appDeployment.Spec.OpId, JobTypeProvision) +} + +func GetTeardownJobName(appDeployment *appv1.AppDeployment) string { + return validJName(appDeployment.Name, appDeployment.Spec.OpId, JobTypeTeardown) +} + +func jobFromAppDeploymentSpec(appDeployment *appv1.AppDeployment, suffix string) *batchv1.Job { + ops := jobOptions{ + name: validJName(appDeployment.Name, appDeployment.Spec.OpId, suffix), + namespace: appDeployment.Namespace, + labels: appDeployment.Labels, + jobSpec: appDeployment.Spec.Provision, + operationID: appDeployment.Spec.OpId, + } + if suffix == JobTypeTeardown { + ops.jobSpec = appDeployment.Spec.Teardown + } + return newJobWithOptions(ops) +} + +type jobOptions struct { + name string + namespace string + labels map[string]string + annotations map[string]string + ownerRefs []metav1.OwnerReference + jobSpec batchv1.JobSpec + operationID string +} + +func newJobWithOptions(options jobOptions) *batchv1.Job { + options.jobSpec.Template.Spec.Containers[0].Env = append(options.jobSpec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: OperationIDEnvKey, + Value: options.operationID, + }) + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: options.name, + Namespace: options.namespace, + Labels: options.labels, + Annotations: options.annotations, + }, + Spec: options.jobSpec, + } + job.Spec.Template.Spec.RestartPolicy = corev1.RestartPolicyOnFailure + job.Spec.BackoffLimit = &backOffLimit + job.Spec.TTLSecondsAfterFinished = &ttlSecondsAfterFinished + + if len(options.ownerRefs) > 0 { + job.OwnerReferences = options.ownerRefs + } + + return job +} + +type JobStatus string + +var ( + JobStatusSucceeded JobStatus = "Succeeded" + JobStatusFailed JobStatus = "Failed" + JobStatusRunning JobStatus = "Running" +) + +func CheckJobStatus(ctx context.Context, job *batchv1.Job) JobStatus { + if job.Status.Succeeded > 0 { + return JobStatusSucceeded + } + if job.Status.Failed > 0 { + return JobStatusFailed + } + return JobStatusRunning +} + +func OperationScopedAppDeployment(appName, opId string) string { + if len(appName) > MaxAppNameLength { + appName = appName[:MaxAppNameLength] + } + if len(opId) > MaxOpIdLength { + opId = opId[:MaxOpIdLength] + } + if opId == "" { + return appName + } + return opId + "-" + appName +} diff --git a/internal/utils/controller/appdeployment/job_test.go b/internal/utils/controller/appdeployment/job_test.go new file mode 100644 index 0000000..34727f4 --- /dev/null +++ b/internal/utils/controller/appdeployment/job_test.go @@ -0,0 +1,230 @@ +package appdeployment + +import ( + "context" + "testing" + + appv1 "github.com/Azure/operation-cache-controller/api/v1" + "github.com/stretchr/testify/assert" + batchv1 "k8s.io/api/batch/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestCheckJobStatus(t *testing.T) { + tests := []struct { + name string + job *batchv1.Job + expected JobStatus + }{ + { + name: "Should return succeeded when job is successful", + job: &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/name": "test-app-name", + }, + Annotations: map[string]string{ + "app.kubernetes.io/version": "test-app-version", + }, + }, + Status: batchv1.JobStatus{ + Failed: 0, + Succeeded: 1, + }, + }, + expected: JobStatusSucceeded, + }, + { + name: "Should return failed when job has failed", + job: &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/name": "test-app-name", + }, + Annotations: map[string]string{ + "app.kubernetes.io/version": "test-app-version", + }, + }, + Status: batchv1.JobStatus{ + Failed: 1, + Succeeded: 0, + }, + }, + expected: JobStatusFailed, + }, + { + name: "Should return in progress when job is still running", + job: &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + Namespace: "test-namespace", + Labels: map[string]string{ + "app.kubernetes.io/name": "test-app-name", + }, + Annotations: map[string]string{ + "app.kubernetes.io/version": "test-app-version", + }, + }, + Status: batchv1.JobStatus{ + Failed: 0, + Succeeded: 0, + }, + }, + expected: JobStatusRunning, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res := CheckJobStatus(context.Background(), tt.job) + assert.Equal(t, tt.expected, res) + }) + } +} +func TestGetProvisionJobName(t *testing.T) { + tests := []struct { + name string + appName string + opId string + expected string + }{ + { + name: "Valid app name and operation ID", + appName: "my-app", + opId: "op123", + expected: "provision-my-app-op123", + }, + { + name: "App name exceeds max length", + appName: "a-very-long-application-name-exceeding-limit", + opId: "op123", + expected: "provision-a-very-long-application-name-exceedi-op123", + }, + { + name: "Operation ID exceeds max length", + appName: "my-app", + opId: "operation-id-exceeding-length", + expected: "provision-my-app-operation-id-excee", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + appDeployment := &appv1.AppDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: tt.appName, + }, + Spec: appv1.AppDeploymentSpec{ + OpId: tt.opId, + }, + } + res := GetProvisionJobName(appDeployment) + assert.Equal(t, tt.expected, res) + }) + } +} + +func TestGetTeardownJobName(t *testing.T) { + tests := []struct { + name string + appName string + opId string + expected string + }{ + { + name: "Valid app name and operation ID", + appName: "my-app", + opId: "op123", + expected: "teardown-my-app-op123", + }, + { + name: "App name exceeds max length", + appName: "a-very-long-application-name-exceeding-limit", + opId: "op123", + expected: "teardown-a-very-long-application-name-exceedi-op123", + }, + { + name: "Operation ID exceeds max length", + appName: "my-app", + opId: "operation-id-exceeding-length", + expected: "teardown-my-app-operation-id-excee", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + appDeployment := &appv1.AppDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: tt.appName, + }, + Spec: appv1.AppDeploymentSpec{ + OpId: tt.opId, + }, + } + res := GetTeardownJobName(appDeployment) + assert.Equal(t, tt.expected, res) + }) + } +} +func TestOperationScopedAppDeployment(t *testing.T) { + tests := []struct { + name string + appName string + opId string + expected string + }{ + { + name: "Both appName and opId within limits", + appName: "my-app", + opId: "op123", + expected: "op123-my-app", + }, + { + name: "appName exceeds max length", + appName: "a-very-long-application-name-exceeding-limit", + opId: "op123", + expected: "op123-a-very-long-application-name-exceedi", + }, + { + name: "opId exceeds max length", + appName: "my-app", + opId: "operation-id-exceeding-length", + expected: "operation-id-excee-my-app", + }, + { + name: "Both appName and opId exceed max length", + appName: "another-very-long-application-name-exceeding-limit", + opId: "another-operation-id-exceeding-length", + expected: "another-operation--another-very-long-application-name-e", + }, + { + name: "Empty appName and opId", + appName: "", + opId: "", + expected: "", + }, + { + name: "Empty appName", + appName: "", + opId: "op123", + expected: "op123-", + }, + { + name: "Empty opId", + appName: "my-app", + opId: "", + expected: "my-app", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res := OperationScopedAppDeployment(tt.appName, tt.opId) + assert.Equal(t, tt.expected, res) + }) + } +} diff --git a/internal/utils/controller/appdeployment/validate.go b/internal/utils/controller/appdeployment/validate.go new file mode 100644 index 0000000..3d3170b --- /dev/null +++ b/internal/utils/controller/appdeployment/validate.go @@ -0,0 +1,139 @@ +package appdeployment + +import ( + "errors" + "fmt" + + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" + + appv1 "github.com/Azure/operation-cache-controller/api/v1" +) + +type Validater func(*appv1.AppDeployment) error + +func Validate(ap *appv1.AppDeployment) error { + var errs error + validaters := []Validater{ + validateJobSpec, + } + for _, v := range validaters { + errs = errors.Join(errs, v(ap)) + } + return errs +} + +// validateJobSpec validates the container count in the AppDeployment Spec +// * container count in AppDeployment Spec should be 1 +// * initCountainer is not allowed +func validateJobSpec(ap *appv1.AppDeployment) error { + if equality.Semantic.DeepEqual(ap.Spec, appv1.AppDeploymentSpec{}) { + return errors.New("spec of appdeployment is nil") + } + // provision job must be present + if equality.Semantic.DeepEqual(ap.Spec.Provision, batchv1.JobSpec{}) { + return errors.New("provision job is nil") + } + if jobConstraint(ap.Spec.Provision) != nil { + return fmt.Errorf("provision: %w", jobConstraint(ap.Spec.Provision)) + } + // teardown job is optional + if !equality.Semantic.DeepEqual(ap.Spec.Teardown, batchv1.JobSpec{}) { + if jobConstraint(ap.Spec.Teardown) != nil { + return fmt.Errorf("teardown: %w", jobConstraint(ap.Spec.Teardown)) + } + } + return nil +} + +// jobConstraint validates the JobSpec +// only container is allowed in the JobSpec +func jobConstraint(js batchv1.JobSpec) error { + if js.ActiveDeadlineSeconds != nil { + return fmt.Errorf("activeDeadlineSeconds is not allowed") + } + if js.BackoffLimit != nil { + return fmt.Errorf("backoffLimit is not allowed") + } + if js.BackoffLimitPerIndex != nil { + return fmt.Errorf("backoffLimitPerIndex is not allowed") + } + if js.Completions != nil { + return fmt.Errorf("completions is not allowed") + } + if js.CompletionMode != nil { + return fmt.Errorf("completionMode is not allowed") + } + if js.ManagedBy != nil { + return fmt.Errorf("managedBy is not allowed") + } + if js.ManualSelector != nil { + return fmt.Errorf("manualSelector is not allowed") + } + if js.MaxFailedIndexes != nil { + return fmt.Errorf("maxFailedIndexes is not allowed") + } + if js.Parallelism != nil { + return fmt.Errorf("parallelism is not allowed") + } + if js.PodFailurePolicy != nil { + return fmt.Errorf("podFailurePolicy is not allowed") + } + if js.PodReplacementPolicy != nil { + return fmt.Errorf("podReplacementPolicy is not allowed") + } + if js.Selector != nil { + return fmt.Errorf("selector is not allowed") + } + if js.TTLSecondsAfterFinished != nil { + return fmt.Errorf("ttlSecondsAfterFinished is not allowed") + } + if js.SuccessPolicy != nil { + return fmt.Errorf("successPolicy is not allowed") + } + if js.Suspend != nil { + return fmt.Errorf("suspend is not allowed") + } + + if podConstraint(js.Template) != nil { + return fmt.Errorf("pod template: %w", podConstraint(js.Template)) + } + + return nil +} + +// podConstraint validates the PodSpec +func podConstraint(pt corev1.PodTemplateSpec) error { + if len(pt.Name) > 0 { + return fmt.Errorf("name is not allowed") + } + if len(pt.Namespace) > 0 { + return fmt.Errorf("namespace is not allowed") + } + if len(pt.Spec.Volumes) > 0 { + return fmt.Errorf("volumes are not allowed") + } + if len(pt.Spec.InitContainers) > 0 { + return fmt.Errorf("initContainers are not allowed") + } + if len(pt.Spec.Containers) != 1 { + return fmt.Errorf("container count should be 1") + } + if containerConstraint(pt.Spec.Containers[0]) != nil { + return fmt.Errorf("container: %w", containerConstraint(pt.Spec.Containers[0])) + } + return nil +} + +// containerConstraint validates the Container +func containerConstraint(c corev1.Container) error { + if c.Image == "" { + return fmt.Errorf("image is empty") + } + if len(c.VolumeMounts) > 0 { + return fmt.Errorf("volumeMounts are not allowed") + } + + return nil +} diff --git a/internal/utils/controller/appdeployment/validate_test.go b/internal/utils/controller/appdeployment/validate_test.go new file mode 100644 index 0000000..a78bf92 --- /dev/null +++ b/internal/utils/controller/appdeployment/validate_test.go @@ -0,0 +1,496 @@ +package appdeployment + +import ( + "testing" + + "github.com/stretchr/testify/assert" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + appv1 "github.com/Azure/operation-cache-controller/api/v1" + "github.com/Azure/operation-cache-controller/internal/utils/ptr" +) + +func TestValidate(t *testing.T) { + tests := []struct { + name string + app appv1.AppDeployment + wantErr bool + }{ + { + name: "Valid AppDeployment", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + Containers: []corev1.Container{ + { + Image: "nginx:latest", + Command: []string{"nginx"}, + }, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "Empty Spec", + app: appv1.AppDeployment{}, + wantErr: true, + }, + { + name: "Invalid Provision Job", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + OpId: "test", + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "nginx:latest", + Command: []string{ + "nginx", + }, + }, + }, + }, + }, + SuccessPolicy: &batchv1.SuccessPolicy{ + Rules: []batchv1.SuccessPolicyRule{ + { + SucceededIndexes: ptr.Of(string("test")), + SucceededCount: ptr.Of(int32(1)), + }, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "Invalid TearDown Job", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + OpId: "test", + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "nginx:latest", + Command: []string{ + "nginx", + }, + }, + }, + }, + }, + }, + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Command: []string{ + "nginx", + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint activeDeadlineSeconds", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + ActiveDeadlineSeconds: ptr.Of(int64(1)), + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint backoffLimit", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + BackoffLimit: ptr.Of(int32(1)), + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint backoffLimitPerIndex", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + BackoffLimitPerIndex: ptr.Of(int32(1)), + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint completions", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Completions: ptr.Of(int32(1)), + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint completionMode", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + CompletionMode: ptr.Of(batchv1.NonIndexedCompletion), + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint managedBy", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + ManagedBy: ptr.Of("test"), + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint manualSelector", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + ManualSelector: ptr.Of(true), + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint maxFailedIndexes", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + MaxFailedIndexes: ptr.Of(int32(1)), + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint parallelism", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Parallelism: ptr.Of(int32(1)), + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint podFailurePolicy", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + PodFailurePolicy: ptr.Of(batchv1.PodFailurePolicy{ + Rules: []batchv1.PodFailurePolicyRule{ + { + Action: batchv1.PodFailurePolicyAction(batchv1.PodFailurePolicyActionFailJob), + }, + }, + }), + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint podReplacementPolicy", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + PodReplacementPolicy: ptr.Of(batchv1.Failed), + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint Selectors", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "test", + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint TTLSecondsAfterFinished", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + TTLSecondsAfterFinished: ptr.Of(int32(1)), + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint SuccessPolicy", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + CompletionMode: new(batchv1.CompletionMode), + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint Suspend", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Suspend: ptr.Of(true), + }, + }, + }, + wantErr: true, + }, + { + name: "jobConstraint PodTemplate", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + OpId: "test", + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{}, + }, + }, + }, + wantErr: true, + }, + { + name: "podConstraint name", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "nginx:latest", + Command: []string{ + "nginx", + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "podConstraint namespace", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "testns", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "nginx:latest", + Command: []string{ + "nginx", + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "podConstraint volumes", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "test", + }, + }, + Containers: []corev1.Container{ + { + Image: "nginx:latest", + Command: []string{ + "nginx", + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "podConstraint initContainers", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + { + Image: "nginx:latest", + Command: []string{ + "nginx", + }, + }, + }, + Containers: []corev1.Container{ + { + Image: "nginx:latest", + Command: []string{ + "nginx", + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "podConstraint container count should be 1", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "nginx:latest", + Command: []string{ + "nginx", + }, + }, + { + Image: "nginx:latest", + Command: []string{ + "nginx", + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "containerConstraint image is empty", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Command: []string{ + "nginx", + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "containerConstraint volumeMounts are not allowed", + app: appv1.AppDeployment{ + Spec: appv1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "nginx:latest", + Command: []string{ + "nginx", + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "test", + }, + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := Validate(&tt.app) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/internal/utils/ptr/ptr.go b/internal/utils/ptr/ptr.go new file mode 100644 index 0000000..63256bd --- /dev/null +++ b/internal/utils/ptr/ptr.go @@ -0,0 +1,13 @@ +package ptr + +func Of[T any](v T) *T { + return &v +} + +func Deref[T any](p *T) T { + if p == nil { + var zero T + return zero + } + return *p +} diff --git a/internal/utils/ptr/ptr_test.go b/internal/utils/ptr/ptr_test.go new file mode 100644 index 0000000..08a7e28 --- /dev/null +++ b/internal/utils/ptr/ptr_test.go @@ -0,0 +1,79 @@ +package ptr + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// test case for Of + +func TestOf(t *testing.T) { + t.Run("string to ptr", func(t *testing.T) { + assert := assert.New(t) + assert.Equal(*Of("a"), "a") + assert.Equal(*Of("b"), "b") + }) + + t.Run("int to ptr", func(t *testing.T) { + assert := assert.New(t) + assert.Equal(*Of(1), 1) + assert.Equal(*Of(2), 2) + }) + + t.Run("bool to ptr", func(t *testing.T) { + assert := assert.New(t) + assert.Equal(*Of(true), true) + assert.Equal(*Of(false), false) + }) + + t.Run("float to ptr", func(t *testing.T) { + assert := assert.New(t) + assert.Equal(*Of(1.1), 1.1) + assert.Equal(*Of(2.2), 2.2) + }) + +} + +// test case for Deref + +func TestDeref(t *testing.T) { + tests := []struct { + name string + input interface{} + expected interface{} + }{ + { + name: "string", + input: "a", + expected: "a", + }, + { + name: "int", + input: 1, + expected: 1, + }, + { + name: "bool true", + input: true, + expected: true, + }, + { + name: "float", + input: 1.1, + expected: 1.1, + }, + { + name: "bool false", + input: false, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert := assert.New(t) + assert.Equal(Deref(Of(tt.input)), tt.expected) + }) + } +} diff --git a/internal/utils/reconciler/operations.go b/internal/utils/reconciler/operations.go new file mode 100644 index 0000000..30e8877 --- /dev/null +++ b/internal/utils/reconciler/operations.go @@ -0,0 +1,85 @@ +package reconciler + +import ( + "context" + "time" +) + +var DefaultRequeueDelay = 10 * time.Second + +type ReconcileOperation func(ctx context.Context) (OperationResult, error) + +type OperationResult struct { + RequeueDelay time.Duration + RequeueRequest bool + CancelRequest bool +} + +func (r OperationResult) RequeueOrCancel() bool { + return r.RequeueRequest || r.CancelRequest +} + +func ContinueOperationResult() OperationResult { + return OperationResult{ + RequeueDelay: 0, + RequeueRequest: false, + CancelRequest: false, + } +} +func StopOperationResult() OperationResult { + return OperationResult{ + RequeueDelay: 0, + RequeueRequest: false, + CancelRequest: true, + } +} + +func StopProcessing() (OperationResult, error) { + result := StopOperationResult() + return result, nil +} + +func Requeue() (result OperationResult, err error) { + result = OperationResult{ + RequeueDelay: DefaultRequeueDelay, + RequeueRequest: true, + CancelRequest: false, + } + return result, nil +} + +func RequeueWithError(err error) (OperationResult, error) { + result := OperationResult{ + RequeueDelay: DefaultRequeueDelay, + RequeueRequest: true, + CancelRequest: false, + } + return result, err +} + +func RequeueOnErrorOrStop(err error) (OperationResult, error) { + return OperationResult{ + RequeueDelay: DefaultRequeueDelay, + RequeueRequest: false, + CancelRequest: true, + }, err +} + +func RequeueOnErrorOrContinue(err error) (OperationResult, error) { + return OperationResult{ + RequeueDelay: DefaultRequeueDelay, + RequeueRequest: false, + CancelRequest: false, + }, err +} + +func RequeueAfter(delay time.Duration, err error) (OperationResult, error) { + return OperationResult{ + RequeueDelay: delay, + RequeueRequest: true, + CancelRequest: false, + }, err +} +func ContinueProcessing() (OperationResult, error) { + return ContinueOperationResult(), nil +} diff --git a/internal/utils/reconciler/operations_test.go b/internal/utils/reconciler/operations_test.go new file mode 100644 index 0000000..774f6fa --- /dev/null +++ b/internal/utils/reconciler/operations_test.go @@ -0,0 +1,120 @@ +package reconciler + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestOperations(t *testing.T) { + t.Run("ContinueOperationResult", func(t *testing.T) { + result := ContinueOperationResult() + assert.Equal(t, OperationResult{ + RequeueDelay: 0, + RequeueRequest: false, + CancelRequest: false, + }, result) + }) + t.Run("StopOperationResult", func(t *testing.T) { + result := StopOperationResult() + assert.Equal(t, OperationResult{ + RequeueDelay: 0, + RequeueRequest: false, + CancelRequest: true, + }, result) + }) + + t.Run("StopProcessing", func(t *testing.T) { + result, err := StopProcessing() + assert.Nil(t, err) + assert.Equal(t, OperationResult{ + RequeueDelay: 0, + RequeueRequest: false, + CancelRequest: true, + }, result) + }) + + t.Run("Requeue", func(t *testing.T) { + result, err := Requeue() + assert.Nil(t, err) + assert.Equal(t, OperationResult{ + RequeueDelay: DefaultRequeueDelay, + RequeueRequest: true, + CancelRequest: false, + }, result) + }) + + t.Run("RequeueWithError", func(t *testing.T) { + result, err := RequeueWithError(errors.New("test")) + assert.NotNil(t, err) + assert.Equal(t, OperationResult{ + RequeueDelay: DefaultRequeueDelay, + RequeueRequest: true, + CancelRequest: false, + }, result) + }) + + t.Run("RequeueOnErrorOrStop", func(t *testing.T) { + result, err := RequeueOnErrorOrStop(errors.New("test")) + assert.NotNil(t, err) + assert.Equal(t, OperationResult{ + RequeueDelay: DefaultRequeueDelay, + RequeueRequest: false, + CancelRequest: true, + }, result) + }) + + t.Run("RequeueOnErrorOrContinue", func(t *testing.T) { + result, err := RequeueOnErrorOrContinue(errors.New("test")) + assert.NotNil(t, err) + assert.Equal(t, OperationResult{ + RequeueDelay: DefaultRequeueDelay, + RequeueRequest: false, + CancelRequest: false, + }, result) + }) + + t.Run("RequeueAfter", func(t *testing.T) { + result, err := RequeueAfter(1, errors.New("test")) + assert.NotNil(t, err) + assert.Equal(t, OperationResult{ + RequeueDelay: 1, + RequeueRequest: true, + CancelRequest: false, + }, result) + }) + + t.Run("ContinueProcessing", func(t *testing.T) { + result, err := ContinueProcessing() + assert.Nil(t, err) + assert.Equal(t, result, OperationResult{ + RequeueDelay: 0, + RequeueRequest: false, + CancelRequest: false, + }) + }) + + t.Run("RequeueOrCancel", func(t *testing.T) { + result1 := OperationResult{ + RequeueDelay: 0, + RequeueRequest: false, + CancelRequest: false, + } + assert.Equal(t, result1.RequeueOrCancel(), false) + + result2 := OperationResult{ + RequeueDelay: 0, + RequeueRequest: true, + CancelRequest: false, + } + assert.Equal(t, result2.RequeueOrCancel(), true) + + result3 := OperationResult{ + RequeueDelay: 0, + RequeueRequest: false, + CancelRequest: true, + } + assert.Equal(t, result3.RequeueOrCancel(), true) + }) +} From 6cea8f66215131e0b34b23a9d3b9e05a1ac22ae5 Mon Sep 17 00:00:00 2001 From: DQ Date: Thu, 10 Apr 2025 16:21:52 +1000 Subject: [PATCH 2/3] fix --- go.mod | 1 + 1 file changed, 1 insertion(+) diff --git a/go.mod b/go.mod index 3c2b96f..e321eba 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ godebug default=go1.23 require ( github.com/onsi/ginkgo/v2 v2.22.0 github.com/onsi/gomega v1.36.1 + github.com/stretchr/testify v1.9.0 k8s.io/api v0.32.1 k8s.io/apimachinery v0.32.1 k8s.io/client-go v0.32.1 From 621af4b58d01c06197ba7be736744b8364052840 Mon Sep 17 00:00:00 2001 From: DQ Date: Thu, 10 Apr 2025 16:23:51 +1000 Subject: [PATCH 3/3] fix --- internal/utils/controller/appdeployment/validate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/utils/controller/appdeployment/validate_test.go b/internal/utils/controller/appdeployment/validate_test.go index a78bf92..48268c0 100644 --- a/internal/utils/controller/appdeployment/validate_test.go +++ b/internal/utils/controller/appdeployment/validate_test.go @@ -218,7 +218,7 @@ func TestValidate(t *testing.T) { PodFailurePolicy: ptr.Of(batchv1.PodFailurePolicy{ Rules: []batchv1.PodFailurePolicyRule{ { - Action: batchv1.PodFailurePolicyAction(batchv1.PodFailurePolicyActionFailJob), + Action: batchv1.PodFailurePolicyActionFailJob, }, }, }),