Skip to content

Commit

Permalink
fix: do not trigger reconcile on DSC if no instance in the cluster
Browse files Browse the repository at this point in the history
- uninstall is handled by UninstallconfigMapPredicates:when no DSC but CM for uninstall is created or updated
- remove check on zeor DSC instance to not continue recontile with dummy name

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
  • Loading branch information
zdtsw committed Oct 31, 2024
1 parent 8ea9ea0 commit 42914a4
Showing 1 changed file with 16 additions and 18 deletions.
34 changes: 16 additions & 18 deletions controllers/datasciencecluster/datasciencecluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,19 @@ var configMapPredicates = predicate.Funcs{
},
}

var UninstallconfigMapPredicates = predicate.Funcs{
UpdateFunc: func(e event.UpdateEvent) bool {
// Trigger reconcile function for updated CM in operator namespace
operatorNs, _ := cluster.GetOperatorNamespace()
return e.ObjectNew.GetNamespace() == operatorNs
},
CreateFunc: func(e event.CreateEvent) bool {
// Trigger reconcile function for created CM in operator namespace
operatorNs, _ := cluster.GetOperatorNamespace()
return e.Object.GetNamespace() == operatorNs
},

Check warning on line 401 in controllers/datasciencecluster/datasciencecluster_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/datasciencecluster/datasciencecluster_controller.go#L392-L401

Added lines #L392 - L401 were not covered by tests
}

// reduce unnecessary reconcile triggered by odh component's deployment change due to ManagedByODHOperator annotation.
var componentDeploymentPredicates = predicate.Funcs{
UpdateFunc: func(e event.UpdateEvent) bool {
Expand Down Expand Up @@ -519,10 +532,8 @@ func (r *DataScienceClusterReconciler) SetupWithManager(ctx context.Context, mgr
)).
Watches(
&corev1.ConfigMap{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []reconcile.Request {
return r.watchDataScienceClusterResources(ctx, a)
}),
builder.WithPredicates(configMapPredicates),
&handler.EnqueueRequestForObject{},
builder.WithPredicates(UninstallconfigMapPredicates),
).
Watches(
&apiextensionsv1.CustomResourceDefinition{},
Expand Down Expand Up @@ -568,19 +579,6 @@ func (r *DataScienceClusterReconciler) watchDataScienceClusterResources(ctx cont
}}
}

// Trigger reconcile function when uninstall configmap is created
operatorNs, err := cluster.GetOperatorNamespace()
if err != nil {
return nil
}
if a.GetNamespace() == operatorNs {
cmLabels := a.GetLabels()
if val, ok := cmLabels[upgrade.DeleteConfigMapLabel]; ok && val == "true" {
return []reconcile.Request{{
NamespacedName: types.NamespacedName{Name: requestName},
}}
}
}
return nil
}

Expand All @@ -595,7 +593,7 @@ func (r *DataScienceClusterReconciler) getRequestName(ctx context.Context) (stri
case len(instanceList.Items) == 1:
return instanceList.Items[0].Name, nil
case len(instanceList.Items) == 0:
return "default-dsc", nil
return "", errors.New("none DataScienceCluster instance found")

Check warning on line 596 in controllers/datasciencecluster/datasciencecluster_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/datasciencecluster/datasciencecluster_controller.go#L596

Added line #L596 was not covered by tests
default:
return "", errors.New("multiple DataScienceCluster instances found")
}
Expand Down

0 comments on commit 42914a4

Please sign in to comment.