improved certificate condition checking and error logging

Signed-off-by: Inteon <42113979+inteon@users.noreply.github.com>
2026-08-09 05:06:38 +10:00 · 2021-08-07 15:42:30 +00:00
parent 34cb511980
commit 1e24c12ead
3 changed files with 112 additions and 73 deletions
@@ -20,6 +20,7 @@ go_library(
        "//pkg/apis/certmanager/v1:go_default_library",
        "//pkg/apis/meta/v1:go_default_library",
        "//pkg/client/clientset/versioned:go_default_library",
+        "//pkg/client/clientset/versioned/typed/certmanager/v1:go_default_library",
        "//pkg/controller/certificatesigningrequests/util:go_default_library",
        "//pkg/util:go_default_library",
        "//pkg/util/pki:go_default_library",
@@ -28,7 +29,6 @@ go_library(
        "//test/e2e/framework/helper/validation/certificates:go_default_library",
        "//test/e2e/framework/helper/validation/certificatesigningrequests:go_default_library",
        "//test/e2e/framework/log:go_default_library",
-        "//test/e2e/util:go_default_library",
        "@com_github_onsi_ginkgo//:go_default_library",
        "@io_k8s_api//certificates/v1:go_default_library",
        "@io_k8s_api//core/v1:go_default_library",
@@ -20,83 +20,151 @@ import (
 	"context"
 	"crypto/x509"
 	"fmt"
+	"os"
 	"sort"
 	"time"

 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/wait"

 	apiutil "github.com/jetstack/cert-manager/pkg/api/util"
 	cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
+	v1 "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
 	cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1"
+	clientset "github.com/jetstack/cert-manager/pkg/client/clientset/versioned/typed/certmanager/v1"
 	"github.com/jetstack/cert-manager/test/e2e/framework/log"
-	e2eutil "github.com/jetstack/cert-manager/test/e2e/util"
 )

-func (h *Helper) handleResult(ns, name string, cert *cmapi.Certificate, state string, err error) (*cmapi.Certificate, error) {
-	if err != nil {
-		log.Logf("Error waiting for Certificate to become %s: %v", state, err)
-		h.Kubectl(ns).DescribeResource("certificate", name)
-		h.Kubectl(ns).Describe("order", "challenge")
-		h.describeCertificateRequestFromCertificate(ns, cert)
-	}
-	return cert, err
-}
+func (h *Helper) waitPollImmediateCertificate(client clientset.CertificateInterface, name string, check func(*v1.Certificate) bool, interval time.Duration, timeout time.Duration) (*cmapi.Certificate, error) {
+	var certificate *v1.Certificate = nil
+	pollErr := wait.PollImmediate(interval, timeout, func() (bool, error) {
+		var err error
+		certificate, err = client.Get(context.TODO(), name, metav1.GetOptions{})
+		if nil != err {
+			certificate = nil
+			return false, fmt.Errorf("error getting Certificate %v: %v", name, err)
+		}

-// waitForCertificateNotIssuing waits for the certificate resource to leave the Issuing state.
-func (h *Helper) waitForCertificateNotIssuing(ns, name string, timeout time.Duration) (*cmapi.Certificate, error) {
-	result, err := e2eutil.WaitForMissingCertificateCondition(h.CMClient.CertmanagerV1().Certificates(ns), name, cmapi.CertificateCondition{
-		Type:   cmapi.CertificateConditionIssuing,
-		Status: cmmeta.ConditionTrue,
-	}, timeout)
-	return h.handleResult(ns, name, result, "Not Issuing", err)
+		return check(certificate), nil
+	})
+
+	if pollErr != nil && certificate != nil {
+		fmt.Fprintf(os.Stderr, "Failed waiting for certificate %v: %v\n", name, pollErr.Error())
+
+		if len(certificate.Status.Conditions) > 0 {
+			fmt.Fprintf(os.Stderr, "Perceived certificate conditions:\n")
+			for _, cond := range certificate.Status.Conditions {
+				fmt.Fprintf(os.Stderr, "- Last Status: '%s' Reason: '%s', Message: '%s'\n", cond.Status, cond.Reason, cond.Message)
+			}
+		}
+
+		fmt.Fprintf(os.Stderr, "Certificate description:\n")
+		h.Kubectl(certificate.Namespace).DescribeResource("certificate", name)
+		fmt.Fprintf(os.Stderr, "Order and challenge descriptions:\n")
+		h.Kubectl(certificate.Namespace).Describe("order", "challenge")
+
+		fmt.Fprintf(os.Stderr, "Certificaterequest description:\n")
+		crName, err := apiutil.ComputeName(certificate.Name, certificate.Spec)
+		if err != nil {
+			log.Logf("Failed to compute CertificateRequest name from certificate: %s", err)
+		} else {
+			h.Kubectl(certificate.Namespace).DescribeResource("certificaterequest", crName)
+		}
+	}
+	return certificate, pollErr
 }

 // WaitForCertificateReady waits for the certificate resource to enter a Ready state and to leave the Issuing state.
 func (h *Helper) WaitForCertificateReady(ns, name string, timeout time.Duration) (*cmapi.Certificate, error) {
-	result, err := e2eutil.WaitForCertificateCondition(h.CMClient.CertmanagerV1().Certificates(ns), name, cmapi.CertificateCondition{
+	ready_true_condition := cmapi.CertificateCondition{
 		Type:   cmapi.CertificateConditionReady,
 		Status: cmmeta.ConditionTrue,
-	}, timeout)
-	if err != nil {
-		return h.handleResult(ns, name, result, "Ready", err)
 	}
-	// Making sure that the Certificate is stable (see #4239) by also waiting for the Issuing state to disappear.
-	// A certificate that has state Ready=True and Issuing not set, is stable and will not change without outside changes.
-	return h.waitForCertificateNotIssuing(ns, name, timeout)
+	issuing_condition := cmapi.CertificateCondition{
+		Type: cmapi.CertificateConditionIssuing,
+	}
+
+	return h.waitPollImmediateCertificate(h.CMClient.CertmanagerV1().Certificates(ns), name, func(certificate *v1.Certificate) bool {
+		if !apiutil.CertificateHasCondition(certificate, ready_true_condition) {
+			log.Logf("Expected Certificate %v condition %v=%v but it has: %v", certificate.Name, ready_true_condition.Type, ready_true_condition.Status, certificate.Status.Conditions)
+			return false
+		}
+
+		if apiutil.CertificateHasCondition(certificate, issuing_condition) {
+			log.Logf("Expected Certificate %v condition %v to be missing but it has: %v", certificate.Name, issuing_condition.Type, certificate.Status.Conditions)
+			return false
+		}
+
+		return true
+	}, 500*time.Millisecond, timeout)
 }

 // WaitForCertificateReadyUpdate waits for the certificate resource to enter a
 // Ready state and to leave the Issuing state. If the provided cert was in a
 // Ready state already, the function waits for a state transition to have happened.
 func (h *Helper) WaitForCertificateReadyUpdate(cert *cmapi.Certificate, timeout time.Duration) (*cmapi.Certificate, error) {
-	result, err := e2eutil.WaitForCertificateConditionWithObservedGeneration(h.CMClient.CertmanagerV1().Certificates(cert.Namespace), cert.Name, cmapi.CertificateCondition{
+	ready_true_condition := cmapi.CertificateCondition{
 		Type:               cmapi.CertificateConditionReady,
 		Status:             cmmeta.ConditionTrue,
 		ObservedGeneration: cert.Generation,
-	}, timeout)
-	if err != nil {
-		return h.handleResult(cert.Namespace, cert.Name, result, "Ready", err)
 	}
-	// Making sure that the Certificate is stable (see #4239) by also waiting for the Issuing state to disappear.
-	// A certificate that has state Ready=True and Issuing not set, is stable and will not change without outside changes.
-	return h.waitForCertificateNotIssuing(cert.Namespace, cert.Name, timeout)
+	issuing_condition := cmapi.CertificateCondition{
+		Type: cmapi.CertificateConditionIssuing,
+	}
+	return h.waitPollImmediateCertificate(h.CMClient.CertmanagerV1().Certificates(cert.Namespace), cert.Name, func(certificate *v1.Certificate) bool {
+		if !apiutil.CertificateHasConditionWithObservedGeneration(certificate, ready_true_condition) {
+			log.Logf(
+				"Expected Certificate %v condition %v=%v (generation >= %v) but it has: %v",
+				certificate.Name,
+				ready_true_condition.Type,
+				ready_true_condition.Status,
+				ready_true_condition.ObservedGeneration,
+				certificate.Status.Conditions,
+			)
+			return false
+		}
+
+		if apiutil.CertificateHasCondition(certificate, issuing_condition) {
+			log.Logf("Expected Certificate %v condition %v to be missing but it has: %v", certificate.Name, issuing_condition.Type, certificate.Status.Conditions)
+			return false
+		}
+
+		return true
+	}, 500*time.Millisecond, timeout)
 }

 // WaitForCertificateReadyUpdate waits for the certificate resource to enter a
 // Ready=False state and to leave the Issuing state. If the provided cert was
 // in a Ready=False state already, the function waits for a state transition to have happened.
 func (h *Helper) WaitForCertificateNotReadyUpdate(cert *cmapi.Certificate, timeout time.Duration) (*cmapi.Certificate, error) {
-	result, err := e2eutil.WaitForCertificateConditionWithObservedGeneration(h.CMClient.CertmanagerV1().Certificates(cert.Namespace), cert.Name, cmapi.CertificateCondition{
+	ready_false_condition := cmapi.CertificateCondition{
 		Type:               cmapi.CertificateConditionReady,
 		Status:             cmmeta.ConditionFalse,
 		ObservedGeneration: cert.Generation,
-	}, timeout)
-	if err != nil {
-		return h.handleResult(cert.Namespace, cert.Name, result, "Not Ready", err)
 	}
-	// Making sure that the Certificate is stable (see #4239) by also waiting for the Issuing state to disappear.
-	// A certificate that has state Ready=False and Issuing not set, is stable and will not change without outside changes.
-	return h.waitForCertificateNotIssuing(cert.Namespace, cert.Name, timeout)
+	issuing_condition := cmapi.CertificateCondition{
+		Type: cmapi.CertificateConditionIssuing,
+	}
+	return h.waitPollImmediateCertificate(h.CMClient.CertmanagerV1().Certificates(cert.Namespace), cert.Name, func(certificate *v1.Certificate) bool {
+		if !apiutil.CertificateHasCondition(certificate, ready_false_condition) {
+			log.Logf(
+				"Expected Certificate %v condition %v=%v (generation >= %v) but it has: %v",
+				certificate.Name,
+				ready_false_condition.Type,
+				ready_false_condition.Status,
+				ready_false_condition.ObservedGeneration,
+				certificate.Status.Conditions,
+			)
+			return false
+		}
+
+		if apiutil.CertificateHasCondition(certificate, issuing_condition) {
+			log.Logf("Expected Certificate %v condition %v to be missing but it has: %v", certificate.Name, issuing_condition.Type, certificate.Status.Conditions)
+			return false
+		}
+
+		return true
+	}, 500*time.Millisecond, timeout)
 }

 func (h *Helper) deduplicateExtKeyUsages(us []x509.ExtKeyUsage) []x509.ExtKeyUsage {
@@ -184,16 +252,3 @@ func (h *Helper) keyUsagesMatch(aKU x509.KeyUsage, aEKU []x509.ExtKeyUsage,

 	return true
 }
-
-func (h *Helper) describeCertificateRequestFromCertificate(ns string, certificate *cmapi.Certificate) {
-	if certificate == nil {
-		return
-	}
-
-	crName, err := apiutil.ComputeName(certificate.Name, certificate.Spec)
-	if err != nil {
-		log.Logf("Failed to compute CertificateRequest name from certificate: %s", err)
-		return
-	}
-	h.Kubectl(ns).DescribeResource("certificaterequest", crName)
-}
@@ -148,6 +148,7 @@ func wrapErrorWithClusterIssuerStatusCondition(client clientset.ClusterIssuerInt

 // WaitForCertificateCondition waits for the status of the named Certificate to contain
 // a condition whose type and status matches the supplied one.
+// Deprecated: this function is not used anymore
 func WaitForCertificateCondition(client clientset.CertificateInterface, name string, condition v1.CertificateCondition, timeout time.Duration) (*v1.Certificate, error) {
 	var certificate *v1.Certificate = nil
 	pollErr := wait.PollImmediate(500*time.Millisecond, timeout,
@@ -167,29 +168,9 @@ func WaitForCertificateCondition(client clientset.CertificateInterface, name str
 	return certificate, wrapErrorWithCertificateStatusCondition(client, pollErr, name, condition.Type)
 }

-// WaitForMissingCertificateCondition waits for the status of the named Certificate to NOT contain
-// a condition whose type and status matches the supplied one.
-func WaitForMissingCertificateCondition(client clientset.CertificateInterface, name string, condition v1.CertificateCondition, timeout time.Duration) (*v1.Certificate, error) {
-	var certificate *v1.Certificate = nil
-	pollErr := wait.PollImmediate(500*time.Millisecond, timeout,
-		func() (bool, error) {
-			log.Logf("Waiting for Certificate %v condition %v=%v to be missing", name, condition.Type, condition.Status)
-			certificate, err := client.Get(context.TODO(), name, metav1.GetOptions{})
-			if nil != err {
-				return false, fmt.Errorf("error getting Certificate %v: %v", name, err)
-			}
-			if apiutil.CertificateHasCondition(certificate, condition) {
-				log.Logf("Expected Certificate %v condition %v=%v to be missing but it has: %v", name, condition.Type, condition.Status, certificate.Status.Conditions)
-				return false, nil
-			}
-			return true, nil
-		},
-	)
-	return certificate, wrapErrorWithCertificateStatusCondition(client, pollErr, name, condition.Type)
-}
-
 // WaitForCertificateConditionWithObservedGeneration waits for the status of the named Certificate to contain
 // a condition whose type and status matches the supplied one.
+// Deprecated: this function is not used anymore
 func WaitForCertificateConditionWithObservedGeneration(client clientset.CertificateInterface, name string, condition v1.CertificateCondition, timeout time.Duration) (*v1.Certificate, error) {
 	var certificate *v1.Certificate = nil
 	pollErr := wait.PollImmediate(500*time.Millisecond, timeout,
@@ -211,6 +192,7 @@ func WaitForCertificateConditionWithObservedGeneration(client clientset.Certific

 // WaitForCertificateEvent waits for an event on the named Certificate to contain
 // an event reason matches the supplied one.
+// Deprecated: this function is not used anymore
 func WaitForCertificateEvent(client kubernetes.Interface, cert *v1.Certificate, reason string, timeout time.Duration) error {
 	return wait.PollImmediate(500*time.Millisecond, timeout,
 		func() (bool, error) {
@@ -225,6 +207,7 @@ func WaitForCertificateEvent(client kubernetes.Interface, cert *v1.Certificate,
 	)
 }

+// Deprecated: this function is not used anymore
 func hasEvent(events *corev1.EventList, reason string) bool {
 	for _, evt := range events.Items {
 		if evt.Reason == reason {
@@ -235,6 +218,7 @@ func hasEvent(events *corev1.EventList, reason string) bool {
 }

 // try to retrieve last condition to help diagnose tests.
+// Deprecated: this function is not used anymore
 func wrapErrorWithCertificateStatusCondition(client clientset.CertificateInterface, pollErr error, name string, conditionType v1.CertificateConditionType) error {
 	if pollErr == nil {
 		return nil