From cb8dbd319a245882b69cb491cffb71ceb8356f21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Haram=20Nyg=C3=A5rd?= Date: Tue, 20 Aug 2024 15:36:23 +0200 Subject: [PATCH 1/5] Remove enabled field from cloudSqlProxy --- api/v1alpha1/podtypes/gcp.go | 6 ------ config/crd/skiperator.kartverket.no_applications.yaml | 3 --- config/crd/skiperator.kartverket.no_skipjobs.yaml | 3 --- pkg/util/helperfunctions.go | 2 +- tests/application/cloudsql-auth-proxy/set-version.yaml | 1 - tests/skipjob/immutable-container/chainsaw-test.yaml | 2 +- 6 files changed, 2 insertions(+), 15 deletions(-) diff --git a/api/v1alpha1/podtypes/gcp.go b/api/v1alpha1/podtypes/gcp.go index e8982bb1..e497d40f 100644 --- a/api/v1alpha1/podtypes/gcp.go +++ b/api/v1alpha1/podtypes/gcp.go @@ -34,12 +34,6 @@ type Auth struct { } type CloudSQLProxySettings struct { - // Enables the sidecar for CloudSQL proxy - - //+kubebuilder:validation:Optional - //+kubebuilder:default:=false - Enabled bool `json:"enabled"` - // Connection name for the CloudSQL instance. Found in the Google Cloud Console under your CloudSQL resource. // The format is "projectName:region:instanceName" E.g. "skip-prod-bda1:europe-north1:my-db". //+kubebuilder:validation:Required diff --git a/config/crd/skiperator.kartverket.no_applications.yaml b/config/crd/skiperator.kartverket.no_applications.yaml index 26e0a27d..8125b94c 100644 --- a/config/crd/skiperator.kartverket.no_applications.yaml +++ b/config/crd/skiperator.kartverket.no_applications.yaml @@ -520,9 +520,6 @@ spec: The format is "projectName:region:instanceName" E.g. "skip-prod-bda1:europe-north1:my-db". pattern: ^[^:]+:[^:]+:[^:]+$ type: string - enabled: - default: false - type: boolean ip: description: The IP address of the CloudSQL instance. This is used to create a serviceentry for the CloudSQL proxy. diff --git a/config/crd/skiperator.kartverket.no_skipjobs.yaml b/config/crd/skiperator.kartverket.no_skipjobs.yaml index c9d51c77..bd9a58b4 100644 --- a/config/crd/skiperator.kartverket.no_skipjobs.yaml +++ b/config/crd/skiperator.kartverket.no_skipjobs.yaml @@ -484,9 +484,6 @@ spec: The format is "projectName:region:instanceName" E.g. "skip-prod-bda1:europe-north1:my-db". pattern: ^[^:]+:[^:]+:[^:]+$ type: string - enabled: - default: false - type: boolean ip: description: The IP address of the CloudSQL instance. This is used to create a serviceentry for the CloudSQL diff --git a/pkg/util/helperfunctions.go b/pkg/util/helperfunctions.go index bf106b43..a3bf824e 100644 --- a/pkg/util/helperfunctions.go +++ b/pkg/util/helperfunctions.go @@ -149,7 +149,7 @@ func EnsurePrefix(s string, prefix string) string { } func IsCloudSqlProxyEnabled(gcp *podtypes.GCP) bool { - return gcp != nil && gcp.CloudSQLProxy != nil && gcp.CloudSQLProxy.Enabled + return gcp != nil && gcp.CloudSQLProxy != nil } func IsGCPAuthEnabled(gcp *podtypes.GCP) bool { diff --git a/tests/application/cloudsql-auth-proxy/set-version.yaml b/tests/application/cloudsql-auth-proxy/set-version.yaml index 47acf911..e8702f00 100644 --- a/tests/application/cloudsql-auth-proxy/set-version.yaml +++ b/tests/application/cloudsql-auth-proxy/set-version.yaml @@ -7,7 +7,6 @@ spec: port: 8080 gcp: cloudSqlProxy: - enabled: true connectionName: test-project-bda1:europe-north1:pg-01-test version: 2.8.12 serviceAccount: grafana@test-project-bda1.iam.gserviceaccount.com diff --git a/tests/skipjob/immutable-container/chainsaw-test.yaml b/tests/skipjob/immutable-container/chainsaw-test.yaml index fe8e2f46..8ff82c94 100644 --- a/tests/skipjob/immutable-container/chainsaw-test.yaml +++ b/tests/skipjob/immutable-container/chainsaw-test.yaml @@ -5,7 +5,7 @@ metadata: name: immutable-container spec: skip: false - concurrent: true + concurrent: false skipDelete: false steps: - try: From 444fd448c101bb21cdbfb9084237ca4cf937b5c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Haram=20Nyg=C3=A5rd?= Date: Wed, 21 Aug 2024 09:23:14 +0200 Subject: [PATCH 2/5] fix attempting to patch immutable fields on jobs --- pkg/resourceprocessor/resource.go | 6 ++-- .../immutable-container/chainsaw-test.yaml | 2 +- .../immutable-container/skipjob-assert.yaml | 28 +++++++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/pkg/resourceprocessor/resource.go b/pkg/resourceprocessor/resource.go index c9685d55..03d36d2d 100644 --- a/pkg/resourceprocessor/resource.go +++ b/pkg/resourceprocessor/resource.go @@ -45,8 +45,10 @@ func preparePatch(new client.Object, old client.Object) { case *batchv1.Job: job := old.(*batchv1.Job) definition := new.(*batchv1.Job) - maps.Copy(definition.Spec.Template.Labels, job.Spec.Template.Labels) //kubernetes adds labels on creation - definition.Spec.Selector = job.Spec.Selector //is set on creation + maps.Copy(definition.Spec.Template.Labels, job.Spec.Template.Labels) // kubernetes adds labels on creation + definition.Spec.Selector = job.Spec.Selector // immutable + definition.Spec.Template = job.Spec.Template // immutable + definition.Spec.Completions = job.Spec.Completions // immutable } } diff --git a/tests/skipjob/immutable-container/chainsaw-test.yaml b/tests/skipjob/immutable-container/chainsaw-test.yaml index 8ff82c94..fe8e2f46 100644 --- a/tests/skipjob/immutable-container/chainsaw-test.yaml +++ b/tests/skipjob/immutable-container/chainsaw-test.yaml @@ -5,7 +5,7 @@ metadata: name: immutable-container spec: skip: false - concurrent: false + concurrent: true skipDelete: false steps: - try: diff --git a/tests/skipjob/immutable-container/skipjob-assert.yaml b/tests/skipjob/immutable-container/skipjob-assert.yaml index 467f10af..86cb728c 100644 --- a/tests/skipjob/immutable-container/skipjob-assert.yaml +++ b/tests/skipjob/immutable-container/skipjob-assert.yaml @@ -13,3 +13,31 @@ spec: status: summary: status: Synced + subresources: + Job[minimal-job]: + message: Job has finished synchronizing + status: Synced + ServiceAccount[minimal-job-skipjob]: + message: ServiceAccount has finished synchronizing + status: Synced + conditions: + - message: Job failed previous run + observedGeneration: 2 + reason: JobFailed + status: 'False' + type: Failed + - message: Job has been created and is now running + observedGeneration: 2 + reason: JobRunning + status: 'False' + type: Running + - message: Job has finished + observedGeneration: 2 + reason: JobFinished + status: 'True' + type: Finished + - message: Internal rules are valid + observedGeneration: 2 + reason: ApplicationReconciled + status: 'True' + type: InternalRulesValid From cd5e4d67357ebdf0df02e684fe1fc03d76793df7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Haram=20Nyg=C3=A5rd?= Date: Wed, 21 Aug 2024 13:20:14 +0200 Subject: [PATCH 3/5] increase timeouts, parallel --- tests/config.yaml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/config.yaml b/tests/config.yaml index cefa3e30..dd9650e8 100644 --- a/tests/config.yaml +++ b/tests/config.yaml @@ -3,11 +3,11 @@ kind: Configuration metadata: name: configuration spec: - parallel: 10 + parallel: 40 timeouts: - delete: 60s - assert: 60s - apply: 60s - cleanup: 60s - error: 60s - exec: 60s + delete: 180s + assert: 180s + apply: 180s + cleanup: 180s + error: 180s + exec: 180s From a7fc542d30c816d23d436259f63c6b062e3b3bae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Haram=20Nyg=C3=A5rd?= Date: Wed, 21 Aug 2024 13:36:26 +0200 Subject: [PATCH 4/5] revert status check for skipjob --- api/v1alpha1/skipjob_types.go | 8 ++--- .../skiperator.kartverket.no_skipjobs.yaml | 32 ++++++++++--------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/api/v1alpha1/skipjob_types.go b/api/v1alpha1/skipjob_types.go index f020e1f3..153e675e 100644 --- a/api/v1alpha1/skipjob_types.go +++ b/api/v1alpha1/skipjob_types.go @@ -36,10 +36,6 @@ type SKIPJobStatus struct { // +kubebuilder:object:generate=true // +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.summary.status` // -// A SKIPJob is either defined as a one-off or a scheduled job. If the Cron field is set for SKIPJob, it may not be removed. If the Cron field is unset, it may not be added. -// The Container field of a SKIPJob is only mutable if the Cron field is set. If unset, you must delete your SKIPJob to change container settings. -// +kubebuilder:validation:XValidation:rule="(has(oldSelf.spec.cron) && has(self.spec.cron)) || (!has(oldSelf.spec.cron) && !has(self.spec.cron))", message="After creation of a SKIPJob you may not remove the Cron field if it was previously present, or add it if it was previously omitted. Please delete the SKIPJob to change its nature from a one-off/scheduled job." -// +kubebuilder:validation:XValidation:rule="(size(self.status.subresources) == 0|| ((!has(self.spec.cron) && (oldSelf.spec.container == self.spec.container)) || has(self.spec.cron)))", message="The field Container is immutable for one-off jobs. Please delete your SKIPJob to change the containers settings." // SKIPJob is the Schema for the skipjobs API type SKIPJob struct { metav1.TypeMeta `json:",inline"` @@ -64,6 +60,10 @@ type SKIPJobList struct { // SKIPJobSpec defines the desired state of SKIPJob // // +kubebuilder:object:generate=true +// A SKIPJob is either defined as a one-off or a scheduled job. If the Cron field is set for SKIPJob, it may not be removed. If the Cron field is unset, it may not be added. +// The Container field of a SKIPJob is only mutable if the Cron field is set. If unset, you must delete your SKIPJob to change container settings. +// +kubebuilder:validation:XValidation:rule="(has(oldSelf.cron) && has(self.cron)) || (!has(oldSelf.cron) && !has(self.cron))", message="After creation of a SKIPJob you may not remove the Cron field if it was previously present, or add it if it was previously omitted. Please delete the SKIPJob to change its nature from a one-off/scheduled job." +// +kubebuilder:validation:XValidation:rule="((!has(self.cron) && (oldSelf.container == self.container)) || has(self.cron))", message="The field Container is immutable for one-off jobs. Please delete your SKIPJob to change the containers settings." type SKIPJobSpec struct { // Settings for the actual Job. If you use a scheduled job, the settings in here will also specify the template of the job. // diff --git a/config/crd/skiperator.kartverket.no_skipjobs.yaml b/config/crd/skiperator.kartverket.no_skipjobs.yaml index bd9a58b4..23cd5b50 100644 --- a/config/crd/skiperator.kartverket.no_skipjobs.yaml +++ b/config/crd/skiperator.kartverket.no_skipjobs.yaml @@ -21,10 +21,7 @@ spec: name: v1alpha1 schema: openAPIV3Schema: - description: |- - A SKIPJob is either defined as a one-off or a scheduled job. If the Cron field is set for SKIPJob, it may not be removed. If the Cron field is unset, it may not be added. - The Container field of a SKIPJob is only mutable if the Cron field is set. If unset, you must delete your SKIPJob to change container settings. - SKIPJob is the Schema for the skipjobs API + description: SKIPJob is the Schema for the skipjobs API properties: apiVersion: description: |- @@ -44,7 +41,11 @@ spec: metadata: type: object spec: - description: SKIPJobSpec defines the desired state of SKIPJob + description: |- + SKIPJobSpec defines the desired state of SKIPJob + + A SKIPJob is either defined as a one-off or a scheduled job. If the Cron field is set for SKIPJob, it may not be removed. If the Cron field is unset, it may not be added. + The Container field of a SKIPJob is only mutable if the Cron field is set. If unset, you must delete your SKIPJob to change container settings. properties: container: description: |- @@ -835,6 +836,17 @@ spec: required: - container type: object + x-kubernetes-validations: + - message: After creation of a SKIPJob you may not remove the Cron field + if it was previously present, or add it if it was previously omitted. + Please delete the SKIPJob to change its nature from a one-off/scheduled + job. + rule: (has(oldSelf.cron) && has(self.cron)) || (!has(oldSelf.cron) && + !has(self.cron)) + - message: The field Container is immutable for one-off jobs. Please delete + your SKIPJob to change the containers settings. + rule: ((!has(self.cron) && (oldSelf.container == self.container)) || + has(self.cron)) status: description: |- SkiperatorStatus @@ -941,16 +953,6 @@ spec: required: - spec type: object - x-kubernetes-validations: - - message: After creation of a SKIPJob you may not remove the Cron field if - it was previously present, or add it if it was previously omitted. Please - delete the SKIPJob to change its nature from a one-off/scheduled job. - rule: (has(oldSelf.spec.cron) && has(self.spec.cron)) || (!has(oldSelf.spec.cron) - && !has(self.spec.cron)) - - message: The field Container is immutable for one-off jobs. Please delete - your SKIPJob to change the containers settings. - rule: (size(self.status.subresources) == 0|| ((!has(self.spec.cron) && (oldSelf.spec.container - == self.spec.container)) || has(self.spec.cron))) served: true storage: true subresources: From 14cdaa17e2d103c39928eb89ed620d4ed5688b56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Haram=20Nyg=C3=A5rd?= Date: Wed, 21 Aug 2024 14:18:05 +0200 Subject: [PATCH 5/5] make ingress test not concurrent --- tests/application/ingress/chainsaw-test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/application/ingress/chainsaw-test.yaml b/tests/application/ingress/chainsaw-test.yaml index 0ef2e0e8..d23d073a 100644 --- a/tests/application/ingress/chainsaw-test.yaml +++ b/tests/application/ingress/chainsaw-test.yaml @@ -4,7 +4,7 @@ metadata: name: ingress spec: skip: false - concurrent: true + concurrent: false skipDelete: false namespace: ingress steps: