Skip to content

Commit

Permalink
Merge pull request #3235 from rikatz/fix-context-logging
Browse files Browse the repository at this point in the history 
🌱 Fix return messages to contain proper VM information
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Oct 28, 2024
2 parents 19f1a8f + 8e95493 commit fea2ad7
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 27 deletions.
2 changes: 1 addition & 1 deletion pkg/services/govmomi/power.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (vms *VMService) getPowerState(ctx context.Context, virtualMachineCtx *virt
case types.VirtualMachinePowerStateSuspended:
return infrav1.VirtualMachinePowerStateSuspended, nil
default:
return "", errors.Errorf("unexpected power state %q for vm %s", powerState, ctx)
return "", errors.Errorf("unexpected power state %q for vm %s", powerState, virtualMachineCtx)
}
}

Expand Down
20 changes: 10 additions & 10 deletions pkg/services/govmomi/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ func (vms *VMService) reconcileMetadata(ctx context.Context, virtualMachineCtx *
log.Info("Updating VM metadata")
taskRef, err := vms.setMetadata(ctx, virtualMachineCtx, newMetadata)
if err != nil {
return false, errors.Wrapf(err, "unable to set metadata on vm %s", ctx)
return false, errors.Wrapf(err, "unable to set metadata on vm %s", virtualMachineCtx)
}

virtualMachineCtx.VSphereVM.Status.TaskRef = taskRef
Expand All @@ -356,7 +356,7 @@ func (vms *VMService) reconcilePowerState(ctx context.Context, virtualMachineCtx
task, err := virtualMachineCtx.Obj.PowerOn(ctx)
if err != nil {
conditions.MarkFalse(virtualMachineCtx.VSphereVM, infrav1.VMProvisionedCondition, infrav1.PoweringOnFailedReason, clusterv1.ConditionSeverityWarning, err.Error())
return false, errors.Wrapf(err, "failed to trigger power on op for vm %s", ctx)
return false, errors.Wrapf(err, "failed to trigger power on op for vm %s", virtualMachineCtx)
}
conditions.MarkFalse(virtualMachineCtx.VSphereVM, infrav1.VMProvisionedCondition, infrav1.PoweringOnReason, clusterv1.ConditionSeverityInfo, "")

Expand All @@ -376,7 +376,7 @@ func (vms *VMService) reconcilePowerState(ctx context.Context, virtualMachineCtx
log.Info("VM is powered on")
return true, nil
default:
return false, errors.Errorf("unexpected power state %q for vm %s", powerState, ctx)
return false, errors.Errorf("unexpected power state %q for vm %s", powerState, virtualMachineCtx)
}
}

Expand Down Expand Up @@ -465,7 +465,7 @@ func (vms *VMService) reconcileStoragePolicy(ctx context.Context, virtualMachine
DeviceChange: changes,
})
if err != nil {
return errors.Wrapf(err, "unable to set storagePolicy on vm %s", ctx)
return errors.Wrapf(err, "unable to set storagePolicy on vm %s", virtualMachineCtx)
}
virtualMachineCtx.VSphereVM.Status.TaskRef = task.Reference().Value
}
Expand All @@ -492,7 +492,7 @@ func (vms *VMService) reconcileHardwareVersion(ctx context.Context, virtualMachi
log.Info("Upgrading hardware version", "fromVersion", virtualMachine.Config.Version, "toVersion", virtualMachineCtx.VSphereVM.Spec.HardwareVersion)
task, err := virtualMachineCtx.Obj.UpgradeVM(ctx, virtualMachineCtx.VSphereVM.Spec.HardwareVersion)
if err != nil {
return false, errors.Wrapf(err, "error trigging upgrade op for machine %s", ctx)
return false, errors.Wrapf(err, "error trigging upgrade op for machine %s", virtualMachineCtx)
}
virtualMachineCtx.VSphereVM.Status.TaskRef = task.Reference().Value
return false, nil
Expand Down Expand Up @@ -535,7 +535,7 @@ func (vms *VMService) reconcilePCIDevices(ctx context.Context, virtualMachineCtx
}
log.Info("PCI devices to be added", "number", len(specsToBeAdded))
if err := virtualMachineCtx.Obj.AddDevice(ctx, pci.ConstructDeviceSpecs(specsToBeAdded)...); err != nil {
return errors.Wrapf(err, "error adding pci devices for %q", ctx)
return errors.Wrapf(err, "error adding pci devices for %q", virtualMachineCtx)
}
}
return nil
Expand All @@ -550,7 +550,7 @@ func (vms *VMService) getMetadata(ctx context.Context, virtualMachineCtx *virtua
)

if err := pc.RetrieveOne(ctx, virtualMachineCtx.Ref, props, &obj); err != nil {
return "", errors.Wrapf(err, "unable to fetch props %v for vm %s", props, ctx)
return "", errors.Wrapf(err, "unable to fetch props %v for vm %s", props, virtualMachineCtx)
}
if obj.Config == nil {
return "", nil
Expand All @@ -571,7 +571,7 @@ func (vms *VMService) getMetadata(ctx context.Context, virtualMachineCtx *virtua

metadataBuf, err := base64.StdEncoding.DecodeString(metadataBase64)
if err != nil {
return "", errors.Wrapf(err, "unable to decode metadata for %s", ctx)
return "", errors.Wrapf(err, "unable to decode metadata for %s", virtualMachineCtx)
}

return string(metadataBuf), nil
Expand Down Expand Up @@ -599,7 +599,7 @@ func (vms *VMService) setMetadata(ctx context.Context, virtualMachineCtx *virtua
ExtraConfig: extraConfig,
})
if err != nil {
return "", errors.Wrapf(err, "unable to set metadata on vm %s", ctx)
return "", errors.Wrapf(err, "unable to set metadata on vm %s", virtualMachineCtx)
}

return task.Reference().Value, nil
Expand Down Expand Up @@ -641,7 +641,7 @@ func (vms *VMService) getBootstrapData(ctx context.Context, vmCtx *capvcontext.V
Name: vmCtx.VSphereVM.Spec.BootstrapRef.Name,
}
if err := vmCtx.Client.Get(ctx, secretKey, secret); err != nil {
return nil, "", errors.Wrapf(err, "failed to get bootstrap data secret for %s", ctx)
return nil, "", errors.Wrapf(err, "failed to get bootstrap data secret for %s", vmCtx)
}

format, ok := secret.Data["format"]
Expand Down
8 changes: 4 additions & 4 deletions pkg/services/govmomi/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,11 @@ func reconcileVSphereVMWhenNetworkIsReady(ctx context.Context, virtualMachineCtx
// Wait for the VM to be powered on.
powerOnTaskInfo, err := powerOnTask.WaitForResult(ctx)
if err != nil && powerOnTaskInfo == nil {
return nil, nil, errors.Wrapf(err, "failed to wait for power on op for vm %s", ctx)
return nil, nil, errors.Wrapf(err, "failed to wait for power on op for vm %s", virtualMachineCtx)
}
powerState, err := virtualMachineCtx.Obj.PowerState(ctx)
if err != nil {
return nil, nil, errors.Wrapf(err, "failed to get power state for vm %s", ctx)
return nil, nil, errors.Wrapf(err, "failed to get power state for vm %s", virtualMachineCtx)
}
if powerState != types.VirtualMachinePowerStatePoweredOn {
return nil, nil, errors.Errorf(
Expand All @@ -208,7 +208,7 @@ func reconcileVSphereVMWhenNetworkIsReady(ctx context.Context, virtualMachineCtx

// Wait for all NICs to have valid MAC addresses.
if err := waitForMacAddresses(ctx, virtualMachineCtx); err != nil {
return nil, nil, errors.Wrapf(err, "failed to wait for mac addresses for vm %s", ctx)
return nil, nil, errors.Wrapf(err, "failed to wait for mac addresses for vm %s", virtualMachineCtx)
}

// Get all the MAC addresses. This is done separately from waiting
Expand All @@ -217,7 +217,7 @@ func reconcileVSphereVMWhenNetworkIsReady(ctx context.Context, virtualMachineCtx
// specs, and not the propery change order.
_, macToDeviceIndex, deviceToMacIndex, err := getMacAddresses(ctx, virtualMachineCtx)
if err != nil {
return nil, nil, errors.Wrapf(err, "failed to get mac addresses for vm %s", ctx)
return nil, nil, errors.Wrapf(err, "failed to get mac addresses for vm %s", virtualMachineCtx)
}

// Wait for the IP addresses to show up for the VM.
Expand Down
24 changes: 12 additions & 12 deletions pkg/services/govmomi/vcenter/clone.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,17 +120,17 @@ func Clone(ctx context.Context, vmCtx *capvcontext.VMContext, bootstrapData []by

folder, err := vmCtx.Session.Finder.FolderOrDefault(ctx, vmCtx.VSphereVM.Spec.Folder)
if err != nil {
return errors.Wrapf(err, "unable to get folder for %q", ctx)
return errors.Wrapf(err, "unable to get folder for %q", vmCtx)
}

pool, err := vmCtx.Session.Finder.ResourcePoolOrDefault(ctx, vmCtx.VSphereVM.Spec.ResourcePool)
if err != nil {
return errors.Wrapf(err, "unable to get resource pool for %q", ctx)
return errors.Wrapf(err, "unable to get resource pool for %q", vmCtx)
}

devices, err := tpl.Device(ctx)
if err != nil {
return errors.Wrapf(err, "error getting devices for %q", ctx)
return errors.Wrapf(err, "error getting devices for %q", vmCtx)
}

// Create a new list of device specs for cloning the VM.
Expand All @@ -140,14 +140,14 @@ func Clone(ctx context.Context, vmCtx *capvcontext.VMContext, bootstrapData []by
if snapshotRef == nil {
diskSpecs, err := getDiskSpec(vmCtx, devices)
if err != nil {
return errors.Wrapf(err, "error getting disk spec for %q", ctx)
return errors.Wrapf(err, "error getting disk spec for %q", vmCtx)
}
deviceSpecs = append(deviceSpecs, diskSpecs...)
}

networkSpecs, err := getNetworkSpecs(ctx, vmCtx, devices)
if err != nil {
return errors.Wrapf(err, "error getting network specs for %q", ctx)
return errors.Wrapf(err, "error getting network specs for %q", vmCtx)
}

deviceSpecs = append(deviceSpecs, networkSpecs...)
Expand Down Expand Up @@ -207,7 +207,7 @@ func Clone(ctx context.Context, vmCtx *capvcontext.VMContext, bootstrapData []by
if vmCtx.VSphereVM.Spec.Datastore != "" {
datastore, err := vmCtx.Session.Finder.Datastore(ctx, vmCtx.VSphereVM.Spec.Datastore)
if err != nil {
return errors.Wrapf(err, "unable to get datastore %s for %q", vmCtx.VSphereVM.Spec.Datastore, ctx)
return errors.Wrapf(err, "unable to get datastore %s for %q", vmCtx.VSphereVM.Spec.Datastore, vmCtx)
}
datastoreRef = types.NewReference(datastore.Reference())
spec.Location.Datastore = datastoreRef
Expand All @@ -217,12 +217,12 @@ func Clone(ctx context.Context, vmCtx *capvcontext.VMContext, bootstrapData []by
if vmCtx.VSphereVM.Spec.StoragePolicyName != "" {
pbmClient, err := pbm.NewClient(ctx, vmCtx.Session.Client.Client)
if err != nil {
return errors.Wrapf(err, "unable to create pbm client for %q", ctx)
return errors.Wrapf(err, "unable to create pbm client for %q", vmCtx)
}

storageProfileID, err = pbmClient.ProfileIDByName(ctx, vmCtx.VSphereVM.Spec.StoragePolicyName)
if err != nil {
return errors.Wrapf(err, "unable to get storageProfileID from name %s for %q", vmCtx.VSphereVM.Spec.StoragePolicyName, ctx)
return errors.Wrapf(err, "unable to get storageProfileID from name %s for %q", vmCtx.VSphereVM.Spec.StoragePolicyName, vmCtx)
}

var hubs []pbmTypes.PbmPlacementHub
Expand Down Expand Up @@ -279,7 +279,7 @@ func Clone(ctx context.Context, vmCtx *capvcontext.VMContext, bootstrapData []by
// if no datastore defined through VM spec or storage policy, use default
datastore, err := vmCtx.Session.Finder.DefaultDatastore(ctx)
if err != nil {
return errors.Wrapf(err, "unable to get default datastore for %q", ctx)
return errors.Wrapf(err, "unable to get default datastore for %q", vmCtx)
}
datastoreRef = types.NewReference(datastore.Reference())
}
Expand All @@ -292,7 +292,7 @@ func Clone(ctx context.Context, vmCtx *capvcontext.VMContext, bootstrapData []by
log.Info(fmt.Sprintf("Cloning Machine with clone mode %s", vmCtx.VSphereVM.Status.CloneMode))
task, err := tpl.Clone(ctx, folder, vmCtx.VSphereVM.Name, spec)
if err != nil {
return errors.Wrapf(err, "error trigging clone op for machine %s", ctx)
return errors.Wrapf(err, "error trigging clone op for machine %s", vmCtx)
}

vmCtx.VSphereVM.Status.TaskRef = task.Reference().Value
Expand Down Expand Up @@ -415,11 +415,11 @@ func getNetworkSpecs(ctx context.Context, vmCtx *capvcontext.VMContext, devices
}
backing, err := ref.EthernetCardBackingInfo(ctx)
if err != nil {
return nil, errors.Wrapf(err, "unable to create new ethernet card backing info for network %q on %q", netSpec.NetworkName, ctx)
return nil, errors.Wrapf(err, "unable to create new ethernet card backing info for network %q on %q", netSpec.NetworkName, vmCtx)
}
dev, err := object.EthernetCardTypes().CreateEthernetCard(ethCardType, backing)
if err != nil {
return nil, errors.Wrapf(err, "unable to create new ethernet card %q for network %q on %q", ethCardType, netSpec.NetworkName, ctx)
return nil, errors.Wrapf(err, "unable to create new ethernet card %q for network %q on %q", ethCardType, netSpec.NetworkName, vmCtx)
}

// Get the actual NIC object. This is safe to assert without a check
Expand Down

0 comments on commit fea2ad7

Please sign in to comment.