Skip to content

Commit

Permalink
Fix peer test flakes.
Browse files Browse the repository at this point in the history 
This commit fixes an issue where the peering tests would flake due
to the fact that we were concurrently modifying a global map. It
also adds in retry logic so that the consul servers have sufficient
time to initialize before attempting to generate peering tokens.
  • Loading branch information
@hashi-derek
hashi-derek committed Aug 22, 2023
1 parent 4c95f8f commit fe97fc0
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 19 deletions.
19 changes: 12 additions & 7 deletions control-plane/connect-inject/controllers/peering/peering_acceptor_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/utils/pointer"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
Expand Down Expand Up @@ -501,7 +500,8 @@ func TestReconcile_CreateUpdatePeeringAcceptor(t *testing.T) {
// Create fake k8s client
k8sObjects := append(tt.k8sObjects(), &ns)

s := scheme.Scheme
s := runtime.NewScheme()
corev1.AddToScheme(s)
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringAcceptor{}, &v1alpha1.PeeringAcceptorList{})
fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build()

Expand Down Expand Up @@ -622,7 +622,8 @@ func TestReconcile_DeletePeeringAcceptor(t *testing.T) {
k8sObjects := []runtime.Object{&ns, acceptor}

// Add peering types to the scheme.
s := scheme.Scheme
s := runtime.NewScheme()
corev1.AddToScheme(s)
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringAcceptor{}, &v1alpha1.PeeringAcceptorList{})
fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build()

Expand Down Expand Up @@ -768,7 +769,8 @@ func TestReconcile_VersionAnnotation(t *testing.T) {
// Create fake k8s client
k8sObjects := []runtime.Object{acceptor, secret, ns}

s := scheme.Scheme
s := runtime.NewScheme()
corev1.AddToScheme(s)
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringAcceptor{}, &v1alpha1.PeeringAcceptorList{})
fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build()

Expand Down Expand Up @@ -1080,7 +1082,8 @@ func TestAcceptorUpdateStatus(t *testing.T) {
k8sObjects = append(k8sObjects, tt.peeringAcceptor)

// Add peering types to the scheme.
s := scheme.Scheme
s := runtime.NewScheme()
corev1.AddToScheme(s)
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringAcceptor{}, &v1alpha1.PeeringAcceptorList{})
fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build()
// Create the peering acceptor controller.
Expand Down Expand Up @@ -1192,7 +1195,8 @@ func TestAcceptorUpdateStatusError(t *testing.T) {
k8sObjects = append(k8sObjects, tt.acceptor)

// Add peering types to the scheme.
s := scheme.Scheme
s := runtime.NewScheme()
corev1.AddToScheme(s)
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringAcceptor{}, &v1alpha1.PeeringAcceptorList{})
fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build()
// Create the peering acceptor controller.
Expand Down Expand Up @@ -1476,7 +1480,8 @@ func TestAcceptor_RequestsForPeeringTokens(t *testing.T) {

for name, tt := range cases {
t.Run(name, func(t *testing.T) {
s := scheme.Scheme
s := runtime.NewScheme()
corev1.AddToScheme(s)
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringAcceptor{}, &v1alpha1.PeeringAcceptorList{})
fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(tt.secret, &tt.acceptors).Build()
controller := AcceptorController{
Expand Down
34 changes: 22 additions & 12 deletions control-plane/connect-inject/controllers/peering/peering_dialer_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/utils/pointer"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
Expand Down Expand Up @@ -280,9 +279,11 @@ func TestReconcile_CreateUpdatePeeringDialer(t *testing.T) {
var encodedPeeringToken string
if tt.peeringName != "" {
// Create the initial token.
baseToken, _, err := acceptorClient.Peerings().GenerateToken(context.Background(), api.PeeringGenerateTokenRequest{PeerName: tt.peeringName}, nil)
require.NoError(t, err)
encodedPeeringToken = baseToken.PeeringToken
retry.Run(t, func(r *retry.R) {
baseToken, _, err := acceptorClient.Peerings().GenerateToken(context.Background(), api.PeeringGenerateTokenRequest{PeerName: tt.peeringName}, nil)
require.NoError(r, err)
encodedPeeringToken = baseToken.PeeringToken
})
}

// If the peering is not supposed to already exist in Consul, then create a secret with the generated token.
Expand Down Expand Up @@ -314,7 +315,8 @@ func TestReconcile_CreateUpdatePeeringDialer(t *testing.T) {
secret.SetResourceVersion("latest-version")
k8sObjects = append(k8sObjects, secret)
}
s := scheme.Scheme
s := runtime.NewScheme()
corev1.AddToScheme(s)
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringDialer{}, &v1alpha1.PeeringDialerList{})
fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build()

Expand Down Expand Up @@ -482,8 +484,11 @@ func TestReconcile_VersionAnnotationPeeringDialer(t *testing.T) {
// Create a peering connection in Consul by generating and establishing a peering connection before calling
// Reconcile().
// Generate a token.
generatedToken, _, err := acceptorClient.Peerings().GenerateToken(context.Background(), api.PeeringGenerateTokenRequest{PeerName: "peering"}, nil)
require.NoError(t, err)
var generatedToken *api.PeeringGenerateTokenResponse
retry.Run(t, func(r *retry.R) {
generatedToken, _, err = acceptorClient.Peerings().GenerateToken(context.Background(), api.PeeringGenerateTokenRequest{PeerName: "peering"}, nil)
require.NoError(r, err)
})

// Create test consul server.
var testServerCfg *testutil.TestServerConfig
Expand Down Expand Up @@ -524,7 +529,8 @@ func TestReconcile_VersionAnnotationPeeringDialer(t *testing.T) {
secret.SetResourceVersion("latest-version")
k8sObjects = append(k8sObjects, secret)

s := scheme.Scheme
s := runtime.NewScheme()
corev1.AddToScheme(s)
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringDialer{}, &v1alpha1.PeeringDialerList{})
fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build()

Expand Down Expand Up @@ -740,7 +746,8 @@ func TestReconcileDeletePeeringDialer(t *testing.T) {
k8sObjects := []runtime.Object{ns, dialer}

// Add peering types to the scheme.
s := scheme.Scheme
s := runtime.NewScheme()
corev1.AddToScheme(s)
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringDialer{}, &v1alpha1.PeeringDialerList{})
fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build()

Expand Down Expand Up @@ -881,7 +888,8 @@ func TestDialerUpdateStatus(t *testing.T) {
k8sObjects = append(k8sObjects, tt.peeringDialer)

// Add peering types to the scheme.
s := scheme.Scheme
s := runtime.NewScheme()
corev1.AddToScheme(s)
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringDialer{}, &v1alpha1.PeeringDialerList{})
fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build()
// Create the peering dialer controller.
Expand Down Expand Up @@ -993,7 +1001,8 @@ func TestDialerUpdateStatusError(t *testing.T) {
k8sObjects = append(k8sObjects, tt.dialer)

// Add peering types to the scheme.
s := scheme.Scheme
s := runtime.NewScheme()
corev1.AddToScheme(s)
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringDialer{}, &v1alpha1.PeeringDialerList{})
fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build()
// Create the peering dialer controller.
Expand Down Expand Up @@ -1277,7 +1286,8 @@ func TestDialer_RequestsForPeeringTokens(t *testing.T) {

for name, tt := range cases {
t.Run(name, func(t *testing.T) {
s := scheme.Scheme
s := runtime.NewScheme()
corev1.AddToScheme(s)
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringDialer{}, &v1alpha1.PeeringDialerList{})
fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(tt.secret, &tt.dialers).Build()
controller := PeeringDialerController{
Expand Down

0 comments on commit fe97fc0

Please sign in to comment.