[GitHub] [trafficcontrol] mattjackson220 commented on a change in pull request #5738: Acme async

[GitBox <gi...@apache.org>]

mattjackson220 commented on a change in pull request #5738:
URL: https://github.com/apache/trafficcontrol/pull/5738#discussion_r616034650



##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/acme.go
##########
@@ -398,23 +471,34 @@ func GetAcmeCertificates(cfg *config.Config, req tc.DeliveryServiceLetsEncryptSS
 	if err := tv.PutDeliveryServiceSSLKeys(dsSSLKeys, tx); err != nil {
 		log.Errorf("Error putting ACME certificate in Traffic Vault: %s", err.Error())
 		api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: FAILED to add SSL keys with "+provider, currentUser, logTx)
+		if err = api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); err != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, err)
+		}
 		return errors.New(deliveryService + ": putting keys in Traffic Vault: " + err.Error())
 	}
 
 	tx2, err := db.Begin()
 	if err != nil {
 		log.Errorf("starting sql transaction for delivery service " + *req.DeliveryService + ": " + err.Error())
+		if err = api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); err != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, err)
+		}
 		return errors.New("starting sql transaction for delivery service " + *req.DeliveryService + ": " + err.Error())
 	}
 
 	if err := updateSSLKeyVersion(*req.DeliveryService, req.Version.ToInt64(), tx2); err != nil {
 		log.Errorf("updating SSL key version for delivery service '" + *req.DeliveryService + "': " + err.Error())
+		if err = api.UpdateAsyncStatus(db, api.AsyncFailed, "ACME renewal failed.", asyncStatusId, true); err != nil {
+			log.Errorf("updating async status for id %v: %v", asyncStatusId, err)
+		}
 		return errors.New("updating SSL key version for delivery service '" + *req.DeliveryService + "': " + err.Error())
 	}
 	tx2.Commit()
 
 	api.CreateChangeLogRawTx(api.ApiChange, "DS: "+*req.DeliveryService+", ID: "+strconv.Itoa(dsID)+", ACTION: Added SSL keys with "+provider, currentUser, logTx)
-
+	if err = api.UpdateAsyncStatus(db, api.AsyncSucceeded, "ACME renewal complete.", asyncStatusId, true); err != nil {
+		log.Errorf("updating async status for id %v: %v", asyncStatusId, err)
+	}
 	return nil

Review comment:
       my thinking on these was that we should just log any errors to the async updates so that it is just a status and doesnt impact the actual workflow. I'm very open to changing it though if we think it would be better to have it return an error if the async part fails




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org