Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱 Get rid of unstructured type in VSphereVM reconciler #2353

Merged
merged 1 commit into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 41 additions & 103 deletions pkg/services/vimmachine.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
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,17 @@
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 err != nil {
return false, errors.Wrapf(err, "unexpected error while reconciling ready state for %s", vimMachineCtx)
}
if !vm.Status.Ready {
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 138 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L138

Added line #L138 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 +144,7 @@
}

// 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 147 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L147

Added line #L147 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 +192,23 @@
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
}
if !ready {
vimMachineCtx.Logger.Info("status.ready is false",
"vmGVK", vm.GroupVersionKind().String(),
"vmNamespace", vm.GetNamespace(),
"vmName", vm.GetName())
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 196 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L195-L196

Added lines #L195 - L196 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 201 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L199-L201

Added lines #L199 - L201 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 211 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L209-L211

Added lines #L209 - L211 were not covered by tests
vimMachineCtx)
}
if vimMachineCtx.VSphereMachine.Spec.ProviderID == nil || *vimMachineCtx.VSphereMachine.Spec.ProviderID != providerID {
Expand All @@ -280,46 +220,44 @@
}

//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 223 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L223

Added line #L223 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 240 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L225-L240

Added lines #L225 - L240 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 243 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L243

Added line #L243 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 246 in pkg/services/vimmachine.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L246

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

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

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L250

Added line #L250 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
machineAddresses := make([]clusterv1.MachineAddress, 0, len(addresses))
for _, addr := range addresses {
machineAddresses = append(machineAddresses, clusterv1.MachineAddress{
Type: clusterv1.MachineExternalIP,
Address: addr,
})

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

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L252-L258

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

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

View check run for this annotation

Codecov / codecov/patch

pkg/services/vimmachine.go#L260

Added line #L260 was not covered by tests

if len(vimMachineCtx.VSphereMachine.Status.Addresses) == 0 {
vimMachineCtx.Logger.Info("waiting on IP addresses")
Expand All @@ -329,7 +267,7 @@
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
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
Loading