Skip to content

Commit

Permalink
apply review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
lukas016 committed Feb 16, 2024
1 parent cbc084d commit d78a06c
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 11 deletions.
44 changes: 34 additions & 10 deletions pkg/controllers/machine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,20 +464,17 @@ func (r *MachineReconciler) reconcileMachine(ctx context.Context, id string) err
log.V(1).Info("Reconciling domain")
state, volumeStates, nicStates, err := r.reconcileDomain(ctx, log, machine)

machine.Status.VolumeStatus = volumeStates
if err != nil {
if _, locErr := r.machines.Update(ctx, machine); locErr != nil {
log.Error(locErr, "failed to update API machine state")
locErr := r.updateAPIMachineStatus(ctx, machine, state, volumeStates, nicStates)
if locErr != nil {
log.Error(locErr, "failed to update API machine")
}
return providerimage.IgnoreImagePulling(err)
}
log.V(1).Info("Reconciled domain")

machine.Status.NetworkInterfaceStatus = nicStates
machine.Status.State = state

if _, err = r.machines.Update(ctx, machine); err != nil {
return fmt.Errorf("failed to update API machine state: %w", err)
if err = r.updateAPIMachineStatus(ctx, machine, state, volumeStates, nicStates); err != nil {
return fmt.Errorf("failed to update API machine: %w", err)
}

return nil
Expand All @@ -491,7 +488,7 @@ func (r *MachineReconciler) reconcileDomain(
log.V(1).Info("Looking up domain")
if _, err := r.libvirt.DomainLookupByUUID(libvirtutils.UUIDStringToBytes(machine.ID)); err != nil {
if !libvirt.IsNotFound(err) {
return "", machine.Status.VolumeStatus, nil, fmt.Errorf("error getting domain %s: %w", machine.ID, err)
return "", nil, nil, fmt.Errorf("error getting domain %s: %w", machine.ID, err)
}

log.V(1).Info("Creating new domain")
Expand All @@ -512,7 +509,7 @@ func (r *MachineReconciler) reconcileDomain(

state, err := r.getMachineState(machine.ID)
if err != nil {
return "", volumeStates, nil, fmt.Errorf("error getting machine state: %w", err)
return "", nil, nil, fmt.Errorf("error getting machine state: %w", err)
}

return state, volumeStates, nicStates, nil
Expand Down Expand Up @@ -903,6 +900,33 @@ func (r *MachineReconciler) getDomainDesc(machineID string) (*libvirtxml.Domain,
return domainXML, nil
}

func (r *MachineReconciler) updateAPIMachineStatus(ctx context.Context, machine *api.Machine, state api.MachineState, volumes []api.VolumeStatus, nics []api.NetworkInterfaceStatus) error {
// TODO: we can rewrite reconcile function for return whole new status structure.
requireUpdate := false

if state != "" && machine.Status.State != state {
requireUpdate = true
machine.Status.State = state
}

if len(volumes) != 0 {
requireUpdate = true
machine.Status.VolumeStatus = volumes
}

if len(nics) != 0 {
requireUpdate = true
machine.Status.NetworkInterfaceStatus = nics
}

if requireUpdate {
_, err := r.machines.Update(ctx, machine)
return err
}

return nil
}

func machineDomain(machineID string) libvirt.Domain {
return libvirt.Domain{
UUID: libvirtutils.UUIDStringToBytes(machineID),
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/machine_controller_volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func convertVolumesMapToListAndNormalize(currentStatus map[string]*api.VolumeSta

// sort is required for avoid reconcile loop triggered by different order of volumes.
sort.SliceStable(newVolumeStatus, func(i, j int) bool {
return strings.Compare(newVolumeStatus[i].Name, newVolumeStatus[j].Name) == -1
return newVolumeStatus[i].Name < newVolumeStatus[j].Name
})

return newVolumeStatus
Expand Down

0 comments on commit d78a06c

Please sign in to comment.