Skip to content

Commit

Permalink
remove controller runtime dependency
Browse files Browse the repository at this point in the history
Signed-off-by: Xudong Liu <xudongl@vmware.com>
  • Loading branch information
Xudong Liu authored and XudongLiuHarold committed Jan 27, 2024
1 parent 58f38be commit bb8c76e
Show file tree
Hide file tree
Showing 12 changed files with 327 additions and 274 deletions.
4 changes: 2 additions & 2 deletions pkg/cloudprovider/vsphereparavirtual/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,17 @@ import (

"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
"sigs.k8s.io/controller-runtime/pkg/client"

cloudprovider "k8s.io/cloud-provider"
"k8s.io/klog/v2"

vmopv1alpha1 "github.com/vmware-tanzu/vm-operator-api/api/v1alpha1"
vmopclient "k8s.io/cloud-provider-vsphere/pkg/cloudprovider/vsphereparavirtual/vmop/clientset/versioned"
"k8s.io/cloud-provider-vsphere/pkg/cloudprovider/vsphereparavirtual/vmservice"
)

type instances struct {
vmClient client.Client
vmClient vmopclient.Interface
namespace string
}

Expand Down
66 changes: 35 additions & 31 deletions pkg/cloudprovider/vsphereparavirtual/instances_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,9 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
clientgotesting "k8s.io/client-go/testing"
cloudprovider "k8s.io/cloud-provider"
"k8s.io/cloud-provider-vsphere/pkg/util"
"sigs.k8s.io/controller-runtime/pkg/client"
fakeClient "sigs.k8s.io/controller-runtime/pkg/client/fake"
fakevmclient "k8s.io/cloud-provider-vsphere/pkg/cloudprovider/vsphereparavirtual/vmop/clientset/versioned/fake"
"sigs.k8s.io/controller-runtime/pkg/envtest"
)

Expand Down Expand Up @@ -73,7 +72,9 @@ func TestNewInstances(t *testing.T) {
expectedErr error
}{
{
name: "NewInstance: when everything is ok",
name: "NewInstance: when everything is ok",
// The above code is declaring a test environment variable of type `envtest.Environment` and
// initializing it with an empty instance of `envtest.Environment`.
testEnv: &envtest.Environment{},
expectedErr: nil,
},
Expand All @@ -94,16 +95,14 @@ func TestNewInstances(t *testing.T) {
}
}

func initTest(testVM *vmopv1alpha1.VirtualMachine) (*instances, *util.FakeClientWrapper) {
scheme := runtime.NewScheme()
_ = vmopv1alpha1.AddToScheme(scheme)
fc := fakeClient.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(testVM).Build()
fcw := util.NewFakeClientWrapper(fc)
func initTest(testVM *vmopv1alpha1.VirtualMachine) (*instances, *fakevmclient.Clientset, error) {
fc := fakevmclient.NewSimpleClientset()
instance := &instances{
vmClient: fcw,
vmClient: fc,
namespace: testClusterNameSpace,
}
return instance, fcw
_, err := fc.VmoperatorV1alpha1().VirtualMachines(testVM.Namespace).Create(context.TODO(), testVM, metav1.CreateOptions{})
return instance, fc, err
}

func TestInstanceID(t *testing.T) {
Expand Down Expand Up @@ -142,7 +141,8 @@ func TestInstanceID(t *testing.T) {

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
instance, _ := initTest(testCase.testVM)
instance, _, err := initTest(testCase.testVM)
assert.NoError(t, err)
instanceID, err := instance.InstanceID(context.Background(), testVMName)
assert.Equal(t, testCase.expectedErr, err)
assert.Equal(t, testCase.expectedInstanceID, instanceID)
Expand All @@ -165,11 +165,11 @@ func TestInstanceIDThrowsErr(t *testing.T) {

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
instance, fcw := initTest(testCase.testVM)
fcw.GetFunc = func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
return fmt.Errorf("Internal error getting VMs")
}

instance, fc, err := initTest(testCase.testVM)
assert.NoError(t, err)
fc.PrependReactor("get", "virtualmachines", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
return true, &vmopv1alpha1.VirtualMachine{}, fmt.Errorf("Internal error getting VMs")
})
instanceID, err := instance.InstanceID(context.Background(), testVMName)
assert.NotEqual(t, nil, err)
assert.NotEqual(t, cloudprovider.InstanceNotFound, err)
Expand Down Expand Up @@ -201,7 +201,8 @@ func TestInstanceExistsByProviderID(t *testing.T) {

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
instance, _ := initTest(testCase.testVM)
instance, _, err := initTest(testCase.testVM)
assert.NoError(t, err)
providerID, err := instance.InstanceExistsByProviderID(context.Background(), testProviderID)
assert.Equal(t, testCase.expectedErr, err)
assert.Equal(t, testCase.expectedResult, providerID)
Expand Down Expand Up @@ -255,7 +256,8 @@ func TestInstanceShutdownByProviderID(t *testing.T) {
testCase.testVM.Status.PowerState = vmopv1alpha1.VirtualMachinePoweredOff
}

instance, _ := initTest(testCase.testVM)
instance, _, err := initTest(testCase.testVM)
assert.NoError(t, err)
ret, err := instance.InstanceShutdownByProviderID(context.Background(), testProviderID)
assert.Equal(t, testCase.expectedErr, err)
assert.Equal(t, testCase.expectedResult, ret)
Expand Down Expand Up @@ -301,7 +303,8 @@ func TestNodeAddressesByProviderID(t *testing.T) {

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
instance, _ := initTest(testCase.testVM)
instance, _, err := initTest(testCase.testVM)
assert.NoError(t, err)
ret, err := instance.NodeAddressesByProviderID(context.Background(), testProviderID)
assert.Equal(t, testCase.expectedErr, err)
assert.Equal(t, testCase.expectedNodeAddress, ret)
Expand All @@ -324,11 +327,11 @@ func TestNodeAddressesByProviderIDInternalErr(t *testing.T) {

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
instance, fcw := initTest(testCase.testVM)
fcw.ListFunc = func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
return fmt.Errorf("Internal error listing VMs")
}

instance, fc, err := initTest(testCase.testVM)
assert.NoError(t, err)
fc.PrependReactor("list", "virtualmachines", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
return true, &vmopv1alpha1.VirtualMachineList{}, fmt.Errorf("Internal error listing VMs")
})
ret, err := instance.NodeAddressesByProviderID(context.Background(), testProviderID)
assert.NotEqual(t, nil, err)
assert.NotEqual(t, cloudprovider.InstanceNotFound, err)
Expand Down Expand Up @@ -375,7 +378,8 @@ func TestNodeAddresses(t *testing.T) {

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
instance, _ := initTest(testCase.testVM)
instance, _, err := initTest(testCase.testVM)
assert.NoError(t, err)
ret, err := instance.NodeAddresses(context.Background(), testVMName)
assert.Equal(t, testCase.expectedErr, err)
assert.Equal(t, testCase.expectedNodeAddress, ret)
Expand All @@ -398,11 +402,11 @@ func TestNodeAddressesInternalErr(t *testing.T) {

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
instance, fcw := initTest(testCase.testVM)
fcw.GetFunc = func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
return fmt.Errorf("Internal error getting VMs")
}

instance, fc, err := initTest(testCase.testVM)
assert.NoError(t, err)
fc.PrependReactor("get", "virtualmachines", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
return true, &vmopv1alpha1.VirtualMachine{}, fmt.Errorf("Internal error getting VMs")
})
ret, err := instance.NodeAddresses(context.Background(), testVMName)
assert.NotEqual(t, nil, err)
assert.NotEqual(t, cloudprovider.InstanceNotFound, err)
Expand Down
80 changes: 39 additions & 41 deletions pkg/cloudprovider/vsphereparavirtual/loadbalancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,12 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
clientgotesting "k8s.io/client-go/testing"
cloudprovider "k8s.io/cloud-provider"
"sigs.k8s.io/controller-runtime/pkg/client"
fakeClient "sigs.k8s.io/controller-runtime/pkg/client/fake"
fakevmclient "k8s.io/cloud-provider-vsphere/pkg/cloudprovider/vsphereparavirtual/vmop/clientset/versioned/fake"
"sigs.k8s.io/controller-runtime/pkg/envtest"

"k8s.io/cloud-provider-vsphere/pkg/cloudprovider/vsphereparavirtual/vmservice"
"k8s.io/cloud-provider-vsphere/pkg/util"

vmopv1alpha1 "github.com/vmware-tanzu/vm-operator-api/api/v1alpha1"
)
Expand All @@ -51,15 +50,13 @@ var (
}
)

func newTestLoadBalancer() (cloudprovider.LoadBalancer, *util.FakeClientWrapper) {
scheme := runtime.NewScheme()
_ = vmopv1alpha1.AddToScheme(scheme)
fc := fakeClient.NewClientBuilder().WithScheme(scheme).Build()
fcw := util.NewFakeClientWrapper(fc)
vms := vmservice.NewVMService(fcw, testClusterNameSpace, &testOwnerReference)
func newTestLoadBalancer() (cloudprovider.LoadBalancer, *fakevmclient.Clientset) {
fc := fakevmclient.NewSimpleClientset()

vms := vmservice.NewVMService(fc, testClusterNameSpace, &testOwnerReference)
return &loadBalancer{
vmService: vms,
}, fcw
}, fc
}

func TestNewLoadBalancer(t *testing.T) {
Expand Down Expand Up @@ -155,7 +152,7 @@ func TestUpdateLoadBalancer(t *testing.T) {

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
lb, fcw := newTestLoadBalancer()
lb, fc := newTestLoadBalancer()
testK8sService := &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: testK8sServiceName,
Expand All @@ -181,9 +178,9 @@ func TestUpdateLoadBalancer(t *testing.T) {

if testCase.expectErr {
// Ensure that the client Update call returns an error on update
fcw.UpdateFunc = func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
return fmt.Errorf("Some undefined update error")
}
fc.PrependReactor("update", "virtualmachineservices", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
return true, &vmopv1alpha1.VirtualMachineService{}, fmt.Errorf("Some undefined update error")
})
err = lb.UpdateLoadBalancer(context.Background(), testClustername, testK8sService, []*v1.Node{})
assert.Error(t, err)
} else {
Expand All @@ -195,7 +192,7 @@ func TestUpdateLoadBalancer(t *testing.T) {
}

func TestEnsureLoadBalancer_VMServiceExternalTrafficPolicyLocal(t *testing.T) {
lb, fcw := newTestLoadBalancer()
lb, fc := newTestLoadBalancer()
testK8sService := &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: testK8sServiceName,
Expand All @@ -205,8 +202,9 @@ func TestEnsureLoadBalancer_VMServiceExternalTrafficPolicyLocal(t *testing.T) {
ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeLocal,
},
}
fcw.CreateFunc = func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error {
vms := &vmopv1alpha1.VirtualMachineService{

fc.PrependReactor("create", "virtualmachineservices", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
return true, &vmopv1alpha1.VirtualMachineService{
Status: vmopv1alpha1.VirtualMachineServiceStatus{
LoadBalancer: vmopv1alpha1.LoadBalancerStatus{
Ingress: []vmopv1alpha1.LoadBalancerIngress{
Expand All @@ -216,10 +214,8 @@ func TestEnsureLoadBalancer_VMServiceExternalTrafficPolicyLocal(t *testing.T) {
},
},
},
}
vms.DeepCopyInto(obj.(*vmopv1alpha1.VirtualMachineService))
return nil
}
}, nil
})

_, ensureErr := lb.EnsureLoadBalancer(context.Background(), testClustername, testK8sService, []*v1.Node{})
assert.NoError(t, ensureErr)
Expand All @@ -231,32 +227,35 @@ func TestEnsureLoadBalancer_VMServiceExternalTrafficPolicyLocal(t *testing.T) {
func TestEnsureLoadBalancer(t *testing.T) {
testCases := []struct {
name string
createFunc func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error
createFunc func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error)
expectErr error
}{
{
name: "when VMService is created but IP not found",
name: "when VMService is created but IP not found",
createFunc: func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
return true, &vmopv1alpha1.VirtualMachineService{}, fmt.Errorf(vmservice.ErrVMServiceIPNotFound.Error())
},
expectErr: vmservice.ErrVMServiceIPNotFound,
},
{
name: "when VMService creation failed",
createFunc: func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error {
return fmt.Errorf(vmservice.ErrCreateVMService.Error())
createFunc: func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
return true, &vmopv1alpha1.VirtualMachineService{}, fmt.Errorf(vmservice.ErrCreateVMService.Error())
},
expectErr: vmservice.ErrCreateVMService,
},
}

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
lb, fcw := newTestLoadBalancer()
lb, fc := newTestLoadBalancer()
testK8sService := &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: testK8sServiceName,
Namespace: testK8sServiceNameSpace,
},
}
fcw.CreateFunc = testCase.createFunc
fc.PrependReactor("create", "virtualmachineservices", testCase.createFunc)

_, ensureErr := lb.EnsureLoadBalancer(context.Background(), testClustername, testK8sService, []*v1.Node{})
assert.Equal(t, ensureErr.Error(), testCase.expectErr.Error())
Expand All @@ -268,16 +267,16 @@ func TestEnsureLoadBalancer(t *testing.T) {
}

func TestEnsureLoadBalancer_VMServiceCreatedIPFound(t *testing.T) {
lb, fcw := newTestLoadBalancer()
lb, fc := newTestLoadBalancer()
testK8sService := &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: testK8sServiceName,
Namespace: testK8sServiceNameSpace,
},
}
// Ensure that the client Create call returns a VMService with a valid IP
fcw.CreateFunc = func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error {
vms := &vmopv1alpha1.VirtualMachineService{
fc.PrependReactor("create", "virtualmachineservices", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
return true, &vmopv1alpha1.VirtualMachineService{
Status: vmopv1alpha1.VirtualMachineServiceStatus{
LoadBalancer: vmopv1alpha1.LoadBalancerStatus{
Ingress: []vmopv1alpha1.LoadBalancerIngress{
Expand Down Expand Up @@ -308,10 +307,8 @@ func TestEnsureLoadBalancer_VMServiceCreatedIPFound(t *testing.T) {
vmservice.NodeSelectorKey: vmservice.NodeRole,
},
},
}
vms.DeepCopyInto(obj.(*vmopv1alpha1.VirtualMachineService))
return nil
}
}, nil
})

status, ensureErr := lb.EnsureLoadBalancer(context.Background(), testClustername, testK8sService, []*v1.Node{})
assert.NoError(t, ensureErr)
Expand All @@ -324,27 +321,27 @@ func TestEnsureLoadBalancer_VMServiceCreatedIPFound(t *testing.T) {
func TestEnsureLoadBalancer_DeleteLB(t *testing.T) {
testCases := []struct {
name string
deleteFunc func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error
deleteFunc func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error)
expectErr string
}{
{
name: "should ignore not found error",
deleteFunc: func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error {
return apierrors.NewNotFound(vmopv1alpha1.Resource("virtualmachineservice"), testClustername)
deleteFunc: func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, apierrors.NewNotFound(vmopv1alpha1.Resource("virtualmachineservice"), testClustername)
},
},
{
name: "should return error",
deleteFunc: func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error {
return fmt.Errorf("an error occurred while deleting load balancer")
deleteFunc: func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, fmt.Errorf("an error occurred while deleting load balancer")
},
expectErr: "an error occurred while deleting load balancer",
},
}

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
lb, fcw := newTestLoadBalancer()
lb, fc := newTestLoadBalancer()
testK8sService := &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: testK8sServiceName,
Expand All @@ -356,7 +353,8 @@ func TestEnsureLoadBalancer_DeleteLB(t *testing.T) {
err := lb.EnsureLoadBalancerDeleted(context.Background(), testClustername, testK8sService)
assert.NoError(t, err)

fcw.DeleteFunc = testCase.deleteFunc
fc.PrependReactor("delete", "virtualmachineservices", testCase.deleteFunc)

err = lb.EnsureLoadBalancerDeleted(context.Background(), "test", testK8sService)
if err != nil {
assert.Equal(t, err.Error(), testCase.expectErr)
Expand Down
Loading

0 comments on commit bb8c76e

Please sign in to comment.