Skip to content

make volumeClient failure fatal only when needed #75

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

Closed
sjenning wants to merge 1 commit into from
Closed

make volumeClient failure fatal only when needed #75

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 6 additions & 4 deletions pkg/cloud/openstack/clients/machineservice.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,12 +226,10 @@ func NewInstanceServiceFromCloud(cloud clientconfig.Cloud) (*InstanceService, er
return nil, fmt.Errorf("Create ImageClient err: %v", err)
}

volumeClient, err := openstack.NewBlockStorageV3(provider, gophercloud.EndpointOpts{
// defer error reporting until the volumeClient would actually be needed
volumeClient, _ := openstack.NewBlockStorageV3(provider, gophercloud.EndpointOpts{
Region: clientOpts.RegionName,
})
if err != nil {
return nil, fmt.Errorf("Create VolumeClient err: %v", err)
}

return &InstanceService{
provider: provider,
Expand Down Expand Up @@ -637,6 +635,10 @@ func (is *InstanceService) InstanceCreate(clusterName string, name string, clust
// if source type is "image" then we have to create a volume from the image first
klog.Infof("Creating a bootable volume from image %v.", config.RootVolume.SourceUUID)

if is.volumeClient == nil {
return nil, fmt.Errorf("Volume requested but block storage service is not available")
Copy link
Copy Markdown

@tomassedovic tomassedovic Jan 2, 2020

Choose a reason for hiding this comment

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

Thanks! I like the change, but I'd prefer to display the original error when creating the client too. Would you be open to adding that?

Copy link
Copy Markdown

@pierreprinetti pierreprinetti Jan 21, 2020

Choose a reason for hiding this comment

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

This could be achieved with client lazy-loading, by storing volumeClient in InstanceService as a closure in NewInstanceServiceFromCloud.

	volumeClient := func() (*gophercloud.ServiceClient, error) {
		return openstack.NewBlockStorageV3(provider, gophercloud.EndpointOpts{
			Region: clientOpts.RegionName,
		})
	}

}

imageID, err := getImageID(is, config.RootVolume.SourceUUID)
if err != nil {
return nil, fmt.Errorf("Create new server err: %v", err)
Expand Down