Skip to content

Commit

Permalink
Refactor unrelated tests to use generalized anti-flake measures - pass
Browse files Browse the repository at this point in the history
  • Loading branch information
analogue committed Jan 8, 2024
1 parent 0a2eedd commit f1120f1
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package serviceaccount
import (
"context"
"testing"
"time"

"github.com/google/go-cmp/cmp"
"google.golang.org/protobuf/proto"
Expand All @@ -30,7 +29,6 @@ import (
"github.com/hashicorp/consul-k8s/control-plane/api/common"
inject "github.com/hashicorp/consul-k8s/control-plane/connect-inject/common"
"github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants"
"github.com/hashicorp/consul-k8s/control-plane/consul"
"github.com/hashicorp/consul-k8s/control-plane/helper/test"
)

Expand Down Expand Up @@ -226,13 +224,6 @@ func runReconcileCase(t *testing.T, tc reconcileCase) {
DenyK8sNamespacesSet: mapset.NewSetWith(),
},
}
resourceClient, err := consul.NewResourceServiceClient(sa.ConsulServerConnMgr)
require.NoError(t, err)

require.Eventually(t, func() bool {
_, _, err := testClient.APIClient.Partitions().Read(context.Background(), constants.DefaultConsulPartition, nil)
return err == nil
}, 5*time.Second, 500*time.Millisecond)

// Default ns and partition if not specified in test.
if tc.targetConsulNs == "" {
Expand All @@ -245,9 +236,9 @@ func runReconcileCase(t *testing.T, tc reconcileCase) {
// If existing resource specified, create it and ensure it exists.
if tc.existingResource != nil {
writeReq := &pbresource.WriteRequest{Resource: tc.existingResource}
_, err = resourceClient.Write(context.Background(), writeReq)
_, err := testClient.ResourceClient.Write(context.Background(), writeReq)
require.NoError(t, err)
test.ResourceHasPersisted(t, context.Background(), resourceClient, tc.existingResource.Id)
test.ResourceHasPersisted(t, context.Background(), testClient.ResourceClient, tc.existingResource.Id)
}

// Run actual reconcile and verify results.
Expand All @@ -264,7 +255,7 @@ func runReconcileCase(t *testing.T, tc reconcileCase) {
}
require.False(t, resp.Requeue)

expectedWorkloadIdentityMatches(t, resourceClient, tc.svcAccountName, tc.targetConsulNs, tc.targetConsulPartition, tc.expectedResource)
expectedWorkloadIdentityMatches(t, testClient.ResourceClient, tc.svcAccountName, tc.targetConsulNs, tc.targetConsulPartition, tc.expectedResource)
}

func expectedWorkloadIdentityMatches(t *testing.T, client pbresource.ResourceServiceClient, name, namespace, partition string, expectedResource *pbresource.Resource) {
Expand Down
4 changes: 2 additions & 2 deletions control-plane/helper/test/test_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ func TestServerWithMockConnMgrWatcher(t *testing.T, callback testutil.ServerConf
// also cause test flakes.
require.Eventually(t,
func() bool {
_, _, err := client.Partitions().Read(context.Background(), constants.DefaultConsulPartition, nil)
return err == nil
partition, _, err := client.Partitions().Read(context.Background(), constants.DefaultConsulPartition, nil)
return err == nil && partition != nil
},
eventuallyWaitFor,
eventuallyTickEvery,
Expand Down
7 changes: 2 additions & 5 deletions control-plane/subcommand/mesh-init/command_ent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/stretchr/testify/require"

"github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants"
"github.com/hashicorp/consul-k8s/control-plane/consul"
"github.com/hashicorp/consul-k8s/control-plane/helper/test"
"github.com/hashicorp/consul-k8s/control-plane/namespaces"
)
Expand Down Expand Up @@ -52,10 +51,8 @@ func TestRun_WithNamespaces(t *testing.T) {
c.Experiments = []string{"resource-apis"}
serverCfg = c
})
resourceClient, err := consul.NewResourceServiceClient(testClient.Watcher)
require.NoError(t, err)

_, err = EnsurePartitionExists(testClient.APIClient, c.consulPartition)
_, err := EnsurePartitionExists(testClient.APIClient, c.consulPartition)
require.NoError(t, err)

partitionedCfg := testClient.Cfg.APIClientConfig
Expand All @@ -68,7 +65,7 @@ func TestRun_WithNamespaces(t *testing.T) {
require.NoError(t, err)

// Register Consul workload.
loadResource(t, resourceClient, getWorkloadID(testPodName, c.consulNamespace, c.consulPartition), getWorkload(), nil)
loadResource(t, testClient.ResourceClient, getWorkloadID(testPodName, c.consulNamespace, c.consulPartition), getWorkload(), nil)

ui := cli.NewMockUi()
cmd := Command{
Expand Down
17 changes: 5 additions & 12 deletions control-plane/subcommand/mesh-init/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"google.golang.org/protobuf/types/known/anypb"

"github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants"
"github.com/hashicorp/consul-k8s/control-plane/consul"
"github.com/hashicorp/consul-k8s/control-plane/helper/test"
)

Expand Down Expand Up @@ -100,11 +99,9 @@ func TestRun_MeshServices(t *testing.T) {
c.Experiments = []string{"resource-apis"}
serverCfg = c
})
resourceClient, err := consul.NewResourceServiceClient(testClient.Watcher)
require.NoError(t, err)

loadResource(t, resourceClient, getWorkloadID(testPodName, constants.DefaultConsulNS, constants.DefaultConsulPartition), tt.workload, nil)
loadResource(t, resourceClient, getProxyConfigurationID(testPodName, constants.DefaultConsulNS, constants.DefaultConsulPartition), tt.proxyConfiguration, nil)
loadResource(t, testClient.ResourceClient, getWorkloadID(testPodName, constants.DefaultConsulNS, constants.DefaultConsulPartition), tt.workload, nil)
loadResource(t, testClient.ResourceClient, getProxyConfigurationID(testPodName, constants.DefaultConsulNS, constants.DefaultConsulPartition), tt.proxyConfiguration, nil)

ui := cli.NewMockUi()
cmd := Command{
Expand Down Expand Up @@ -164,8 +161,6 @@ func TestRun_RetryServicePolling(t *testing.T) {
c.Experiments = []string{"resource-apis"}
serverCfg = c
})
resourceClient, err := consul.NewResourceServiceClient(testClient.Watcher)
require.NoError(t, err)

// Start the consul service registration in a go func and delay it so that it runs
// after the cmd.Run() starts.
Expand All @@ -176,7 +171,7 @@ func TestRun_RetryServicePolling(t *testing.T) {
// Wait a moment, this ensures that we are already in the retry logic.
time.Sleep(time.Second * 2)
// Register counting service.
loadResource(t, resourceClient, getWorkloadID(testPodName, constants.DefaultConsulNS, constants.DefaultConsulPartition), getWorkload(), nil)
loadResource(t, testClient.ResourceClient, getWorkloadID(testPodName, constants.DefaultConsulNS, constants.DefaultConsulPartition), getWorkload(), nil)
}()

ui := cli.NewMockUi()
Expand Down Expand Up @@ -240,16 +235,14 @@ func TestRun_TrafficRedirection(t *testing.T) {
c.Experiments = []string{"resource-apis"}
serverCfg = c
})
resourceClient, err := consul.NewResourceServiceClient(testClient.Watcher)
require.NoError(t, err)

// Add additional proxy configuration either to a config entry or to the service itself.
if c.registerProxyConfiguration {
loadResource(t, resourceClient, getProxyConfigurationID(testPodName, constants.DefaultConsulNS, constants.DefaultConsulPartition), getProxyConfiguration(), nil)
loadResource(t, testClient.ResourceClient, getProxyConfigurationID(testPodName, constants.DefaultConsulNS, constants.DefaultConsulPartition), getProxyConfiguration(), nil)
}

// Register Consul workload.
loadResource(t, resourceClient, getWorkloadID(testPodName, constants.DefaultConsulNS, constants.DefaultConsulPartition), getWorkload(), nil)
loadResource(t, testClient.ResourceClient, getWorkloadID(testPodName, constants.DefaultConsulNS, constants.DefaultConsulPartition), getWorkload(), nil)

iptablesProvider := &fakeIptablesProvider{}
iptablesCfg := iptables.Config{
Expand Down
16 changes: 3 additions & 13 deletions control-plane/subcommand/partition-init/command_ent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
pbtenancy "github.com/hashicorp/consul/proto-public/pbtenancy/v2beta1"
"github.com/hashicorp/consul/sdk/testutil"

"github.com/hashicorp/consul-k8s/control-plane/consul"
"github.com/hashicorp/consul-k8s/control-plane/helper/test"
)

Expand Down Expand Up @@ -96,10 +95,7 @@ func TestRun_PartitionCreate(t *testing.T) {
v2tenancy: true,
experiments: []string{"resource-apis", "v2tenancy"},
requirePartitionCreated: func(testClient *test.TestServerClient) {
resourceClient, err := consul.NewResourceServiceClient(testClient.Watcher)
require.NoError(t, err)

_, err = resourceClient.Read(context.Background(), &pbresource.ReadRequest{
_, err := testClient.ResourceClient.Read(context.Background(), &pbresource.ReadRequest{
Id: &pbresource.ID{
Name: partitionName,
Type: pbtenancy.PartitionType,
Expand Down Expand Up @@ -180,13 +176,10 @@ func TestRun_PartitionExists(t *testing.T) {
v2tenancy: true,
experiments: []string{"resource-apis", "v2tenancy"},
preCreatePartition: func(testClient *test.TestServerClient) {
resourceClient, err := consul.NewResourceServiceClient(testClient.Watcher)
require.NoError(t, err)

data, err := anypb.New(&pbtenancy.Partition{Description: partitionDesc})
require.NoError(t, err)

_, err = resourceClient.Write(context.Background(), &pbresource.WriteRequest{
_, err = testClient.ResourceClient.Write(context.Background(), &pbresource.WriteRequest{
Resource: &pbresource.Resource{
Id: &pbresource.ID{
Name: partitionName,
Expand All @@ -198,10 +191,7 @@ func TestRun_PartitionExists(t *testing.T) {
require.NoError(t, err)
},
requirePartitionNotCreated: func(testClient *test.TestServerClient) {
resourceClient, err := consul.NewResourceServiceClient(testClient.Watcher)
require.NoError(t, err)

rsp, err := resourceClient.Read(context.Background(), &pbresource.ReadRequest{
rsp, err := testClient.ResourceClient.Read(context.Background(), &pbresource.ReadRequest{
Id: &pbresource.ID{
Name: partitionName,
Type: pbtenancy.PartitionType,
Expand Down

0 comments on commit f1120f1

Please sign in to comment.