Improve error messages returned with pending & failing requests

Signed-off-by: James Munnelly <[email protected]>

Author: munnerz

manager/manager.go

@@ -339,7 +339,7 @@ func (m *Manager) issue(ctx context.Context, volumeID string) error {
 
 	// Poll every 1s for the CertificateRequest to be ready
 	lastFailureReason := ""
-	if err := wait.PollUntil(time.Second, func() (done bool, err error) {
+	if err := wait.PollUntilWithContext(ctx, time.Second, func(ctx context.Context) (done bool, err error) {
 		updatedReq, err := m.lister.CertificateRequests(req.Namespace).Get(req.Name)
 		if apierrors.IsNotFound(err) {
 			// A NotFound error implies something deleted the resource - fail
@@ -363,8 +363,9 @@ func (m *Manager) issue(ctx context.Context, volumeID string) error {
 			return false, fmt.Errorf("request %q has been denied by the approval plugin: %s", updatedReq.Name, cond.Message)
 		}
 
-		if !apiutil.CertificateRequestIsApproved(updatedReq) {
-			lastFailureReason = "request has not yet been approved by approval plugin"
+		isApproved := apiutil.CertificateRequestIsApproved(updatedReq)
+		if !isApproved {
+			lastFailureReason = fmt.Sprintf("request %q has not yet been approved by approval plugin", updatedReq.Name)
 			// we don't stop execution here, as some versions of cert-manager (and some external issuer plugins)
 			// may not be aware/utilise approval.
 			// If the certificate is still issued despite never being approved, the CSI driver should continue
@@ -373,11 +374,11 @@ func (m *Manager) issue(ctx context.Context, volumeID string) error {
 
 		readyCondition := apiutil.GetCertificateRequestCondition(updatedReq, cmapi.CertificateRequestConditionReady)
 		if readyCondition == nil {
-			if apiutil.CertificateRequestIsApproved(updatedReq) {
-				lastFailureReason = "request has no ready condition"
+			// only overwrite the approval failure message if the request is actually approved
+			// otherwise we may hide more useful information from the user by accident.
+			if isApproved {
+				lastFailureReason = fmt.Sprintf("request %q has no ready condition", updatedReq.Name)
 			}
-			log.V(2).Info("CertificateRequest is still pending")
-			// Issuance is still pending
 			return false, nil
 		}
 
@@ -387,11 +388,12 @@ func (m *Manager) issue(ctx context.Context, volumeID string) error {
 		case cmapi.CertificateRequestReasonFailed:
 			return false, fmt.Errorf("request %q has failed: %s", updatedReq.Name, readyCondition.Message)
 		case cmapi.CertificateRequestReasonPending:
-			lastFailureReason = fmt.Sprintf("request pending: %v", readyCondition.Message)
-			log.V(2).Info("CertificateRequest is still pending")
+			if isApproved {
+				lastFailureReason = fmt.Sprintf("request %q is pending: %v", updatedReq.Name, readyCondition.Message)
+			}
 			return false, nil
 		default:
-			lastFailureReason = fmt.Sprintf("unrecognised Ready condition state (%s): %s", readyCondition.Reason, readyCondition.Message)
+			lastFailureReason = fmt.Sprintf("request %q has unrecognised Ready condition state (%s): %s", updatedReq.Name, readyCondition.Reason, readyCondition.Message)
 			log.Info("unrecognised state for Ready condition", "request_namespace", updatedReq.Namespace, "request_name", updatedReq.Name, "condition", *readyCondition)
 			return false, nil
 		}
@@ -401,7 +403,7 @@ func (m *Manager) issue(ctx context.Context, volumeID string) error {
 		// writing out signed certificate
 		req = updatedReq
 		return true, nil
-	}, ctx.Done()); err != nil {
+	}); err != nil {
 		if errors.Is(err, wait.ErrWaitTimeout) {
 			// try and return a more helpful error message than "timed out waiting for the condition"
 			return fmt.Errorf("waiting for request: %s", lastFailureReason)