Merge pull request #84 from tboerger/openstack-fixes

Detect openstack ip without floating ips, add image and flavor by id

Author: bradrydzewski

cmd/drone-autoscaler/main.go

@@ -315,6 +315,7 @@ func setupProvider(c config.Config) (autoscaler.Provider, error) {
 			openstack.WithImage(c.OpenStack.Image),
 			openstack.WithRegion(c.OpenStack.Region),
 			openstack.WithFlavor(c.OpenStack.Flavor),
+			openstack.WithNetwork(c.OpenStack.Network),
 			openstack.WithFloatingIpPool(c.OpenStack.Pool),
 			openstack.WithSSHKey(c.OpenStack.SSHKey),
 			openstack.WithSecurityGroup(c.OpenStack.SecurityGroup...),

config/config.go

@@ -192,6 +192,7 @@ type (
 			Region        string `envconfig:"OS_REGION_NAME"`
 			Image         string
 			Flavor        string
+			Network       string
 			Pool          string   `envconfig:"DRONE_OPENSTACK_IP_POOL"`
 			SecurityGroup []string `split_words:"true"`
 			SSHKey        string

config/load_test.go

@@ -143,6 +143,7 @@ func TestLoad(t *testing.T) {
 		"DRONE_PACKET_USERDATA_FILE":       "/path/to/cloud/init.yml",
 		"DRONE_PACKET_HOSTNAME":            "agent",
 		"DRONE_PACKET_TAGS":                "drone,agent,prod",
+		"DRONE_OPENSTACK_NETWORK":          "my-subnet-1",
 		"DRONE_OPENSTACK_IP_POOL":          "ext-ips-1",
 		"DRONE_OPENSTACK_SSHKEY":           "drone-ci",
 		"DRONE_OPENSTACK_SECURITY_GROUP":   "secgrp-feedface",
@@ -321,6 +322,7 @@ var jsonConfig = []byte(`{
     "Region": "sto-01",
     "Image": "ubuntu-16.04-server-latest",
     "Flavor": "t1.medium",
+    "Network": "my-subnet-1",
     "Pool": "ext-ips-1",
     "SecurityGroup": [
       "secgrp-feedface"

drivers/openstack/create.go

@@ -10,10 +10,11 @@ import (
 
 	"github.com/drone/autoscaler"
 	"github.com/drone/autoscaler/logger"
-
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/floatingips"
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/keypairs"
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
+	"github.com/gophercloud/gophercloud/openstack/networking/v2/networks"
+	"github.com/gophercloud/gophercloud/pagination"
 )
 
 // Create creates an OpenStack instance
@@ -27,18 +28,37 @@ func (p *provider) Create(ctx context.Context, opts autoscaler.InstanceCreateOpt
 	if err != nil {
 		return nil, err
 	}
-	// Make a floating ip to attach.
-	ip, err := floatingips.Create(p.computeClient, floatingips.CreateOpts{
-		Pool: p.pool,
-	}).Extract()
-	if err != nil {
-		return nil, err
+
+	logger := logger.FromContext(ctx).
+		WithField("region", p.region).
+		WithField("image", p.image).
+		WithField("flavor", p.flavor).
+		WithField("network", p.network).
+		WithField("pool", p.pool).
+		WithField("name", opts.Name)
+
+	logger.Debugln("instance create")
+
+	nets := make([]servers.Network, 0)
+
+	if p.network != "" {
+		network, err := networks.Get(p.networkClient, p.network).Extract()
+		if err != nil {
+			logger.WithError(err).
+				Debugln("failed to find network")
+			return nil, err
+		}
+
+		nets = append(nets, servers.Network{
+			UUID: network.ID,
+		})
 	}
 
 	serverCreateOpts := servers.CreateOpts{
 		Name:           opts.Name,
-		ImageName:      p.image,
-		FlavorName:     p.flavor,
+		ImageRef:       p.image,
+		FlavorRef:      p.flavor,
+		Networks:       nets,
 		UserData:       buf.Bytes(),
 		ServiceClient:  p.computeClient,
 		Metadata:       p.metadata,
@@ -50,35 +70,80 @@ func (p *provider) Create(ctx context.Context, opts autoscaler.InstanceCreateOpt
 	}
 	server, err := servers.Create(p.computeClient, createOpts).Extract()
 	if err != nil {
-		floatingips.Delete(p.computeClient, ip.ID)
+		logger.WithError(err).
+			Debugln("failed to create server")
 		return nil, err
 	}
-	logger := logger.FromContext(ctx).
-		WithField("region", p.region).
-		WithField("image", p.image).
-		WithField("sizes", p.flavor).
-		WithField("name", opts.Name)
 
 	err = servers.WaitForStatus(p.computeClient, server.ID, "ACTIVE", 300)
 	if err != nil {
+		logger.WithError(err).
+			Debugln("failed waiting for server")
 		return nil, err
 	}
-	floatingips.AssociateInstance(p.computeClient, server.ID, floatingips.AssociateOpts{
-		FloatingIP: ip.IP,
-	})
-
-	logger.Debugln("instance create")
 
 	instance := &autoscaler.Instance{
 		Provider: autoscaler.ProviderOpenStack,
 		ID:       server.ID,
 		Name:     server.Name,
 		Region:   p.region,
-		Address:  ip.IP,
 		Image:    p.image,
 		Size:     p.flavor,
 	}
 
+	if p.network != "" {
+		network, err := networks.Get(p.networkClient, p.network).Extract()
+		if err != nil {
+			logger.WithError(err).
+				Debugln("failed to find network")
+			return nil, err
+		}
+
+		if err := servers.ListAddresses(p.computeClient, server.ID).EachPage(func(page pagination.Page) (bool, error) {
+			result, err := servers.ExtractAddresses(page)
+			if err != nil {
+				return false, err
+			}
+
+			for name, addresses := range result {
+				if name == network.Name {
+					for _, address := range addresses {
+						instance.Address = address.Address
+						return true, nil
+					}
+				}
+
+			}
+
+			return false, nil
+		}); err != nil {
+			logger.WithError(err).
+				Debugln("failed to fetch address")
+			return nil, err
+		}
+	}
+
+	if p.pool != "" {
+		ip, err := floatingips.Create(p.computeClient, floatingips.CreateOpts{
+			Pool: p.pool,
+		}).Extract()
+		if err != nil {
+			logger.WithError(err).
+				Debugln("failed to create floating ip")
+			return nil, err
+		}
+
+		if err := floatingips.AssociateInstance(p.computeClient, server.ID, floatingips.AssociateOpts{
+			FloatingIP: ip.IP,
+		}).ExtractErr(); err != nil {
+			logger.WithError(err).
+				Debugln("failed to associate floating ip")
+			return nil, err
+		}
+
+		instance.Address = ip.IP
+	}
+
 	logger.
 		WithField("name", instance.Name).
 		WithField("ip", instance.Address).

drivers/openstack/create_test.go

@@ -6,10 +6,11 @@ package openstack
 
 import (
 	"context"
-	"github.com/drone/autoscaler"
-	"github.com/h2non/gock"
 	"os"
 	"testing"
+
+	"github.com/drone/autoscaler"
+	"github.com/h2non/gock"
 )
 
 func TestCreate(t *testing.T) {
@@ -31,6 +32,21 @@ func TestCreate(t *testing.T) {
 		SetHeader("X-Subject-Token", authToken).
 		BodyString(string(tokenResp1))
 
+	authResp2 := helperLoad(t, "authresp1.json")
+	gock.New("http://ops.my.cloud").
+		Get("/identity").
+		Reply(300).
+		SetHeader("Content-Type", "application/json").
+		BodyString(string(authResp2))
+
+	tokenResp2 := helperLoad(t, "tokenresp1.json")
+	gock.New("http://ops.my.cloud").
+		Post("/identity/v3/auth/tokens").
+		Reply(201).
+		SetHeader("Content-Type", "application/json").
+		SetHeader("X-Subject-Token", authToken).
+		BodyString(string(tokenResp2))
+
 	fipResp1 := helperLoad(t, "fipresp1.json")
 	gock.New("http://ops.my.cloud").
 		Post("/compute/v2.1/os-floating-ips").
@@ -113,6 +129,7 @@ func TestAuthFail(t *testing.T) {
 	if err != nil {
 		t.Error("Unable to set OS_PASSWORD")
 	}
+
 	authResp1 := helperLoad(t, "authresp1.json")
 	gock.New("http://ops.my.cloud").
 		Get("/identity").
@@ -160,13 +177,20 @@ func TestCreateFail(t *testing.T) {
 		SetHeader("X-Subject-Token", authToken).
 		BodyString(string(tokenResp1))
 
-	fipResp1 := helperLoad(t, "fipresp1.json")
+	authResp2 := helperLoad(t, "authresp1.json")
 	gock.New("http://ops.my.cloud").
-		Post("/compute/v2.1/os-floating-ips").
-		MatchHeader("X-Auth-Token", authToken).
-		Reply(200).
+		Get("/identity").
+		Reply(300).
 		SetHeader("Content-Type", "application/json").
-		BodyString(string(fipResp1))
+		BodyString(string(authResp2))
+
+	tokenResp2 := helperLoad(t, "tokenresp1.json")
+	gock.New("http://ops.my.cloud").
+		Post("/identity/v3/auth/tokens").
+		Reply(201).
+		SetHeader("Content-Type", "application/json").
+		SetHeader("X-Subject-Token", authToken).
+		BodyString(string(tokenResp2))
 
 	imageListResp := helperLoad(t, "imagelistresp1.json")
 	gock.New("http://ops.my.cloud").
@@ -240,7 +264,7 @@ func testInstance(instance *autoscaler.Instance) func(t *testing.T) {
 		if want, got := instance.Address, "172.24.4.5"; got != want {
 			t.Errorf("Want instance IP %q, got %q", want, got)
 		}
-		if want, got := instance.Image, "ubuntu-16.04-server-latest"; got != want {
+		if want, got := instance.Image, "4ef19958-ee2d-44a7-a100-de0b8afdbc8e"; got != want {
 			t.Errorf("Want instance ID %q, got %q", want, got)
 		}
 		if want, got := instance.ID, "56046f6d-3184-495b-938b-baa450db970d"; got != want {
@@ -255,7 +279,7 @@ func testInstance(instance *autoscaler.Instance) func(t *testing.T) {
 		if want, got := instance.Region, "RegionOne"; got != want {
 			t.Errorf("Want instance Region %q, got %q", want, got)
 		}
-		if want, got := instance.Size, "m1.small"; got != want {
+		if want, got := instance.Size, "29e3cce3-d771-4220-80fe-3edf0e8dd466"; got != want {
 			t.Errorf("Want instance Size %q, got %q", want, got)
 		}
 	}

drivers/openstack/destroy.go

@@ -6,62 +6,87 @@ package openstack
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/drone/autoscaler"
 	"github.com/drone/autoscaler/logger"
 
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/floatingips"
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
+	"github.com/gophercloud/gophercloud/pagination"
 )
 
 func (p *provider) Destroy(ctx context.Context, instance *autoscaler.Instance) error {
 	logger := logger.FromContext(ctx).
 		WithField("region", instance.Region).
 		WithField("image", instance.Image).
-		WithField("size", instance.Size).
+		WithField("flavor", instance.Size).
 		WithField("name", instance.Name)
 
 	logger.Debugln("deleting instance")
 
-	_ = p.deleteFloatingIps(instance)
+	err := p.deleteFloatingIps(instance)
+	if err != nil {
+		logger.WithError(err).
+			Debugln("failed to delete floating ips")
 
-	err := servers.Delete(p.computeClient, instance.ID).ExtractErr()
+		return err
+	}
+
+	err = servers.Delete(p.computeClient, instance.ID).ExtractErr()
 	if err == nil {
 		logger.Debugln("instance deleted")
 		return nil
 	}
 
+	if err.Error() == "Resource not found" {
+		logger.WithError(err).
+			Debugln("instance does not exist")
+		return autoscaler.ErrInstanceNotFound
+	}
+
 	logger.WithError(err).
-		Errorln("deleting instance failed, attempting to force")
+		Errorln("attempting to force delete")
 
 	err = servers.ForceDelete(p.computeClient, instance.ID).ExtractErr()
-
 	if err == nil {
 		logger.Debugln("instance deleted")
 		return nil
 	}
 
+	if err.Error() == "Resource not found" {
+		logger.WithError(err).
+			Debugln("instance does not exist")
+		return autoscaler.ErrInstanceNotFound
+	}
+
 	logger.WithError(err).
 		Errorln("force-deleting instance failed")
 
 	return err
 }
 
 func (p *provider) deleteFloatingIps(instance *autoscaler.Instance) error {
-	floatingips.DisassociateInstance(p.computeClient, instance.ID, floatingips.DisassociateOpts{
-		FloatingIP: instance.Address,
-	})
-	// Remove our allocated ip from the pool.
-	allPages, err := floatingips.List(p.computeClient).AllPages()
-	ips, err := floatingips.ExtractFloatingIPs(allPages)
-	if err != nil {
-		return err
-	}
-	for _, fip := range ips {
-		if fip.InstanceID == instance.ID {
-			floatingips.Delete(p.computeClient, fip.ID)
+	return floatingips.List(p.computeClient).EachPage(func(page pagination.Page) (bool, error) {
+		ips, err := floatingips.ExtractFloatingIPs(page)
+		if err != nil {
+			return false, err
 		}
-	}
 
-	return nil
+		for _, ip := range ips {
+			if ip.InstanceID == instance.ID {
+				if err := floatingips.DisassociateInstance(p.computeClient, instance.ID, floatingips.DisassociateOpts{
+					FloatingIP: ip.IP,
+				}).ExtractErr(); err != nil {
+					return false, fmt.Errorf("failed to disassociate floating ip: %s", err)
+				}
+
+				if err := floatingips.Delete(p.computeClient, ip.ID).ExtractErr(); err != nil {
+					return false, fmt.Errorf("failed to delete floating ip: %s", err)
+				}
+			}
+		}
+
+		return true, nil
+	})
 }