Skip to content

Commit

Permalink
Update after code review
Browse files Browse the repository at this point in the history
  • Loading branch information
mszadkow committed Oct 24, 2024
1 parent eb0238c commit 0618ad7
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 3 deletions.
4 changes: 2 additions & 2 deletions pkg/controller/jobs/mpijob/mpijob_multikueue_adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ func (b *multikueueAdapter) KeepAdmissionCheckPending() bool {
return false
}

func (b *multikueueAdapter) IsJobManagedByKueue(ctx context.Context, remoteClient client.Client, key types.NamespacedName) (bool, string, error) {
func (b *multikueueAdapter) IsJobManagedByKueue(ctx context.Context, c client.Client, key types.NamespacedName) (bool, string, error) {
job := kfmpi.MPIJob{}
err := remoteClient.Get(ctx, key, &job)
err := c.Get(ctx, key, &job)
if err != nil {
return false, "", err
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/controller/jobs/mpijob/mpijob_multikueue_adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,14 @@ func TestMultikueueAdapter(t *testing.T) {
Obj(),
},
},
"missing job is not considered managed": {
operation: func(ctx context.Context, adapter *multikueueAdapter, managerClient, workerClient client.Client) error {
if isManged, _, _ := adapter.IsJobManagedByKueue(ctx, managerClient, types.NamespacedName{Name: "mpijob1", Namespace: TestNamespace}); isManged {
return errors.New("expecting false")
}
return nil
},
},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/jobs/mpijob/mpijob_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,6 @@ func (w *MpiJobWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runti
}

// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type
func (w *MpiJobWebhook) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
func (w *MpiJobWebhook) ValidateDelete(context.Context, runtime.Object) (admission.Warnings, error) {
return nil, nil
}

0 comments on commit 0618ad7

Please sign in to comment.