More robust WaitForServiceLatestRevision function for rolling upgrades

knative · Nov 6, 2019 · 7c5dfc4 · 7c5dfc4
1 parent a200148
commit 7c5dfc4
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 6 deletions.
diff --git a/test/v1/revision.go b/test/v1/revision.go
@@ -75,6 +75,14 @@ func IsRevisionReady(r *v1.Revision) (bool, error) {
 	return r.Generation == r.Status.ObservedGeneration && r.Status.IsReady(), nil
 }
 
+// IsRevisionPinned will check if the revision is pinned to a route.
+func IsRevisionPinned(r *v1.Revision) (bool, error) {
+	if _, ok := r.Annotations[serving.RevisionLastPinnedAnnotationKey]; !ok {
+		return false, fmt.Errorf("Revision %s not pinned", r.Name)
+	}
+	return true, nil
+}
+
 // IsRevisionAtExpectedGeneration returns a function that will check if the annotations
 // on the revision include an annotation for the generation and that the annotation is
 // set to the expected value.

diff --git a/test/v1/service.go b/test/v1/service.go
@@ -174,18 +174,24 @@ func WaitForServiceLatestRevision(clients *test.Clients, names test.ResourceName
 	err := WaitForServiceState(clients.ServingClient, names.Service, func(s *v1.Service) (bool, error) {
 		if s.Status.LatestCreatedRevisionName != names.Revision {
 			revisionName = s.Status.LatestCreatedRevisionName
+			// We also check that the revision is pinned, meaning it's not a stale revision.
+			// Without this it might happen that the latest created revision is later overriden by a newer one
+			// and the following check for LatestReadyRevisionName would fail.
+			if revErr := CheckRevisionState(clients.ServingClient, revisionName, IsRevisionPinned); revErr != nil {
+				return false, nil
+			}
 			return true, nil
 		}
 		return false, nil
 	}, "ServiceUpdatedWithRevision")
 	if err != nil {
-		return "", err
+		return "", errors.Wrapf(err, "LatestCreatedRevisionName not updated")
 	}
 	err = WaitForServiceState(clients.ServingClient, names.Service, func(s *v1.Service) (bool, error) {
 		return (s.Status.LatestReadyRevisionName == revisionName), nil
 	}, "ServiceReadyWithRevision")
 
-	return revisionName, err
+	return revisionName, errors.Wrapf(err, "LatestReadyRevisionName not updated with %s", revisionName)
 }
 
 // Service returns a Service object in namespace with the name names.Service

diff --git a/test/v1alpha1/revision.go b/test/v1alpha1/revision.go
@@ -75,6 +75,14 @@ func IsRevisionReady(r *v1alpha1.Revision) (bool, error) {
 	return r.Generation == r.Status.ObservedGeneration && r.Status.IsReady(), nil
 }
 
+// IsRevisionPinned will check if the revision is pinned to a route.
+func IsRevisionPinned(r *v1alpha1.Revision) (bool, error) {
+	if _, ok := r.Annotations[serving.RevisionLastPinnedAnnotationKey]; !ok {
+		return false, fmt.Errorf("Revision %s not pinned", r.Name)
+	}
+	return true, nil
+}
+
 // IsRevisionAtExpectedGeneration returns a function that will check if the annotations
 // on the revision include an annotation for the generation and that the annotation is
 // set to the expected value.

diff --git a/test/v1alpha1/service.go b/test/v1alpha1/service.go
@@ -252,18 +252,24 @@ func WaitForServiceLatestRevision(clients *test.Clients, names test.ResourceName
 	err := WaitForServiceState(clients.ServingAlphaClient, names.Service, func(s *v1alpha1.Service) (bool, error) {
 		if s.Status.LatestCreatedRevisionName != names.Revision {
 			revisionName = s.Status.LatestCreatedRevisionName
+			// We also check that the revision is pinned, meaning it's not a stale revision.
+			// Without this it might happen that the latest created revision is later overriden by a newer one
+			// and the following check for LatestReadyRevisionName would fail.
+			if revErr := CheckRevisionState(clients.ServingAlphaClient, revisionName, IsRevisionPinned); revErr != nil {
+				return false, nil
+			}
 			return true, nil
 		}
 		return false, nil
 	}, "ServiceUpdatedWithRevision")
 	if err != nil {
-		return "", err
+		return "", errors.Wrapf(err, "LatestCreatedRevisionName not updated")
 	}
 	err = WaitForServiceState(clients.ServingAlphaClient, names.Service, func(s *v1alpha1.Service) (bool, error) {
 		return (s.Status.LatestReadyRevisionName == revisionName), nil
 	}, "ServiceReadyWithRevision")
 
-	return revisionName, err
+	return revisionName, errors.Wrapf(err, "LatestReadyRevisionName not updated with %s", revisionName)
 }
 
 // LatestService returns a Service object in namespace with the name names.Service

diff --git a/test/v1beta1/revision.go b/test/v1beta1/revision.go
@@ -75,6 +75,14 @@ func IsRevisionReady(r *v1beta1.Revision) (bool, error) {
 	return r.Generation == r.Status.ObservedGeneration && r.Status.IsReady(), nil
 }
 
+// IsRevisionPinned will check if the revision is pinned to a route.
+func IsRevisionPinned(r *v1beta1.Revision) (bool, error) {
+	if _, ok := r.Annotations[serving.RevisionLastPinnedAnnotationKey]; !ok {
+		return false, fmt.Errorf("Revision %s not pinned", r.Name)
+	}
+	return true, nil
+}
+
 // IsRevisionAtExpectedGeneration returns a function that will check if the annotations
 // on the revision include an annotation for the generation and that the annotation is
 // set to the expected value.

diff --git a/test/v1beta1/service.go b/test/v1beta1/service.go
@@ -173,18 +173,24 @@ func WaitForServiceLatestRevision(clients *test.Clients, names test.ResourceName
 	err := WaitForServiceState(clients.ServingBetaClient, names.Service, func(s *v1beta1.Service) (bool, error) {
 		if s.Status.LatestCreatedRevisionName != names.Revision {
 			revisionName = s.Status.LatestCreatedRevisionName
+			// We also check that the revision is pinned, meaning it's not a stale revision.
+			// Without this it might happen that the latest created revision is later overriden by a newer one
+			// and the following check for LatestReadyRevisionName would fail.
+			if revErr := CheckRevisionState(clients.ServingBetaClient, revisionName, IsRevisionPinned); revErr != nil {
+				return false, nil
+			}
 			return true, nil
 		}
 		return false, nil
 	}, "ServiceUpdatedWithRevision")
 	if err != nil {
-		return "", err
+		return "", errors.Wrapf(err, "LatestCreatedRevisionName not updated")
 	}
 	err = WaitForServiceState(clients.ServingBetaClient, names.Service, func(s *v1beta1.Service) (bool, error) {
 		return (s.Status.LatestReadyRevisionName == revisionName), nil
 	}, "ServiceReadyWithRevision")
 
-	return revisionName, err
+	return revisionName, errors.Wrapf(err, "LatestReadyRevisionName not updated with %s", revisionName)
 }
 
 // Service returns a Service object in namespace with the name names.Service