fix deletion of precreated security groups (#76)

fix deletion of precreated security groups Sometimes security groups was still there after machine deletion Reviewed-by: Artem Lifshits
opentelekomcloud · Apr 12, 2024 · 726240d · 726240d
1 parent d82aa6e
commit 726240d
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 16 deletions.
diff --git a/driver/compute.go b/driver/compute.go
@@ -168,14 +168,28 @@ func (d *Driver) deleteInstance() error {
 	if err := d.initComputeV2(); err != nil {
 		return err
 	}
+	sGroups, err := d.client.GetInstanceSG(d.InstanceID)
+	if err != nil {
+		return fmt.Errorf("failed to get ECS security groups: %s", err)
+	}
 	if err := d.client.DeleteInstance(d.InstanceID); err != nil {
 		return fmt.Errorf("failed to delete instance: %s", logHTTP500(err))
 	}
-	err := d.client.WaitForInstanceStatus(d.InstanceID, "")
+	err = d.client.WaitForInstanceStatus(d.InstanceID, "")
 	switch err.(type) {
 	case golangsdk.ErrDefault404:
 	default:
 		return fmt.Errorf("failed to wait for instance status after deletion: %s", logHTTP500(err))
 	}
+	for _, group := range sGroups {
+		if group.Description == services.DefaultSecurityGroupDescription {
+			if err := d.client.DeleteSecurityGroup(group.ID); err != nil {
+				return fmt.Errorf("failed to delete security group: %s", logHTTP500(err))
+			}
+			if err := d.client.WaitForGroupDeleted(group.ID); err != nil {
+				return fmt.Errorf("failed to wait for security group status after deletion: %s", logHTTP500(err))
+			}
+		}
+	}
 	return nil
 }
diff --git a/driver/network.go b/driver/network.go
@@ -3,6 +3,7 @@ package opentelekomcloud
 import (
 	"fmt"
 
+	"github.com/hashicorp/go-multierror"
 	"github.com/opentelekomcloud/docker-machine-opentelekomcloud/driver/services"
 	golangsdk "github.com/opentelekomcloud/gophertelekomcloud"
 )
@@ -148,18 +149,21 @@ func (d *Driver) deleteSubnet() error {
 }
 
 func (d *Driver) deleteSecGroups() error {
+	mErr := &multierror.Error{}
 	if err := d.initComputeV2(); err != nil {
 		return err
 	}
 	id := d.ManagedSecurityGroupID
 	if id == "" {
 		return nil
 	}
-	if err := d.client.DeleteSecurityGroup(id); err != nil {
-		return fmt.Errorf("failed to delete security group: %s", logHTTP500(err))
-	}
-	if err := d.client.WaitForGroupDeleted(id); err != nil {
-		return fmt.Errorf("failed to wait for security group status after deletion: %s", logHTTP500(err))
+	if d.client.SecurityGroupExist(id) {
+		if err := d.client.DeleteSecurityGroup(id); err != nil {
+			mErr = multierror.Append(mErr, logHTTP500(err))
+		}
+		if err := d.client.WaitForGroupDeleted(id); err != nil {
+			mErr = multierror.Append(mErr, logHTTP500(err))
+		}
 	}
-	return nil
+	return mErr.ErrorOrNil()
 }
diff --git a/driver/opentelekomcloud_test.go b/driver/opentelekomcloud_test.go
@@ -238,9 +238,6 @@ func TestDriver_CreateWithExistingSecGroups(t *testing.T) {
 	newSG := utils.RandomString(10, "nsg-")
 	sg, err := preDriver.client.CreateSecurityGroup(newSG, services.PortRange{From: 24})
 	assert.NoError(t, err)
-	defer func() {
-		assert.NoError(t, preDriver.client.DeleteSecurityGroup(sg.ID))
-	}()
 
 	driver, err := newDriverFromFlags(
 		map[string]interface{}{

diff --git a/driver/services/compute.go b/driver/services/compute.go
@@ -21,8 +21,9 @@ import (
 
 // Instance statuses
 const (
-	InstanceStatusStopped = "SHUTOFF"
-	InstanceStatusRunning = "ACTIVE"
+	InstanceStatusStopped           = "SHUTOFF"
+	InstanceStatusRunning           = "ACTIVE"
+	DefaultSecurityGroupDescription = "Automatically created by docker-machine for OTC"
 )
 
 // InitCompute initializes Compute v2 service
@@ -141,6 +142,19 @@ func (c *Client) DeleteInstance(instanceID string) error {
 	return servers.Delete(c.ComputeV2, instanceID).Err
 }
 
+// GetInstanceSG get details of ECS instance security groups
+func (c *Client) GetInstanceSG(instanceID string) ([]secgroups.SecurityGroup, error) {
+	allPages, err := secgroups.ListByServer(c.ComputeV2, instanceID).AllPages()
+	if err != nil {
+		return nil, err
+	}
+	securityGroups, err := secgroups.ExtractSecurityGroups(allPages)
+	if err != nil {
+		return nil, err
+	}
+	return securityGroups, nil
+}
+
 // FindInstance returns instance ID by instance Name
 func (c *Client) FindInstance(name string) (string, error) {
 	listOpts := servers.ListOpts{Name: name}
@@ -306,7 +320,7 @@ type PortRange struct {
 func (c *Client) CreateSecurityGroup(securityGroupName string, ports ...PortRange) (*secgroups.SecurityGroup, error) {
 	opts := secgroups.CreateOpts{
 		Name:        securityGroupName,
-		Description: "Automatically created by docker-machine for OTC",
+		Description: DefaultSecurityGroupDescription,
 	}
 	sg, err := secgroups.Create(c.ComputeV2, opts).Extract()
 	if err != nil {
@@ -364,6 +378,12 @@ func (c *Client) FindSecurityGroups(secGroups []string) ([]string, error) {
 	return secGroupIDs, nil
 }
 
+// SecurityGroupExist check if security group still exist
+func (c *Client) SecurityGroupExist(id string) bool {
+	err := secgroups.Get(c.ComputeV2, id).Err
+	return err == nil
+}
+
 // DeleteSecurityGroup deletes managed security group
 func (c *Client) DeleteSecurityGroup(securityGroupID string) error {
 	return secgroups.Delete(c.ComputeV2, securityGroupID).Err

diff --git a/go.mod b/go.mod
@@ -4,7 +4,7 @@ go 1.14
 
 require (
 	github.com/apparentlymart/go-cidr v1.1.0
-	github.com/docker/docker v20.10.24+incompatible // indirect
+	github.com/docker/docker v20.10.27+incompatible // indirect
 	github.com/docker/machine v0.16.2
 	github.com/getlantern/deepcopy v0.0.0-20160317154340-7f45deb8130a
 	github.com/hashicorp/go-multierror v1.1.0

diff --git a/go.sum b/go.sum
@@ -7,8 +7,8 @@ github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/docker/docker v20.10.24+incompatible h1:Ugvxm7a8+Gz6vqQYQQ2W7GYq5EUPaAiuPgIfVyI3dYE=
-github.com/docker/docker v20.10.24+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
+github.com/docker/docker v20.10.27+incompatible h1:Id/ZooynV4ZlD6xX20RCd3SR0Ikn7r4QZDa2ECK2TgA=
+github.com/docker/docker v20.10.27+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
 github.com/docker/machine v0.16.2 h1:jyF9k3Zg+oIGxxSdYKPScyj3HqFZ6FjgA/3sblcASiU=
 github.com/docker/machine v0.16.2/go.mod h1:I8mPNDeK1uH+JTcUU7X0ZW8KiYz0jyAgNaeSJ1rCfDI=
 github.com/getlantern/deepcopy v0.0.0-20160317154340-7f45deb8130a h1:yU/FENpkHYISWsQrbr3pcZOBj0EuRjPzNc1+dTCLu44=