Skip to content

Commit

Permalink
resolve comment
Browse files Browse the repository at this point in the history
Signed-off-by: Xudong Liu <xudongl@vmware.com>
  • Loading branch information
Xudong Liu committed Feb 2, 2024
1 parent 1071d14 commit 1006123
Show file tree
Hide file tree
Showing 14 changed files with 269 additions and 153 deletions.
2 changes: 1 addition & 1 deletion pkg/cloudprovider/vsphereparavirtual/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import (
)

type instances struct {
vmClient vmop.V1alpha1Interface
vmClient vmop.Interface
namespace string
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/cloudprovider/vsphereparavirtual/instances_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,12 @@ func initTest(testVM *vmopv1alpha1.VirtualMachine) (*instances, *dynamicfake.Fak
scheme := runtime.NewScheme()
_ = vmopv1alpha1.AddToScheme(scheme)
fc := dynamicfake.NewSimpleDynamicClient(scheme)
fcw := vmopclient.NewFakeClient(fc)
fcw := vmopclient.NewFakeClientSet(fc)
instance := &instances{
vmClient: fcw,
namespace: testClusterNameSpace,
}
_, err := fcw.VirtualMachines(testVM.Namespace).Create(context.TODO(), testVM, metav1.CreateOptions{})
_, err := fcw.V1alpha1().VirtualMachines(testVM.Namespace).Create(context.TODO(), testVM, metav1.CreateOptions{})
return instance, fc, err
}

Expand Down
26 changes: 13 additions & 13 deletions pkg/cloudprovider/vsphereparavirtual/loadbalancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@ var (
}
)

func newTestLoadBalancer() (cloudprovider.LoadBalancer, *vmopclient.FakeClient) {
func newTestLoadBalancer() (cloudprovider.LoadBalancer, *dynamicfake.FakeDynamicClient) {
scheme := runtime.NewScheme()
_ = vmopv1alpha1.AddToScheme(scheme)
fc := dynamicfake.NewSimpleDynamicClient(scheme)
fcw := vmopclient.NewFakeClient(fc)
fcw := vmopclient.NewFakeClientSet(fc)

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

func TestNewLoadBalancer(t *testing.T) {
Expand Down Expand Up @@ -150,7 +150,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 @@ -176,7 +176,7 @@ func TestUpdateLoadBalancer(t *testing.T) {

if testCase.expectErr {
// Ensure that the client Update call returns an error on update
fcw.DynamicClient.PrependReactor("update", "virtualmachineservices", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
fc.PrependReactor("update", "virtualmachineservices", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, fmt.Errorf("Some undefined update error")
})
err = lb.UpdateLoadBalancer(context.Background(), testClustername, testK8sService, []*v1.Node{})
Expand All @@ -190,7 +190,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 @@ -201,7 +201,7 @@ func TestEnsureLoadBalancer_VMServiceExternalTrafficPolicyLocal(t *testing.T) {
},
}

fcw.DynamicClient.PrependReactor("create", "virtualmachineservices", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
fc.PrependReactor("create", "virtualmachineservices", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
unstructuredObj, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(&vmopv1alpha1.VirtualMachineService{
Status: vmopv1alpha1.VirtualMachineServiceStatus{
LoadBalancer: vmopv1alpha1.LoadBalancerStatus{
Expand Down Expand Up @@ -247,14 +247,14 @@ func TestEnsureLoadBalancer(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,
Namespace: testK8sServiceNameSpace,
},
}
fcw.DynamicClient.PrependReactor("create", "virtualmachineservices", 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 @@ -266,15 +266,15 @@ 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.DynamicClient.PrependReactor("create", "virtualmachineservices", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
fc.PrependReactor("create", "virtualmachineservices", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
unstructuredObj, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(&vmopv1alpha1.VirtualMachineService{
Status: vmopv1alpha1.VirtualMachineServiceStatus{
LoadBalancer: vmopv1alpha1.LoadBalancerStatus{
Expand Down Expand Up @@ -342,7 +342,7 @@ func TestEnsureLoadBalancer_DeleteLB(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 @@ -354,7 +354,7 @@ func TestEnsureLoadBalancer_DeleteLB(t *testing.T) {
err := lb.EnsureLoadBalancerDeleted(context.Background(), testClustername, testK8sService)
assert.NoError(t, err)

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

err = lb.EnsureLoadBalancerDeleted(context.Background(), "test", testK8sService)
if err != nil {
Expand Down
8 changes: 4 additions & 4 deletions pkg/cloudprovider/vsphereparavirtual/vmoperator.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (

// 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 vmop.V1alpha1Interface) (*vmopv1alpha1.VirtualMachine, error) {
func discoverNodeByProviderID(ctx context.Context, providerID string, namespace string, vmClient vmop.Interface) (*vmopv1alpha1.VirtualMachine, error) {
var discoveredNode *vmopv1alpha1.VirtualMachine = nil

// Adding Retry here because there is no retry in caller from node controller
Expand All @@ -24,7 +24,7 @@ func discoverNodeByProviderID(ctx context.Context, providerID string, namespace
checkError,
func() error {
uuid := GetUUIDFromProviderID(providerID)
vms, err := vmClient.VirtualMachines(namespace).List(ctx, metav1.ListOptions{})
vms, err := vmClient.V1alpha1().VirtualMachines(namespace).List(ctx, metav1.ListOptions{})
if err != nil {
return err
}
Expand All @@ -44,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 vmop.V1alpha1Interface) (*vmopv1alpha1.VirtualMachine, error) {
func discoverNodeByName(ctx context.Context, name types.NodeName, namespace string, vmClient vmop.Interface) (*vmopv1alpha1.VirtualMachine, error) {
var discoveredNode *vmopv1alpha1.VirtualMachine = nil

// Adding Retry here because there is no retry in caller from node controller
Expand All @@ -53,7 +53,7 @@ func discoverNodeByName(ctx context.Context, name types.NodeName, namespace stri
DiscoverNodeBackoff,
checkError,
func() error {
vm, err := vmClient.VirtualMachines(namespace).Get(ctx, string(name), metav1.GetOptions{})
vm, err := vmClient.V1alpha1().VirtualMachines(namespace).Get(ctx, string(name), metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
return nil
Expand Down
21 changes: 17 additions & 4 deletions pkg/cloudprovider/vsphereparavirtual/vmoperator/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,17 @@ var (
}
)

// Clientset contains the clients for groups. Each group has exactly one
// version included in a Clientset.
type Clientset struct {
vmopv1alpha1 *VmoperatorV1alpha1Client
}

// V1alpha1 retrieves the VmoperatorV1alpha1Client
func (c *Clientset) V1alpha1() vmoperator.V1alpha1Interface {
return c.vmopv1alpha1
}

// VmoperatorV1alpha1Client contains the dynamic client for vm operator group
type VmoperatorV1alpha1Client struct {
dynamicClient *dynamic.DynamicClient
Expand All @@ -50,7 +61,7 @@ func (c *VmoperatorV1alpha1Client) Client() dynamic.Interface {
}

// NewForConfig creates a new client for the given config.
func NewForConfig(c *rest.Config) (*VmoperatorV1alpha1Client, error) {
func NewForConfig(c *rest.Config) (*Clientset, error) {
scheme := runtime.NewScheme()
_ = vmopv1alpha1.AddToScheme(scheme)

Expand All @@ -59,8 +70,10 @@ func NewForConfig(c *rest.Config) (*VmoperatorV1alpha1Client, error) {
return nil, err
}

client := &VmoperatorV1alpha1Client{
dynamicClient: dynamicClient,
clientSet := &Clientset{
vmopv1alpha1: &VmoperatorV1alpha1Client{
dynamicClient: dynamicClient,
},
}
return client, nil
return clientSet, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,32 @@ import (
"k8s.io/cloud-provider-vsphere/pkg/cloudprovider/vsphereparavirtual/vmoperator"
)

// FakeClientSet contains the fake clients for groups. Each group has exactly one
// version included in a Clientset.
type FakeClientSet struct {
FakeClient *FakeClient
}

// V1alpha1 retrieves the fake VmoperatorV1alpha1Client
func (c *FakeClientSet) V1alpha1() vmoperator.V1alpha1Interface {
return c.FakeClient
}

// NewFakeClientSet creates a FakeClientWrapper
func NewFakeClientSet(fakeClient *dynamicfake.FakeDynamicClient) *FakeClientSet {
fcw := &FakeClientSet{
FakeClient: &FakeClient{
DynamicClient: fakeClient,
},
}
return fcw
}

// FakeClient contains the fake dynamic client for vm operator group
type FakeClient struct {
DynamicClient *dynamicfake.FakeDynamicClient
}

// NewFakeClient creates a FakeClientWrapper
func NewFakeClient(fakeClient *dynamicfake.FakeDynamicClient) *FakeClient {
fcw := FakeClient{}
fcw.DynamicClient = fakeClient
return &fcw
}

// VirtualMachines retrieves the virtualmachine client
func (c *FakeClient) VirtualMachines(namespace string) vmoperator.VirtualMachineInterface {
return newVirtualMachines(c, namespace)
Expand Down
Loading

0 comments on commit 1006123

Please sign in to comment.