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 <[email protected]>
  • Loading branch information
XudongLiuHarold committed Jan 26, 2024
1 parent 58f38be commit d6d4f9e
Show file tree
Hide file tree
Showing 12 changed files with 273 additions and 229 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
30 changes: 14 additions & 16 deletions pkg/cloudprovider/vsphereparavirtual/instances_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,10 @@ import (
vmopv1alpha1 "github.com/vmware-tanzu/vm-operator-api/api/v1alpha1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
cloudprovider "k8s.io/cloud-provider"
faketvmclient "k8s.io/cloud-provider-vsphere/pkg/cloudprovider/vsphereparavirtual/vmop/clientset/versioned/fake"
"k8s.io/cloud-provider-vsphere/pkg/util"
"sigs.k8s.io/controller-runtime/pkg/client"
fakeClient "sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/envtest"
)

Expand Down Expand Up @@ -73,7 +71,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,13 +94,11 @@ 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, *util.FakeVMClientWrapper) {
fc := faketvmclient.NewSimpleClientset()
fcw := util.NewFakeVMClientWrapper(fc)
instance := &instances{
vmClient: fcw,
vmClient: fc,
namespace: testClusterNameSpace,
}
return instance, fcw
Expand Down Expand Up @@ -166,8 +164,8 @@ 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")
fcw.GetFunc = func(ctx context.Context, namespace, name string, opts metav1.GetOptions) (result *vmopv1alpha1.VirtualMachine, err error) {
return nil, fmt.Errorf("Internal error getting VMs")
}

instanceID, err := instance.InstanceID(context.Background(), testVMName)
Expand Down Expand Up @@ -325,8 +323,8 @@ 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")
fcw.ListFunc = func(ctx context.Context, namespace string, opts metav1.ListOptions) (result *vmopv1alpha1.VirtualMachineList, err error) {
return nil, fmt.Errorf("Internal error listing VMs")
}

ret, err := instance.NodeAddressesByProviderID(context.Background(), testProviderID)
Expand Down Expand Up @@ -399,8 +397,8 @@ 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")
fcw.GetFunc = func(ctx context.Context, namespace, name string, opts metav1.GetOptions) (result *vmopv1alpha1.VirtualMachine, err error) {
return nil, fmt.Errorf("Internal error getting VMs")
}

ret, err := instance.NodeAddresses(context.Background(), testVMName)
Expand Down
41 changes: 19 additions & 22 deletions pkg/cloudprovider/vsphereparavirtual/loadbalancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,8 @@ import (
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
cloudprovider "k8s.io/cloud-provider"
"sigs.k8s.io/controller-runtime/pkg/client"
fakeClient "sigs.k8s.io/controller-runtime/pkg/client/fake"
faketvmclient "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"
Expand All @@ -51,11 +49,10 @@ var (
}
)

func newTestLoadBalancer() (cloudprovider.LoadBalancer, *util.FakeClientWrapper) {
scheme := runtime.NewScheme()
_ = vmopv1alpha1.AddToScheme(scheme)
fc := fakeClient.NewClientBuilder().WithScheme(scheme).Build()
fcw := util.NewFakeClientWrapper(fc)
func newTestLoadBalancer() (cloudprovider.LoadBalancer, *util.FakeVMServiceClientWrapper) {
fc := faketvmclient.NewSimpleClientset()
fcw := util.NewFakeVMServiceClientWrapper(fc)

vms := vmservice.NewVMService(fcw, testClusterNameSpace, &testOwnerReference)
return &loadBalancer{
vmService: vms,
Expand Down Expand Up @@ -181,8 +178,8 @@ 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")
fcw.UpdateFunc = func(ctx context.Context, vm *vmopv1alpha1.VirtualMachineService, opts metav1.UpdateOptions) (result *vmopv1alpha1.VirtualMachineService, err error) {
return nil, fmt.Errorf("Some undefined update error")
}
err = lb.UpdateLoadBalancer(context.Background(), testClustername, testK8sService, []*v1.Node{})
assert.Error(t, err)
Expand All @@ -205,7 +202,7 @@ func TestEnsureLoadBalancer_VMServiceExternalTrafficPolicyLocal(t *testing.T) {
ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeLocal,
},
}
fcw.CreateFunc = func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error {
fcw.CreateFunc = func(ctx context.Context, vm *vmopv1alpha1.VirtualMachineService, opts metav1.CreateOptions) (result *vmopv1alpha1.VirtualMachineService, err error) {
vms := &vmopv1alpha1.VirtualMachineService{
Status: vmopv1alpha1.VirtualMachineServiceStatus{
LoadBalancer: vmopv1alpha1.LoadBalancerStatus{
Expand All @@ -217,8 +214,8 @@ func TestEnsureLoadBalancer_VMServiceExternalTrafficPolicyLocal(t *testing.T) {
},
},
}
vms.DeepCopyInto(obj.(*vmopv1alpha1.VirtualMachineService))
return nil
vms.DeepCopyInto(vm)
return vm, nil
}

_, ensureErr := lb.EnsureLoadBalancer(context.Background(), testClustername, testK8sService, []*v1.Node{})
Expand All @@ -231,7 +228,7 @@ 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(ctx context.Context, vm *vmopv1alpha1.VirtualMachineService, opts metav1.CreateOptions) (result *vmopv1alpha1.VirtualMachineService, err error)
expectErr error
}{
{
Expand All @@ -240,8 +237,8 @@ func TestEnsureLoadBalancer(t *testing.T) {
},
{
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(ctx context.Context, vm *vmopv1alpha1.VirtualMachineService, opts metav1.CreateOptions) (result *vmopv1alpha1.VirtualMachineService, err error) {
return nil, fmt.Errorf(vmservice.ErrCreateVMService.Error())
},
expectErr: vmservice.ErrCreateVMService,
},
Expand Down Expand Up @@ -276,7 +273,7 @@ func TestEnsureLoadBalancer_VMServiceCreatedIPFound(t *testing.T) {
},
}
// 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 {
fcw.CreateFunc = func(ctx context.Context, vm *vmopv1alpha1.VirtualMachineService, opts metav1.CreateOptions) (result *vmopv1alpha1.VirtualMachineService, err error) {
vms := &vmopv1alpha1.VirtualMachineService{
Status: vmopv1alpha1.VirtualMachineServiceStatus{
LoadBalancer: vmopv1alpha1.LoadBalancerStatus{
Expand Down Expand Up @@ -309,8 +306,8 @@ func TestEnsureLoadBalancer_VMServiceCreatedIPFound(t *testing.T) {
},
},
}
vms.DeepCopyInto(obj.(*vmopv1alpha1.VirtualMachineService))
return nil
vms.DeepCopyInto(vm)
return vm, nil
}

status, ensureErr := lb.EnsureLoadBalancer(context.Background(), testClustername, testK8sService, []*v1.Node{})
Expand All @@ -324,18 +321,18 @@ 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(ctx context.Context, namespace, name string, opts metav1.DeleteOptions) error
expectErr string
}{
{
name: "should ignore not found error",
deleteFunc: func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error {
deleteFunc: func(ctx context.Context, namespace, name string, opts metav1.DeleteOptions) error {
return apierrors.NewNotFound(vmopv1alpha1.Resource("virtualmachineservice"), testClustername)
},
},
{
name: "should return error",
deleteFunc: func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error {
deleteFunc: func(ctx context.Context, namespace, name string, opts metav1.DeleteOptions) error {
return fmt.Errorf("an error occurred while deleting load balancer")
},
expectErr: "an error occurred while deleting load balancer",
Expand Down
19 changes: 8 additions & 11 deletions pkg/cloudprovider/vsphereparavirtual/vmoperator.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@ import (

vmopv1alpha1 "github.com/vmware-tanzu/vm-operator-api/api/v1alpha1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/cloud-provider-vsphere/pkg/util"
"sigs.k8s.io/controller-runtime/pkg/client"

vmopclient "k8s.io/cloud-provider-vsphere/pkg/cloudprovider/vsphereparavirtual/vmop/clientset/versioned"
)

// discoverNodeByProviderID takes a ProviderID and returns a VirtualMachine if one exists, or nil otherwise
// VirtualMachine not found is not an error
func discoverNodeByProviderID(ctx context.Context, providerID string, namespace string, vmClient client.Client) (*vmopv1alpha1.VirtualMachine, error) {
func discoverNodeByProviderID(ctx context.Context, providerID string, namespace string, vmClient vmopclient.Interface) (*vmopv1alpha1.VirtualMachine, error) {
var discoveredNode *vmopv1alpha1.VirtualMachine = nil

// Adding Retry here because there is no retry in caller from node controller
Expand All @@ -22,10 +24,7 @@ func discoverNodeByProviderID(ctx context.Context, providerID string, namespace
checkError,
func() error {
uuid := GetUUIDFromProviderID(providerID)
vms := vmopv1alpha1.VirtualMachineList{}
err := vmClient.List(ctx, &vms, &client.ListOptions{
Namespace: namespace,
})
vms, err := vmClient.VmoperatorV1alpha1().VirtualMachines(namespace).List(ctx, metav1.ListOptions{})
if err != nil {
return err
}
Expand All @@ -45,7 +44,7 @@ func discoverNodeByProviderID(ctx context.Context, providerID string, namespace

// discoverNodeByName takes a node name and returns a VirtualMachine if one exists, or nil otherwise
// VirtualMachine not found is not an error
func discoverNodeByName(ctx context.Context, name types.NodeName, namespace string, vmClient client.Client) (*vmopv1alpha1.VirtualMachine, error) {
func discoverNodeByName(ctx context.Context, name types.NodeName, namespace string, vmClient vmopclient.Interface) (*vmopv1alpha1.VirtualMachine, error) {
var discoveredNode *vmopv1alpha1.VirtualMachine = nil

// Adding Retry here because there is no retry in caller from node controller
Expand All @@ -54,16 +53,14 @@ func discoverNodeByName(ctx context.Context, name types.NodeName, namespace stri
DiscoverNodeBackoff,
checkError,
func() error {
vmKey := types.NamespacedName{Name: string(name), Namespace: namespace}
vm := vmopv1alpha1.VirtualMachine{}
err := vmClient.Get(ctx, vmKey, &vm)
vm, err := vmClient.VmoperatorV1alpha1().VirtualMachines(namespace).Get(ctx, string(name), metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
return nil
}
return err
}
discoveredNode = &vm
discoveredNode = vm
return nil
})

Expand Down
4 changes: 2 additions & 2 deletions pkg/cloudprovider/vsphereparavirtual/vmservice/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ import (
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/klog/v2/klogr"
"sigs.k8s.io/controller-runtime/pkg/client"

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

var log = klogr.New().WithName("vmservice")
Expand All @@ -41,7 +41,7 @@ type VMService interface {

// vmService takes care of mapping of LB type of service to VM service in supervisor cluster
type vmService struct {
vmClient client.Client
vmClient vmopclient.Interface
namespace string
ownerReference *metav1.OwnerReference
}
34 changes: 13 additions & 21 deletions pkg/cloudprovider/vsphereparavirtual/vmservice/vmservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,10 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
runtime "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
rest "k8s.io/client-go/rest"
client "sigs.k8s.io/controller-runtime/pkg/client"

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

const (
Expand Down Expand Up @@ -89,17 +88,14 @@ var (

// GetVmopClient gets a vm-operator-api client
// This is separate from NewVMService so that a fake client can be injected for testing
func GetVmopClient(config *rest.Config) (client.Client, error) {
func GetVmopClient(config *rest.Config) (vmopclient.Interface, error) {
scheme := runtime.NewScheme()
_ = vmopv1alpha1.AddToScheme(scheme)
client, err := client.New(config, client.Options{
Scheme: scheme,
})
return client, err
return vmopclient.NewForConfig(config)
}

// NewVMService creates a vmService object
func NewVMService(vmClient client.Client, ns string, ownerRef *metav1.OwnerReference) VMService {
func NewVMService(vmClient vmopclient.Interface, ns string, ownerRef *metav1.OwnerReference) VMService {
return &vmService{
vmClient: vmClient,
namespace: ns,
Expand Down Expand Up @@ -135,18 +131,16 @@ func (s *vmService) Get(ctx context.Context, service *v1.Service, clusterName st
logger := log.WithValues("name", service.Name, "namespace", service.Namespace)
logger.V(2).Info("Attempting to get VirtualMachineService")

vmService := vmopv1alpha1.VirtualMachineService{}
vmServiceKey := types.NamespacedName{Name: s.GetVMServiceName(service, clusterName), Namespace: s.namespace}

if err := s.vmClient.Get(ctx, vmServiceKey, &vmService); err != nil {
vmService, err := s.vmClient.VmoperatorV1alpha1().VirtualMachineServices(s.namespace).Get(ctx, s.GetVMServiceName(service, clusterName), metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
return nil, nil
}
logger.Error(ErrGetVMService, fmt.Sprintf("%v", err))
return nil, err
}

return &vmService, nil
return vmService, nil
}

// Create creates a vmservice to map to the given lb type of service, it should be called if vmservice not found
Expand All @@ -160,8 +154,8 @@ func (s *vmService) Create(ctx context.Context, service *v1.Service, clusterName
return nil, err
}

vmService.Namespace = s.namespace
if err = s.vmClient.Create(ctx, vmService); err != nil {
vmService, err = s.vmClient.VmoperatorV1alpha1().VirtualMachineServices(s.namespace).Create(ctx, vmService, metav1.CreateOptions{})
if err != nil {
logger.Error(ErrCreateVMService, fmt.Sprintf("%v", err))
return nil, err
}
Expand Down Expand Up @@ -258,7 +252,8 @@ func (s *vmService) Update(ctx context.Context, service *v1.Service, clusterName
}

if needsUpdate {
if err := s.vmClient.Update(ctx, newVMService); err != nil {
newVMService, err = s.vmClient.VmoperatorV1alpha1().VirtualMachineServices(s.namespace).Update(ctx, newVMService, metav1.UpdateOptions{})
if err != nil {
logger.Error(ErrUpdateVMService, fmt.Sprintf("%v", err))
return nil, err
}
Expand All @@ -275,11 +270,8 @@ func (s *vmService) Delete(ctx context.Context, service *v1.Service, clusterName
logger := log.WithValues("name", service.Name, "namespace", service.Namespace)
logger.V(2).Info("Attempting to delete VirtualMachineService")

vmService := vmopv1alpha1.VirtualMachineService{}
vmService.Name = s.GetVMServiceName(service, clusterName)
vmService.Namespace = s.namespace

if err := s.vmClient.Delete(ctx, &vmService); err != nil {
err := s.vmClient.VmoperatorV1alpha1().VirtualMachineServices(s.namespace).Delete(ctx, s.GetVMServiceName(service, clusterName), metav1.DeleteOptions{})
if err != nil {
logger.Error(ErrDeleteVMService, fmt.Sprintf("%v", err))
return err
}
Expand Down
Loading

0 comments on commit d6d4f9e

Please sign in to comment.