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

Flaky ENT Tests fix #1886

Merged
merged 3 commits into from
Feb 26, 2024
Merged

Flaky ENT Tests fix #1886

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
072920f
- fixing flaky tests, yay!
kkavish Feb 23, 2024
4ab95bf
- fixing flaky tests, yay!
kkavish Feb 23, 2024
0d772e4
- fixing flaky tests, yay!
kkavish Feb 23, 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
64 changes: 4 additions & 60 deletions dependency/catalog_node_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func TestCatalogNodeQuery_Fetch(t *testing.T) {
cases := tenancyHelper.GenerateNonDefaultTenancyTests(func(tenancy *test.Tenancy) []interface{} {
return []interface{}{
testCase{
tenancyHelper.AppendTenancyInfo("local", tenancy),
tenancyHelper.AppendTenancyInfo("local", tenancy), // the agent has nothing registered, we use fake node name for registration
"",
&CatalogNode{
Node: &Node{
Expand All @@ -152,50 +152,22 @@ func TestCatalogNodeQuery_Fetch(t *testing.T) {
},
},
Services: []*CatalogNodeService{
{
ID: "conn-enabled-service-default-default",
Service: "conn-enabled-service-default-default",
Tags: ServiceTags([]string{}),
Meta: map[string]string{},
Port: 12345,
},
{
ID: "conn-enabled-service-proxy-default-default",
Service: "conn-enabled-service-proxy-default-default",
Tags: ServiceTags([]string{}),
Meta: map[string]string{},
Port: 21999,
},
{
ID: "consul",
Service: "consul",
Port: testConsul.Config.Ports.Server,
Tags: ServiceTags([]string{}),
Meta: map[string]string{},
},
{
ID: "service-meta-default-default",
Service: "service-meta-default-default",
Tags: ServiceTags([]string{"tag1"}),
Meta: map[string]string{
"meta1": "value1",
},
},
{
ID: "service-taggedAddresses-default-default",
Service: "service-taggedAddresses-default-default",
Tags: ServiceTags([]string{}),
Meta: map[string]string{},
},
},
},
},
testCase{
tenancyHelper.AppendTenancyInfo("partition and ns", tenancy),
fmt.Sprintf("%s?partition=%s&ns=%s", testConsul.Config.NodeName, tenancy.Partition, tenancy.Namespace),
fmt.Sprintf("%s?partition=%s&ns=%s", "node"+tenancy.Partition, tenancy.Partition, tenancy.Namespace),
&CatalogNode{
Node: &Node{
Node: testConsul.Config.NodeName,
Node: "node" + tenancy.Partition,
Address: testConsul.Config.Bind,
Datacenter: "dc1",
TaggedAddresses: map[string]string{
Expand Down Expand Up @@ -250,7 +222,7 @@ func TestCatalogNodeQuery_Fetch(t *testing.T) {
cases = append(cases, tenancyHelper.GenerateDefaultTenancyTests(func(tenancy *test.Tenancy) []interface{} {
return []interface{}{
testCase{
tenancyHelper.AppendTenancyInfo("local", tenancy),
tenancyHelper.AppendTenancyInfo("local", tenancy), // the agent has nothing registered, we use fake node name for registration
"",
&CatalogNode{
Node: &Node{
Expand All @@ -266,41 +238,13 @@ func TestCatalogNodeQuery_Fetch(t *testing.T) {
},
},
Services: []*CatalogNodeService{
{
ID: "conn-enabled-service-default-default",
Service: "conn-enabled-service-default-default",
Tags: ServiceTags([]string{}),
Meta: map[string]string{},
Port: 12345,
},
{
ID: "conn-enabled-service-proxy-default-default",
Service: "conn-enabled-service-proxy-default-default",
Tags: ServiceTags([]string{}),
Meta: map[string]string{},
Port: 21999,
},
{
ID: "consul",
Service: "consul",
Port: testConsul.Config.Ports.Server,
Tags: ServiceTags([]string{}),
Meta: map[string]string{},
},
{
ID: "service-meta-default-default",
Service: "service-meta-default-default",
Tags: ServiceTags([]string{"tag1"}),
Meta: map[string]string{
"meta1": "value1",
},
},
{
ID: "service-taggedAddresses-default-default",
Service: "service-taggedAddresses-default-default",
Tags: ServiceTags([]string{}),
Meta: map[string]string{},
},
},
},
},
Expand Down
40 changes: 38 additions & 2 deletions dependency/catalog_nodes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,26 @@ func TestCatalogNodesQuery_Fetch(t *testing.T) {
//"consul-network-segment": "",
},
},
{
Node: "nodedefault",
Address: testConsul.Config.Bind,
Datacenter: "dc1",
TaggedAddresses: map[string]string{
//"lan": "127.0.0.1",
//"wan": "127.0.0.1",
},
Meta: map[string]string{
//"consul-network-segment": "",
},
},
},
},
testCase{
tenancyHelper.AppendTenancyInfo("partition and namespace", tenancy),
fmt.Sprintf("?partition=%s&ns=%s@dc1", tenancy.Partition, tenancy.Namespace),
[]*Node{
{
Node: testConsul.Config.NodeName,
Node: "node" + tenancy.Partition,
Address: testConsul.Config.Bind,
Datacenter: "dc1",
TaggedAddresses: map[string]string{
Expand Down Expand Up @@ -198,14 +210,26 @@ func TestCatalogNodesQuery_Fetch(t *testing.T) {
//"consul-network-segment": "",
},
},
{
Node: "nodedefault",
Address: testConsul.Config.Bind,
Datacenter: "dc1",
TaggedAddresses: map[string]string{
//"lan": "127.0.0.1",
//"wan": "127.0.0.1",
},
Meta: map[string]string{
//"consul-network-segment": "",
},
},
},
},
testCase{
tenancyHelper.AppendTenancyInfo("partition", tenancy),
fmt.Sprintf("?partition=%s@dc1", tenancy.Partition),
[]*Node{
{
Node: testConsul.Config.NodeName,
Node: "node" + tenancy.Partition,
Address: testConsul.Config.Bind,
Datacenter: "dc1",
TaggedAddresses: map[string]string{
Expand Down Expand Up @@ -239,6 +263,18 @@ func TestCatalogNodesQuery_Fetch(t *testing.T) {
//"consul-network-segment": "",
},
},
{
Node: "node" + tenancy.Partition,
Address: testConsul.Config.Bind,
Datacenter: "dc1",
TaggedAddresses: map[string]string{
//"lan": "127.0.0.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is commented. I feel that we should remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could, but I want to keep it to investigate why it's not working intermittently.

//"wan": "127.0.0.1",
},
Meta: map[string]string{
//"consul-network-segment": "",
},
},
},
},
}
Expand Down
4 changes: 2 additions & 2 deletions dependency/catalog_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func TestCatalogServiceQuery_Fetch(t *testing.T) {
"service-meta-default-default",
[]*CatalogService{
{
Node: testConsul.Config.NodeName,
Node: "node" + tenancy.Partition,
Address: testConsul.Config.Bind,
Datacenter: "dc1",
TaggedAddresses: map[string]string{
Expand Down Expand Up @@ -253,7 +253,7 @@ func TestCatalogServiceQuery_Fetch(t *testing.T) {
fmt.Sprintf("service-meta-%s-%s?ns=%s&partition=%s", tenancy.Partition, tenancy.Namespace, tenancy.Namespace, tenancy.Partition),
[]*CatalogService{
{
Node: testConsul.Config.NodeName,
Node: "node" + tenancy.Partition,
Address: testConsul.Config.Bind,
Datacenter: "dc1",
TaggedAddresses: map[string]string{
Expand Down
9 changes: 2 additions & 7 deletions dependency/dependency_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,19 +139,14 @@ func TestMain(m *testing.M) {

func (c *ClientSet) createConsulTestResources() error {
catalog := testClients.Consul().Catalog()

node, err := testClients.Consul().Agent().NodeName()
if err != nil {
return err
}

for _, tenancy := range tenancyHelper.TestTenancies() {
partition := ""
namespace := ""
if tenancyHelper.IsConsulEnterprise() {
partition = tenancy.Partition
namespace = tenancy.Namespace
}
node := "node" + tenancy.Partition
// service with meta data
serviceMetaService := &api.AgentService{
ID: fmt.Sprintf("service-meta-%s-%s", tenancy.Partition, tenancy.Namespace),
Expand Down Expand Up @@ -241,6 +236,7 @@ func (c *ClientSet) createConsulTestResources() error {
if err := testClients.createConsulPeerings(tenancy); err != nil {
return err
}
time.Sleep(200 * time.Millisecond)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this needed? The creation of resources should be in sync right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is to make sure that consul reaches a state of entropy.
the falky tests were due to wrong way of creating resource but the consul servers were not reaching a state of entropy and thus were not evicting the resources sometimes.

Copy link
Member

Choose a reason for hiding this comment

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

If you changed all the registrations so they're not to the local agent but to a fake node then agent entropy won't matter so there should be no need for this sleep

Copy link
Contributor Author

Choose a reason for hiding this comment

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

noted, I had it in place so that if someone accidentally writes to the local agent, the tests would keep on failing, without getting successful intermittently.

What do you suggest?

}

return nil
Expand Down Expand Up @@ -562,7 +558,6 @@ func (c *ClientSet) createConsulPartitions() error {

return nil
}

func (c *ClientSet) createConsulNs() error {
for _, tenancy := range tenancyHelper.TestTenancies() {
if tenancy.Namespace != "" && tenancy.Namespace != "default" {
Expand Down
8 changes: 4 additions & 4 deletions dependency/health_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ func TestHealthServiceQuery_Fetch(t *testing.T) {
"service-meta-default-default",
[]*HealthService{
{
Node: testConsul.Config.NodeName,
Node: "node" + tenancy.Partition,
NodeAddress: testConsul.Config.Bind,
NodeTaggedAddresses: map[string]string{
//"lan": "127.0.0.1",
Expand Down Expand Up @@ -477,7 +477,7 @@ func TestHealthServiceQuery_Fetch(t *testing.T) {
"service-taggedAddresses-default-default",
[]*HealthService{
{
Node: testConsul.Config.NodeName,
Node: "node" + tenancy.Partition,
NodeAddress: testConsul.Config.Bind,
NodeTaggedAddresses: map[string]string{
//"lan": "127.0.0.1",
Expand Down Expand Up @@ -580,7 +580,7 @@ func TestHealthServiceQuery_Fetch(t *testing.T) {
fmt.Sprintf("service-meta-%s-%s?partition=%s&ns=%s", tenancy.Partition, tenancy.Namespace, tenancy.Partition, tenancy.Namespace),
[]*HealthService{
{
Node: testConsul.Config.NodeName,
Node: "node" + tenancy.Partition,
NodeAddress: testConsul.Config.Bind,
NodeTaggedAddresses: map[string]string{
//"lan": "127.0.0.1",
Expand Down Expand Up @@ -609,7 +609,7 @@ func TestHealthServiceQuery_Fetch(t *testing.T) {
fmt.Sprintf("service-taggedAddresses-%s-%s?partition=%s&ns=%s", tenancy.Partition, tenancy.Namespace, tenancy.Partition, tenancy.Namespace),
[]*HealthService{
{
Node: testConsul.Config.NodeName,
Node: "node" + tenancy.Partition,
NodeAddress: testConsul.Config.Bind,
NodeTaggedAddresses: map[string]string{
//"lan": "127.0.0.1",
Expand Down
2 changes: 1 addition & 1 deletion dependency/kv_get_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func TestKVGetQuery_Fetch(t *testing.T) {
testCase{
tenancyHelper.AppendTenancyInfo("exists", tenancy),
"test-kv-get/key",
fmt.Sprintf("value-%s-%s", tenancy.Partition, tenancy.Namespace),
"value-default-default",
},
testCase{
tenancyHelper.AppendTenancyInfo("exists_empty_string", tenancy),
Expand Down
12 changes: 6 additions & 6 deletions dependency/kv_keys_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,18 +272,18 @@ func TestKVKeysQuery_Fetch(t *testing.T) {
tenancyHelper.AppendTenancyInfo("exists", tenancy),
"test-kv-keys/prefix",
[]string{
fmt.Sprintf("foo-%s-%s", tenancy.Partition, tenancy.Namespace),
fmt.Sprintf("wave/ocean-%s-%s", tenancy.Partition, tenancy.Namespace),
fmt.Sprintf("zip-%s-%s", tenancy.Partition, tenancy.Namespace),
"foo-default-default",
"wave/ocean-default-default",
"zip-default-default",
},
},
testCase{
tenancyHelper.AppendTenancyInfo("trailing", tenancy),
"test-kv-keys/prefix/",
[]string{
fmt.Sprintf("foo-%s-%s", tenancy.Partition, tenancy.Namespace),
fmt.Sprintf("wave/ocean-%s-%s", tenancy.Partition, tenancy.Namespace),
fmt.Sprintf("zip-%s-%s", tenancy.Partition, tenancy.Namespace),
"foo-default-default",
"wave/ocean-default-default",
"zip-default-default",
},
},
testCase{
Expand Down
12 changes: 6 additions & 6 deletions dependency/kv_list_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,17 +299,17 @@ func TestKVListQuery_Fetch(t *testing.T) {
{
Path: "test-kv-list/prefix/foo",
Key: "foo",
Value: fmt.Sprintf("bar-%s-%s", tenancy.Partition, tenancy.Namespace),
Value: "bar-default-default",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this change needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, segregated the default cases for CE.
but the resource creation is common and uses tenancy name.

},
{
Path: "test-kv-list/prefix/wave/ocean",
Key: "wave/ocean",
Value: fmt.Sprintf("sleek-%s-%s", tenancy.Partition, tenancy.Namespace),
Value: "sleek-default-default",
},
{
Path: "test-kv-list/prefix/zip",
Key: "zip",
Value: fmt.Sprintf("zap-%s-%s", tenancy.Partition, tenancy.Namespace),
Value: "zap-default-default",
},
},
},
Expand All @@ -320,17 +320,17 @@ func TestKVListQuery_Fetch(t *testing.T) {
{
Path: "test-kv-list/prefix/foo",
Key: "foo",
Value: fmt.Sprintf("bar-%s-%s", tenancy.Partition, tenancy.Namespace),
Value: "bar-default-default",
},
{
Path: "test-kv-list/prefix/wave/ocean",
Key: "wave/ocean",
Value: fmt.Sprintf("sleek-%s-%s", tenancy.Partition, tenancy.Namespace),
Value: "sleek-default-default",
},
{
Path: "test-kv-list/prefix/zip",
Key: "zip",
Value: fmt.Sprintf("zap-%s-%s", tenancy.Partition, tenancy.Namespace),
Value: "zap-default-default",
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions test/tenancy_helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func (t *TenancyHelper) GenerateTenancyTests(generationFunc func(tenancy *Tenanc
func (t *TenancyHelper) GenerateNonDefaultTenancyTests(generationFunc func(tenancy *Tenancy) []interface{}) []interface{} {
cases := make([]interface{}, 0)
for _, tenancy := range t.TestTenancies() {
if tenancy.Partition != "default" || tenancy.Namespace != "default" {
if tenancy.Partition != "default" {
cases = append(cases, generationFunc(tenancy)...)
}
}
Expand All @@ -135,7 +135,7 @@ func (t *TenancyHelper) GenerateNonDefaultTenancyTests(generationFunc func(tenan
func (t *TenancyHelper) GenerateDefaultTenancyTests(generationFunc func(tenancy *Tenancy) []interface{}) []interface{} {
cases := make([]interface{}, 0)
for _, tenancy := range t.TestTenancies() {
if tenancy.Partition == "default" && tenancy.Namespace == "default" {
if tenancy.Partition == "default" {
cases = append(cases, generationFunc(tenancy)...)
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/tenancy_helper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func TestGenerateDefaultTenancyTests(t *testing.T) {
fakeTest{name: "CE2"},
}
}},
{name: "ENT", isConsulEnterprise: true, expectedTests: 2, input: func(tenancy *Tenancy) []interface{} {
{name: "ENT", isConsulEnterprise: true, expectedTests: 4, input: func(tenancy *Tenancy) []interface{} {
return []interface{}{
fakeTest{name: "ENT1"},
fakeTest{name: "ENT2"},
Expand Down Expand Up @@ -179,7 +179,7 @@ func TestGenerateNonDefaultTenancyTests(t *testing.T) {
fakeTest{name: "CE2"},
}
}},
{name: "ENT", isConsulEnterprise: true, expectedTests: 6, input: func(tenancy *Tenancy) []interface{} {
{name: "ENT", isConsulEnterprise: true, expectedTests: 4, input: func(tenancy *Tenancy) []interface{} {
return []interface{}{
fakeTest{name: "ENT1"},
fakeTest{name: "ENT2"},
Expand Down
Loading