Skip to content

Commit 2abe65e

Browse files
tpantelistpantelis
committed
Conformance tests: improve awaitServiceImport
We can improve it by utilizing the form of 'Eventually' that accepts a function that that takes a single Gomega argument which is used to make assertions. 'Eventually' succeeds only if all the assertions in the polled function pass. This is simpler than first polling to find the ServiceImport based on initial checks then making separate assertions to provide better error output that typically overlap with the initial checks. Signed-off-by: Tom Pantelis <[email protected]>
1 parent a23ff80 commit 2abe65e

File tree

3 files changed

+68
-81
lines changed

3 files changed

+68
-81
lines changed

‎conformance/clusterip_service_dns.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,10 @@ var _ = Describe("", Label(OptionalLabel, DNSLabel, ClusterIPLabel), func() {
4444

4545
serviceImports := []*v1alpha1.ServiceImport{}
4646
for _, client := range clients {
47-
serviceImport := t.awaitServiceImport(&client, t.helloService.Name, func(serviceImport *v1alpha1.ServiceImport) bool {
48-
return len(serviceImport.Spec.IPs) > 0
49-
})
50-
Expect(serviceImport).NotTo(BeNil(), "ServiceImport was not found on cluster %q", client.name)
51-
Expect(serviceImport.Spec.IPs).ToNot(BeEmpty(), "ServiceImport on cluster %q does not contain an IP", client.name)
47+
serviceImport := t.awaitServiceImport(&client, t.helloService.Name, false,
48+
func(g Gomega, serviceImport *v1alpha1.ServiceImport) {
49+
g.Expect(serviceImport.Spec.IPs).ToNot(BeEmpty(), "ServiceImport on cluster %q does not contain an IP", client.name)
50+
})
5251
serviceImports = append(serviceImports, serviceImport)
5352
}
5453

‎conformance/conformance_suite.go

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import (
3737
apierrors "k8s.io/apimachinery/pkg/api/errors"
3838
"k8s.io/apimachinery/pkg/api/meta"
3939
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
40-
"k8s.io/apimachinery/pkg/util/wait"
4140
"k8s.io/client-go/kubernetes"
4241
rest "k8s.io/client-go/rest"
4342
"k8s.io/client-go/tools/clientcmd"
@@ -227,22 +226,31 @@ func (t *testDriver) getServiceImport(c *clusterClients, name string) *v1alpha1.
227226
return si
228227
}
229228

230-
func (t *testDriver) awaitServiceImport(c *clusterClients, name string, verify func(*v1alpha1.ServiceImport) bool) *v1alpha1.ServiceImport {
229+
func (t *testDriver) awaitServiceImport(c *clusterClients, name string, reportNonConformanceOnMissing bool,
230+
verify func(Gomega, *v1alpha1.ServiceImport)) *v1alpha1.ServiceImport {
231231
var serviceImport *v1alpha1.ServiceImport
232232

233-
_ = wait.PollUntilContextTimeout(ctx, 100*time.Millisecond,
234-
20*time.Second, true, func(ctx context.Context) (bool, error) {
235-
defer GinkgoRecover()
233+
Eventually(func(g Gomega) {
234+
si := t.getServiceImport(c, name)
236235

237-
si := t.getServiceImport(c, name)
238-
if si == nil {
239-
return false, nil
240-
}
236+
missingMsg := fmt.Sprintf("ServiceImport was not found on cluster %q", c.name)
241237

242-
serviceImport = si
238+
var missing any = missingMsg
239+
if reportNonConformanceOnMissing {
240+
missing = reportNonConformant(missingMsg)
241+
}
243242

244-
return verify == nil || verify(serviceImport), nil
245-
})
243+
g.Expect(si).NotTo(BeNil(), missing)
244+
245+
serviceImport = si
246+
247+
if verify != nil {
248+
verify(g, serviceImport)
249+
}
250+
251+
// The final run succeeded so cancel any prior non-conformance reported.
252+
cancelNonConformanceReport()
253+
}).Within(20 * time.Second).WithPolling(100 * time.Millisecond).Should(Succeed())
246254

247255
return serviceImport
248256
}

‎conformance/service_import.go

Lines changed: 44 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,15 @@ func testGeneralServiceImport() {
4747
t.createServiceExport(&clients[0], helloServiceExport)
4848
})
4949

50-
assertHasKeyValues := func(actual, expected map[string]string) {
50+
assertHasKeyValues := func(g Gomega, actual, expected map[string]string) {
5151
for k, v := range expected {
52-
Expect(actual).To(HaveKeyWithValue(k, v), reportNonConformant(""))
52+
g.Expect(actual).To(HaveKeyWithValue(k, v), reportNonConformant(""))
5353
}
5454
}
55-
assertNotHasKeyValues := func(actual, expected map[string]string) {
55+
56+
assertNotHasKeyValues := func(g Gomega, actual, expected map[string]string) {
5657
for k, v := range expected {
57-
Expect(actual).ToNot(HaveKeyWithValue(k, v), reportNonConformant(""))
58+
g.Expect(actual).ToNot(HaveKeyWithValue(k, v), reportNonConformant(""))
5859
}
5960
}
6061

@@ -66,10 +67,7 @@ func testGeneralServiceImport() {
6667

6768
AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/blob/master/keps/sig-multicluster/1645-multi-cluster-services-api/README.md#importing-services")
6869

69-
serviceImport := t.awaitServiceImport(&clients[0], t.helloService.Name, func(serviceImport *v1alpha1.ServiceImport) bool {
70-
return true
71-
})
72-
Expect(serviceImport).NotTo(BeNil(), "ServiceImport was not found")
70+
t.awaitServiceImport(&clients[0], t.helloService.Name, false, nil)
7371

7472
By(fmt.Sprintf("Exporting the service on the second cluster %q", clients[1].name))
7573

@@ -109,16 +107,14 @@ func testGeneralServiceImport() {
109107
Label(OptionalLabel), Label(ExportedLabelsLabel), func() {
110108
AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#labels-and-annotations")
111109

112-
serviceImport := t.awaitServiceImport(&clients[0], helloServiceName, func(serviceImport *v1alpha1.ServiceImport) bool {
113-
return len(serviceImport.Labels) > 0
114-
})
115-
Expect(serviceImport).NotTo(BeNil(), "ServiceImport was not found")
110+
t.awaitServiceImport(&clients[0], helloServiceName, false,
111+
func(g Gomega, serviceImport *v1alpha1.ServiceImport) {
112+
assertHasKeyValues(g, serviceImport.Annotations, helloServiceExport.Annotations)
113+
assertNotHasKeyValues(g, serviceImport.Annotations, t.helloService.Annotations)
116114

117-
assertHasKeyValues(serviceImport.Annotations, helloServiceExport.Annotations)
118-
assertNotHasKeyValues(serviceImport.Annotations, t.helloService.Annotations)
119-
120-
assertHasKeyValues(serviceImport.Labels, helloServiceExport.Labels)
121-
assertNotHasKeyValues(serviceImport.Labels, t.helloService.Labels)
115+
assertHasKeyValues(g, serviceImport.Labels, helloServiceExport.Labels)
116+
assertNotHasKeyValues(g, serviceImport.Labels, t.helloService.Labels)
117+
})
122118
})
123119
})
124120

@@ -141,16 +137,14 @@ func testGeneralServiceImport() {
141137
t.awaitServiceExportCondition(&clients[0], v1alpha1.ServiceExportConflict, metav1.ConditionTrue)
142138
t.awaitServiceExportCondition(&clients[1], v1alpha1.ServiceExportConflict, metav1.ConditionTrue)
143139

144-
serviceImport := t.awaitServiceImport(&clients[0], t.helloService.Name, func(serviceImport *v1alpha1.ServiceImport) bool {
145-
return len(serviceImport.Labels) > 0
146-
})
147-
Expect(serviceImport).NotTo(BeNil(), "ServiceImport was not found")
148-
149-
assertHasKeyValues(serviceImport.Annotations, helloServiceExport.Annotations)
150-
assertNotHasKeyValues(serviceImport.Annotations, tt.helloServiceExport2.Annotations)
140+
t.awaitServiceImport(&clients[0], t.helloService.Name, false,
141+
func(g Gomega, serviceImport *v1alpha1.ServiceImport) {
142+
assertHasKeyValues(g, serviceImport.Annotations, helloServiceExport.Annotations)
143+
assertNotHasKeyValues(g, serviceImport.Annotations, tt.helloServiceExport2.Annotations)
151144

152-
assertHasKeyValues(serviceImport.Labels, helloServiceExport.Labels)
153-
assertNotHasKeyValues(serviceImport.Labels, tt.helloServiceExport2.Labels)
145+
assertHasKeyValues(g, serviceImport.Labels, helloServiceExport.Labels)
146+
assertNotHasKeyValues(g, serviceImport.Labels, tt.helloServiceExport2.Labels)
147+
})
154148
})
155149
})
156150
})
@@ -173,9 +167,7 @@ func testClusterIPServiceImport() {
173167
AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#importing-services")
174168

175169
for i := range clients {
176-
serviceImport := t.awaitServiceImport(&clients[i], helloServiceName, nil)
177-
Expect(serviceImport).NotTo(BeNil(), reportNonConformant(fmt.Sprintf("ServiceImport was not found on cluster %q",
178-
clients[i].name)))
170+
serviceImport := t.awaitServiceImport(&clients[i], helloServiceName, true, nil)
179171

180172
Expect(serviceImport.Spec.Type).To(Equal(v1alpha1.ClusterSetIP), reportNonConformant(
181173
fmt.Sprintf("ServiceImport on cluster %q has type %q", clients[i].name, serviceImport.Spec.Type)))
@@ -195,37 +187,32 @@ func testClusterIPServiceImport() {
195187
Label(RequiredLabel), func() {
196188
AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#session-affinity")
197189

198-
serviceImport := t.awaitServiceImport(&clients[0], helloServiceName, nil)
199-
Expect(serviceImport).NotTo(BeNil(), "ServiceImport was not found")
190+
t.awaitServiceImport(&clients[0], helloServiceName, false, func(g Gomega, serviceImport *v1alpha1.ServiceImport) {
191+
g.Expect(serviceImport.Spec.SessionAffinity).To(Equal(t.helloService.Spec.SessionAffinity), reportNonConformant(""))
200192

201-
Expect(serviceImport.Spec.SessionAffinity).To(Equal(t.helloService.Spec.SessionAffinity), reportNonConformant(""))
202-
203-
Expect(serviceImport.Spec.SessionAffinityConfig).To(Equal(t.helloService.Spec.SessionAffinityConfig), reportNonConformant(
204-
"The SessionAffinityConfig of the ServiceImport does not match the exported Service's SessionAffinityConfig"))
193+
g.Expect(serviceImport.Spec.SessionAffinityConfig).To(Equal(t.helloService.Spec.SessionAffinityConfig), reportNonConformant(
194+
"The SessionAffinityConfig of the ServiceImport does not match the exported Service's SessionAffinityConfig"))
195+
})
205196
})
206197

207198
Specify("An IP should be allocated for a ClusterSetIP ServiceImport", Label(RequiredLabel), func() {
208199
AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#clustersetip")
209200

210-
serviceImport := t.awaitServiceImport(&clients[0], t.helloService.Name, func(serviceImport *v1alpha1.ServiceImport) bool {
211-
return len(serviceImport.Spec.IPs) > 0
212-
})
213-
Expect(serviceImport).NotTo(BeNil(), "ServiceImport was not found")
201+
serviceImport := t.awaitServiceImport(&clients[0], t.helloService.Name, false,
202+
func(g Gomega, serviceImport *v1alpha1.ServiceImport) {
203+
g.Expect(serviceImport.Spec.IPs).ToNot(BeEmpty(), reportNonConformant(""))
204+
})
214205

215-
Expect(serviceImport.Spec.IPs).ToNot(BeEmpty(), reportNonConformant(""))
216206
Expect(net.ParseIP(serviceImport.Spec.IPs[0])).ToNot(BeNil(),
217207
reportNonConformant(fmt.Sprintf("The value %q is not a valid IP", serviceImport.Spec.IPs[0])))
218208
})
219209

220210
Specify("The ports for a ClusterSetIP ServiceImport should match those of the exported service", Label(RequiredLabel), func() {
221211
AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#service-port")
222212

223-
serviceImport := t.awaitServiceImport(&clients[0], helloServiceName, func(serviceImport *v1alpha1.ServiceImport) bool {
224-
return len(serviceImport.Spec.Ports) > 0
213+
t.awaitServiceImport(&clients[0], helloServiceName, false, func(g Gomega, serviceImport *v1alpha1.ServiceImport) {
214+
g.Expect(sortMCSPorts(serviceImport.Spec.Ports)).To(Equal(toMCSPorts(t.helloService.Spec.Ports)), reportNonConformant(""))
225215
})
226-
Expect(serviceImport).NotTo(BeNil(), "ServiceImport was not found")
227-
228-
Expect(sortMCSPorts(serviceImport.Spec.Ports)).To(Equal(toMCSPorts(t.helloService.Spec.Ports)), reportNonConformant(""))
229216
})
230217

231218
Context("A ClusterIP service exported on two clusters", func() {
@@ -246,13 +233,11 @@ func testClusterIPServiceImport() {
246233
Specify("should expose the union of the constituent service ports", Label(RequiredLabel), func() {
247234
AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#service-port")
248235

249-
serviceImport := t.awaitServiceImport(&clients[0], t.helloService.Name, func(serviceImport *v1alpha1.ServiceImport) bool {
250-
return len(serviceImport.Spec.Ports) == 3
251-
})
252-
Expect(serviceImport).NotTo(BeNil(), "ServiceImport was not found")
253-
254-
Expect(sortMCSPorts(serviceImport.Spec.Ports)).To(Equal(toMCSPorts(
255-
append(t.helloService.Spec.Ports, tt.helloService2.Spec.Ports[1]))), reportNonConformant(""))
236+
t.awaitServiceImport(&clients[0], t.helloService.Name, false,
237+
func(g Gomega, serviceImport *v1alpha1.ServiceImport) {
238+
g.Expect(sortMCSPorts(serviceImport.Spec.Ports)).To(Equal(toMCSPorts(
239+
append(t.helloService.Spec.Ports, tt.helloService2.Spec.Ports[1]))), reportNonConformant(""))
240+
})
256241
})
257242
})
258243

@@ -268,13 +253,11 @@ func testClusterIPServiceImport() {
268253
t.awaitServiceExportCondition(&clients[0], v1alpha1.ServiceExportConflict, metav1.ConditionTrue)
269254
t.awaitServiceExportCondition(&clients[1], v1alpha1.ServiceExportConflict, metav1.ConditionTrue)
270255

271-
serviceImport := t.awaitServiceImport(&clients[0], t.helloService.Name, func(serviceImport *v1alpha1.ServiceImport) bool {
272-
return len(serviceImport.Spec.Ports) == len(t.helloService.Spec.Ports)
273-
})
274-
Expect(serviceImport).NotTo(BeNil(), "ServiceImport was not found")
275-
276-
Expect(sortMCSPorts(serviceImport.Spec.Ports)).To(Equal(toMCSPorts(t.helloService.Spec.Ports)),
277-
reportNonConformant("The service ports were not resolved correctly"))
256+
t.awaitServiceImport(&clients[0], t.helloService.Name, false,
257+
func(g Gomega, serviceImport *v1alpha1.ServiceImport) {
258+
g.Expect(sortMCSPorts(serviceImport.Spec.Ports)).To(Equal(toMCSPorts(t.helloService.Spec.Ports)),
259+
reportNonConformant("The service ports were not resolved correctly"))
260+
})
278261
})
279262
})
280263
})
@@ -296,9 +279,7 @@ func testHeadlessServiceImport() {
296279
AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#service-types")
297280

298281
for i := range clients {
299-
serviceImport := t.awaitServiceImport(&clients[i], helloServiceName, nil)
300-
Expect(serviceImport).NotTo(BeNil(), reportNonConformant(fmt.Sprintf("ServiceImport was not found on cluster %q",
301-
clients[i].name)))
282+
serviceImport := t.awaitServiceImport(&clients[i], helloServiceName, true, nil)
302283

303284
Expect(serviceImport.Spec.Type).To(Equal(v1alpha1.Headless), reportNonConformant(
304285
fmt.Sprintf("ServiceImport on cluster %q has type %q", clients[i].name, serviceImport.Spec.Type)))
@@ -317,8 +298,7 @@ func testHeadlessServiceImport() {
317298
Specify("No clusterset IP should be allocated for a Headless ServiceImport", Label(RequiredLabel), func() {
318299
AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#clustersetip")
319300

320-
serviceImport := t.awaitServiceImport(&clients[0], t.helloService.Name, nil)
321-
Expect(serviceImport).NotTo(BeNil(), "ServiceImport was not found")
301+
t.awaitServiceImport(&clients[0], t.helloService.Name, false, nil)
322302

323303
Consistently(func() []string {
324304
return t.getServiceImport(&clients[0], t.helloService.Name).Spec.IPs

0 commit comments

Comments
 (0)