Skip to content

Commit

Permalink
Get rid of unstructured type in VSphereVM reconciler
Browse files Browse the repository at this point in the history 
Signed-off-by: Gong Zhang <[email protected]>
  • Loading branch information
@zhanggbj
zhanggbj committed Sep 14, 2023
1 parent 21665a2 commit 4f81588
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 94 deletions.
138 changes: 46 additions & 92 deletions pkg/services/vimmachine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ import (
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
kerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/utils/integer"
Expand Down Expand Up @@ -127,32 +125,20 @@ func (v *VimMachineService) ReconcileNormal(machineCtx capvcontext.MachineContex
return false, err
}

// Convert the VM resource to unstructured data.
vmData, err := runtime.DefaultUnstructuredConverter.ToUnstructured(vm)
if err != nil {
return false, errors.Wrapf(err,
"failed to convert %s to unstructured data",
vm.GetObjectKind().GroupVersionKind().String())
}
vmObj := &unstructured.Unstructured{Object: vmData}
vmObj.SetGroupVersionKind(vm.GetObjectKind().GroupVersionKind())
vmObj.SetAPIVersion(vm.GetObjectKind().GroupVersionKind().GroupVersion().String())
vmObj.SetKind(vm.GetObjectKind().GroupVersionKind().Kind)

// Waits the VM's ready state.
if ok, err := v.waitReadyState(vimMachineCtx, vmObj); !ok {
if ok, err := v.waitReadyState(vimMachineCtx, vm); !ok {
if err != nil {
return false, errors.Wrapf(err, "unexpected error while reconciling ready state for %s", vimMachineCtx)
}
vimMachineCtx.Logger.Info("waiting for ready state")
// VSphereMachine wraps a VMSphereVM, so we are mirroring status from the underlying VMSphereVM
// in order to provide evidences about machine provisioning while provisioning is actually happening.
conditions.SetMirror(vimMachineCtx.VSphereMachine, infrav1.VMProvisionedCondition, conditions.UnstructuredGetter(vmObj))
conditions.SetMirror(vimMachineCtx.VSphereMachine, infrav1.VMProvisionedCondition, vm)
return true, nil
}

// Reconcile the VSphereMachine's provider ID using the VM's BIOS UUID.
if ok, err := v.reconcileProviderID(vimMachineCtx, vmObj); !ok {
if ok, err := v.reconcileProviderID(vimMachineCtx, vm); !ok {

Check warning on line 141 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L141 

Added line #L141 was not covered by tests
if err != nil {
return false, errors.Wrapf(err, "unexpected error while reconciling provider ID for %s", vimMachineCtx)
}
Expand All @@ -161,7 +147,7 @@ func (v *VimMachineService) ReconcileNormal(machineCtx capvcontext.MachineContex
}

// Reconcile the VSphereMachine's node addresses from the VM's IP addresses.
if ok, err := v.reconcileNetwork(vimMachineCtx, vmObj); !ok {
if ok, err := v.reconcileNetwork(vimMachineCtx, vm); !ok {

Check warning on line 150 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L150 

Added line #L150 was not covered by tests
if err != nil {
return false, errors.Wrapf(err, "unexpected error while reconciling network for %s", vimMachineCtx)
}
Expand Down Expand Up @@ -209,66 +195,36 @@ func (v *VimMachineService) findVSphereVM(vimMachineCtx *capvcontext.VIMMachineC
return vm, nil
}

func (v *VimMachineService) waitReadyState(vimMachineCtx *capvcontext.VIMMachineContext, vm *unstructured.Unstructured) (bool, error) {
ready, ok, err := unstructured.NestedBool(vm.Object, "status", "ready")
if !ok {
if err != nil {
return false, errors.Wrapf(err,
"unexpected error when getting status.ready from %s %s/%s for %s",
vm.GroupVersionKind(),
vm.GetNamespace(),
vm.GetName(),
vimMachineCtx)
}
vimMachineCtx.Logger.Info("status.ready not found",
"vmGVK", vm.GroupVersionKind().String(),
"vmNamespace", vm.GetNamespace(),
"vmName", vm.GetName())
return false, nil
}
func (v *VimMachineService) waitReadyState(vimMachineCtx *capvcontext.VIMMachineContext, vm *infrav1.VSphereVM) (bool, error) {
ready := vm.Status.Ready
if !ready {
vimMachineCtx.Logger.Info("status.ready is false",
"vmGVK", vm.GroupVersionKind().String(),
"vmNamespace", vm.GetNamespace(),
"vmName", vm.GetName())
"vmKind", vm.Kind,
"vmNamespace", vm.Namespace,
"vmName", vm.Name)
return false, nil
}

return true, nil
}

func (v *VimMachineService) reconcileProviderID(vimMachineCtx *capvcontext.VIMMachineContext, vm *unstructured.Unstructured) (bool, error) {
biosUUID, ok, err := unstructured.NestedString(vm.Object, "spec", "biosUUID")
if !ok {
if err != nil {
return false, errors.Wrapf(err,
"unexpected error when getting spec.biosUUID from %s %s/%s for %s",
vm.GroupVersionKind(),
vm.GetNamespace(),
vm.GetName(),
vimMachineCtx)
}
vimMachineCtx.Logger.Info("spec.biosUUID not found",
"vmGVK", vm.GroupVersionKind().String(),
"vmNamespace", vm.GetNamespace(),
"vmName", vm.GetName())
return false, nil
}
func (v *VimMachineService) reconcileProviderID(vimMachineCtx *capvcontext.VIMMachineContext, vm *infrav1.VSphereVM) (bool, error) {
biosUUID := vm.Spec.BiosUUID

Check warning on line 212 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L211-L212 

Added lines #L211 - L212 were not covered by tests
if biosUUID == "" {
vimMachineCtx.Logger.Info("spec.biosUUID is empty",
"vmGVK", vm.GroupVersionKind().String(),
"vmNamespace", vm.GetNamespace(),
"vmName", vm.GetName())
"vmKind", vm.Kind,
"vmNamespace", vm.Namespace,
"vmName", vm.Name)

Check warning on line 217 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L215-L217 

Added lines #L215 - L217 were not covered by tests
return false, nil
}

providerID := infrautilv1.ConvertUUIDToProviderID(biosUUID)
if providerID == "" {
return false, errors.Errorf("invalid BIOS UUID %s from %s %s/%s for %s",
biosUUID,
vm.GroupVersionKind(),
vm.GetNamespace(),
vm.GetName(),
vm.Kind,
vm.Namespace,
vm.Name,

Check warning on line 227 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L225-L227 

Added lines #L225 - L227 were not covered by tests
vimMachineCtx)
}
if vimMachineCtx.VSphereMachine.Spec.ProviderID == nil || *vimMachineCtx.VSphereMachine.Spec.ProviderID != providerID {
Expand All @@ -280,46 +236,44 @@ func (v *VimMachineService) reconcileProviderID(vimMachineCtx *capvcontext.VIMMa
}

//nolint:nestif
func (v *VimMachineService) reconcileNetwork(vimMachineCtx *capvcontext.VIMMachineContext, vm *unstructured.Unstructured) (bool, error) {
func (v *VimMachineService) reconcileNetwork(vimMachineCtx *capvcontext.VIMMachineContext, vm *infrav1.VSphereVM) (bool, error) {

Check warning on line 239 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L239 

Added line #L239 was not covered by tests
var errs []error
if networkStatusListOfIfaces, ok, _ := unstructured.NestedSlice(vm.Object, "status", "network"); ok {
var networkStatusList []infrav1.NetworkStatus
for i, networkStatusListMemberIface := range networkStatusListOfIfaces {
if buf, err := json.Marshal(networkStatusListMemberIface); err != nil {
networkStatusListOfIfaces := vm.Status.Network
var networkStatusList []infrav1.NetworkStatus
for i, networkStatusListMemberIface := range networkStatusListOfIfaces {
if buf, err := json.Marshal(networkStatusListMemberIface); err != nil {
vimMachineCtx.Logger.Error(err,
"unsupported data for member of status.network list",
"index", i)
errs = append(errs, err)
} else {
var networkStatus infrav1.NetworkStatus
err := json.Unmarshal(buf, &networkStatus)
if err == nil && networkStatus.MACAddr == "" {
err = errors.New("macAddr is required")
errs = append(errs, err)
}
if err != nil {

Check warning on line 256 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L241-L256 

Added lines #L241 - L256 were not covered by tests
vimMachineCtx.Logger.Error(err,
"unsupported data for member of status.network list",
"index", i)
"index", i, "data", string(buf))

Check warning on line 259 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L259 

Added line #L259 was not covered by tests
errs = append(errs, err)
} else {
var networkStatus infrav1.NetworkStatus
err := json.Unmarshal(buf, &networkStatus)
if err == nil && networkStatus.MACAddr == "" {
err = errors.New("macAddr is required")
errs = append(errs, err)
}
if err != nil {
vimMachineCtx.Logger.Error(err,
"unsupported data for member of status.network list",
"index", i, "data", string(buf))
errs = append(errs, err)
} else {
networkStatusList = append(networkStatusList, networkStatus)
}
networkStatusList = append(networkStatusList, networkStatus)

Check warning on line 262 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L262 

Added line #L262 was not covered by tests
}
}
vimMachineCtx.VSphereMachine.Status.Network = networkStatusList
}
vimMachineCtx.VSphereMachine.Status.Network = networkStatusList

Check warning on line 266 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L266 

Added line #L266 was not covered by tests

if addresses, ok, _ := unstructured.NestedStringSlice(vm.Object, "status", "addresses"); ok {
var machineAddresses []clusterv1.MachineAddress
for _, addr := range addresses {
machineAddresses = append(machineAddresses, clusterv1.MachineAddress{
Type: clusterv1.MachineExternalIP,
Address: addr,
})
}
vimMachineCtx.VSphereMachine.Status.Addresses = machineAddresses
addresses := vm.Status.Addresses
var machineAddresses []clusterv1.MachineAddress
for _, addr := range addresses {
machineAddresses = append(machineAddresses, clusterv1.MachineAddress{
Type: clusterv1.MachineExternalIP,
Address: addr,
})

Check warning on line 274 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L268-L274 

Added lines #L268 - L274 were not covered by tests
}
vimMachineCtx.VSphereMachine.Status.Addresses = machineAddresses

Check warning on line 276 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L276 

Added line #L276 was not covered by tests

if len(vimMachineCtx.VSphereMachine.Status.Addresses) == 0 {
vimMachineCtx.Logger.Info("waiting on IP addresses")
Expand All @@ -329,7 +283,7 @@ func (v *VimMachineService) reconcileNetwork(vimMachineCtx *capvcontext.VIMMachi
return true, nil
}

func (v *VimMachineService) createOrPatchVSphereVM(vimMachineCtx *capvcontext.VIMMachineContext, vsphereVM *infrav1.VSphereVM) (runtime.Object, error) {
func (v *VimMachineService) createOrPatchVSphereVM(vimMachineCtx *capvcontext.VIMMachineContext, vsphereVM *infrav1.VSphereVM) (*infrav1.VSphereVM, error) {
// Create or update the VSphereVM resource.
vm := &infrav1.VSphereVM{
ObjectMeta: metav1.ObjectMeta{
Expand Down
4 changes: 2 additions & 2 deletions pkg/services/vimmachine_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ var _ = Describe("VimMachineService_createOrPatchVSphereVM", func() {
})
It("returns a renamed vspherevm object", func() {
vm, err := vimMachineService.createOrPatchVSphereVM(machineCtx, getVSphereVM(hostAddr, corev1.ConditionTrue))
vmName := vm.(*infrav1.VSphereVM).GetName()
vmName := vm.Name
Expect(err).NotTo(HaveOccurred())
Expect(vmName).To(Equal("fake-long-rname"))
})
Expand All @@ -287,7 +287,7 @@ var _ = Describe("VimMachineService_createOrPatchVSphereVM", func() {
})
It("returns the same vspherevm name", func() {
vm, err := vimMachineService.createOrPatchVSphereVM(machineCtx, getVSphereVM(hostAddr, corev1.ConditionTrue))
vmName := vm.(*infrav1.VSphereVM).GetName()
vmName := vm.Name
Expect(err).NotTo(HaveOccurred())
Expect(vmName).To(Equal(fakeLongClusterName))
})
Expand Down

0 comments on commit 4f81588

Please sign in to comment.