Skip to content

Commit 49a922c

Browse files
sdowellsdowell
authored
test: re-introduce e2e syncing condition validation (#1577)
This was previously commented out due to flakiness.
1 parent 7e75ecf commit 49a922c

21 files changed

+172
-196
lines changed

‎e2e/nomostest/testpredicates/predicates.go‎

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ package testpredicates
1717
import (
1818
"errors"
1919
"fmt"
20-
"reflect"
2120
"sort"
2221
"strings"
2322
"sync"
@@ -986,7 +985,7 @@ func RepoSyncHasRenderingError(errCode, errMessage string) Predicate {
986985

987986
// RootSyncHasSyncError returns an error if the RootSync does not have the
988987
// specified Sync error code and (optional, partial) message.
989-
func RootSyncHasSyncError(errCode, errMessage string) Predicate {
988+
func RootSyncHasSyncError(errCode, errMessage string, resources []v1beta1.ResourceRef) Predicate {
990989
return func(o client.Object) error {
991990
if o == nil {
992991
return ErrObjectNotFound
@@ -995,13 +994,13 @@ func RootSyncHasSyncError(errCode, errMessage string) Predicate {
995994
if !ok {
996995
return WrongTypeErr(o, &v1beta1.RootSync{})
997996
}
998-
return ValidateError(rs.Status.Sync.Errors, errCode, errMessage, nil)
997+
return ValidateError(rs.Status.Sync.Errors, errCode, errMessage, resources)
999998
}
1000999
}
10011000

10021001
// RepoSyncHasSyncError returns an error if the RootSync does not have the
10031002
// specified Sync error code and (optional, partial) message.
1004-
func RepoSyncHasSyncError(errCode, errMessage string) Predicate {
1003+
func RepoSyncHasSyncError(errCode, errMessage string, resources []v1beta1.ResourceRef) Predicate {
10051004
return func(o client.Object) error {
10061005
if o == nil {
10071006
return ErrObjectNotFound
@@ -1010,7 +1009,7 @@ func RepoSyncHasSyncError(errCode, errMessage string) Predicate {
10101009
if !ok {
10111010
return WrongTypeErr(o, &v1beta1.RepoSync{})
10121011
}
1013-
return ValidateError(rs.Status.Sync.Errors, errCode, errMessage, nil)
1012+
return ValidateError(rs.Status.Sync.Errors, errCode, errMessage, resources)
10141013
}
10151014
}
10161015

@@ -1481,7 +1480,7 @@ func ValidateError(errs []v1beta1.ConfigSyncError, code, message string, resourc
14811480
for _, e := range errs {
14821481
if e.Code == code {
14831482
hasMessage = message == "" || strings.Contains(e.ErrorMessage, message)
1484-
hasResources = len(resources) == 0 || reflect.DeepEqual(resources, e.Resources)
1483+
hasResources = len(resources) == 0 || equality.Semantic.DeepEqual(resources, e.Resources)
14851484

14861485
if hasMessage && hasResources {
14871486
return nil
@@ -1496,6 +1495,58 @@ func ValidateError(errs []v1beta1.ConfigSyncError, code, message string, resourc
14961495
return fmt.Errorf("error %s not present: %s", code, log.AsJSON(errs))
14971496
}
14981497

1498+
// RootSyncHasErrorSourceRefs validates that the RootSync has the Syncing condition
1499+
// with the provided errorSourceRefs
1500+
func RootSyncHasErrorSourceRefs(expectedErrorSourceRefs []v1beta1.ErrorSource) Predicate {
1501+
return func(obj client.Object) error {
1502+
if obj == nil {
1503+
return ErrObjectNotFound
1504+
}
1505+
rs, ok := obj.(*v1beta1.RootSync)
1506+
if !ok {
1507+
return WrongTypeErr(obj, &v1beta1.RootSync{})
1508+
}
1509+
1510+
syncingCondition := rootsync.GetCondition(rs.Status.Conditions, v1beta1.RootSyncSyncing)
1511+
if syncingCondition == nil {
1512+
return fmt.Errorf("syncingCondition is nil")
1513+
}
1514+
if syncingCondition.Status == metav1.ConditionTrue {
1515+
return fmt.Errorf("status.conditions['Syncing'].status is True, expected false")
1516+
}
1517+
if !equality.Semantic.DeepEqual(syncingCondition.ErrorSourceRefs, expectedErrorSourceRefs) {
1518+
return fmt.Errorf("status.conditions['Syncing'].errorSourceRefs is %v, expected %v", syncingCondition.ErrorSourceRefs, expectedErrorSourceRefs)
1519+
}
1520+
return nil
1521+
}
1522+
}
1523+
1524+
// RepoSyncHasErrorSourceRefs validates that the RepoSync has the Syncing condition
1525+
// with the provided errorSourceRefs
1526+
func RepoSyncHasErrorSourceRefs(expectedErrorSourceRefs []v1beta1.ErrorSource) Predicate {
1527+
return func(obj client.Object) error {
1528+
if obj == nil {
1529+
return ErrObjectNotFound
1530+
}
1531+
rs, ok := obj.(*v1beta1.RepoSync)
1532+
if !ok {
1533+
return WrongTypeErr(obj, &v1beta1.RepoSync{})
1534+
}
1535+
1536+
syncingCondition := reposync.GetCondition(rs.Status.Conditions, v1beta1.RepoSyncSyncing)
1537+
if syncingCondition == nil {
1538+
return fmt.Errorf("syncingCondition is nil")
1539+
}
1540+
if syncingCondition.Status == metav1.ConditionTrue {
1541+
return fmt.Errorf("status.conditions['Syncing'].status is True, expected false")
1542+
}
1543+
if !equality.Semantic.DeepEqual(syncingCondition.ErrorSourceRefs, expectedErrorSourceRefs) {
1544+
return fmt.Errorf("status.conditions['Syncing'].errorSourceRefs is %v, expected %v", syncingCondition.ErrorSourceRefs, expectedErrorSourceRefs)
1545+
}
1546+
return nil
1547+
}
1548+
}
1549+
14991550
// ConfigMapHasData returns an error if the ConfigMap doesn't contain the given key value pair
15001551
func ConfigMapHasData(key string, value string) Predicate {
15011552
return func(o client.Object) error {

‎e2e/nomostest/testwatcher/watch.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ import (
3535
"kpt.dev/configsync/e2e/nomostest/testkubeclient"
3636
"kpt.dev/configsync/e2e/nomostest/testlogger"
3737
"kpt.dev/configsync/e2e/nomostest/testpredicates"
38+
"kpt.dev/configsync/pkg/api/configsync"
39+
"kpt.dev/configsync/pkg/api/configsync/v1beta1"
3840
"kpt.dev/configsync/pkg/kinds"
3941
"kpt.dev/configsync/pkg/util/log"
4042
watchutil "kpt.dev/configsync/pkg/util/watch"
@@ -403,3 +405,43 @@ func (w *Watcher) WatchForNotFound(gvk schema.GroupVersionKind, name, namespace
403405
opts = append(opts, WatchPredicates(testpredicates.ObjectNotFoundPredicate(w.scheme)))
404406
return w.WatchObject(gvk, name, namespace, opts...)
405407
}
408+
409+
// WatchForRootSyncSourceError waits until the given error (code and message) is present on the RootSync resource
410+
func (w *Watcher) WatchForRootSyncSourceError(rsName, code string, message string, opts ...WatchOption) error {
411+
predicates := []testpredicates.Predicate{
412+
testpredicates.RootSyncHasSourceError(code, message),
413+
testpredicates.RootSyncHasErrorSourceRefs([]v1beta1.ErrorSource{v1beta1.SourceError}),
414+
}
415+
opts = append(opts, WatchPredicates(predicates...))
416+
return w.WatchObject(kinds.RootSyncV1Beta1(), rsName, configsync.ControllerNamespace, opts...)
417+
}
418+
419+
// WatchForRootSyncSyncError waits until the given error (code and message) is present on the RootSync resource
420+
func (w *Watcher) WatchForRootSyncSyncError(rsName, code string, message string, resources []v1beta1.ResourceRef, opts ...WatchOption) error {
421+
predicates := []testpredicates.Predicate{
422+
testpredicates.RootSyncHasSyncError(code, message, resources),
423+
testpredicates.RootSyncHasErrorSourceRefs([]v1beta1.ErrorSource{v1beta1.SyncError}),
424+
}
425+
opts = append(opts, WatchPredicates(predicates...))
426+
return w.WatchObject(kinds.RootSyncV1Beta1(), rsName, configsync.ControllerNamespace, opts...)
427+
}
428+
429+
// WatchForRepoSyncSyncError waits until the given error (code and message) is present on the RepoSync resource
430+
func (w *Watcher) WatchForRepoSyncSyncError(ns, rsName, code string, message string, resources []v1beta1.ResourceRef, opts ...WatchOption) error {
431+
predicates := []testpredicates.Predicate{
432+
testpredicates.RepoSyncHasSyncError(code, message, resources),
433+
testpredicates.RepoSyncHasErrorSourceRefs([]v1beta1.ErrorSource{v1beta1.SyncError}),
434+
}
435+
opts = append(opts, WatchPredicates(predicates...))
436+
return w.WatchObject(kinds.RepoSyncV1Beta1(), rsName, ns, opts...)
437+
}
438+
439+
// WatchForRepoSyncSourceError waits until the given error (code and message) is present on the RepoSync resource
440+
func (w *Watcher) WatchForRepoSyncSourceError(ns, rsName, code, message string, opts ...WatchOption) error {
441+
predicates := []testpredicates.Predicate{
442+
testpredicates.RepoSyncHasSourceError(code, message),
443+
testpredicates.RepoSyncHasErrorSourceRefs([]v1beta1.ErrorSource{v1beta1.SourceError}),
444+
}
445+
opts = append(opts, WatchPredicates(predicates...))
446+
return w.WatchObject(kinds.RepoSyncV1Beta1(), rsName, ns, opts...)
447+
}

‎e2e/nomostest/wait_for_sync.go‎

Lines changed: 0 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -232,89 +232,6 @@ func (nt *NT) WatchForSync(
232232
return nil
233233
}
234234

235-
// WaitForRootSyncSourceError waits until the given error (code and message) is present on the RootSync resource
236-
func (nt *NT) WaitForRootSyncSourceError(rsName, code string, message string, opts ...WaitOption) {
237-
nt.T.Helper()
238-
Wait(nt.T, fmt.Sprintf("RootSync %s source error code %s", rsName, code), nt.DefaultWaitTimeout,
239-
func() error {
240-
nt.T.Helper()
241-
rs := k8sobjects.RootSyncObjectV1Beta1(rsName)
242-
if err := nt.KubeClient.Get(rs.GetName(), rs.GetNamespace(), rs); err != nil {
243-
return err
244-
}
245-
// Only validate the rendering status, not the Syncing condition
246-
// TODO: Remove this hack once async sync status updates are fixed to reflect only the latest commit.
247-
return testpredicates.ValidateError(rs.Status.Source.Errors, code, message, nil)
248-
// syncingCondition := rootsync.GetCondition(rs.Status.Conditions, v1beta1.RootSyncSyncing)
249-
// return validateRootSyncError(rs.Status.Source.Errors, syncingCondition, code, message, []v1beta1.ErrorSource{v1beta1.SourceError})
250-
},
251-
opts...,
252-
)
253-
}
254-
255-
// WaitForRootSyncSyncError waits until the given error (code and message) is present on the RootSync resource
256-
func (nt *NT) WaitForRootSyncSyncError(rsName, code string, message string, resources []v1beta1.ResourceRef, opts ...WaitOption) {
257-
nt.T.Helper()
258-
Wait(nt.T, fmt.Sprintf("RootSync %s sync error code %s", rsName, code), nt.DefaultWaitTimeout,
259-
func() error {
260-
nt.T.Helper()
261-
rs := k8sobjects.RootSyncObjectV1Beta1(rsName)
262-
err := nt.KubeClient.Get(rs.GetName(), rs.GetNamespace(), rs)
263-
if err != nil {
264-
return err
265-
}
266-
// Only validate the sync status, not the Syncing condition
267-
// TODO: Remove this hack once async sync status updates are fixed to reflect only the latest commit.
268-
return testpredicates.ValidateError(rs.Status.Sync.Errors, code, message, resources)
269-
// syncingCondition := rootsync.GetCondition(rs.Status.Conditions, v1beta1.RootSyncSyncing)
270-
// return validateRootSyncError(rs.Status.Sync.Errors, syncingCondition, code, message, []v1beta1.ErrorSource{v1beta1.SyncError})
271-
},
272-
opts...,
273-
)
274-
}
275-
276-
// WaitForRepoSyncSyncError waits until the given error (code and message) is present on the RepoSync resource
277-
func (nt *NT) WaitForRepoSyncSyncError(ns, rsName, code string, message string, resources []v1beta1.ResourceRef, opts ...WaitOption) {
278-
nt.T.Helper()
279-
Wait(nt.T, fmt.Sprintf("RepoSync %s/%s sync error code %s", ns, rsName, code), nt.DefaultWaitTimeout,
280-
func() error {
281-
nt.T.Helper()
282-
rs := k8sobjects.RepoSyncObjectV1Beta1(ns, rsName)
283-
err := nt.KubeClient.Get(rs.GetName(), rs.GetNamespace(), rs)
284-
if err != nil {
285-
return err
286-
}
287-
// Only validate the sync status, not the Syncing condition
288-
// TODO: Remove this hack once async sync status updates are fixed to reflect only the latest commit.
289-
return testpredicates.ValidateError(rs.Status.Sync.Errors, code, message, resources)
290-
// syncingCondition := reposync.GetCondition(rs.Status.Conditions, v1beta1.RepoSyncSyncing)
291-
// return validateRepoSyncError(rs.Status.Sync.Errors, syncingCondition, code, message, []v1beta1.ErrorSource{v1beta1.SyncError})
292-
},
293-
opts...,
294-
)
295-
}
296-
297-
// WaitForRepoSyncSourceError waits until the given error (code and message) is present on the RepoSync resource
298-
func (nt *NT) WaitForRepoSyncSourceError(ns, rsName, code, message string, opts ...WaitOption) {
299-
nt.T.Helper()
300-
Wait(nt.T, fmt.Sprintf("RepoSync %s/%s source error code %s", ns, rsName, code), nt.DefaultWaitTimeout,
301-
func() error {
302-
nt.T.Helper()
303-
rs := k8sobjects.RepoSyncObjectV1Beta1(ns, rsName)
304-
err := nt.KubeClient.Get(rs.GetName(), rs.GetNamespace(), rs)
305-
if err != nil {
306-
return err
307-
}
308-
// Only validate the rendering status, not the Syncing condition
309-
// TODO: Remove this hack once async sync status updates are fixed to reflect only the latest commit.
310-
return testpredicates.ValidateError(rs.Status.Source.Errors, code, message, nil)
311-
// syncingCondition := reposync.GetCondition(rs.Status.Conditions, v1beta1.RepoSyncSyncing)
312-
// return validateRepoSyncError(rs.Status.Source.Errors, syncingCondition, code, message, []v1beta1.ErrorSource{v1beta1.SourceError})
313-
},
314-
opts...,
315-
)
316-
}
317-
318235
// WaitForRootSyncStalledError waits until the given Stalled error is present on the RootSync resource.
319236
func (nt *NT) WaitForRootSyncStalledError(rsName, reason, message string) {
320237
nt.T.Helper()
@@ -378,37 +295,3 @@ func (nt *NT) WaitForRepoSyncStalledError(rsNamespace, rsName, reason, message s
378295
return nil
379296
})
380297
}
381-
382-
// TODO: Uncomment when Syncing condition consistency is fixed
383-
// func validateRootSyncError(statusErrs []v1beta1.ConfigSyncError, syncingCondition *v1beta1.RootSyncCondition, code string, message string, expectedErrorSourceRefs []v1beta1.ErrorSource) error {
384-
// if err := testpredicates.ValidateError(statusErrs, code, message); err != nil {
385-
// return err
386-
// }
387-
// if syncingCondition == nil {
388-
// return fmt.Errorf("syncingCondition is nil")
389-
// }
390-
// if syncingCondition.Status == metav1.ConditionTrue {
391-
// return fmt.Errorf("status.conditions['Syncing'].status is True, expected false")
392-
// }
393-
// if !reflect.DeepEqual(syncingCondition.ErrorSourceRefs, expectedErrorSourceRefs) {
394-
// return fmt.Errorf("status.conditions['Syncing'].errorSourceRefs is %v, expected %v", syncingCondition.ErrorSourceRefs, expectedErrorSourceRefs)
395-
// }
396-
// return nil
397-
// }
398-
399-
// TODO: Uncomment when Syncing condition consistency is fixed
400-
// func validateRepoSyncError(statusErrs []v1beta1.ConfigSyncError, syncingCondition *v1beta1.RepoSyncCondition, code string, message string, expectedErrorSourceRefs []v1beta1.ErrorSource) error {
401-
// if err := testpredicates.ValidateError(statusErrs, code, message); err != nil {
402-
// return err
403-
// }
404-
// if syncingCondition == nil {
405-
// return fmt.Errorf("syncingCondition is nil")
406-
// }
407-
// if syncingCondition.Status == metav1.ConditionTrue {
408-
// return fmt.Errorf("status.conditions['Syncing'].status is True, expected false")
409-
// }
410-
// if !reflect.DeepEqual(syncingCondition.ErrorSourceRefs, expectedErrorSourceRefs) {
411-
// return fmt.Errorf("status.conditions['Syncing'].errorSourceRefs is %v, expected %v", syncingCondition.ErrorSourceRefs, expectedErrorSourceRefs)
412-
// }
413-
// return nil
414-
// }

‎e2e/testcases/cluster_selectors_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -802,7 +802,7 @@ func TestClusterSelectorAnnotationConflicts(t *testing.T) {
802802
})
803803
nt.Must(rootSyncGitRepo.Add("acme/namespaces/eng/backend/bob-rolebinding.yaml", rb))
804804
nt.Must(rootSyncGitRepo.CommitAndPush("Add both cluster selector annotations to a role binding"))
805-
nt.WaitForRootSyncSourceError(configsync.RootSyncName, selectors.ClusterSelectorAnnotationConflictErrorCode, "")
805+
nt.Must(nt.Watcher.WatchForRootSyncSourceError(configsync.RootSyncName, selectors.ClusterSelectorAnnotationConflictErrorCode, ""))
806806

807807
rootSyncNN := nomostest.RootSyncNN(configsync.RootSyncName)
808808
rootSyncLabels, err := nomostest.MetricLabelsForRootSync(nt, rootSyncNN)

‎e2e/testcases/custom_resources_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func TestCRDDeleteBeforeRemoveCustomResourceV1(t *testing.T) {
133133
nt.Must(rootSyncGitRepo.CommitAndPush("Modify Anvil CR"))
134134
secondCommitHash := rootSyncGitRepo.MustHash(nt.T)
135135

136-
nt.WaitForRootSyncSourceError(configsync.RootSyncName, status.UnknownKindErrorCode, "")
136+
nt.Must(nt.Watcher.WatchForRootSyncSourceError(configsync.RootSyncName, status.UnknownKindErrorCode, ""))
137137

138138
rootSyncLabels, err = nomostest.MetricLabelsForRootSync(nt, rootSyncNN)
139139
if err != nil {

‎e2e/testcases/gatekeeper_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ func TestConstraintTemplateAndConstraintInSameCommit(t *testing.T) {
8080
}
8181
})
8282

83-
nt.WaitForRootSyncSourceError(configsync.RootSyncName, status.UnknownKindErrorCode,
84-
`No CustomResourceDefinition is defined for the type "K8sAllowedRepos.constraints.gatekeeper.sh" in the cluster`)
83+
nt.Must(nt.Watcher.WatchForRootSyncSourceError(configsync.RootSyncName, status.UnknownKindErrorCode,
84+
`No CustomResourceDefinition is defined for the type "K8sAllowedRepos.constraints.gatekeeper.sh" in the cluster`))
8585

8686
// Simulate Gatekeeper's controller behavior.
8787
// Wait for the ConstraintTemplate to be applied, then apply the Constraint CRD.

‎e2e/testcases/github_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ func TestGithubAppRootSync(t *testing.T) {
7171
Auth: configsync.AuthNone,
7272
}
7373
nt.Must(nt.KubeClient.Apply(rs))
74-
nt.WaitForRootSyncSourceError(rootSyncNN.Name, status.SourceErrorCode,
75-
`Authentication failed`)
74+
nt.Must(nt.Watcher.WatchForRootSyncSourceError(rootSyncNN.Name, status.SourceErrorCode,
75+
`Authentication failed`))
7676
// Verify reconciler-manager checks existence of Secret
7777
nt.T.Log("The reconciler-manager should report a validation error for missing Secret")
7878
emptySecretNN := types.NamespacedName{Name: "empty-secret", Namespace: rootSyncNN.Namespace}
@@ -169,8 +169,8 @@ func TestGithubAppRepoSync(t *testing.T) {
169169
Auth: configsync.AuthNone,
170170
}
171171
nt.Must(nt.KubeClient.Apply(rs))
172-
nt.WaitForRepoSyncSourceError(repoSyncNN.Namespace, repoSyncNN.Name, status.SourceErrorCode,
173-
`Authentication failed`)
172+
nt.Must(nt.Watcher.WatchForRepoSyncSourceError(repoSyncNN.Namespace, repoSyncNN.Name, status.SourceErrorCode,
173+
`Authentication failed`))
174174
// Verify reconciler-manager checks existence of Secret
175175
nt.T.Log("The reconciler-manager should report a validation error for missing Secret")
176176
emptySecretNN := types.NamespacedName{

‎e2e/testcases/helm_sync_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ func TestHelmNamespaceRepo(t *testing.T) {
482482
rs := nt.RepoSyncObjectHelm(repoSyncNN, chart.HelmChartID)
483483
nt.Must(rootSyncGitRepo.Add(nomostest.StructuredNSPath(repoSyncNN.Namespace, repoSyncNN.Name), rs))
484484
nt.Must(rootSyncGitRepo.CommitAndPush("Update RepoSync to sync from a Helm Chart with cluster-scoped resources"))
485-
nt.WaitForRepoSyncSourceError(repoSyncNN.Namespace, repoSyncNN.Name, nonhierarchical.BadScopeErrCode, "must be Namespace-scoped type")
485+
nt.Must(nt.Watcher.WatchForRepoSyncSourceError(repoSyncNN.Namespace, repoSyncNN.Name, nonhierarchical.BadScopeErrCode, "must be Namespace-scoped type"))
486486

487487
nt.T.Log("Update the helm chart with only a namespace-scope resource")
488488
validChart, err := nt.BuildAndPushHelmPackage(repoSyncNN,

‎e2e/testcases/invalid_auth_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestInvalidAuth(t *testing.T) {
4848
if err := nomostest.SetupFakeSSHCreds(nt, rs.Kind, nomostest.RootSyncNN(rs.Name), configsync.AuthSSH, controllers.GitCredentialVolume); err != nil {
4949
nt.T.Fatal(err)
5050
}
51-
nt.WaitForRootSyncSourceError(configsync.RootSyncName, status.SourceErrorCode, "Permission denied")
51+
nt.Must(nt.Watcher.WatchForRootSyncSourceError(configsync.RootSyncName, status.SourceErrorCode, "Permission denied"))
5252

5353
rootSyncNN := nomostest.RootSyncNN(configsync.RootSyncName)
5454
rootSyncLabels, err := nomostest.MetricLabelsForRootSync(nt, rootSyncNN)

0 commit comments

Comments
 (0)