From 28367eb10ed67df890c0f31e32fbacdb9535d409 Mon Sep 17 00:00:00 2001 From: Jason Parraga Date: Thu, 18 Jul 2024 04:27:22 -0700 Subject: [PATCH] Avoid nil pointer when resources is nil (#302) Signed-off-by: Jason Parraga --- .../install/scheduler_controller.go | 2 +- .../install/scheduler_controller_test.go | 166 ++++++++++++++++++ 2 files changed, 167 insertions(+), 1 deletion(-) diff --git a/internal/controller/install/scheduler_controller.go b/internal/controller/install/scheduler_controller.go index 8efffea4..56c91fc9 100644 --- a/internal/controller/install/scheduler_controller.go +++ b/internal/controller/install/scheduler_controller.go @@ -358,8 +358,8 @@ func createSchedulerDeployment(scheduler *installv1alpha1.Scheduler) (*appsv1.De if scheduler.Spec.Resources != nil { deployment.Spec.Template.Spec.Containers[0].Resources = *scheduler.Spec.Resources + deployment.Spec.Template.Spec.Containers[0].Env = addGoMemLimit(deployment.Spec.Template.Spec.Containers[0].Env, *scheduler.Spec.Resources) } - deployment.Spec.Template.Spec.Containers[0].Env = addGoMemLimit(deployment.Spec.Template.Spec.Containers[0].Env, *scheduler.Spec.Resources) return &deployment, nil } diff --git a/internal/controller/install/scheduler_controller_test.go b/internal/controller/install/scheduler_controller_test.go index 0b9e1e28..d2079c74 100644 --- a/internal/controller/install/scheduler_controller_test.go +++ b/internal/controller/install/scheduler_controller_test.go @@ -289,6 +289,172 @@ func TestSchedulerReconciler_ReconcileErrorDueToApplicationConfig(t *testing.T) assert.Error(t, err) } +func TestSchedulerReconciler_ReconcileMissingResources(t *testing.T) { + t.Parallel() + + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + scheme, err := v1alpha1.SchemeBuilder.Build() + if err != nil { + t.Fatalf("should not return error when building schema") + } + + expectedNamespacedName := types.NamespacedName{Namespace: "default", Name: "scheduler"} + dbPruningEnabled := true + dbPruningSchedule := "1d" + terminationGracePeriod := int64(20) + expectedScheduler := v1alpha1.Scheduler{ + TypeMeta: metav1.TypeMeta{ + Kind: "Scheduler", + APIVersion: "install.armadaproject.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "scheduler"}, + Spec: v1alpha1.SchedulerSpec{ + Replicas: ptr.To[int32](2), + CommonSpecBase: installv1alpha1.CommonSpecBase{ + Labels: nil, + Image: v1alpha1.Image{ + Repository: "testrepo", + Tag: "1.0.0", + }, + ApplicationConfig: runtime.RawExtension{}, + Prometheus: &installv1alpha1.PrometheusConfig{Enabled: true, ScrapeInterval: &metav1.Duration{Duration: 1 * time.Second}}, + TerminationGracePeriodSeconds: &terminationGracePeriod, + }, + ClusterIssuer: "test", + HostNames: []string{"localhost"}, + Ingress: &installv1alpha1.IngressConfig{ + IngressClass: "nginx", + Labels: map[string]string{"test": "hello"}, + Annotations: map[string]string{"test": "hello"}, + }, + Pruner: &installv1alpha1.PrunerConfig{ + Enabled: dbPruningEnabled, + Schedule: dbPruningSchedule, + }, + }, + } + + scheduler, err := generateSchedulerInstallComponents(&expectedScheduler, scheme) + if err != nil { + t.Fatal("We should not fail on generating scheduler") + } + + mockK8sClient := k8sclient.NewMockClient(mockCtrl) + mockK8sClient. + EXPECT(). + Get(gomock.Any(), expectedNamespacedName, gomock.AssignableToTypeOf(&v1alpha1.Scheduler{})). + Return(nil). + SetArg(2, expectedScheduler) + + // Finalizer + mockK8sClient. + EXPECT(). + Update(gomock.Any(), gomock.AssignableToTypeOf(&installv1alpha1.Scheduler{})). + Return(nil) + + // ServiceAccount + mockK8sClient. + EXPECT(). + Get(gomock.Any(), expectedNamespacedName, gomock.AssignableToTypeOf(&corev1.ServiceAccount{})). + Return(errors.NewNotFound(schema.GroupResource{}, "scheduler")) + mockK8sClient. + EXPECT(). + Create(gomock.Any(), gomock.AssignableToTypeOf(&corev1.ServiceAccount{})). + Return(nil). + SetArg(1, *scheduler.ServiceAccount) + + mockK8sClient. + EXPECT(). + Get(gomock.Any(), expectedNamespacedName, gomock.AssignableToTypeOf(&corev1.Secret{})). + Return(errors.NewNotFound(schema.GroupResource{}, "scheduler")) + mockK8sClient. + EXPECT(). + Create(gomock.Any(), gomock.AssignableToTypeOf(&corev1.Secret{})). + Return(nil). + SetArg(1, *scheduler.Secret) + + expectedJobName := types.NamespacedName{Namespace: "default", Name: "scheduler-migration"} + scheduler.Jobs[0].Status = batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{{ + Type: batchv1.JobComplete, + Status: corev1.ConditionTrue, + }}, + } + mockK8sClient. + EXPECT(). + Get(gomock.Any(), expectedJobName, gomock.AssignableToTypeOf(&batchv1.Job{})). + Return(errors.NewNotFound(schema.GroupResource{}, "scheduler")) + mockK8sClient. + EXPECT(). + Create(gomock.Any(), gomock.AssignableToTypeOf(&batchv1.Job{})). + Return(nil). + SetArg(1, *scheduler.Jobs[0]) + mockK8sClient. + EXPECT(). + Get(gomock.Any(), expectedJobName, gomock.AssignableToTypeOf(&batchv1.Job{})). + Return(nil). + SetArg(2, *scheduler.Jobs[0]) + + mockK8sClient. + EXPECT(). + Get(gomock.Any(), expectedNamespacedName, gomock.AssignableToTypeOf(&appsv1.Deployment{})). + Return(errors.NewNotFound(schema.GroupResource{}, "scheduler")) + mockK8sClient. + EXPECT(). + Create(gomock.Any(), gomock.AssignableToTypeOf(&appsv1.Deployment{})). + Return(nil). + SetArg(1, *scheduler.Deployment) + + mockK8sClient. + EXPECT(). + Get(gomock.Any(), expectedNamespacedName, gomock.AssignableToTypeOf(&corev1.Service{})). + Return(errors.NewNotFound(schema.GroupResource{}, "scheduler")) + mockK8sClient. + EXPECT(). + Create(gomock.Any(), gomock.AssignableToTypeOf(&corev1.Service{})). + Return(nil). + SetArg(1, *scheduler.Service) + + // IngressGrpc + expectedIngressName := expectedNamespacedName + expectedIngressName.Name = expectedIngressName.Name + "-grpc" + mockK8sClient. + EXPECT(). + Get(gomock.Any(), expectedIngressName, gomock.AssignableToTypeOf(&networkingv1.Ingress{})). + Return(errors.NewNotFound(schema.GroupResource{}, "scheduler")) + mockK8sClient. + EXPECT(). + Create(gomock.Any(), gomock.AssignableToTypeOf(&networkingv1.Ingress{})). + Return(nil). + SetArg(1, *scheduler.IngressGrpc) + + // ServiceMonitor + mockK8sClient. + EXPECT(). + Get(gomock.Any(), expectedNamespacedName, gomock.AssignableToTypeOf(&monitoringv1.ServiceMonitor{})). + Return(errors.NewNotFound(schema.GroupResource{}, "scheduler")) + mockK8sClient. + EXPECT(). + Create(gomock.Any(), gomock.AssignableToTypeOf(&monitoringv1.ServiceMonitor{})). + Return(nil) + + r := SchedulerReconciler{ + Client: mockK8sClient, + Scheme: scheme, + } + + req := ctrl.Request{ + NamespacedName: types.NamespacedName{Namespace: "default", Name: "scheduler"}, + } + + _, err = r.Reconcile(context.Background(), req) + if err != nil { + t.Fatalf("reconcile should not return error") + } +} + func TestSchedulerReconciler_createSchedulerCronJob(t *testing.T) { t.Parallel()