🐛 fix: considers objects in kube-system for cert-manager to avoid upgrading twice (#11351)

* fix: considers objects in kube-system for cert-manager to avoid upgrading twice

* fix: removes use of slices.DeleteFunc because it zeros the elements and doesn't remove those from obj list

* fix: do not reassign slice as it is used in different methods

Author: faiq

cmd/clusterctl/client/cluster/cert_manager.go

@@ -25,6 +25,7 @@ import (
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -53,6 +54,10 @@ const (
 var (
 	//go:embed assets/cert-manager-test-resources.yaml
 	certManagerTestManifest []byte
+	// namespaces for all relevant objects in a cert-manager installation.
+	// It also includes relevant resources in the kube-system namespace, which is used by cert-manager
+	// for leader election (https://github.com/cert-manager/cert-manager/issues/6716).
+	certManagerNamespaces = []string{certManagerNamespace, metav1.NamespaceSystem}
 )
 
 // CertManagerUpgradePlan defines the upgrade plan if cert-manager needs to be
@@ -202,9 +207,9 @@ func (cm *certManagerClient) install(ctx context.Context, version string, objs [
 func (cm *certManagerClient) PlanUpgrade(ctx context.Context) (CertManagerUpgradePlan, error) {
 	log := logf.Log
 
-	objs, err := cm.proxy.ListResources(ctx, map[string]string{clusterctlv1.ClusterctlCoreLabel: clusterctlv1.ClusterctlCoreLabelCertManagerValue}, certManagerNamespace)
+	objs, err := cm.proxy.ListResources(ctx, map[string]string{clusterctlv1.ClusterctlCoreLabel: clusterctlv1.ClusterctlCoreLabelCertManagerValue}, certManagerNamespaces...)
 	if err != nil {
-		return CertManagerUpgradePlan{}, errors.Wrap(err, "failed get cert manager components")
+		return CertManagerUpgradePlan{}, errors.Wrap(err, "failed to get cert-manager components")
 	}
 
 	// If there are no cert manager components with the clusterctl labels, it means that cert-manager is externally managed.
@@ -240,12 +245,10 @@ func (cm *certManagerClient) PlanUpgrade(ctx context.Context) (CertManagerUpgrad
 // older than the version currently suggested by clusterctl, upgrades it.
 func (cm *certManagerClient) EnsureLatestVersion(ctx context.Context) error {
 	log := logf.Log
-
-	objs, err := cm.proxy.ListResources(ctx, map[string]string{clusterctlv1.ClusterctlCoreLabel: clusterctlv1.ClusterctlCoreLabelCertManagerValue}, certManagerNamespace)
+	objs, err := cm.proxy.ListResources(ctx, map[string]string{clusterctlv1.ClusterctlCoreLabel: clusterctlv1.ClusterctlCoreLabelCertManagerValue}, certManagerNamespaces...)
 	if err != nil {
-		return errors.Wrap(err, "failed get cert manager components")
+		return errors.Wrap(err, "failed to get cert-manager components")
 	}
-
 	// If there are no cert manager components with the clusterctl labels, it means that cert-manager is externally managed.
 	if len(objs) == 0 {
 		log.V(5).Info("Skipping cert-manager upgrade because externally managed")
@@ -338,13 +341,19 @@ func (cm *certManagerClient) shouldUpgrade(desiredVersion string, objs, installO
 
 	needUpgrade := false
 	currentVersion := ""
-	for i := range objs {
-		obj := objs[i]
 
-		// Endpoints and EndpointSlices are generated by Kubernetes without the version annotation, so we are skipping them
-		if obj.GetKind() == "Endpoints" || obj.GetKind() == "EndpointSlice" {
-			continue
+	// creates a new list removing resources that are generated by the kubernetes API
+	// this is relevant if the versions are the same, because we compare
+	// the number of objects when version of objects are equal
+	relevantObjs := []unstructured.Unstructured{}
+	for _, o := range objs {
+		if !(o.GetKind() == "Endpoints" || o.GetKind() == "EndpointSlice") {
+			relevantObjs = append(relevantObjs, o)
 		}
+	}
+
+	for i := range relevantObjs {
+		obj := relevantObjs[i]
 
 		// if there is no version annotation, this means the obj is cert-manager v0.11.0 (installed with older version of clusterctl)
 		objVersion, ok := obj.GetAnnotations()[clusterctlv1.CertManagerVersionAnnotation]
@@ -374,7 +383,7 @@ func (cm *certManagerClient) shouldUpgrade(desiredVersion string, objs, installO
 			// The installed version is equal to the desired version. Upgrade is required only if the number
 			// of available objects and objects to install differ. This would act as a re-install.
 			currentVersion = objVersion
-			needUpgrade = len(objs) != len(installObjs)
+			needUpgrade = len(relevantObjs) != len(installObjs)
 		case c > 0:
 			// The installed version is greater than the desired version. Upgrade is not required.
 			currentVersion = objVersion