Skip to content

Commit ed7d2a1

Browse files
committed
Misc NIMPipeline fixes
* Error out if there is a conflict with the NIM services in the pipeline vs pre-existing service * Clenup NIM service if the spec is removed from the pipeline Signed-off-by: Shiva Krishna, Merla <smerla@nvidia.com>
1 parent 0f23095 commit ed7d2a1

File tree

1 file changed

+21
-23
lines changed

1 file changed

+21
-23
lines changed

internal/controller/nimpipeline_controller.go

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ func (r *NIMPipelineReconciler) reconcileNIMService(ctx context.Context, nimPipe
193193

194194
// Sync NIMService with the desired spec
195195
namespacedName := types.NamespacedName{Name: nimService.Name, Namespace: nimService.Namespace}
196-
err := r.syncResource(ctx, namespacedName, nimService)
196+
err := r.syncResource(ctx, namespacedName, nimPipeline, nimService)
197197
if err != nil {
198198
logger.Error(err, "Failed to sync NIMService", "name", nimService.Name)
199199
return err
@@ -202,7 +202,7 @@ func (r *NIMPipelineReconciler) reconcileNIMService(ctx context.Context, nimPipe
202202
return nil
203203
}
204204

205-
func (r *NIMPipelineReconciler) syncResource(ctx context.Context, currentNamespacedName types.NamespacedName, desired *appsv1alpha1.NIMService) error {
205+
func (r *NIMPipelineReconciler) syncResource(ctx context.Context, currentNamespacedName types.NamespacedName, nimPipeline *appsv1alpha1.NIMPipeline, desired *appsv1alpha1.NIMService) error {
206206
logger := log.FromContext(ctx)
207207

208208
current := &appsv1alpha1.NIMService{}
@@ -224,7 +224,11 @@ func (r *NIMPipelineReconciler) syncResource(ctx context.Context, currentNamespa
224224
return err
225225
}
226226
} else {
227-
// Resource exists, so update it
227+
// Resource exists, so update it only if the current nimservice is owned by the pipeline
228+
if !r.isOwnedByNIMPipeline(current, nimPipeline) {
229+
return fmt.Errorf("NIMservice %s already exists and is not owned by the NIMPipeline %s", current.Name, nimPipeline.Name)
230+
}
231+
228232
// Ensure the resource version is carried over to the desired object
229233
desired.ResourceVersion = current.ResourceVersion
230234

@@ -248,21 +252,13 @@ func (r *NIMPipelineReconciler) cleanupDisabledNIMs(ctx context.Context, nimPipe
248252
var allErrors []error
249253

250254
for _, svc := range serviceList.Items {
251-
owned := false
252-
for _, ownerRef := range svc.GetOwnerReferences() {
253-
if ownerRef.Kind == "NIMPipeline" && ownerRef.UID == nimPipeline.UID {
254-
owned = true
255-
break
256-
}
257-
}
258-
259255
// Ignore NIM services not owned by the NIM pipeline
260-
if !owned {
256+
if !r.isOwnedByNIMPipeline(&svc, nimPipeline) {
261257
continue
262258
}
263259

264-
// Cleanup any stale NIM services if they are part of the pipeline but are disabled
265-
if enabled, exists := enabledServices[svc.Name]; exists && !enabled {
260+
// Cleanup any stale NIM services if they were previously part of the pipeline but are removed/disabled
261+
if enabled, exists := enabledServices[svc.Name]; !exists || !enabled {
266262
if err := r.deleteService(ctx, &svc); err != nil {
267263
logger.Error(err, "Unable to delete disabled NIM service", "Name", svc.Name)
268264
allErrors = append(allErrors, fmt.Errorf("failed to delete service %s: %w", svc.Name, err))
@@ -297,15 +293,7 @@ func (r *NIMPipelineReconciler) updateStatus(ctx context.Context, nimPipeline *a
297293
foundServices := make(map[string]bool)
298294

299295
for _, svc := range serviceList.Items {
300-
owned := false
301-
for _, ownerRef := range svc.GetOwnerReferences() {
302-
if ownerRef.Kind == "NIMPipeline" && ownerRef.UID == nimPipeline.UID {
303-
owned = true
304-
break
305-
}
306-
}
307-
308-
if enabled, exists := enabledServices[svc.Name]; !owned || !exists || !enabled {
296+
if enabled, exists := enabledServices[svc.Name]; !exists || !enabled || !r.isOwnedByNIMPipeline(&svc, nimPipeline) {
309297
continue
310298
}
311299

@@ -409,3 +397,13 @@ func (r *NIMPipelineReconciler) refreshMetrics(ctx context.Context) {
409397
}
410398
refreshNIMPipelineMetrics(nimPipelineList)
411399
}
400+
401+
// isOwnedByNIMPipeline checks if the given NIMService is owned by the specified NIMPipeline.
402+
func (r *NIMPipelineReconciler) isOwnedByNIMPipeline(svc *appsv1alpha1.NIMService, pipeline *appsv1alpha1.NIMPipeline) bool {
403+
for _, ownerRef := range svc.GetOwnerReferences() {
404+
if ownerRef.Kind == "NIMPipeline" && ownerRef.UID == pipeline.UID {
405+
return true
406+
}
407+
}
408+
return false
409+
}

0 commit comments

Comments
 (0)