Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

tests: Fix flaky TestWatchNamespaceSelector #10160

Merged
merged 4 commits into from
Oct 3, 2024
Merged

tests: Fix flaky TestWatchNamespaceSelector #10160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8184ce2
tests: Fix flaky TestWatchNamespaceSelector
davidjumani Oct 3, 2024
c93fc9b
add changelog
davidjumani Oct 3, 2024
576eec5
skip flakey test to be fixed in subsequent PR
davidjumani Oct 3, 2024
f5c4dfd
add flake attempts
davidjumani Oct 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ analyze:

GINKGO_VERSION ?= $(shell echo $(shell go list -m github.com/onsi/ginkgo/v2) | cut -d' ' -f2)
GINKGO_ENV ?= GOLANG_PROTOBUF_REGISTRATION_CONFLICT=ignore ACK_GINKGO_RC=true ACK_GINKGO_DEPRECATIONS=$(GINKGO_VERSION)
GINKGO_FLAGS ?= -tags=purego --trace -progress -race --fail-fast -fail-on-pending --randomize-all --compilers=5
GINKGO_FLAGS ?= -tags=purego --trace -progress -race --fail-fast -fail-on-pending --randomize-all --compilers=5 --flake-attempts=3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will affect all times we invoke these tests, on PRs and nightlies. I'm good with this, though initially i had envisioned we would only add this for PRs, so that nightlies still give us a good signal. wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I assume the plan is to eventually move away from ginkgo, this should be fine as long as we make strides to migrate tests

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm in favor of reverting this change so we can catch flakes more easily

GINKGO_REPORT_FLAGS ?= --json-report=test-report.json --junit-report=junit.xml -output-dir=$(OUTPUT_DIR)
GINKGO_COVERAGE_FLAGS ?= --cover --covermode=atomic --coverprofile=coverage.cov
TEST_PKG ?= ./... # Default to run all tests
Expand Down
4 changes: 4 additions & 0 deletions changelog/v1.18.0-beta25/fix-watch-ns.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
changelog:
- type: NON_USER_FACING
description: Fixes the flaky TestMatchLabels that could be caused by not removing the label on a namespace after the test.

4 changes: 4 additions & 0 deletions test/kube2e/gloo/bootstrap_clients_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,10 @@ var _ = Describe("Bootstrap Clients", func() {
var verifyTranslation func()

BeforeEach(func() {

// TODO (davidjumani) : Fix in a followup - https://solo-io-corp.slack.com/archives/G01EERAK3KJ/p1727966518844649?thread_ts=1727959118.508609&cid=G01EERAK3KJ
Skip("Skipping flakey test.")

deploymentClient = resourceClientset.KubeClients().AppsV1().Deployments(testHelper.InstallNamespace)

// verifyTranslation creates a VS with a directActionRoute and verifies it has been accepted
Expand Down
5 changes: 5 additions & 0 deletions test/kubernetes/e2e/features/watch_namespace_selector/suite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ func (s *testingSuite) testWatchNamespaceSelector(key, value string) {

// The VS defined in the random namespace should be translated
s.TestInstallation.Assertions.CurlEventuallyRespondsWithStatus(s.Ctx, "random/", http.StatusOK)

s.unwatchNamespace(key)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a "cleanup" step and therefore we should consider performing it in the AfterTest method, or in a deferred function so it is always executed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's only for this specific test and it also serves as a way to verify that unlabelling a namespace leads to the resources defined in it to not be translated


// Ensure CRs defined in non watched-namespaces are not translated
s.TestInstallation.Assertions.CurlConsistentlyRespondsWithStatus(s.Ctx, "random/", http.StatusNotFound)
}

func (s *testingSuite) TestUnwatchedNamespaceValidation() {
Expand Down
13 changes: 10 additions & 3 deletions test/kubernetes/e2e/features/watch_namespace_selector/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/solo-io/gloo/test/kubernetes/e2e/tests/base"
"github.com/solo-io/skv2/codegen/util"
corev1 "k8s.io/api/core/v1"
"sigs.k8s.io/controller-runtime/pkg/client"

gatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1/kube/apis/gateway.solo.io/v1"
Expand All @@ -16,7 +17,13 @@ var (
installNamespaceVSManifest = filepath.Join(util.MustGetThisDir(), "testdata", "vs-install-ns.yaml")

unlabeledRandomNamespaceManifest = filepath.Join(util.MustGetThisDir(), "testdata", "random-ns-unlabeled.yaml")
randomVSManifest = filepath.Join(util.MustGetThisDir(), "testdata", "vs-random.yaml")
randomNamespace = &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "random",
},
}

randomVSManifest = filepath.Join(util.MustGetThisDir(), "testdata", "vs-random.yaml")

randomUpstreamManifest = filepath.Join(util.MustGetThisDir(), "testdata", "upstream-random.yaml")
installNamespaceWithRandomUpstreamVSManifest = filepath.Join(util.MustGetThisDir(), "testdata", "vs-upstream.yaml")
Expand All @@ -36,13 +43,13 @@ var (
"TestMatchLabels": {
SimpleTestCase: base.SimpleTestCase{
Manifests: []string{unlabeledRandomNamespaceManifest, randomVSManifest},
Resources: []client.Object{randomNamespaceVS},
Resources: []client.Object{randomNamespace, randomNamespaceVS},
},
},
"TestMatchExpressions": {
SimpleTestCase: base.SimpleTestCase{
Manifests: []string{unlabeledRandomNamespaceManifest, randomVSManifest},
Resources: []client.Object{randomNamespaceVS},
Resources: []client.Object{randomNamespace, randomNamespaceVS},
},
},
"TestUnwatchedNamespaceValidation": {
Expand Down
Loading