Merge pull request #10178 from zetaab/portinuse

OpenStack Reset deviceID status if needed
kubernetes · Nov 6, 2020 · d13ae5a · d13ae5a
2 parents 7b26ec4 + bd2dcc9
commit d13ae5a
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 8 deletions.
diff --git a/pkg/instancegroups/rollingupdate_os_test.go b/pkg/instancegroups/rollingupdate_os_test.go
@@ -172,7 +172,7 @@ func makeGroupOS(t *testing.T, groups map[string]*cloudinstances.CloudInstanceGr
 					Port: port.ID,
 				},
 			},
-		})
+		}, "")
 		if err != nil {
 			t.Fatalf("Failed to make group: %v", err)
 		}

diff --git a/upup/pkg/fi/cloudup/openstack/cloud.go b/upup/pkg/fi/cloudup/openstack/cloud.go
@@ -108,7 +108,7 @@ type OpenstackCloud interface {
 	ListInstances(servers.ListOptsBuilder) ([]servers.Server, error)
 
 	// CreateInstance will create an openstack server provided create opts
-	CreateInstance(servers.CreateOptsBuilder) (*servers.Server, error)
+	CreateInstance(servers.CreateOptsBuilder, string) (*servers.Server, error)
 
 	//DeleteInstanceWithID will delete instance
 	DeleteInstanceWithID(instanceID string) error
@@ -216,6 +216,9 @@ type OpenstackCloud interface {
 	//GetPort will return a Neutron port by ID
 	GetPort(id string) (*ports.Port, error)
 
+	//UpdatePort will update a Neutron port by ID and options
+	UpdatePort(id string, opt ports.UpdateOptsBuilder) (*ports.Port, error)
+
 	//ListPorts will return the Neutron ports which match the options
 	ListPorts(opt ports.ListOptsBuilder) ([]ports.Port, error)
 

diff --git a/upup/pkg/fi/cloudup/openstack/instance.go b/upup/pkg/fi/cloudup/openstack/instance.go
@@ -22,7 +22,9 @@ import (
 
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/flavors"
 
+	"github.com/gophercloud/gophercloud"
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
+	"github.com/gophercloud/gophercloud/openstack/networking/v2/ports"
 	"github.com/mitchellh/mapstructure"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/klog/v2"
@@ -47,16 +49,40 @@ var floatingBackoff = wait.Backoff{
 	Steps:    20,
 }
 
-func (c *openstackCloud) CreateInstance(opt servers.CreateOptsBuilder) (*servers.Server, error) {
-	return createInstance(c, opt)
+func (c *openstackCloud) CreateInstance(opt servers.CreateOptsBuilder, portID string) (*servers.Server, error) {
+	return createInstance(c, opt, portID)
 }
 
-func createInstance(c OpenstackCloud, opt servers.CreateOptsBuilder) (*servers.Server, error) {
+func IsPortInUse(err error) bool {
+	if _, ok := err.(gophercloud.ErrDefault409); ok {
+		return true
+	}
+	return false
+}
+
+func createInstance(c OpenstackCloud, opt servers.CreateOptsBuilder, portID string) (*servers.Server, error) {
 	var server *servers.Server
 
 	done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
 		v, err := servers.Create(c.ComputeClient(), opt).Extract()
 		if err != nil {
+			if IsPortInUse(err) && portID != "" {
+				port, err := c.GetPort(portID)
+				if err != nil {
+					return false, fmt.Errorf("error finding port %s: %v", portID, err)
+				}
+				// port is attached to deleted instance, we need reset the status of the DeviceID
+				// this is bug in OpenStack APIs
+				if port.DeviceID != "" && port.DeviceOwner == "" {
+					klog.Warningf("Port %s is attached to Device that does not exist anymore, reseting the status of DeviceID", portID)
+					_, err := c.UpdatePort(portID, ports.UpdateOpts{
+						DeviceID: fi.String(""),
+					})
+					if err != nil {
+						return false, fmt.Errorf("error updating port %s deviceid: %v", portID, err)
+					}
+				}
+			}
 			return false, fmt.Errorf("error creating server %v: %v", opt, err)
 		}
 		server = v

diff --git a/upup/pkg/fi/cloudup/openstack/mock_cloud.go b/upup/pkg/fi/cloudup/openstack/mock_cloud.go
@@ -169,8 +169,8 @@ func (c *MockCloud) AttachVolume(serverID string, opts volumeattach.CreateOpts)
 	return attachVolume(c, serverID, opts)
 }
 
-func (c *MockCloud) CreateInstance(opt servers.CreateOptsBuilder) (*servers.Server, error) {
-	return createInstance(c, opt)
+func (c *MockCloud) CreateInstance(opt servers.CreateOptsBuilder, portID string) (*servers.Server, error) {
+	return createInstance(c, opt, portID)
 }
 
 func (c *MockCloud) CreateKeypair(opt keypairs.CreateOptsBuilder) (*keypairs.KeyPair, error) {
@@ -355,6 +355,10 @@ func (c *MockCloud) GetPort(id string) (*ports.Port, error) {
 	return getPort(c, id)
 }
 
+func (c *MockCloud) UpdatePort(id string, opt ports.UpdateOptsBuilder) (*ports.Port, error) {
+	return updatePort(c, id, opt)
+}
+
 func (c *MockCloud) GetStorageAZFromCompute(computeAZ string) (*az.AvailabilityZone, error) {
 	return getStorageAZFromCompute(c, computeAZ)
 }

diff --git a/upup/pkg/fi/cloudup/openstack/port.go b/upup/pkg/fi/cloudup/openstack/port.go
@@ -48,6 +48,30 @@ func createPort(c OpenstackCloud, opt ports.CreateOptsBuilder) (*ports.Port, err
 	}
 }
 
+func (c *openstackCloud) UpdatePort(id string, opt ports.UpdateOptsBuilder) (*ports.Port, error) {
+	return updatePort(c, id, opt)
+}
+
+func updatePort(c OpenstackCloud, id string, opt ports.UpdateOptsBuilder) (*ports.Port, error) {
+	var p *ports.Port
+
+	done, err := vfs.RetryWithBackoff(readBackoff, func() (bool, error) {
+		port, err := ports.Update(c.NetworkingClient(), id, opt).Extract()
+		if err != nil {
+			return false, err
+		}
+		p = port
+		return true, nil
+	})
+	if err != nil {
+		return p, err
+	} else if done {
+		return p, nil
+	} else {
+		return p, wait.ErrWaitTimeout
+	}
+}
+
 func (c *openstackCloud) GetPort(id string) (*ports.Port, error) {
 	return getPort(c, id)
 }

diff --git a/upup/pkg/fi/cloudup/openstacktasks/instance.go b/upup/pkg/fi/cloudup/openstacktasks/instance.go
@@ -292,7 +292,7 @@ func (_ *Instance) RenderOpenstack(t *openstack.OpenstackAPITarget, a, e, change
 			return err
 		}
 
-		v, err := t.Cloud.CreateInstance(opts)
+		v, err := t.Cloud.CreateInstance(opts, fi.StringValue(e.Port.ID))
 		if err != nil {
 			return fmt.Errorf("Error creating instance: %v", err)
 		}