Skip to content

Commit

Permalink
Merge pull request #11455 from k8s-infra-cherrypick-robot/cherry-pick…
Browse files Browse the repository at this point in the history
…-11351-to-release-1.7

[release-1.7] 🐛 fix: considers objects in kube-system for cert-manager to avoid upgrading twice
  • Loading branch information
k8s-ci-robot authored Nov 21, 2024
2 parents a6242ba + 176b2e7 commit 4a54410
Showing 1 changed file with 21 additions and 12 deletions.
33 changes: 21 additions & 12 deletions cmd/clusterctl/client/cluster/cert_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 4a54410

Please sign in to comment.