Merge pull request #9645 from rifelpet/openstack-cloudmock-part1

Refactor openstackCloud to be mockable, add a MockCloud

upup/pkg/fi/cloudup/openstack/BUILD.bazel

@@ -12,6 +12,7 @@ go_library(
         "instance.go",
         "keypair.go",
         "loadbalancer.go",
+        "mock_cloud.go",
         "network.go",
         "port.go",
         "router.go",

upup/pkg/fi/cloudup/openstack/availability_zone.go

@@ -26,6 +26,10 @@ import (
 )
 
 func (c *openstackCloud) ListAvailabilityZones(serviceClient *gophercloud.ServiceClient) (azList []az.AvailabilityZone, err error) {
+	return listAvailabilityZones(c, serviceClient)
+}
+
+func listAvailabilityZones(c OpenstackCloud, serviceClient *gophercloud.ServiceClient) (azList []az.AvailabilityZone, err error) {
 
 	done, err := vfs.RetryWithBackoff(readBackoff, func() (bool, error) {
 		azPage, err := az.List(serviceClient).AllPages()
@@ -50,6 +54,10 @@ func (c *openstackCloud) ListAvailabilityZones(serviceClient *gophercloud.Servic
 }
 
 func (c *openstackCloud) GetStorageAZFromCompute(computeAZ string) (*az.AvailabilityZone, error) {
+	return getStorageAZFromCompute(c, computeAZ)
+}
+
+func getStorageAZFromCompute(c OpenstackCloud, computeAZ string) (*az.AvailabilityZone, error) {
 	// TODO: This is less than desirable, but openstack differs here
 	// Check to see if the availability zone exists.
 	azList, err := c.ListAvailabilityZones(c.BlockStorageClient())

upup/pkg/fi/cloudup/openstack/cloud.go

@@ -96,6 +96,7 @@ type OpenstackCloud interface {
 	NetworkingClient() *gophercloud.ServiceClient
 	LoadBalancerClient() *gophercloud.ServiceClient
 	DNSClient() *gophercloud.ServiceClient
+	ImageClient() *gophercloud.ServiceClient
 	UseOctavia() bool
 	UseZones([]string)
 
@@ -320,9 +321,22 @@ type openstackCloud struct {
 
 var _ fi.Cloud = &openstackCloud{}
 
+var openstackCloudInstances map[string]OpenstackCloud = make(map[string]OpenstackCloud)
+
 func NewOpenstackCloud(tags map[string]string, spec *kops.ClusterSpec) (OpenstackCloud, error) {
+
 	config := vfs.OpenstackConfig{}
 
+	region, err := config.GetRegion()
+	if err != nil {
+		return nil, fmt.Errorf("error finding openstack region: %v", err)
+	}
+
+	raw := openstackCloudInstances[region]
+	if raw != nil {
+		return raw, nil
+	}
+
 	authOption, err := config.GetCredential()
 	if err != nil {
 		return nil, err
@@ -333,11 +347,6 @@ func NewOpenstackCloud(tags map[string]string, spec *kops.ClusterSpec) (Openstac
 		return nil, fmt.Errorf("error building openstack provider client: %v", err)
 	}
 
-	region, err := config.GetRegion()
-	if err != nil {
-		return nil, fmt.Errorf("error finding openstack region: %v", err)
-	}
-
 	if spec != nil && spec.CloudConfig != nil && spec.CloudConfig.Openstack != nil && spec.CloudConfig.Openstack.InsecureSkipVerify != nil {
 		tlsconfig := &tls.Config{}
 		tlsconfig.InsecureSkipVerify = fi.BoolValue(spec.CloudConfig.Openstack.InsecureSkipVerify)
@@ -470,6 +479,8 @@ func NewOpenstackCloud(tags map[string]string, spec *kops.ClusterSpec) (Openstac
 		}
 	}
 	c.lbClient = lbClient
+	openstackCloudInstances[region] = c
+
 	return c, nil
 }
 
@@ -502,6 +513,10 @@ func (c *openstackCloud) DNSClient() *gophercloud.ServiceClient {
 	return c.dnsClient
 }
 
+func (c *openstackCloud) ImageClient() *gophercloud.ServiceClient {
+	return c.glanceClient
+}
+
 func (c *openstackCloud) Region() string {
 	return c.region
 }
@@ -520,10 +535,14 @@ func (c *openstackCloud) DNS() (dnsprovider.Interface, error) {
 
 // FindVPCInfo list subnets in network
 func (c *openstackCloud) FindVPCInfo(id string) (*fi.VPCInfo, error) {
+	return findVPCInfo(c, id, c.zones)
+}
+
+func findVPCInfo(c OpenstackCloud, id string, zones []string) (*fi.VPCInfo, error) {
 	vpcInfo := &fi.VPCInfo{}
 	// Find subnets in the network
 	{
-		if len(c.zones) == 0 {
+		if len(zones) == 0 {
 			return nil, fmt.Errorf("could not initialize zones")
 		}
 		klog.V(2).Infof("Calling ListSubnets for subnets in Network %q", id)
@@ -536,7 +555,7 @@ func (c *openstackCloud) FindVPCInfo(id string) (*fi.VPCInfo, error) {
 		}
 
 		for index, subnet := range subnets {
-			zone := c.zones[int(index)%len(c.zones)]
+			zone := zones[int(index)%len(zones)]
 			subnetInfo := &fi.SubnetInfo{
 				ID:   subnet.ID,
 				CIDR: subnet.CIDR,
@@ -550,6 +569,10 @@ func (c *openstackCloud) FindVPCInfo(id string) (*fi.VPCInfo, error) {
 
 // DeleteGroup in openstack will delete servergroup, instances and ports
 func (c *openstackCloud) DeleteGroup(g *cloudinstances.CloudInstanceGroup) error {
+	return deleteGroup(c, g)
+}
+
+func deleteGroup(c OpenstackCloud, g *cloudinstances.CloudInstanceGroup) error {
 	grp := g.Raw.(*servergroups.ServerGroup)
 
 	for _, id := range grp.Members {
@@ -582,6 +605,10 @@ func (c *openstackCloud) DeleteGroup(g *cloudinstances.CloudInstanceGroup) error
 }
 
 func (c *openstackCloud) GetCloudGroups(cluster *kops.Cluster, instancegroups []*kops.InstanceGroup, warnUnmatched bool, nodes []v1.Node) (map[string]*cloudinstances.CloudInstanceGroup, error) {
+	return getCloudGroups(c, cluster, instancegroups, warnUnmatched, nodes)
+}
+
+func getCloudGroups(c OpenstackCloud, cluster *kops.Cluster, instancegroups []*kops.InstanceGroup, warnUnmatched bool, nodes []v1.Node) (map[string]*cloudinstances.CloudInstanceGroup, error) {
 	nodeMap := cloudinstances.GetNodeMap(nodes, cluster)
 	groups := make(map[string]*cloudinstances.CloudInstanceGroup)
 
@@ -602,7 +629,7 @@ func (c *openstackCloud) GetCloudGroups(cluster *kops.Cluster, instancegroups []
 			}
 			continue
 		}
-		groups[instancegroup.ObjectMeta.Name], err = c.osBuildCloudInstanceGroup(cluster, instancegroup, &grp, nodeMap)
+		groups[instancegroup.ObjectMeta.Name], err = osBuildCloudInstanceGroup(c, cluster, instancegroup, &grp, nodeMap)
 		if err != nil {
 			return nil, fmt.Errorf("error getting cloud instance group %q: %v", instancegroup.ObjectMeta.Name, err)
 		}
@@ -620,6 +647,10 @@ type Address struct {
 }
 
 func (c *openstackCloud) GetApiIngressStatus(cluster *kops.Cluster) ([]kops.ApiIngressStatus, error) {
+	return getApiIngressStatus(c, cluster)
+}
+
+func getApiIngressStatus(c OpenstackCloud, cluster *kops.Cluster) ([]kops.ApiIngressStatus, error) {
 	var ingresses []kops.ApiIngressStatus
 	if cluster.Spec.CloudConfig.Openstack.Loadbalancer != nil {
 		if cluster.Spec.MasterPublicName != "" {

upup/pkg/fi/cloudup/openstack/dns.go

@@ -27,6 +27,10 @@ import (
 
 // ListDNSZones will list available DNS zones
 func (c *openstackCloud) ListDNSZones(opt zones.ListOptsBuilder) ([]zones.Zone, error) {
+	return listDNSZones(c, opt)
+}
+
+func listDNSZones(c OpenstackCloud, opt zones.ListOptsBuilder) ([]zones.Zone, error) {
 	var zs []zones.Zone
 
 	done, err := vfs.RetryWithBackoff(readBackoff, func() (bool, error) {
@@ -52,6 +56,10 @@ func (c *openstackCloud) ListDNSZones(opt zones.ListOptsBuilder) ([]zones.Zone,
 
 // ListDNSRecordsets will list DNS recordsets
 func (c *openstackCloud) ListDNSRecordsets(zoneID string, opt recordsets.ListOptsBuilder) ([]recordsets.RecordSet, error) {
+	return listDNSRecordsets(c, zoneID, opt)
+}
+
+func listDNSRecordsets(c OpenstackCloud, zoneID string, opt recordsets.ListOptsBuilder) ([]recordsets.RecordSet, error) {
 	var rrs []recordsets.RecordSet
 
 	done, err := vfs.RetryWithBackoff(readBackoff, func() (bool, error) {

upup/pkg/fi/cloudup/openstack/floatingip.go

@@ -26,6 +26,10 @@ import (
 )
 
 func (c *openstackCloud) GetFloatingIP(id string) (fip *floatingips.FloatingIP, err error) {
+	return getFloatingIP(c, id)
+}
+
+func getFloatingIP(c OpenstackCloud, id string) (fip *floatingips.FloatingIP, err error) {
 	done, err := vfs.RetryWithBackoff(readBackoff, func() (bool, error) {
 
 		fip, err = floatingips.Get(c.ComputeClient(), id).Extract()
@@ -43,7 +47,33 @@ func (c *openstackCloud) GetFloatingIP(id string) (fip *floatingips.FloatingIP,
 	return fip, nil
 }
 
+func (c *openstackCloud) CreateFloatingIP(opts floatingips.CreateOpts) (fip *floatingips.FloatingIP, err error) {
+	return createFloatingIP(c, opts)
+}
+
+func createFloatingIP(c OpenstackCloud, opts floatingips.CreateOpts) (fip *floatingips.FloatingIP, err error) {
+	done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
+
+		fip, err = floatingips.Create(c.ComputeClient(), opts).Extract()
+		if err != nil {
+			return false, fmt.Errorf("CreateFloatingIP: create floating IP failed: %v", err)
+		}
+		return true, nil
+	})
+	if !done {
+		if err == nil {
+			err = wait.ErrWaitTimeout
+		}
+		return fip, err
+	}
+	return fip, nil
+}
+
 func (c *openstackCloud) AssociateFloatingIPToInstance(serverID string, opts floatingips.AssociateOpts) (err error) {
+	return associateFloatingIPToInstance(c, serverID, opts)
+}
+
+func associateFloatingIPToInstance(c OpenstackCloud, serverID string, opts floatingips.AssociateOpts) (err error) {
 	done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
 		err = floatingips.AssociateInstance(c.ComputeClient(), serverID, opts).ExtractErr()
 		if err != nil {
@@ -59,6 +89,10 @@ func (c *openstackCloud) AssociateFloatingIPToInstance(serverID string, opts flo
 }
 
 func (c *openstackCloud) CreateL3FloatingIP(opts l3floatingip.CreateOpts) (fip *l3floatingip.FloatingIP, err error) {
+	return createL3FloatingIP(c, opts)
+}
+
+func createL3FloatingIP(c OpenstackCloud, opts l3floatingip.CreateOpts) (fip *l3floatingip.FloatingIP, err error) {
 	done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
 
 		fip, err = l3floatingip.Create(c.NetworkingClient(), opts).Extract()
@@ -77,6 +111,10 @@ func (c *openstackCloud) CreateL3FloatingIP(opts l3floatingip.CreateOpts) (fip *
 }
 
 func (c *openstackCloud) ListFloatingIPs() (fips []floatingips.FloatingIP, err error) {
+	return listFloatingIPs(c)
+}
+
+func listFloatingIPs(c OpenstackCloud) (fips []floatingips.FloatingIP, err error) {
 
 	done, err := vfs.RetryWithBackoff(readBackoff, func() (bool, error) {
 		pages, err := floatingips.List(c.ComputeClient()).AllPages()
@@ -99,6 +137,10 @@ func (c *openstackCloud) ListFloatingIPs() (fips []floatingips.FloatingIP, err e
 }
 
 func (c *openstackCloud) ListL3FloatingIPs(opts l3floatingip.ListOpts) (fips []l3floatingip.FloatingIP, err error) {
+	return listL3FloatingIPs(c, opts)
+}
+
+func listL3FloatingIPs(c OpenstackCloud, opts l3floatingip.ListOpts) (fips []l3floatingip.FloatingIP, err error) {
 
 	done, err := vfs.RetryWithBackoff(readBackoff, func() (bool, error) {
 		page, err := l3floatingip.List(c.NetworkingClient(), opts).AllPages()
@@ -121,6 +163,10 @@ func (c *openstackCloud) ListL3FloatingIPs(opts l3floatingip.ListOpts) (fips []l
 }
 
 func (c *openstackCloud) DeleteFloatingIP(id string) (err error) {
+	return deleteFloatingIP(c, id)
+}
+
+func deleteFloatingIP(c OpenstackCloud, id string) (err error) {
 
 	done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
 		err = l3floatingip.Delete(c.ComputeClient(), id).ExtractErr()
@@ -136,6 +182,10 @@ func (c *openstackCloud) DeleteFloatingIP(id string) (err error) {
 }
 
 func (c *openstackCloud) DeleteL3FloatingIP(id string) (err error) {
+	return deleteL3FloatingIP(c, id)
+}
+
+func deleteL3FloatingIP(c OpenstackCloud, id string) (err error) {
 
 	done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
 		err = l3floatingip.Delete(c.NetworkingClient(), id).ExtractErr()

upup/pkg/fi/cloudup/openstack/image.go

@@ -23,8 +23,12 @@ import (
 )
 
 func (c *openstackCloud) GetImage(name string) (*images.Image, error) {
+	return getImage(c, name)
+}
+
+func getImage(c OpenstackCloud, name string) (*images.Image, error) {
 	opts := images.ListOpts{Name: name}
-	pager := images.List(c.glanceClient, opts)
+	pager := images.List(c.ImageClient(), opts)
 	page, err := pager.AllPages()
 	if err != nil {
 		return nil, fmt.Errorf("failed to list images: %v", err)

upup/pkg/fi/cloudup/openstack/instance.go

@@ -46,6 +46,10 @@ var floatingBackoff = wait.Backoff{
 }
 
 func (c *openstackCloud) CreateInstance(opt servers.CreateOptsBuilder) (*servers.Server, error) {
+	return createInstance(c, opt)
+}
+
+func createInstance(c OpenstackCloud, opt servers.CreateOptsBuilder) (*servers.Server, error) {
 	var server *servers.Server
 
 	done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
@@ -66,6 +70,11 @@ func (c *openstackCloud) CreateInstance(opt servers.CreateOptsBuilder) (*servers
 }
 
 func (c *openstackCloud) ListServerFloatingIPs(instanceID string) ([]*string, error) {
+	return listServerFloatingIPs(c, instanceID, c.floatingEnabled)
+}
+
+func listServerFloatingIPs(c OpenstackCloud, instanceID string, floatingEnabled bool) ([]*string, error) {
+
 	var result []*string
 	_, err := vfs.RetryWithBackoff(floatingBackoff, func() (bool, error) {
 		server, err := c.GetInstance(instanceID)
@@ -81,7 +90,7 @@ func (c *openstackCloud) ListServerFloatingIPs(instanceID string) ([]*string, er
 
 		for _, addrList := range addresses {
 			for _, props := range addrList {
-				if c.floatingEnabled {
+				if floatingEnabled {
 					if props.IPType == "floating" {
 						result = append(result, fi.String(props.Addr))
 					}
@@ -102,21 +111,37 @@ func (c *openstackCloud) ListServerFloatingIPs(instanceID string) ([]*string, er
 }
 
 func (c *openstackCloud) DeleteInstance(i *cloudinstances.CloudInstanceGroupMember) error {
+	return deleteInstance(c, i)
+}
+
+func deleteInstance(c OpenstackCloud, i *cloudinstances.CloudInstanceGroupMember) error {
 	klog.Warning("This does not work without running kops update cluster --yes in another terminal")
-	return c.DeleteInstanceWithID(i.ID)
+	return deleteInstanceWithID(c, i.ID)
 }
 
 func (c *openstackCloud) DeleteInstanceWithID(instanceID string) error {
+	return deleteInstanceWithID(c, instanceID)
+}
+
+func deleteInstanceWithID(c OpenstackCloud, instanceID string) error {
 	return servers.Delete(c.ComputeClient(), instanceID).ExtractErr()
 }
 
 // DetachInstance is not implemented yet. It needs to cause a cloud instance to no longer be counted against the group's size limits.
 func (c *openstackCloud) DetachInstance(i *cloudinstances.CloudInstanceGroupMember) error {
+	return detachInstance(c, i)
+}
+
+func detachInstance(c OpenstackCloud, i *cloudinstances.CloudInstanceGroupMember) error {
 	klog.V(8).Info("openstack cloud provider DetachInstance not implemented yet")
 	return fmt.Errorf("openstack cloud provider does not support surging")
 }
 
 func (c *openstackCloud) GetInstance(id string) (*servers.Server, error) {
+	return getInstance(c, id)
+}
+
+func getInstance(c OpenstackCloud, id string) (*servers.Server, error) {
 	var server *servers.Server
 
 	done, err := vfs.RetryWithBackoff(readBackoff, func() (bool, error) {
@@ -137,6 +162,10 @@ func (c *openstackCloud) GetInstance(id string) (*servers.Server, error) {
 }
 
 func (c *openstackCloud) ListInstances(opt servers.ListOptsBuilder) ([]servers.Server, error) {
+	return listInstances(c, opt)
+}
+
+func listInstances(c OpenstackCloud, opt servers.ListOptsBuilder) ([]servers.Server, error) {
 	var instances []servers.Server
 
 	done, err := vfs.RetryWithBackoff(readBackoff, func() (bool, error) {