diff --git a/pkg/cloudprovider/providers/openstack/BUILD b/pkg/cloudprovider/providers/openstack/BUILD index 42a185e2f4c..7840fd00a6a 100644 --- a/pkg/cloudprovider/providers/openstack/BUILD +++ b/pkg/cloudprovider/providers/openstack/BUILD @@ -26,6 +26,7 @@ go_library( "//pkg/controller:go_default_library", "//pkg/util/mount:go_default_library", "//pkg/volume:go_default_library", + "//pkg/volume/util:go_default_library", "//vendor/github.com/golang/glog:go_default_library", "//vendor/github.com/gophercloud/gophercloud:go_default_library", "//vendor/github.com/gophercloud/gophercloud/openstack:go_default_library", diff --git a/pkg/cloudprovider/providers/openstack/openstack.go b/pkg/cloudprovider/providers/openstack/openstack.go index 00e15d228b2..759412a9f01 100644 --- a/pkg/cloudprovider/providers/openstack/openstack.go +++ b/pkg/cloudprovider/providers/openstack/openstack.go @@ -319,6 +319,22 @@ func mapNodeNameToServerName(nodeName types.NodeName) string { return string(nodeName) } +// getNodeNameByID maps instanceid to types.NodeName +func (os *OpenStack) GetNodeNameByID(instanceID string) (types.NodeName, error) { + client, err := os.NewComputeV2() + var nodeName types.NodeName + if err != nil { + return nodeName, err + } + + server, err := servers.Get(client, instanceID).Extract() + if err != nil { + return nodeName, err + } + nodeName = mapServerToNodeName(server) + return nodeName, nil +} + // mapServerToNodeName maps an OpenStack Server to a k8s NodeName func mapServerToNodeName(server *servers.Server) types.NodeName { // Node names are always lowercase, and (at least) @@ -346,11 +362,14 @@ func foreachServer(client *gophercloud.ServiceClient, opts servers.ListOptsBuild return err } -func getServerByName(client *gophercloud.ServiceClient, name types.NodeName) (*servers.Server, error) { +func getServerByName(client *gophercloud.ServiceClient, name types.NodeName, showOnlyActive bool) (*servers.Server, error) { opts := servers.ListOpts{ - Name: fmt.Sprintf("^%s$", regexp.QuoteMeta(mapNodeNameToServerName(name))), - Status: "ACTIVE", + Name: fmt.Sprintf("^%s$", regexp.QuoteMeta(mapNodeNameToServerName(name))), } + if showOnlyActive { + opts.Status = "ACTIVE" + } + pager := servers.List(client, opts) serverList := make([]servers.Server, 0, 1) @@ -432,7 +451,7 @@ func nodeAddresses(srv *servers.Server) ([]v1.NodeAddress, error) { } func getAddressesByName(client *gophercloud.ServiceClient, name types.NodeName) ([]v1.NodeAddress, error) { - srv, err := getServerByName(client, name) + srv, err := getServerByName(client, name, true) if err != nil { return nil, err } @@ -582,7 +601,7 @@ func (os *OpenStack) GetZoneByNodeName(nodeName types.NodeName) (cloudprovider.Z return cloudprovider.Zone{}, err } - srv, err := getServerByName(compute, nodeName) + srv, err := getServerByName(compute, nodeName, true) if err != nil { if err == ErrNotFound { return cloudprovider.Zone{}, cloudprovider.InstanceNotFound diff --git a/pkg/cloudprovider/providers/openstack/openstack_instances.go b/pkg/cloudprovider/providers/openstack/openstack_instances.go index 3cf1733b322..981ff7b9f89 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_instances.go +++ b/pkg/cloudprovider/providers/openstack/openstack_instances.go @@ -103,7 +103,7 @@ func (i *Instances) NodeAddressesByProviderID(providerID string) ([]v1.NodeAddre // ExternalID returns the cloud provider ID of the specified instance (deprecated). func (i *Instances) ExternalID(name types.NodeName) (string, error) { - srv, err := getServerByName(i.compute, name) + srv, err := getServerByName(i.compute, name, true) if err != nil { if err == ErrNotFound { return "", cloudprovider.InstanceNotFound @@ -151,7 +151,7 @@ func (os *OpenStack) InstanceID() (string, error) { // InstanceID returns the cloud provider ID of the specified instance. func (i *Instances) InstanceID(name types.NodeName) (string, error) { - srv, err := getServerByName(i.compute, name) + srv, err := getServerByName(i.compute, name, true) if err != nil { if err == ErrNotFound { return "", cloudprovider.InstanceNotFound @@ -184,7 +184,7 @@ func (i *Instances) InstanceTypeByProviderID(providerID string) (string, error) // InstanceType returns the type of the specified instance. func (i *Instances) InstanceType(name types.NodeName) (string, error) { - srv, err := getServerByName(i.compute, name) + srv, err := getServerByName(i.compute, name, true) if err != nil { return "", err diff --git a/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go b/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go index 2089c14f4ce..036af670bc7 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go +++ b/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go @@ -551,7 +551,7 @@ func getNodeSecurityGroupIDForLB(compute *gophercloud.ServiceClient, nodes []*v1 for _, node := range nodes { nodeName := types.NodeName(node.Name) - srv, err := getServerByName(compute, nodeName) + srv, err := getServerByName(compute, nodeName, true) if err != nil { return nodeSecurityGroupIDs.List(), err } diff --git a/pkg/cloudprovider/providers/openstack/openstack_routes.go b/pkg/cloudprovider/providers/openstack/openstack_routes.go index c5f0974dadd..c5a8ba6d212 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_routes.go +++ b/pkg/cloudprovider/providers/openstack/openstack_routes.go @@ -288,7 +288,7 @@ func (r *Routes) DeleteRoute(clusterName string, route *cloudprovider.Route) err } func getPortIDByIP(compute *gophercloud.ServiceClient, targetNode types.NodeName, ipAddress string) (string, error) { - srv, err := getServerByName(compute, targetNode) + srv, err := getServerByName(compute, targetNode, true) if err != nil { return "", err } diff --git a/pkg/cloudprovider/providers/openstack/openstack_volumes.go b/pkg/cloudprovider/providers/openstack/openstack_volumes.go index eab5b7c9b5d..8a530592845 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_volumes.go +++ b/pkg/cloudprovider/providers/openstack/openstack_volumes.go @@ -17,6 +17,7 @@ limitations under the License. package openstack import ( + "errors" "fmt" "io/ioutil" "path" @@ -26,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" k8s_volume "k8s.io/kubernetes/pkg/volume" + volumeutil "k8s.io/kubernetes/pkg/volume/util" "github.com/gophercloud/gophercloud" volumeexpand "github.com/gophercloud/gophercloud/openstack/blockstorage/extensions/volumeactions" @@ -317,8 +319,33 @@ func (os *OpenStack) AttachDisk(instanceID, volumeID string) (string, error) { if instanceID == volume.AttachedServerId { glog.V(4).Infof("Disk %s is already attached to instance %s", volumeID, instanceID) return volume.ID, nil + } else { + nodeName, err := os.GetNodeNameByID(volume.AttachedServerId) + attachErr := fmt.Sprintf("disk %s path %s is attached to a different instance (%s)", volumeID, volume.AttachedDevice, volume.AttachedServerId) + if err != nil { + glog.Error(attachErr) + return "", errors.New(attachErr) + } + // using volume.AttachedDevice may cause problems because cinder does not report device path correctly see issue #33128 + devicePath := volume.AttachedDevice + danglingErr := volumeutil.NewDanglingError(attachErr, nodeName, devicePath) + glog.V(4).Infof("volume %s is already attached to node %s path %s", volumeID, nodeName, devicePath) + // check special case, if node is deleted from cluster but exist still in openstack + // we need to check can we detach the cinder, node is deleted from cluster if state is not ACTIVE + srv, err := getServerByName(cClient, nodeName, false) + if err != nil { + return "", err + } + if srv.Status != "ACTIVE" { + err = os.DetachDisk(volume.AttachedServerId, volumeID) + if err != nil { + glog.Error(err) + return "", err + } + glog.V(4).Infof("detached volume %s node state was %s", volumeID, srv.Status) + } + return "", danglingErr } - return "", fmt.Errorf("disk %s is attached to a different instance (%s)", volumeID, volume.AttachedServerId) } startTime := time.Now()