Remove temporary helm values

* `statefulsetRunnerTemporarySetPodSeccompProfile` * `jobTaskRunnerTemporarySetPodSeccompProfile` Those helm values were introduced to avoid workloads restart on Korifi upgrade. With the upcoming release workloads would be restarted by an unrelated change anyway, so it is a convenient moment to get rid of those. issue #3164 Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
cloudfoundry · Oct 17, 2024 · 82c2d66 · 82c2d66
1 parent 8b2cf15
commit 82c2d66
Show file tree

Hide file tree

Showing 14 changed files with 50 additions and 130 deletions.
diff --git a/README.helm.md b/README.helm.md
@@ -92,7 +92,6 @@ Here are all the values that can be set for the chart:
     - `requests`: Resource requests.
       - `cpu` (_String_): CPU request.
       - `memory` (_String_): Memory request.
-  - `temporarySetPodSeccompProfile` (_Boolean_): Sets the pod .spec.securityContext.seccompProfile to RuntimeDefault. Setting this flag to true will cause a restart of all previously running pods.
 - `kpackImageBuilder`:
   - `builderReadinessTimeout` (_String_): The time that the kpack Builder will be waited for if not in ready state, berfore the build workload fails. See [`time.ParseDuration`](https://pkg.go.dev/time#ParseDuration) for details on the format, an additional `d` suffix for days is supported.
   - `builderRepository` (_String_): Container image repository to store the `ClusterBuilder` image. Required when `clusterBuilderName` is not provided.
@@ -134,5 +133,4 @@ Here are all the values that can be set for the chart:
     - `requests`: Resource requests.
       - `cpu` (_String_): CPU request.
       - `memory` (_String_): Memory request.
-  - `temporarySetPodSeccompProfile` (_Boolean_): Sets the pod .spec.securityContext.seccompProfile to RuntimeDefault. Setting this flag to true will cause a restart of all previously running pods.
 - `systemImagePullSecrets` (_Array_): List of `Secret` names to be used when pulling Korifi system images from private registries
diff --git a/controllers/config/config.go b/controllers/config/config.go
@@ -30,11 +30,7 @@ type ControllerConfig struct {
 	SpaceFinalizerAppDeletionTimeout *int32             `yaml:"spaceFinalizerAppDeletionTimeout"`
 
 	// job-task-runner
-	JobTTL                                     string `yaml:"jobTTL"`
-	JobTaskRunnerTemporarySetPodSeccompProfile bool   `yaml:"jobTaskRunnerTemporarySetPodSeccompProfile"`
-
-	// statefulset-runner
-	StatefulsetRunnerTemporarySetPodSeccompProfile bool `yaml:"statefulsetRunnerTemporarySetPodSeccompProfile"`
+	JobTTL string `yaml:"jobTTL"`
 
 	// kpack-image-builder
 	ClusterBuilderName        string     `yaml:"clusterBuilderName"`

diff --git a/controllers/main.go b/controllers/main.go
@@ -393,7 +393,6 @@ func main() {
 				mgr.GetScheme(),
 				jobtaskrunnercontrollers.NewStatusGetter(mgr.GetClient()),
 				jobTTL,
-				controllerConfig.JobTaskRunnerTemporarySetPodSeccompProfile,
 			)
 			if err = taskWorkloadReconciler.SetupWithManager(mgr); err != nil {
 				setupLog.Error(err, "unable to create controller", "controller", "TaskWorkload")
@@ -405,10 +404,7 @@ func main() {
 			if err = statefulsetcontrollers.NewAppWorkloadReconciler(
 				mgr.GetClient(),
 				mgr.GetScheme(),
-				statefulsetcontrollers.NewAppWorkloadToStatefulsetConverter(
-					mgr.GetScheme(),
-					controllerConfig.StatefulsetRunnerTemporarySetPodSeccompProfile,
-				),
+				statefulsetcontrollers.NewAppWorkloadToStatefulsetConverter(mgr.GetScheme()),
 				statefulsetcontrollers.NewPDBUpdater(mgr.GetClient()),
 				controllersLog,
 			).SetupWithManager(mgr); err != nil {

diff --git a/helm/korifi/controllers/configmap.yaml b/helm/korifi/controllers/configmap.yaml
@@ -53,10 +53,6 @@ data:
     {{- end }}
     {{- if .Values.jobTaskRunner.include }}
     jobTTL: {{ required "jobTTL is required" .Values.jobTaskRunner.jobTTL }}
-    jobTaskRunnerTemporarySetPodSeccompProfile: {{ .Values.jobTaskRunner.temporarySetPodSeccompProfile }}
-    {{- end }}
-    {{- if .Values.statefulsetRunner.include }}
-    statefulsetRunnerTemporarySetPodSeccompProfile: {{ .Values.statefulsetRunner.temporarySetPodSeccompProfile }}
     {{- end }}
     networking:
       gatewayNamespace: {{ .Release.Namespace }}-gateway

diff --git a/helm/korifi/values.schema.json b/helm/korifi/values.schema.json
@@ -469,10 +469,6 @@
           "description": "Number of replicas.",
           "type": "integer"
         },
-        "temporarySetPodSeccompProfile": {
-          "description": "Sets the pod .spec.securityContext.seccompProfile to RuntimeDefault. Setting this flag to true will cause a restart of all previously running pods.",
-          "type": "boolean"
-        },
         "resources": {
           "description": "[`ResourceRequirements`](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#resourcerequirements-v1-core) for the API.",
           "type": "object",
@@ -521,10 +517,6 @@
           "description": "Number of replicas.",
           "type": "integer"
         },
-        "temporarySetPodSeccompProfile": {
-          "description": "Sets the pod .spec.securityContext.seccompProfile to RuntimeDefault. Setting this flag to true will cause a restart of all previously running pods.",
-          "type": "boolean"
-        },
         "resources": {
           "description": "[`ResourceRequirements`](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#resourcerequirements-v1-core) for the API.",
           "type": "object",

diff --git a/helm/korifi/values.yaml b/helm/korifi/values.yaml
@@ -112,7 +112,6 @@ kpackImageBuilder:
 statefulsetRunner:
   include: true
   replicas: 1
-  temporarySetPodSeccompProfile: false
   resources:
     limits:
       cpu: 500m
@@ -124,7 +123,6 @@ statefulsetRunner:
 jobTaskRunner:
   include: true
   replicas: 1
-  temporarySetPodSeccompProfile: false
   resources:
     limits:
       cpu: 500m

diff --git a/job-task-runner/controllers/integration/suite_test.go b/job-task-runner/controllers/integration/suite_test.go
@@ -82,7 +82,6 @@ var _ = BeforeSuite(func() {
 		k8sManager.GetScheme(),
 		controllers.NewStatusGetter(k8sManager.GetClient()),
 		time.Minute,
-		false,
 	)
 	err = taskWorkloadReconciler.SetupWithManager(k8sManager)
 	Expect(err).NotTo(HaveOccurred())

diff --git a/job-task-runner/controllers/integration/taskworkload_controller_test.go b/job-task-runner/controllers/integration/taskworkload_controller_test.go
@@ -77,6 +77,9 @@ var _ = Describe("Job TaskWorkload Controller Integration Test", func() {
 		Expect(podSpec.RestartPolicy).To(Equal(corev1.RestartPolicyNever))
 		Expect(podSpec.SecurityContext).To(Equal(&corev1.PodSecurityContext{
 			RunAsNonRoot: tools.PtrTo(true),
+			SeccompProfile: &corev1.SeccompProfile{
+				Type: corev1.SeccompProfileTypeRuntimeDefault,
+			},
 		}))
 		Expect(podSpec.AutomountServiceAccountToken).To(Equal(tools.PtrTo(false)))
 		Expect(podSpec.ImagePullSecrets).To(ConsistOf(corev1.LocalObjectReference{Name: "my-image-secret"}))

diff --git a/job-task-runner/controllers/taskworkload_controller.go b/job-task-runner/controllers/taskworkload_controller.go
@@ -51,12 +51,11 @@ type TaskStatusGetter interface {
 
 // TaskWorkloadReconciler reconciles a TaskWorkload object
 type TaskWorkloadReconciler struct {
-	k8sClient                                  client.Client
-	logger                                     logr.Logger
-	scheme                                     *runtime.Scheme
-	statusGetter                               TaskStatusGetter
-	jobTTL                                     time.Duration
-	jobTaskRunnerTemporarySetPodSeccompProfile bool
+	k8sClient    client.Client
+	logger       logr.Logger
+	scheme       *runtime.Scheme
+	statusGetter TaskStatusGetter
+	jobTTL       time.Duration
 }
 
 func NewTaskWorkloadReconciler(
@@ -65,15 +64,13 @@ func NewTaskWorkloadReconciler(
 	scheme *runtime.Scheme,
 	statusGetter TaskStatusGetter,
 	jobTTL time.Duration,
-	jobTaskRunnerTemporarySetPodSeccompProfile bool,
 ) *k8s.PatchingReconciler[korifiv1alpha1.TaskWorkload, *korifiv1alpha1.TaskWorkload] {
 	taskReconciler := TaskWorkloadReconciler{
 		k8sClient:    k8sClient,
 		logger:       logger,
 		scheme:       scheme,
 		statusGetter: statusGetter,
 		jobTTL:       jobTTL,
-		jobTaskRunnerTemporarySetPodSeccompProfile: jobTaskRunnerTemporarySetPodSeccompProfile,
 	}
 
 	return k8s.NewPatchingReconciler[korifiv1alpha1.TaskWorkload, *korifiv1alpha1.TaskWorkload](logger, k8sClient, &taskReconciler)
@@ -135,9 +132,9 @@ func (r TaskWorkloadReconciler) getOrCreateJob(ctx context.Context, logger logr.
 }
 
 func (r TaskWorkloadReconciler) createJob(ctx context.Context, logger logr.Logger, taskWorkload *korifiv1alpha1.TaskWorkload) (*batchv1.Job, error) {
-	job := WorkloadToJob(taskWorkload, int32(r.jobTTL.Seconds()), r.jobTaskRunnerTemporarySetPodSeccompProfile)
-	err := controllerutil.SetControllerReference(taskWorkload, job, r.scheme)
+	job, err := r.workloadToJob(taskWorkload)
 	if err != nil {
+		logger.Info("failed to convert task workload to job", "reason", err)
 		return nil, err
 	}
 
@@ -154,11 +151,7 @@ func (r TaskWorkloadReconciler) createJob(ctx context.Context, logger logr.Logge
 	return job, nil
 }
 
-func WorkloadToJob(
-	taskWorkload *korifiv1alpha1.TaskWorkload,
-	jobTTL int32,
-	jobTaskRunnerTemporarySetPodSeccompProfile bool,
-) *batchv1.Job {
+func (r *TaskWorkloadReconciler) workloadToJob(taskWorkload *korifiv1alpha1.TaskWorkload) (*batchv1.Job, error) {
 	job := &batchv1.Job{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      taskWorkload.Name,
@@ -168,12 +161,15 @@ func WorkloadToJob(
 			BackoffLimit:            tools.PtrTo(int32(0)),
 			Parallelism:             tools.PtrTo(int32(1)),
 			Completions:             tools.PtrTo(int32(1)),
-			TTLSecondsAfterFinished: tools.PtrTo(jobTTL),
+			TTLSecondsAfterFinished: tools.PtrTo(int32(r.jobTTL.Seconds())),
 			Template: corev1.PodTemplateSpec{
 				Spec: corev1.PodSpec{
 					RestartPolicy: corev1.RestartPolicyNever,
 					SecurityContext: &corev1.PodSecurityContext{
 						RunAsNonRoot: tools.PtrTo(true),
+						SeccompProfile: &corev1.SeccompProfile{
+							Type: corev1.SeccompProfileTypeRuntimeDefault,
+						},
 					},
 					AutomountServiceAccountToken: tools.PtrTo(false),
 					ImagePullSecrets:             taskWorkload.Spec.ImagePullSecrets,
@@ -199,12 +195,12 @@ func WorkloadToJob(
 		},
 	}
 
-	if jobTaskRunnerTemporarySetPodSeccompProfile {
-		job.Spec.Template.Spec.SecurityContext.SeccompProfile = &corev1.SeccompProfile{
-			Type: corev1.SeccompProfileTypeRuntimeDefault,
-		}
+	err := controllerutil.SetControllerReference(taskWorkload, job, r.scheme)
+	if err != nil {
+		return nil, err
 	}
-	return job
+
+	return job, nil
 }
 
 func (r *TaskWorkloadReconciler) updateTaskWorkloadStatus(ctx context.Context, taskWorkload *korifiv1alpha1.TaskWorkload, job *batchv1.Job) error {

diff --git a/job-task-runner/controllers/taskworkload_controller_test.go b/job-task-runner/controllers/taskworkload_controller_test.go
@@ -8,10 +8,10 @@ import (
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
 	"code.cloudfoundry.org/korifi/job-task-runner/controllers"
 	"code.cloudfoundry.org/korifi/job-task-runner/controllers/fake"
+	"code.cloudfoundry.org/korifi/tools/k8s"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	batchv1 "k8s.io/api/batch/v1"
-	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -27,16 +27,16 @@ var _ = Describe("TaskworkloadController", func() {
 	var (
 		statusGetter *fake.TaskStatusGetter
 
-		reconcileResult                            ctrl.Result
-		reconcileErr                               error
-		req                                        ctrl.Request
-		taskWorkload                               *korifiv1alpha1.TaskWorkload
-		getTaskWorkloadError                       error
-		createdJob                                 *batchv1.Job
-		existingJob                                *batchv1.Job
-		getExistingJobError                        error
-		createJobError                             error
-		jobTaskRunnerTemporarySetPodSeccompProfile bool
+		reconciler           *k8s.PatchingReconciler[korifiv1alpha1.TaskWorkload, *korifiv1alpha1.TaskWorkload]
+		reconcileResult      ctrl.Result
+		reconcileErr         error
+		req                  ctrl.Request
+		taskWorkload         *korifiv1alpha1.TaskWorkload
+		getTaskWorkloadError error
+		createdJob           *batchv1.Job
+		existingJob          *batchv1.Job
+		getExistingJobError  error
+		createJobError       error
 	)
 
 	BeforeEach(func() {
@@ -98,13 +98,12 @@ var _ = Describe("TaskworkloadController", func() {
 			Reason:             "something",
 		}}, nil)
 
-		jobTaskRunnerTemporarySetPodSeccompProfile = false
+		reconciler = controllers.NewTaskWorkloadReconciler(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)), fakeClient, scheme.Scheme, statusGetter, time.Hour)
 
 		req = ctrl.Request{NamespacedName: client.ObjectKeyFromObject(taskWorkload)}
 	})
 
 	JustBeforeEach(func() {
-		reconciler := controllers.NewTaskWorkloadReconciler(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)), fakeClient, scheme.Scheme, statusGetter, time.Hour, jobTaskRunnerTemporarySetPodSeccompProfile)
 		reconcileResult, reconcileErr = reconciler.Reconcile(context.Background(), req)
 	})
 
@@ -225,33 +224,4 @@ var _ = Describe("TaskworkloadController", func() {
 			Expect(reconcileErr).To(MatchError(ContainSubstring("get-conditions-error")))
 		})
 	})
-
-	Describe("jobTaskRunnerTemporarySetPodSeccompProfile", func() {
-		var (
-			job                                        *batchv1.Job
-			jobTaskRunnerTemporarySetPodSeccompProfile bool
-		)
-
-		BeforeEach(func() {
-			jobTaskRunnerTemporarySetPodSeccompProfile = false
-		})
-
-		JustBeforeEach(func() {
-			job = controllers.WorkloadToJob(taskWorkload, 123, jobTaskRunnerTemporarySetPodSeccompProfile)
-		})
-
-		It("does not set spec.securityContext.seccompProfile", func() {
-			Expect(job.Spec.Template.Spec.SecurityContext.SeccompProfile).To(BeNil())
-		})
-
-		When("jobTaskRunnerTemporarySetPodSeccompProfile is set to true", func() {
-			BeforeEach(func() {
-				jobTaskRunnerTemporarySetPodSeccompProfile = true
-			})
-
-			It("sets spec.securityContext.seccompProfile to RuntimeDefault", func() {
-				Expect(job.Spec.Template.Spec.SecurityContext.SeccompProfile.Type).To(Equal(corev1.SeccompProfileTypeRuntimeDefault))
-			})
-		})
-	})
 })
diff --git a/kpack-image-builder/controllers/buildworkload_controller_test.go b/kpack-image-builder/controllers/buildworkload_controller_test.go
@@ -1153,9 +1153,7 @@ var _ = Describe("BuildWorkloadReconciler", func() {
 	})
 
 	When("failure during generateDropletStatus call", func() {
-		var (
-			build *buildv1alpha2.Build
-		)
+		var build *buildv1alpha2.Build
 		BeforeEach(func() {
 			fakeImageConfigGetter.ConfigReturns(image.Config{}, errors.New("fake error"))
 			buildWorkload = buildWorkloadObject(buildWorkloadGUID, namespaceGUID, source, env, services, reconcilerName, buildpacks)

diff --git a/statefulset-runner/controllers/appworkload_to_stset.go b/statefulset-runner/controllers/appworkload_to_stset.go
@@ -20,17 +20,12 @@ import (
 )
 
 type AppWorkloadToStatefulsetConverter struct {
-	scheme                                         *runtime.Scheme
-	statefulsetRunnerTemporarySetPodSeccompProfile bool
+	scheme *runtime.Scheme
 }
 
-func NewAppWorkloadToStatefulsetConverter(
-	scheme *runtime.Scheme,
-	statefulsetRunnerTemporarySetPodSeccompProfile bool,
-) *AppWorkloadToStatefulsetConverter {
+func NewAppWorkloadToStatefulsetConverter(scheme *runtime.Scheme) *AppWorkloadToStatefulsetConverter {
 	return &AppWorkloadToStatefulsetConverter{
 		scheme: scheme,
-		statefulsetRunnerTemporarySetPodSeccompProfile: statefulsetRunnerTemporarySetPodSeccompProfile,
 	}
 }
 
@@ -147,19 +142,16 @@ func (r *AppWorkloadToStatefulsetConverter) Convert(appWorkload *korifiv1alpha1.
 					ImagePullSecrets: appWorkload.Spec.ImagePullSecrets,
 					SecurityContext: &corev1.PodSecurityContext{
 						RunAsNonRoot: tools.PtrTo(true),
+						SeccompProfile: &corev1.SeccompProfile{
+							Type: corev1.SeccompProfileTypeRuntimeDefault,
+						},
 					},
 					ServiceAccountName: ServiceAccountName,
 				},
 			},
 		},
 	}
 
-	if r.statefulsetRunnerTemporarySetPodSeccompProfile {
-		statefulSet.Spec.Template.Spec.SecurityContext.SeccompProfile = &corev1.SeccompProfile{
-			Type: corev1.SeccompProfileTypeRuntimeDefault,
-		}
-	}
-
 	statefulSet.Spec.Template.Spec.AutomountServiceAccountToken = tools.PtrTo(false)
 	statefulSet.Spec.Selector = statefulSetLabelSelector(appWorkload)