Skip to content

Commit

Permalink
🌱 Use client-go global scheme as much as possible
Browse files Browse the repository at this point in the history 
A test is currently flaking and reporting a race condition because we
use the scheme in a way that shouldn't be used. A runtime.Scheme object
is not safe to use concurrently (add and read at the same time), but
it's meant to be used in an immutable way after the first registration
happens. In some tests we were reusing and adding to the same scheme
while other tests were reading from it, this caused a race condition to
trigger given that the scheme uses maps internally to store its
information.

Going forward, we should standardize on using the global scheme more and
more to keep things simple. This would not only simplify our tests, but
also simplify our code.

Signed-off-by: Vince Prignano <vincepri@vmware.com>
  • Loading branch information
@vincepri
vincepri committed May 27, 2021
1 parent 0e1629b commit 29333cd
Show file tree
Hide file tree
Showing 46 changed files with 365 additions and 504 deletions.
19 changes: 3 additions & 16 deletions api/v1alpha3/conversion_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,54 +20,41 @@ import (
"testing"

fuzz "github.com/google/gofuzz"
. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/api/apitesting/fuzzer"
"sigs.k8s.io/controller-runtime/pkg/conversion"

"k8s.io/apimachinery/pkg/runtime"
runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer"
"sigs.k8s.io/cluster-api/api/v1alpha4"
utilconversion "sigs.k8s.io/cluster-api/util/conversion"
"sigs.k8s.io/controller-runtime/pkg/conversion"
)

func TestFuzzyConversion(t *testing.T) {
g := NewWithT(t)
scheme := runtime.NewScheme()
g.Expect(AddToScheme(scheme)).To(Succeed())
g.Expect(v1alpha4.AddToScheme(scheme)).To(Succeed())

t.Run("for Cluster", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Scheme: scheme,
Hub: &v1alpha4.Cluster{},
Spoke: &Cluster{},
SpokeAfterMutation: clusterSpokeAfterMutation,
}))

t.Run("for Machine", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Scheme: scheme,
Hub: &v1alpha4.Machine{},
Spoke: &Machine{},
FuzzerFuncs: []fuzzer.FuzzerFuncs{BootstrapFuzzFuncs},
}))

t.Run("for MachineSet", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Scheme: scheme,
Hub: &v1alpha4.MachineSet{},
Spoke: &MachineSet{},
FuzzerFuncs: []fuzzer.FuzzerFuncs{BootstrapFuzzFuncs, CustomObjectMetaFuzzFunc},
}))

t.Run("for MachineDeployment", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Scheme: scheme,
Hub: &v1alpha4.MachineDeployment{},
Spoke: &MachineDeployment{},
FuzzerFuncs: []fuzzer.FuzzerFuncs{BootstrapFuzzFuncs, CustomObjectMetaFuzzFunc},
}))

t.Run("for MachineHealthCheckSpec", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Scheme: scheme,
Hub: &v1alpha4.MachineHealthCheck{},
Spoke: &MachineHealthCheck{},
Hub: &v1alpha4.MachineHealthCheck{},
Spoke: &MachineHealthCheck{},
}))
}

Expand Down
10 changes: 0 additions & 10 deletions bootstrap/kubeadm/api/v1alpha3/conversion_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,20 @@ import (
"testing"

fuzz "github.com/google/gofuzz"
. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/api/apitesting/fuzzer"
runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer"

"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha4"
"sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta1"
utilconversion "sigs.k8s.io/cluster-api/util/conversion"
)

func TestFuzzyConversion(t *testing.T) {
g := NewWithT(t)
scheme := runtime.NewScheme()
g.Expect(AddToScheme(scheme)).To(Succeed())
g.Expect(v1alpha4.AddToScheme(scheme)).To(Succeed())

t.Run("for KubeadmConfig", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Scheme: scheme,
Hub: &v1alpha4.KubeadmConfig{},
Spoke: &KubeadmConfig{},
FuzzerFuncs: []fuzzer.FuzzerFuncs{fuzzFuncs},
}))
t.Run("for KubeadmConfigTemplate", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Scheme: scheme,
Hub: &v1alpha4.KubeadmConfigTemplate{},
Spoke: &KubeadmConfigTemplate{},
FuzzerFuncs: []fuzzer.FuzzerFuncs{fuzzFuncs},
Expand Down
67 changes: 25 additions & 42 deletions bootstrap/kubeadm/controllers/kubeadmconfig_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import (
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
bootstrapapi "k8s.io/cluster-bootstrap/token/api"
"k8s.io/utils/pointer"
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
Expand All @@ -46,23 +46,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

func setupScheme() *runtime.Scheme {
scheme := runtime.NewScheme()
if err := clusterv1.AddToScheme(scheme); err != nil {
panic(err)
}
if err := expv1.AddToScheme(scheme); err != nil {
panic(err)
}
if err := bootstrapv1.AddToScheme(scheme); err != nil {
panic(err)
}
if err := corev1.AddToScheme(scheme); err != nil {
panic(err)
}
return scheme
}

// MachineToBootstrapMapFunc return kubeadm bootstrap configref name when configref exists.
func TestKubeadmConfigReconciler_MachineToBootstrapMapFuncReturn(t *testing.T) {
g := NewWithT(t)
Expand All @@ -83,7 +66,7 @@ func TestKubeadmConfigReconciler_MachineToBootstrapMapFuncReturn(t *testing.T) {
}
machineObjs = append(machineObjs, m)
}
fakeClient := helpers.NewFakeClientWithScheme(setupScheme(), objs...)
fakeClient := helpers.NewFakeClientWithScheme(scheme.Scheme, objs...)
reconciler := &KubeadmConfigReconciler{
Client: fakeClient,
}
Expand All @@ -108,7 +91,7 @@ func TestKubeadmConfigReconciler_Reconcile_ReturnEarlyIfKubeadmConfigIsReady(t *
objects := []client.Object{
config,
}
myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)

k := &KubeadmConfigReconciler{
Client: myclient,
Expand Down Expand Up @@ -137,7 +120,7 @@ func TestKubeadmConfigReconciler_Reconcile_ReturnNilIfReferencedMachineIsNotFoun
// intentionally omitting machine
config,
}
myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)

k := &KubeadmConfigReconciler{
Client: myclient,
Expand Down Expand Up @@ -165,7 +148,7 @@ func TestKubeadmConfigReconciler_Reconcile_ReturnEarlyIfMachineHasDataSecretName
machine,
config,
}
myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)

k := &KubeadmConfigReconciler{
Client: myclient,
Expand Down Expand Up @@ -200,7 +183,7 @@ func TestKubeadmConfigReconciler_ReturnEarlyIfClusterInfraNotReady(t *testing.T)
machine,
config,
}
myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)

k := &KubeadmConfigReconciler{
Client: myclient,
Expand Down Expand Up @@ -231,7 +214,7 @@ func TestKubeadmConfigReconciler_Reconcile_ReturnEarlyIfMachineHasNoCluster(t *t
machine,
config,
}
myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)

k := &KubeadmConfigReconciler{
Client: myclient,
Expand All @@ -258,7 +241,7 @@ func TestKubeadmConfigReconciler_Reconcile_ReturnNilIfMachineDoesNotHaveAssociat
machine,
config,
}
myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)

k := &KubeadmConfigReconciler{
Client: myclient,
Expand Down Expand Up @@ -287,7 +270,7 @@ func TestKubeadmConfigReconciler_Reconcile_ReturnNilIfAssociatedClusterIsNotFoun
machine,
config,
}
myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)

k := &KubeadmConfigReconciler{
Client: myclient,
Expand Down Expand Up @@ -352,7 +335,7 @@ func TestKubeadmConfigReconciler_Reconcile_RequeueJoiningNodesIfControlPlaneNotI
t.Run(tc.name, func(t *testing.T) {
g := NewWithT(t)

myclient := helpers.NewFakeClientWithScheme(setupScheme(), tc.objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, tc.objects...)

k := &KubeadmConfigReconciler{
Client: myclient,
Expand Down Expand Up @@ -385,7 +368,7 @@ func TestKubeadmConfigReconciler_Reconcile_GenerateCloudConfigData(t *testing.T)
}
objects = append(objects, createSecrets(t, cluster, controlPlaneInitConfig)...)

myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)

k := &KubeadmConfigReconciler{
Client: myclient,
Expand Down Expand Up @@ -437,7 +420,7 @@ func TestKubeadmConfigReconciler_Reconcile_ErrorIfJoiningControlPlaneHasInvalidC
controlPlaneJoinConfig,
}
objects = append(objects, createSecrets(t, cluster, controlPlaneInitConfig)...)
myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)

k := &KubeadmConfigReconciler{
Client: myclient,
Expand Down Expand Up @@ -475,7 +458,7 @@ func TestKubeadmConfigReconciler_Reconcile_RequeueIfControlPlaneIsMissingAPIEndp
}
objects = append(objects, createSecrets(t, cluster, controlPlaneInitConfig)...)

myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)

k := &KubeadmConfigReconciler{
Client: myclient,
Expand Down Expand Up @@ -547,7 +530,7 @@ func TestReconcileIfJoinNodesAndControlPlaneIsReady(t *testing.T) {
config,
}
objects = append(objects, createSecrets(t, cluster, config)...)
myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)
k := &KubeadmConfigReconciler{
Client: myclient,
KubeadmInitLock: &myInitLocker{},
Expand Down Expand Up @@ -623,7 +606,7 @@ func TestReconcileIfJoinNodePoolsAndControlPlaneIsReady(t *testing.T) {
config,
}
objects = append(objects, createSecrets(t, cluster, config)...)
myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)
k := &KubeadmConfigReconciler{
Client: myclient,
KubeadmInitLock: &myInitLocker{},
Expand Down Expand Up @@ -677,7 +660,7 @@ func TestKubeadmConfigSecretCreatedStatusNotPatched(t *testing.T) {
}

objects = append(objects, createSecrets(t, cluster, initConfig)...)
myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)
k := &KubeadmConfigReconciler{
Client: myclient,
KubeadmInitLock: &myInitLocker{},
Expand Down Expand Up @@ -750,7 +733,7 @@ func TestBootstrapTokenTTLExtension(t *testing.T) {
}

objects = append(objects, createSecrets(t, cluster, initConfig)...)
myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)
k := &KubeadmConfigReconciler{
Client: myclient,
KubeadmInitLock: &myInitLocker{},
Expand Down Expand Up @@ -896,7 +879,7 @@ func TestBootstrapTokenRotationMachinePool(t *testing.T) {
}

objects = append(objects, createSecrets(t, cluster, initConfig)...)
myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)
k := &KubeadmConfigReconciler{
Client: myclient,
KubeadmInitLock: &myInitLocker{},
Expand Down Expand Up @@ -1020,7 +1003,7 @@ func TestBootstrapTokenRotationMachinePool(t *testing.T) {
// Ensure the discovery portion of the JoinConfiguration gets generated correctly.
func TestKubeadmConfigReconciler_Reconcile_DiscoveryReconcileBehaviors(t *testing.T) {
k := &KubeadmConfigReconciler{
Client: helpers.NewFakeClientWithScheme(setupScheme()),
Client: helpers.NewFakeClientWithScheme(scheme.Scheme),
KubeadmInitLock: &myInitLocker{},
remoteClientGetter: fakeremote.NewClusterClient,
}
Expand Down Expand Up @@ -1353,7 +1336,7 @@ func TestKubeadmConfigReconciler_Reconcile_AlwaysCheckCAVerificationUnlessReques
t.Run(tc.name, func(t *testing.T) {
g := NewWithT(t)

myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)
reconciler := KubeadmConfigReconciler{
Client: myclient,
KubeadmInitLock: &myInitLocker{},
Expand Down Expand Up @@ -1401,7 +1384,7 @@ func TestKubeadmConfigReconciler_ClusterToKubeadmConfigs(t *testing.T) {
expectedNames = append(expectedNames, configName)
objs = append(objs, mp, c)
}
fakeClient := helpers.NewFakeClientWithScheme(setupScheme(), objs...)
fakeClient := helpers.NewFakeClientWithScheme(scheme.Scheme, objs...)
reconciler := &KubeadmConfigReconciler{
Client: fakeClient,
}
Expand Down Expand Up @@ -1440,7 +1423,7 @@ func TestKubeadmConfigReconciler_Reconcile_DoesNotFailIfCASecretsAlreadyExist(t
"tls.key": []byte("hello world"),
},
}
fakec := helpers.NewFakeClientWithScheme(setupScheme(), []client.Object{cluster, m, c, scrt}...)
fakec := helpers.NewFakeClientWithScheme(scheme.Scheme, []client.Object{cluster, m, c, scrt}...)
reconciler := &KubeadmConfigReconciler{
Client: fakec,
KubeadmInitLock: &myInitLocker{},
Expand Down Expand Up @@ -1472,7 +1455,7 @@ func TestKubeadmConfigReconciler_Reconcile_ExactlyOneControlPlaneMachineInitiali
controlPlaneInitMachineSecond,
controlPlaneInitConfigSecond,
}
myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)
k := &KubeadmConfigReconciler{
Client: myclient,
KubeadmInitLock: &myInitLocker{},
Expand Down Expand Up @@ -1527,7 +1510,7 @@ func TestKubeadmConfigReconciler_Reconcile_PatchWhenErrorOccurred(t *testing.T)
objects = append(objects, s)
}

myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, objects...)
k := &KubeadmConfigReconciler{
Client: myclient,
KubeadmInitLock: &myInitLocker{},
Expand Down Expand Up @@ -1663,7 +1646,7 @@ func TestKubeadmConfigReconciler_ResolveFiles(t *testing.T) {
t.Run(name, func(t *testing.T) {
g := NewWithT(t)

myclient := helpers.NewFakeClientWithScheme(setupScheme(), tc.objects...)
myclient := helpers.NewFakeClientWithScheme(scheme.Scheme, tc.objects...)
k := &KubeadmConfigReconciler{
Client: myclient,
KubeadmInitLock: &myInitLocker{},
Expand Down
Loading

0 comments on commit 29333cd

Please sign in to comment.