Skip to content

Commit 34a7680

Browse files
mattmoorgoogle-prow-robot
authored andcommitted
Create Check methods parallel to WaitFor methods. (#1188)
Every check we perform shouldn't be a WaitFor; we should WaitFor readiness and then assert things work in-the-now. This creates parallel methods for checking things, which has the same signature as their `WaitFor` siblings. This uses these methods for the recently sunk checks in the routing conformance test. Related: #1178
1 parent d1c1633 commit 34a7680

File tree

3 files changed

+72
-22
lines changed

3 files changed

+72
-22
lines changed

test/adding_tests.md

+19-6
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ You can:
2727
* [Output log](#output-log)
2828
* [Get access to client objects](#get-access-to-client-objects)
2929
* [Make requests against deployed services](#make-requests-against-deployed-services)
30-
* [Poll Knative Serving resources](#poll-knative-serving-resources)
30+
* [Check Knative Serving resources](#check-knative-serving-resources)
3131
* [Verify resource state transitions](#verify-resource-state-transitions)
3232
* [Generate boilerplate CRDs](#generate-boilerplate-crds)
3333
* [Ensure test cleanup](#ensure-test-cleanup)
@@ -114,13 +114,13 @@ should be used or the domain should be used directly.
114114

115115
_See [request.go](./request.go)._
116116

117-
### Poll Knative Serving resources
117+
### Check Knative Serving resources
118118

119119
After creating Knative Serving resources or making changes to them, you will need to wait for the system
120-
to realize those chnages. You can use the Knative Serving CRD polling methods to poll the resources until
121-
they get into the desired state or time out.
120+
to realize those changes. You can use the Knative Serving CRD check and polling methods to check the
121+
resources are either in or reach the desired state.
122122

123-
These functions use the kubernetes [`wait` package](https://godoc.org/k8s.io/apimachinery/pkg/util/wait).
123+
The `WaitFor*` functions use the kubernetes [`wait` package](https://godoc.org/k8s.io/apimachinery/pkg/util/wait).
124124
To poll they use [`PollImmediate`](https://godoc.org/k8s.io/apimachinery/pkg/util/wait#PollImmediate)
125125
and the return values of the function you provide behave the same as
126126
[`ConditionFunc`](https://godoc.org/k8s.io/apimachinery/pkg/util/wait#ConditionFunc):
@@ -141,7 +141,20 @@ err := test.WaitForConfigurationState(clients.Configs, configName, func(c *v1alp
141141
})
142142
```
143143

144-
_See [crd_polling.go](./crd_polling.go)._
144+
We also have `Check*` variants of many of these methods with identical signatures, same example:
145+
146+
```go
147+
var revisionName string
148+
err := test.CheckConfigurationState(clients.Configs, configName, func(c *v1alpha1.Configuration) (bool, error) {
149+
if c.Status.LatestCreatedRevisionName != "" {
150+
revisionName = c.Status.LatestCreatedRevisionName
151+
return true, nil
152+
}
153+
return false, nil
154+
})
155+
```
156+
157+
_See [crd_checks.go](./crd_checks.go)._
145158

146159
### Verify resource state transitions
147160

test/conformance/route_test.go

+4-16
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,6 @@ func createRouteAndConfig(clients *test.Clients, names test.ResourceNames, image
5151
return err
5252
}
5353

54-
func getFirstRevisionName(clients *test.Clients, names test.ResourceNames) (string, error) {
55-
var revisionName string
56-
err := test.WaitForConfigurationState(clients.Configs, names.Config, func(c *v1alpha1.Configuration) (bool, error) {
57-
if c.Status.LatestCreatedRevisionName != "" {
58-
revisionName = c.Status.LatestCreatedRevisionName
59-
return true, nil
60-
}
61-
return false, nil
62-
})
63-
return revisionName, err
64-
}
65-
6654
func updateConfigWithImage(clients *test.Clients, names test.ResourceNames, imagePaths []string) error {
6755
patches := []jsonpatch.JsonPatchOperation{
6856
jsonpatch.JsonPatchOperation{
@@ -110,19 +98,19 @@ func assertResourcesUpdatedWhenRevisionIsReady(t *testing.T, clients *test.Clien
11098

11199
// We want to verify that the endpoint works as soon as Ready: True, but there are a bunch of other pieces of state that we validate for conformance.
112100
glog.Infof("The Revision will be marked as Ready when it can serve traffic")
113-
err = test.WaitForRevisionState(clients.Revisions, names.Revision, test.IsRevisionReady(names.Revision))
101+
err = test.CheckRevisionState(clients.Revisions, names.Revision, test.IsRevisionReady(names.Revision))
114102
if err != nil {
115103
t.Fatalf("Revision %s did not become ready to serve traffic: %v", names.Revision, err)
116104
}
117105
glog.Infof("Updates the Configuration that the Revision is ready")
118-
err = test.WaitForConfigurationState(clients.Configs, names.Config, func(c *v1alpha1.Configuration) (bool, error) {
106+
err = test.CheckConfigurationState(clients.Configs, names.Config, func(c *v1alpha1.Configuration) (bool, error) {
119107
return c.Status.LatestReadyRevisionName == names.Revision, nil
120108
})
121109
if err != nil {
122110
t.Fatalf("The Configuration %s was not updated indicating that the Revision %s was ready: %v\n", names.Config, names.Revision, err)
123111
}
124112
glog.Infof("Updates the Route to route traffic to the Revision")
125-
err = test.WaitForRouteState(clients.Routes, names.Route, test.AllRouteTrafficAtRevision(names.Route, names.Revision))
113+
err = test.CheckRouteState(clients.Routes, names.Route, test.AllRouteTrafficAtRevision(names.Route, names.Revision))
126114
if err != nil {
127115
t.Fatalf("The Route %s was not updated to route traffic to the Revision %s: %v", names.Route, names.Revision, err)
128116
}
@@ -175,7 +163,7 @@ func TestRouteCreation(t *testing.T) {
175163
}
176164

177165
glog.Infof("The Configuration will be updated with the name of the Revision once it is created")
178-
revisionName, err := getFirstRevisionName(clients, names)
166+
revisionName, err := getNextRevisionName(clients, names)
179167
if err != nil {
180168
t.Fatalf("Configuration %s was not updated with the new revision: %v", names.Config, err)
181169
}

test/crd_polling.go renamed to test/crd_checks.go

+49
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ limitations under the License.
1919
package test
2020

2121
import (
22+
"fmt"
2223
"time"
2324

2425
"github.com/knative/serving/pkg/apis/serving/v1alpha1"
@@ -47,6 +48,22 @@ func WaitForRouteState(client elatyped.RouteInterface, name string, inState func
4748
})
4849
}
4950

51+
// CheckRouteState verifies the status of the Route called name from client
52+
// is in a particular state by calling `inState` and expecting `true`.
53+
// This is the non-polling variety of WaitForRouteState
54+
func CheckRouteState(client elatyped.RouteInterface, name string, inState func(r *v1alpha1.Route) (bool, error)) error {
55+
r, err := client.Get(name, metav1.GetOptions{})
56+
if err != nil {
57+
return err
58+
}
59+
if done, err := inState(r); err != nil {
60+
return err
61+
} else if !done {
62+
return fmt.Errorf("route %q is not in desired state: %+v", name, r)
63+
}
64+
return nil
65+
}
66+
5067
// WaitForConfigurationState polls the status of the Configuration called name
5168
// from client every interval until inState returns `true` indicating it
5269
// is done, returns an error or timeout.
@@ -60,6 +77,22 @@ func WaitForConfigurationState(client elatyped.ConfigurationInterface, name stri
6077
})
6178
}
6279

80+
// CheckConfigurationState verifies the status of the Configuration called name from client
81+
// is in a particular state by calling `inState` and expecting `true`.
82+
// This is the non-polling variety of WaitForConfigurationState
83+
func CheckConfigurationState(client elatyped.ConfigurationInterface, name string, inState func(r *v1alpha1.Configuration) (bool, error)) error {
84+
c, err := client.Get(name, metav1.GetOptions{})
85+
if err != nil {
86+
return err
87+
}
88+
if done, err := inState(c); err != nil {
89+
return err
90+
} else if !done {
91+
return fmt.Errorf("configuration %q is not in desired state: %+v", name, c)
92+
}
93+
return nil
94+
}
95+
6396
// WaitForRevisionState polls the status of the Revision called name
6497
// from client every interval until inState returns `true` indicating it
6598
// is done, returns an error or timeout.
@@ -73,6 +106,22 @@ func WaitForRevisionState(client elatyped.RevisionInterface, name string, inStat
73106
})
74107
}
75108

109+
// CheckRevisionState verifies the status of the Revision called name from client
110+
// is in a particular state by calling `inState` and expecting `true`.
111+
// This is the non-polling variety of WaitForRevisionState
112+
func CheckRevisionState(client elatyped.RevisionInterface, name string, inState func(r *v1alpha1.Revision) (bool, error)) error {
113+
r, err := client.Get(name, metav1.GetOptions{})
114+
if err != nil {
115+
return err
116+
}
117+
if done, err := inState(r); err != nil {
118+
return err
119+
} else if !done {
120+
return fmt.Errorf("revision %q is not in desired state: %+v", name, r)
121+
}
122+
return nil
123+
}
124+
76125
// WaitForIngressState polls the status of the Ingress called name
77126
// from client every interval until inState returns `true` indicating it
78127
// is done, returns an error or timeout.

0 commit comments

Comments
 (0)