simplify deletion logic (open-telemetry#2971)

ItielOlenick · Jun 6, 2024 · eaa4e63 · eaa4e63
1 parent 2a9db5f
commit eaa4e63
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 67 deletions.
diff --git a/controllers/common.go b/controllers/common.go
@@ -22,11 +22,13 @@ import (
 	"github.com/go-logr/logr"
 	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/retry"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
@@ -77,6 +79,25 @@ func BuildOpAMPBridge(params manifests.Params) ([]client.Object, error) {
 	return resources, nil
 }
 
+// getList queries the Kubernetes API to list the requested resource, setting the list l of type T.
+func getList[T client.Object](ctx context.Context, cl client.Client, l T, options ...client.ListOption) (map[types.UID]client.Object, error) {
+	ownedObjects := map[types.UID]client.Object{}
+	list := &unstructured.UnstructuredList{}
+	gvk, err := apiutil.GVKForObject(l, cl.Scheme())
+	if err != nil {
+		return nil, err
+	}
+	list.SetGroupVersionKind(gvk)
+	err = cl.List(ctx, list, options...)
+	if err != nil {
+		return ownedObjects, fmt.Errorf("error listing %T: %w", l, err)
+	}
+	for i := range list.Items {
+		ownedObjects[list.Items[i].GetUID()] = &list.Items[i]
+	}
+	return ownedObjects, nil
+}
+
 // reconcileDesiredObjects runs the reconcile process using the mutateFn over the given list of objects.
 func reconcileDesiredObjects(ctx context.Context, kubeClient client.Client, logger logr.Logger, owner metav1.Object, scheme *runtime.Scheme, desiredObjects []client.Object, ownedObjects map[types.UID]client.Object) error {
 	var errs []error

diff --git a/controllers/opentelemetrycollector_controller.go b/controllers/opentelemetrycollector_controller.go
@@ -17,7 +17,6 @@ package controllers
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/go-logr/logr"
 	routev1 "github.com/openshift/api/route/v1"
@@ -49,6 +48,13 @@ import (
 	"github.com/open-telemetry/opentelemetry-operator/pkg/featuregate"
 )
 
+var (
+	ownedClusterObjectTypes = []client.Object{
+		&rbacv1.ClusterRole{},
+		&rbacv1.ClusterRoleBinding{},
+	}
+)
+
 // OpenTelemetryCollectorReconciler reconciles a OpenTelemetryCollector object.
 type OpenTelemetryCollectorReconciler struct {
 	client.Client
@@ -69,72 +75,40 @@ type Params struct {
 
 func (r *OpenTelemetryCollectorReconciler) findOtelOwnedObjects(ctx context.Context, params manifests.Params) (map[types.UID]client.Object, error) {
 	ownedObjects := map[types.UID]client.Object{}
-
+	ownedObjectTypes := []client.Object{
+		&autoscalingv2.HorizontalPodAutoscaler{},
+		&networkingv1.Ingress{},
+		&policyV1.PodDisruptionBudget{},
+	}
 	listOps := &client.ListOptions{
 		Namespace:     params.OtelCol.Namespace,
 		LabelSelector: labels.SelectorFromSet(manifestutils.SelectorLabels(params.OtelCol.ObjectMeta, collector.ComponentOpenTelemetryCollector)),
 	}
-	hpaList := &autoscalingv2.HorizontalPodAutoscalerList{}
-	err := r.List(ctx, hpaList, listOps)
-	if err != nil {
-		return nil, fmt.Errorf("error listing HorizontalPodAutoscalers: %w", err)
-	}
-	for i := range hpaList.Items {
-		ownedObjects[hpaList.Items[i].GetUID()] = &hpaList.Items[i]
-	}
 	if featuregate.PrometheusOperatorIsAvailable.IsEnabled() && r.config.PrometheusCRAvailability() == prometheus.Available {
-		servicemonitorList := &monitoringv1.ServiceMonitorList{}
-		err = r.List(ctx, servicemonitorList, listOps)
-		if err != nil {
-			return nil, fmt.Errorf("error listing ServiceMonitors: %w", err)
-		}
-		for i := range servicemonitorList.Items {
-			ownedObjects[servicemonitorList.Items[i].GetUID()] = servicemonitorList.Items[i]
-		}
-
-		podMonitorList := &monitoringv1.PodMonitorList{}
-		err = r.List(ctx, podMonitorList, listOps)
-		if err != nil {
-			return nil, fmt.Errorf("error listing PodMonitors: %w", err)
-		}
-		for i := range podMonitorList.Items {
-			ownedObjects[podMonitorList.Items[i].GetUID()] = podMonitorList.Items[i]
-		}
-	}
-	ingressList := &networkingv1.IngressList{}
-	err = r.List(ctx, ingressList, listOps)
-	if err != nil {
-		return nil, fmt.Errorf("error listing Ingresses: %w", err)
+		ownedObjectTypes = append(ownedObjectTypes,
+			&monitoringv1.ServiceMonitor{},
+			&monitoringv1.PodMonitor{},
+		)
 	}
-	for i := range ingressList.Items {
-		ownedObjects[ingressList.Items[i].GetUID()] = &ingressList.Items[i]
-	}
-
 	if params.Config.OpenShiftRoutesAvailability() == openshift.RoutesAvailable {
-		routesList := &routev1.RouteList{}
-		err = r.List(ctx, routesList, listOps)
+		ownedObjectTypes = append(ownedObjectTypes, &routev1.Route{})
+	}
+	for _, objectType := range ownedObjectTypes {
+		objs, err := getList(ctx, r, objectType, listOps)
 		if err != nil {
-			return nil, fmt.Errorf("error listing Routes: %w", err)
+			return nil, err
 		}
-		for i := range routesList.Items {
-			ownedObjects[routesList.Items[i].GetUID()] = &routesList.Items[i]
+		for uid, object := range objs {
+			ownedObjects[uid] = object
 		}
 	}
-	pdbList := &policyV1.PodDisruptionBudgetList{}
-	err = r.List(ctx, pdbList, listOps)
-	if err != nil {
-		return nil, fmt.Errorf("error listing PodDisruptionBudgets: %w", err)
-	}
-	for i := range pdbList.Items {
-		ownedObjects[pdbList.Items[i].GetUID()] = &pdbList.Items[i]
-	}
 	if params.Config.CreateRBACPermissions() == rbac.Available {
-		clusterObjects, err := r.findClusterRoleObjects(ctx, params)
+		objs, err := r.findClusterRoleObjects(ctx, params)
 		if err != nil {
 			return nil, err
 		}
-		for k, v := range clusterObjects {
-			ownedObjects[k] = v
+		for uid, object := range objs {
+			ownedObjects[uid] = object
 		}
 	}
 	return ownedObjects, nil
@@ -148,21 +122,14 @@ func (r *OpenTelemetryCollectorReconciler) findClusterRoleObjects(ctx context.Co
 	listOpsCluster := &client.ListOptions{
 		LabelSelector: labels.SelectorFromSet(manifestutils.SelectorLabels(params.OtelCol.ObjectMeta, collector.ComponentOpenTelemetryCollector)),
 	}
-	clusterroleList := &rbacv1.ClusterRoleList{}
-	err := r.List(ctx, clusterroleList, listOpsCluster)
-	if err != nil {
-		return nil, fmt.Errorf("error listing ClusterRoles: %w", err)
-	}
-	for i := range clusterroleList.Items {
-		ownedObjects[clusterroleList.Items[i].GetUID()] = &clusterroleList.Items[i]
-	}
-	clusterrolebindingList := &rbacv1.ClusterRoleBindingList{}
-	err = r.List(ctx, clusterrolebindingList, listOpsCluster)
-	if err != nil {
-		return nil, fmt.Errorf("error listing ClusterRoleBIndings: %w", err)
-	}
-	for i := range clusterrolebindingList.Items {
-		ownedObjects[clusterrolebindingList.Items[i].GetUID()] = &clusterrolebindingList.Items[i]
+	for _, objectType := range ownedClusterObjectTypes {
+		objs, err := getList(ctx, r, objectType, listOpsCluster)
+		if err != nil {
+			return nil, err
+		}
+		for uid, object := range objs {
+			ownedObjects[uid] = object
+		}
 	}
 	return ownedObjects, nil
 }