Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Removes test related flags from binary
The binary was exposing flags used specifically by the tests which is
misleading since the functioanlity of the binary is not altered by those
flags.
Two main causes of the flags being exposed were:
1. ginkgo and gomega packages used in code required by the controllers
2. legacy code add flags to only run unit tests or integration tests.

This patch refactors the code to remove the flags from showing up on the
binary.
  • Loading branch information
srm09 committed Sep 9, 2022
1 parent 623cd59 commit 5a82e33
Show file tree
Hide file tree
Showing 15 changed files with 73 additions and 314 deletions.
3 changes: 0 additions & 3 deletions controllers/controllers_suite_test.go
Expand Up @@ -68,9 +68,6 @@ func setup() {
if err := AddClusterControllerToManager(testEnv.GetContext(), testEnv.Manager, &infrav1.VSphereCluster{}); err != nil {
panic(fmt.Sprintf("unable to setup VsphereCluster controller: %v", err))
}
if err := AddClusterControllerToManager(testEnv.GetContext(), testEnv.Manager, &vmwarev1.VSphereCluster{}); err != nil {
panic(fmt.Sprintf("unable to setup supervisor VsphereCluster controller: %v", err))
}
if err := AddMachineControllerToManager(testEnv.GetContext(), testEnv.Manager, &infrav1.VSphereMachine{}); err != nil {
panic(fmt.Sprintf("unable to setup VsphereMachine controller: %v", err))
}
Expand Down
5 changes: 0 additions & 5 deletions controllers/serviceaccount_controller.go
Expand Up @@ -44,7 +44,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/source"

vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/builder"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
vmwarecontext "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context/vmware"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/record"
Expand Down Expand Up @@ -97,10 +96,6 @@ func AddServiceAccountProviderControllerToManager(ctx *context.ControllerManager
Complete(r)
}

func NewServiceAccountReconciler() builder.Reconciler {
return ServiceAccountReconciler{}
}

type ServiceAccountReconciler struct {
*context.ControllerContext

Expand Down
6 changes: 3 additions & 3 deletions controllers/serviceaccount_controller_intg_test.go
Expand Up @@ -31,16 +31,16 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/builder"
helpers "sigs.k8s.io/cluster-api-provider-vsphere/test/helpers/vmware"
)

var _ = Describe("ProviderServiceAccount controller integration tests", func() {
var (
intCtx *builder.IntegrationTestContext
intCtx *helpers.IntegrationTestContext
)

BeforeEach(func() {
intCtx = ServiceAccountProviderTestsuite.NewIntegrationTestContextWithClusters(ctx, testEnv.Manager.GetClient())
intCtx = helpers.NewIntegrationTestContextWithClusters(ctx, testEnv.Manager.GetClient())
testSystemSvcAcctCM := "test-system-svc-acct-cm"
cfgMap := getSystemServiceAccountsConfigMap(intCtx.VSphereCluster.Namespace, testSystemSvcAcctCM)
Expect(intCtx.Client.Create(intCtx, cfgMap)).To(Succeed())
Expand Down
12 changes: 4 additions & 8 deletions controllers/serviceaccount_controller_suite_test.go
Expand Up @@ -31,13 +31,9 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/builder"
helpers "sigs.k8s.io/cluster-api-provider-vsphere/test/helpers/vmware"
)

// This object is used for unit tests setup only
// Integration tests will be run using the existing envTest setup.
var ServiceAccountProviderTestsuite = builder.NewTestSuiteForController(NewServiceAccountReconciler)

const (
testProviderSvcAccountName = "test-pvcsi"

Expand Down Expand Up @@ -119,7 +115,7 @@ func assertTargetSecret(ctx goctx.Context, guestClient client.Client, namespace,
}).Should(Equal([]byte(testSecretToken)))
}

func assertTargetNamespace(ctx *builder.UnitTestContextForController, guestClient client.Client, namespaceName string, isExist bool) {
func assertTargetNamespace(ctx *helpers.UnitTestContextForController, guestClient client.Client, namespaceName string, isExist bool) {
namespace := &corev1.Namespace{}
err := guestClient.Get(ctx, client.ObjectKey{Name: namespaceName}, namespace)
if isExist {
Expand All @@ -129,7 +125,7 @@ func assertTargetNamespace(ctx *builder.UnitTestContextForController, guestClien
}
}

func assertRoleWithGetPVC(ctx *builder.UnitTestContextForController, ctrlClient client.Client, namespace, name string) {
func assertRoleWithGetPVC(ctx *helpers.UnitTestContextForController, ctrlClient client.Client, namespace, name string) {
var roleList rbacv1.RoleList
opts := &client.ListOptions{
Namespace: namespace,
Expand All @@ -147,7 +143,7 @@ func assertRoleWithGetPVC(ctx *builder.UnitTestContextForController, ctrlClient
}))
}

func assertRoleBinding(_ *builder.UnitTestContextForController, ctrlClient client.Client, namespace, name string) {
func assertRoleBinding(_ *helpers.UnitTestContextForController, ctrlClient client.Client, namespace, name string) {
var roleBindingList rbacv1.RoleBindingList
opts := &client.ListOptions{
Namespace: namespace,
Expand Down
24 changes: 17 additions & 7 deletions controllers/serviceaccount_controller_unit_test.go
Expand Up @@ -26,25 +26,34 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/builder"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/context/fake"
helpers "sigs.k8s.io/cluster-api-provider-vsphere/test/helpers/vmware"
)

var _ = Describe("ServiceAccountReconciler ReconcileNormal", unitTestsReconcileNormal)

func unitTestsReconcileNormal() {
var (
ctx *builder.UnitTestContextForController
ctx *helpers.UnitTestContextForController
vsphereCluster *vmwarev1.VSphereCluster
initObjects []client.Object
namespace string
reconciler ServiceAccountReconciler
)

JustBeforeEach(func() {
// Note: The service account provider requires a reference to the vSphereCluster hence the need to create
// a fake vSphereCluster in the test and pass it to during context setup.
ctx = ServiceAccountProviderTestsuite.NewUnitTestContextForControllerWithVSphereCluster(namespace, vsphereCluster, false, initObjects...)
reconciler = ServiceAccountReconciler{}
ctx = helpers.NewUnitTestContextForController(namespace, vsphereCluster, false, initObjects, nil)
_, err := reconciler.ReconcileNormal(ctx.GuestClusterContext)
Expect(err).NotTo(HaveOccurred())

// Update the VSphereCluster and its status in the fake client.
Expect(ctx.Client.Update(ctx, ctx.VSphereCluster)).To(Succeed())
Expect(ctx.Client.Status().Update(ctx, ctx.VSphereCluster)).To(Succeed())
})

AfterEach(func() {
ctx = nil
})
Expand Down Expand Up @@ -73,7 +82,7 @@ func unitTestsReconcileNormal() {
Context("When serviceaccount secret is created", func() {
It("Should reconcile", func() {
assertTargetNamespace(ctx, ctx.GuestClient, testTargetNS, false)
updateServiceAccountSecretAndReconcileNormal(ctx, vsphereCluster)
updateServiceAccountSecretAndReconcileNormal(ctx, reconciler, vsphereCluster)
assertTargetNamespace(ctx, ctx.GuestClient, testTargetNS, true)
By("Creating the target secret in the target namespace")
assertTargetSecret(ctx, ctx.GuestClient, testTargetNS, testTargetSecret)
Expand All @@ -84,7 +93,7 @@ func unitTestsReconcileNormal() {
It("Should reconcile", func() {
// This is to simulate an outdated token that will be replaced when the serviceaccount secret is created.
createTargetSecretWithInvalidToken(ctx, ctx.GuestClient, testTargetNS)
updateServiceAccountSecretAndReconcileNormal(ctx, vsphereCluster)
updateServiceAccountSecretAndReconcileNormal(ctx, reconciler, vsphereCluster)
By("Updating the target secret in the target namespace")
assertTargetSecret(ctx, ctx.GuestClient, testTargetNS, testTargetSecret)
assertProviderServiceAccountsCondition(ctx.VSphereCluster, corev1.ConditionTrue, "", "", "")
Expand Down Expand Up @@ -113,7 +122,8 @@ func unitTestsReconcileNormal() {

// Updates the service account secret similar to how a token controller would act upon a service account
// and then re-invokes reconcileNormal.
func updateServiceAccountSecretAndReconcileNormal(ctx *builder.UnitTestContextForController, object client.Object) {
func updateServiceAccountSecretAndReconcileNormal(ctx *helpers.UnitTestContextForController, reconciler ServiceAccountReconciler, object client.Object) {
assertServiceAccountAndUpdateSecret(ctx, ctx.Client, object.GetNamespace(), object.GetName())
Expect(ctx.ReconcileNormal()).Should(Succeed())
_, err := reconciler.ReconcileNormal(ctx.GuestClusterContext)
Expect(err).NotTo(HaveOccurred())
}
5 changes: 0 additions & 5 deletions controllers/servicediscovery_controller.go
Expand Up @@ -47,7 +47,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/source"

vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/builder"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
vmwarecontext "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context/vmware"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/record"
Expand Down Expand Up @@ -121,10 +120,6 @@ func AddServiceDiscoveryControllerToManager(ctx *context.ControllerManagerContex
Complete(r)
}

func newServiceDiscoveryReconciler() builder.Reconciler {
return serviceDiscoveryReconciler{}
}

type serviceDiscoveryReconciler struct {
*context.ControllerContext

Expand Down
6 changes: 3 additions & 3 deletions controllers/svcdiscovery_controller_intg_test.go
Expand Up @@ -22,16 +22,16 @@ import (
corev1 "k8s.io/api/core/v1"
"sigs.k8s.io/controller-runtime/pkg/client"

"sigs.k8s.io/cluster-api-provider-vsphere/pkg/builder"
helpers "sigs.k8s.io/cluster-api-provider-vsphere/test/helpers/vmware"
)

var _ = Describe("Service Discovery controller integration tests", func() {
var (
intCtx *builder.IntegrationTestContext
intCtx *helpers.IntegrationTestContext
initObjects []client.Object
)
BeforeEach(func() {
intCtx = serviceDiscoveryTestSuite.NewIntegrationTestContextWithClusters(ctx, testEnv.Manager.GetClient())
intCtx = helpers.NewIntegrationTestContextWithClusters(ctx, testEnv.Manager.GetClient())
})
AfterEach(func() {
intCtx.AfterEach()
Expand Down
4 changes: 0 additions & 4 deletions controllers/svcdiscovery_controller_suite_test.go
Expand Up @@ -32,12 +32,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

vmwarev1beta1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/builder"
)

// serviceAccountProviderTestsuite is used for unit and integration testing this controller.
var serviceDiscoveryTestSuite = builder.NewTestSuiteForController(newServiceDiscoveryReconciler)

const (
testSupervisorAPIServerVIP = "10.0.0.100"
testSupervisorAPIServerVIP2 = "10.0.0.200"
Expand Down
13 changes: 10 additions & 3 deletions controllers/svcdiscovery_controller_unit_test.go
Expand Up @@ -26,22 +26,29 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

vmwarev1b1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/builder"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/context/fake"
helpers "sigs.k8s.io/cluster-api-provider-vsphere/test/helpers/vmware"
)

var _ = Describe("ServiceDiscoveryReconciler ReconcileNormal", serviceDiscoveryUnitTestsReconcileNormal)

func serviceDiscoveryUnitTestsReconcileNormal() {
var (
ctx *builder.UnitTestContextForController
ctx *helpers.UnitTestContextForController
vsphereCluster vmwarev1b1.VSphereCluster
initObjects []client.Object
reconciler serviceDiscoveryReconciler
)
namespace := capiutil.RandomString(6)
JustBeforeEach(func() {
vsphereCluster = fake.NewVSphereCluster(namespace)
ctx = serviceDiscoveryTestSuite.NewUnitTestContextForController(namespace, &vsphereCluster, initObjects...)
ctx = helpers.NewUnitTestContextForController(namespace, &vsphereCluster, false, initObjects, nil)
_, err := reconciler.ReconcileNormal(ctx.GuestClusterContext)
Expect(err).NotTo(HaveOccurred())

// Update the VSphereCluster and its status in the fake client.
Expect(ctx.Client.Update(ctx, ctx.VSphereCluster)).To(Succeed())
Expect(ctx.Client.Status().Update(ctx, ctx.VSphereCluster)).To(Succeed())
})
JustAfterEach(func() {
ctx = nil
Expand Down
3 changes: 1 addition & 2 deletions feature/gates.go
Expand Up @@ -18,15 +18,14 @@ package feature

import (
"k8s.io/component-base/featuregate"
"sigs.k8s.io/cluster-api/feature"
)

var (
// MutableGates is a mutable version of DefaultFeatureGate.
// Only top-level commands/options setup and the k8s.io/component-base/featuregate/testing package should make use of this.
// Tests that need to modify featuregate gates for the duration of their test should use:
// defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.<FeatureName>, <value>)()
MutableGates = feature.MutableGates
MutableGates = featuregate.NewFeatureGate()

// Gates is a shared global FeatureGate.
// Top-level commands/options setup that needs to modify this featuregate gate should use DefaultMutableFeatureGate.
Expand Down
70 changes: 0 additions & 70 deletions pkg/builder/flags.go

This file was deleted.

0 comments on commit 5a82e33

Please sign in to comment.