Skip to content

Commit 28367eb

Browse files
authored
Avoid nil pointer when resources is nil (#302)
Signed-off-by: Jason Parraga <sovietaced@gmail.com>
1 parent afc6591 commit 28367eb

File tree

2 files changed

+167
-1
lines changed

2 files changed

+167
-1
lines changed

internal/controller/install/scheduler_controller.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,8 @@ func createSchedulerDeployment(scheduler *installv1alpha1.Scheduler) (*appsv1.De
358358

359359
if scheduler.Spec.Resources != nil {
360360
deployment.Spec.Template.Spec.Containers[0].Resources = *scheduler.Spec.Resources
361+
deployment.Spec.Template.Spec.Containers[0].Env = addGoMemLimit(deployment.Spec.Template.Spec.Containers[0].Env, *scheduler.Spec.Resources)
361362
}
362-
deployment.Spec.Template.Spec.Containers[0].Env = addGoMemLimit(deployment.Spec.Template.Spec.Containers[0].Env, *scheduler.Spec.Resources)
363363

364364
return &deployment, nil
365365
}

internal/controller/install/scheduler_controller_test.go

+166
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,172 @@ func TestSchedulerReconciler_ReconcileErrorDueToApplicationConfig(t *testing.T)
289289
assert.Error(t, err)
290290
}
291291

292+
func TestSchedulerReconciler_ReconcileMissingResources(t *testing.T) {
293+
t.Parallel()
294+
295+
mockCtrl := gomock.NewController(t)
296+
defer mockCtrl.Finish()
297+
298+
scheme, err := v1alpha1.SchemeBuilder.Build()
299+
if err != nil {
300+
t.Fatalf("should not return error when building schema")
301+
}
302+
303+
expectedNamespacedName := types.NamespacedName{Namespace: "default", Name: "scheduler"}
304+
dbPruningEnabled := true
305+
dbPruningSchedule := "1d"
306+
terminationGracePeriod := int64(20)
307+
expectedScheduler := v1alpha1.Scheduler{
308+
TypeMeta: metav1.TypeMeta{
309+
Kind: "Scheduler",
310+
APIVersion: "install.armadaproject.io/v1alpha1",
311+
},
312+
ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "scheduler"},
313+
Spec: v1alpha1.SchedulerSpec{
314+
Replicas: ptr.To[int32](2),
315+
CommonSpecBase: installv1alpha1.CommonSpecBase{
316+
Labels: nil,
317+
Image: v1alpha1.Image{
318+
Repository: "testrepo",
319+
Tag: "1.0.0",
320+
},
321+
ApplicationConfig: runtime.RawExtension{},
322+
Prometheus: &installv1alpha1.PrometheusConfig{Enabled: true, ScrapeInterval: &metav1.Duration{Duration: 1 * time.Second}},
323+
TerminationGracePeriodSeconds: &terminationGracePeriod,
324+
},
325+
ClusterIssuer: "test",
326+
HostNames: []string{"localhost"},
327+
Ingress: &installv1alpha1.IngressConfig{
328+
IngressClass: "nginx",
329+
Labels: map[string]string{"test": "hello"},
330+
Annotations: map[string]string{"test": "hello"},
331+
},
332+
Pruner: &installv1alpha1.PrunerConfig{
333+
Enabled: dbPruningEnabled,
334+
Schedule: dbPruningSchedule,
335+
},
336+
},
337+
}
338+
339+
scheduler, err := generateSchedulerInstallComponents(&expectedScheduler, scheme)
340+
if err != nil {
341+
t.Fatal("We should not fail on generating scheduler")
342+
}
343+
344+
mockK8sClient := k8sclient.NewMockClient(mockCtrl)
345+
mockK8sClient.
346+
EXPECT().
347+
Get(gomock.Any(), expectedNamespacedName, gomock.AssignableToTypeOf(&v1alpha1.Scheduler{})).
348+
Return(nil).
349+
SetArg(2, expectedScheduler)
350+
351+
// Finalizer
352+
mockK8sClient.
353+
EXPECT().
354+
Update(gomock.Any(), gomock.AssignableToTypeOf(&installv1alpha1.Scheduler{})).
355+
Return(nil)
356+
357+
// ServiceAccount
358+
mockK8sClient.
359+
EXPECT().
360+
Get(gomock.Any(), expectedNamespacedName, gomock.AssignableToTypeOf(&corev1.ServiceAccount{})).
361+
Return(errors.NewNotFound(schema.GroupResource{}, "scheduler"))
362+
mockK8sClient.
363+
EXPECT().
364+
Create(gomock.Any(), gomock.AssignableToTypeOf(&corev1.ServiceAccount{})).
365+
Return(nil).
366+
SetArg(1, *scheduler.ServiceAccount)
367+
368+
mockK8sClient.
369+
EXPECT().
370+
Get(gomock.Any(), expectedNamespacedName, gomock.AssignableToTypeOf(&corev1.Secret{})).
371+
Return(errors.NewNotFound(schema.GroupResource{}, "scheduler"))
372+
mockK8sClient.
373+
EXPECT().
374+
Create(gomock.Any(), gomock.AssignableToTypeOf(&corev1.Secret{})).
375+
Return(nil).
376+
SetArg(1, *scheduler.Secret)
377+
378+
expectedJobName := types.NamespacedName{Namespace: "default", Name: "scheduler-migration"}
379+
scheduler.Jobs[0].Status = batchv1.JobStatus{
380+
Conditions: []batchv1.JobCondition{{
381+
Type: batchv1.JobComplete,
382+
Status: corev1.ConditionTrue,
383+
}},
384+
}
385+
mockK8sClient.
386+
EXPECT().
387+
Get(gomock.Any(), expectedJobName, gomock.AssignableToTypeOf(&batchv1.Job{})).
388+
Return(errors.NewNotFound(schema.GroupResource{}, "scheduler"))
389+
mockK8sClient.
390+
EXPECT().
391+
Create(gomock.Any(), gomock.AssignableToTypeOf(&batchv1.Job{})).
392+
Return(nil).
393+
SetArg(1, *scheduler.Jobs[0])
394+
mockK8sClient.
395+
EXPECT().
396+
Get(gomock.Any(), expectedJobName, gomock.AssignableToTypeOf(&batchv1.Job{})).
397+
Return(nil).
398+
SetArg(2, *scheduler.Jobs[0])
399+
400+
mockK8sClient.
401+
EXPECT().
402+
Get(gomock.Any(), expectedNamespacedName, gomock.AssignableToTypeOf(&appsv1.Deployment{})).
403+
Return(errors.NewNotFound(schema.GroupResource{}, "scheduler"))
404+
mockK8sClient.
405+
EXPECT().
406+
Create(gomock.Any(), gomock.AssignableToTypeOf(&appsv1.Deployment{})).
407+
Return(nil).
408+
SetArg(1, *scheduler.Deployment)
409+
410+
mockK8sClient.
411+
EXPECT().
412+
Get(gomock.Any(), expectedNamespacedName, gomock.AssignableToTypeOf(&corev1.Service{})).
413+
Return(errors.NewNotFound(schema.GroupResource{}, "scheduler"))
414+
mockK8sClient.
415+
EXPECT().
416+
Create(gomock.Any(), gomock.AssignableToTypeOf(&corev1.Service{})).
417+
Return(nil).
418+
SetArg(1, *scheduler.Service)
419+
420+
// IngressGrpc
421+
expectedIngressName := expectedNamespacedName
422+
expectedIngressName.Name = expectedIngressName.Name + "-grpc"
423+
mockK8sClient.
424+
EXPECT().
425+
Get(gomock.Any(), expectedIngressName, gomock.AssignableToTypeOf(&networkingv1.Ingress{})).
426+
Return(errors.NewNotFound(schema.GroupResource{}, "scheduler"))
427+
mockK8sClient.
428+
EXPECT().
429+
Create(gomock.Any(), gomock.AssignableToTypeOf(&networkingv1.Ingress{})).
430+
Return(nil).
431+
SetArg(1, *scheduler.IngressGrpc)
432+
433+
// ServiceMonitor
434+
mockK8sClient.
435+
EXPECT().
436+
Get(gomock.Any(), expectedNamespacedName, gomock.AssignableToTypeOf(&monitoringv1.ServiceMonitor{})).
437+
Return(errors.NewNotFound(schema.GroupResource{}, "scheduler"))
438+
mockK8sClient.
439+
EXPECT().
440+
Create(gomock.Any(), gomock.AssignableToTypeOf(&monitoringv1.ServiceMonitor{})).
441+
Return(nil)
442+
443+
r := SchedulerReconciler{
444+
Client: mockK8sClient,
445+
Scheme: scheme,
446+
}
447+
448+
req := ctrl.Request{
449+
NamespacedName: types.NamespacedName{Namespace: "default", Name: "scheduler"},
450+
}
451+
452+
_, err = r.Reconcile(context.Background(), req)
453+
if err != nil {
454+
t.Fatalf("reconcile should not return error")
455+
}
456+
}
457+
292458
func TestSchedulerReconciler_createSchedulerCronJob(t *testing.T) {
293459
t.Parallel()
294460

0 commit comments

Comments
 (0)