Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove temporary helm values #3523

Merged
merged 1 commit into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions README.helm.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
6 changes: 1 addition & 5 deletions controllers/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
6 changes: 1 addition & 5 deletions controllers/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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 {
Expand Down
4 changes: 0 additions & 4 deletions helm/korifi/controllers/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions helm/korifi/values.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
2 changes: 0 additions & 2 deletions helm/korifi/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ kpackImageBuilder:
statefulsetRunner:
include: true
replicas: 1
temporarySetPodSeccompProfile: false
resources:
limits:
cpu: 500m
Expand All @@ -124,7 +123,6 @@ statefulsetRunner:
jobTaskRunner:
include: true
replicas: 1
temporarySetPodSeccompProfile: false
resources:
limits:
cpu: 500m
Expand Down
1 change: 0 additions & 1 deletion job-task-runner/controllers/integration/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ var _ = BeforeSuite(func() {
k8sManager.GetScheme(),
controllers.NewStatusGetter(k8sManager.GetClient()),
time.Minute,
false,
)
err = taskWorkloadReconciler.SetupWithManager(k8sManager)
Expect(err).NotTo(HaveOccurred())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"}))
Expand Down
38 changes: 17 additions & 21 deletions job-task-runner/controllers/taskworkload_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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)
Expand Down Expand Up @@ -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
}

Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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 {
Expand Down
54 changes: 12 additions & 42 deletions job-task-runner/controllers/taskworkload_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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() {
Expand Down Expand Up @@ -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)
})

Expand Down Expand Up @@ -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))
})
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 5 additions & 13 deletions statefulset-runner/controllers/appworkload_to_stset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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)

Expand Down
Loading
Loading