Cloudrun: Check knative observed generation when polling (#4793) (#9213)

* Check knative observed generation when polling * Rework to return errors * PR feedback, rename var Signed-off-by: Modular Magician <[email protected]>
hashicorp · May 21, 2021 · 465b2c4 · 465b2c4
1 parent 7d115b8
commit 465b2c4
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 25 deletions.
diff --git a/.changelog/4793.txt b/.changelog/4793.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+cloudrun: fixed a bug where resources would return successfully due to responses based on a previous version of the resource
+```
diff --git a/google/cloudrun_polling.go b/google/cloudrun_polling.go
@@ -26,37 +26,63 @@ type KnativeStatus struct {
 		SelfLink  string
 	}
 	Status struct {
-		Conditions []Condition
+		Conditions         []Condition
+		ObservedGeneration float64
 	}
 }
 
-func PollCheckKnativeStatus(resp map[string]interface{}, respErr error) PollResult {
-	if respErr != nil {
-		return ErrorPollResult(respErr)
+func getGeneration(res map[string]interface{}) (int, error) {
+	metadata, ok := res["metadata"]
+	if !ok {
+		return 0, fmt.Errorf("Unable to find knative metadata")
 	}
-	s := KnativeStatus{}
-	if err := Convert(resp, &s); err != nil {
-		return ErrorPollResult(errwrap.Wrapf("unable to get KnativeStatus: {{err}}", err))
+	m, ok := metadata.(map[string]interface{})
+	if !ok {
+		return 0, fmt.Errorf("Unable to find generation in knative metadata")
 	}
+	gen, ok := m["generation"]
+	if !ok {
+		return 0, fmt.Errorf("Unable to find generation in knative metadata")
+	}
+	return int(gen.(float64)), nil
+}
 
-	for _, condition := range s.Status.Conditions {
-		if condition.Type == readyStatusType {
-			log.Printf("[DEBUG] checking KnativeStatus Ready condition %s: %s", condition.Status, condition.Message)
-			switch condition.Status {
-			case "True":
-				// Resource is ready
-				return SuccessPollResult()
-			case "Unknown":
-				// DomainMapping can enter a 'terminal' state where "Ready" status is "Unknown"
-				// but the resource is waiting for external verification of DNS records.
-				if condition.Reason == pendingCertificateReason {
+func PollCheckKnativeStatusFunc(knativeRestResponse map[string]interface{}) func(resp map[string]interface{}, respErr error) PollResult {
+	return func(resp map[string]interface{}, respErr error) PollResult {
+		if respErr != nil {
+			return ErrorPollResult(respErr)
+		}
+		s := KnativeStatus{}
+		if err := Convert(resp, &s); err != nil {
+			return ErrorPollResult(errwrap.Wrapf("unable to get KnativeStatus: {{err}}", err))
+		}
+
+		gen, err := getGeneration(knativeRestResponse)
+		if err != nil {
+			return ErrorPollResult(errwrap.Wrapf("unable to find Knative generation: {{err}}", err))
+		}
+		if int(s.Status.ObservedGeneration) != gen {
+			return PendingStatusPollResult("waiting for observed generation to match")
+		}
+		for _, condition := range s.Status.Conditions {
+			if condition.Type == readyStatusType {
+				log.Printf("[DEBUG] checking KnativeStatus Ready condition %s: %s", condition.Status, condition.Message)
+				switch condition.Status {
+				case "True":
+					// Resource is ready
 					return SuccessPollResult()
+				case "Unknown":
+					// DomainMapping can enter a 'terminal' state where "Ready" status is "Unknown"
+					// but the resource is waiting for external verification of DNS records.
+					if condition.Reason == pendingCertificateReason {
+						return SuccessPollResult()
+					}
+					return PendingStatusPollResult(fmt.Sprintf("%s:%s", condition.Status, condition.Message))
+				case "False":
+					return ErrorPollResult(fmt.Errorf(`resource is in failed state "Ready:False", message: %s`, condition.Message))
 				}
-				return PendingStatusPollResult(fmt.Sprintf("%s:%s", condition.Status, condition.Message))
-			case "False":
-				return ErrorPollResult(fmt.Errorf(`resource is in failed state "Ready:False", message: %s`, condition.Message))
 			}
 		}
+		return PendingStatusPollResult("no status yet")
 	}
-	return PendingStatusPollResult("no status yet")
 }
diff --git a/google/resource_cloud_run_domain_mapping.go b/google/resource_cloud_run_domain_mapping.go
@@ -335,7 +335,7 @@ func resourceCloudRunDomainMappingCreate(d *schema.ResourceData, meta interface{
 	}
 	d.SetId(id)
 
-	err = PollingWaitTime(resourceCloudRunDomainMappingPollRead(d, meta), PollCheckKnativeStatus, "Creating DomainMapping", d.Timeout(schema.TimeoutCreate), 1)
+	err = PollingWaitTime(resourceCloudRunDomainMappingPollRead(d, meta), PollCheckKnativeStatusFunc(res), "Creating DomainMapping", d.Timeout(schema.TimeoutCreate), 1)
 	if err != nil {
 		return fmt.Errorf("Error waiting to create DomainMapping: %s", err)
 	}

diff --git a/google/resource_cloud_run_service.go b/google/resource_cloud_run_service.go
@@ -750,7 +750,7 @@ func resourceCloudRunServiceCreate(d *schema.ResourceData, meta interface{}) err
 	}
 	d.SetId(id)
 
-	err = PollingWaitTime(resourceCloudRunServicePollRead(d, meta), PollCheckKnativeStatus, "Creating Service", d.Timeout(schema.TimeoutCreate), 1)
+	err = PollingWaitTime(resourceCloudRunServicePollRead(d, meta), PollCheckKnativeStatusFunc(res), "Creating Service", d.Timeout(schema.TimeoutCreate), 1)
 	if err != nil {
 		return fmt.Errorf("Error waiting to create Service: %s", err)
 	}
@@ -938,7 +938,7 @@ func resourceCloudRunServiceUpdate(d *schema.ResourceData, meta interface{}) err
 		log.Printf("[DEBUG] Finished updating Service %q: %#v", d.Id(), res)
 	}
 
-	err = PollingWaitTime(resourceCloudRunServicePollRead(d, meta), PollCheckKnativeStatus, "Updating Service", d.Timeout(schema.TimeoutUpdate), 1)
+	err = PollingWaitTime(resourceCloudRunServicePollRead(d, meta), PollCheckKnativeStatusFunc(res), "Updating Service", d.Timeout(schema.TimeoutUpdate), 1)
 	if err != nil {
 		return err
 	}