Skip to content

Add more context to vsphere errors #354

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 14, 2018
Merged

Add more context to vsphere errors #354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b4c57d9
Add more context to vsphere errors
alvaroaleman Oct 12, 2018
63ea478
Do not return an error if there was none
alvaroaleman Oct 12, 2018
5369281
Disable autoupgrade for coreos in e2e tests to speed up testing
alvaroaleman Oct 12, 2018
9e9f232
Merge branch 'master' into vsphere-context
alvaroaleman Oct 14, 2018
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
34 changes: 18 additions & 16 deletions pkg/cloudprovider/provider/vsphere/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ func createLinkClonedVM(vmName, vmImage, datacenter, clusterName, folder string,

dc, err := f.Datacenter(ctx, datacenter)
if err != nil {
return err
return fmt.Errorf("failed to get datacenter: %v", err)
}
f.SetDatacenter(dc)

templateVM, err := f.VirtualMachine(ctx, vmImage)
if err != nil {
return err
return fmt.Errorf("failed to get virtualmachine: %v", err)
}

glog.V(3).Infof("Template VM ref is %+v", templateVM)
Expand Down Expand Up @@ -99,7 +99,7 @@ func createLinkClonedVM(vmName, vmImage, datacenter, clusterName, folder string,
var mvm mo.VirtualMachine
err = templateVM.Properties(ctx, templateVM.Reference(), []string{"config", "config.vAppConfig", "config.vAppConfig.property"}, &mvm)
if err != nil {
return err
return fmt.Errorf("failed to extract vapp properties for coreos: %v", err)
}

var propertySpecs []types.VAppPropertySpec
Expand Down Expand Up @@ -153,22 +153,24 @@ func createLinkClonedVM(vmName, vmImage, datacenter, clusterName, folder string,
// Create a link cloned VM from the template VM's snapshot
clonedVMTask, err := templateVM.Clone(ctx, targetVMFolder, vmName, *cloneSpec)
if err != nil {
return err
return fmt.Errorf("failed to clone template vm: %v", err)
}

_, err = clonedVMTask.WaitForResult(ctx, nil)
return err
if _, err = clonedVMTask.WaitForResult(ctx, nil); err != nil {
return fmt.Errorf("error when waiting for result of clone task: %v", err)
}
return nil
}

func updateNetworkForVM(ctx context.Context, vm *object.VirtualMachine, currentNetName string, newNetName string) error {
newNet, err := getNetworkFromVM(ctx, vm, newNetName)
if err != nil {
return err
return fmt.Errorf("failed to get network from vm: %v", err)
}

availableData, err := getNetworkDevicesAndBackingsFromVM(ctx, vm, currentNetName)
if err != nil {
return err
return fmt.Errorf("failed to get network devices for vm: %v", err)
}
if len(availableData) == 0 {
return errors.New("found no matching network adapter")
Expand Down Expand Up @@ -233,12 +235,12 @@ func getNetworkFromVM(ctx context.Context, vm *object.VirtualMachine, netName st
func createSnapshot(ctx context.Context, vm *object.VirtualMachine, snapshotName string, snapshotDesc string) (object.Reference, error) {
task, err := vm.CreateSnapshot(ctx, snapshotName, snapshotDesc, false, false)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to create snapshot: %v", err)
}

taskInfo, err := task.WaitForResult(ctx, nil)
if err != nil {
return nil, err
return nil, fmt.Errorf("error waiting for task completion: %v", err)
}
glog.Infof("taskInfo.Result is %s", taskInfo.Result)
return taskInfo.Result.(object.Reference), nil
Expand All @@ -249,7 +251,7 @@ func findSnapshot(ctx context.Context, vm *object.VirtualMachine, name string) (

err := vm.Properties(ctx, vm.Reference(), []string{"snapshot"}, &moVirtualMachine)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get vm properties: %v", err)
}

if moVirtualMachine.Snapshot == nil {
Expand Down Expand Up @@ -288,29 +290,29 @@ func uploadAndAttachISO(f *find.Finder, vmRef *object.VirtualMachine, localIsoFi

datastore, err := f.Datastore(ctx, datastoreName)
if err != nil {
return err
return fmt.Errorf("failed to get datastore: %v", err)
}
p := soap.DefaultUpload
remoteIsoFilePath := fmt.Sprintf("%s/%s", vmRef.Name(), "cloud-init.iso")
glog.V(3).Infof("Uploading userdata ISO to datastore %+v, destination iso is %s\n", datastore, remoteIsoFilePath)
err = datastore.UploadFile(ctx, localIsoFilePath, remoteIsoFilePath, &p)
if err != nil {
return err
return fmt.Errorf("failed to upload iso: %v", err)
}
glog.V(3).Infof("Uploaded ISO file %s", localIsoFilePath)

// Find the cd-rom devide and insert the cloud init iso file into it.
devices, err := vmRef.Device(ctx)
if err != nil {
return err
return fmt.Errorf("failed to get devices: %v", err)
}

// passing empty cd-rom name so that the first one gets returned
cdrom, err := devices.FindCdrom("")
cdrom.Connectable.StartConnected = true
if err != nil {
return err
return fmt.Errorf("failed to find cdrom device: %v", err)
}
cdrom.Connectable.StartConnected = true
Copy link
Contributor

@mrIncompetent mrIncompetent Oct 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary?

Copy link
Contributor Author

@alvaroaleman alvaroaleman Oct 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its the device that contains the iso in which we put the userdata, I'd like it to be connected ;) This PR just moved that line after the error handling

Copy link
Contributor

@mrIncompetent mrIncompetent Oct 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, didn't spotted the removal above 🤦‍♂️

iso := datastore.Path(remoteIsoFilePath)
return vmRef.EditDevice(ctx, devices.InsertIso(cdrom, iso))
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/cloudprovider/provider/vsphere/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,17 +125,17 @@ func (p *provider) AddDefaults(spec v1alpha1.MachineSpec) (v1alpha1.MachineSpec,

finder, err := getDatacenterFinder(cfg.Datacenter, client)
if err != nil {
return spec, changed, err
return spec, changed, fmt.Errorf("failed to get datacenter finder: %v", err)
}

templateVM, err := finder.VirtualMachine(ctx, cfg.TemplateVMName)
if err != nil {
return spec, changed, err
return spec, changed, fmt.Errorf("failed to get virtual machine: %v", err)
}

availableNetworkDevices, err := getNetworkDevicesAndBackingsFromVM(ctx, templateVM, "")
if err != nil {
return spec, changed, err
return spec, changed, fmt.Errorf("failed to get network devices for vm: %v", err)
}

if len(availableNetworkDevices) == 0 {
Expand Down Expand Up @@ -381,7 +381,7 @@ func (p *provider) Create(machine *v1alpha1.Machine, _ cloud.MachineUpdater, use
// Upstream issue: https://bugs.launchpad.net/cloud-images/+bug/1573095
err = removeFloppyDevice(virtualMachine)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to remove floppy device: %v", err)
}

powerOnTask, err := virtualMachine.PowerOn(context.TODO())
Expand All @@ -404,7 +404,7 @@ func (p *provider) Delete(machine *v1alpha1.Machine, _ cloud.MachineUpdater) err
if err == cloudprovidererrors.ErrInstanceNotFound {
return nil
}
return err
return fmt.Errorf("failed to get instance: %v", err)
}

config, pc, _, err := p.getConfig(machine.Spec.ProviderConfig)
Expand Down Expand Up @@ -499,7 +499,7 @@ func (p *provider) Get(machine *v1alpha1.Machine) (instance.Instance, error) {

finder, err := getDatacenterFinder(config.Datacenter, client)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get datacenter finder: %v", err)
}
virtualMachine, err := finder.VirtualMachine(context.TODO(), machine.Spec.Name)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions test/e2e/provisioning/testdata/machinedeployment-aws.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ spec:
# Can be 'ubuntu', 'coreos' or 'centos'
operatingSystem: "<< OS_NAME >>"
operatingSystemSpec:
distUpgradeOnBoot: false
disableAutoUpdate: true
containerRuntimeInfo:
name: "<< CONTAINER_RUNTIME >>"
Expand Down
1 change: 1 addition & 0 deletions test/e2e/provisioning/testdata/machinedeployment-azure.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ spec:
operatingSystem: "<< OS_NAME >>"
operatingSystemSpec:
distUpgradeOnBoot: false
disableAutoUpdate: true
containerRuntimeInfo:
name: "<< CONTAINER_RUNTIME >>"
versions:
Expand Down
1 change: 1 addition & 0 deletions test/e2e/provisioning/testdata/machinedeployment-digitalocean.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ spec:
operatingSystem: "<< OS_NAME >>"
operatingSystemSpec:
distUpgradeOnBoot: false
disableAutoUpdate: true
containerRuntimeInfo:
name: "<< CONTAINER_RUNTIME >>"
versions:
Expand Down
1 change: 1 addition & 0 deletions test/e2e/provisioning/testdata/machinedeployment-hetzner.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ spec:
operatingSystem: "<< OS_NAME >>"
operatingSystemSpec:
distUpgradeOnBoot: false
disableAutoUpdate: true
containerRuntimeInfo:
name: "<< CONTAINER_RUNTIME >>"
versions:
Expand Down
1 change: 1 addition & 0 deletions test/e2e/provisioning/testdata/machinedeployment-openstack-upgrade.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ spec:
operatingSystem: "<< OS_NAME >>"
operatingSystemSpec:
distUpgradeOnBoot: true
disableAutoUpdate: true
containerRuntimeInfo:
name: "<< CONTAINER_RUNTIME >>"
versions:
Expand Down
1 change: 1 addition & 0 deletions test/e2e/provisioning/testdata/machinedeployment-openstack.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ spec:
operatingSystem: "<< OS_NAME >>"
operatingSystemSpec:
distUpgradeOnBoot: false
disableAutoUpdate: true
containerRuntimeInfo:
name: "<< CONTAINER_RUNTIME >>"
versions:
Expand Down
1 change: 1 addition & 0 deletions test/e2e/provisioning/testdata/machinedeployment-vsphere-static-ip.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ spec:
operatingSystem: "<< OS_NAME >>"
operatingSystemSpec:
distUpgradeOnBoot: false
disableAutoUpdate: true
network:
cidr: "192.168.44.<< IP_OCTET >>/20"
gateway: "192.168.32.1"
Expand Down
1 change: 1 addition & 0 deletions test/e2e/provisioning/testdata/machinedeployment-vsphere.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ spec:
operatingSystem: "<< OS_NAME >>"
operatingSystemSpec:
distUpgradeOnBoot: false
disableAutoUpdate: true
containerRuntimeInfo:
name: "<< CONTAINER_RUNTIME >>"
versions:
Expand Down