Merge pull request #587 from ystia/bugfix/GH-584-openstack-networks

Use "0" for default instance name for Openstack network

CHANGELOG.md

@@ -55,6 +55,7 @@ func GetWorkflow(ctx context.Context, deploymentID, workflowName string) (*tosca
 * Can have current deployment and undeployment on the same application on specific conditions ([GH-567](https://github.com/ystia/yorc/issues/567))
 * API calls to deploy and update a deployment will now prevent other API calls that may modify a deployment to run at the same time
 * Yorc HTTP health check is defined on localhost address ([GH-585](https://github.com/ystia/yorc/issues/585))
+* Unable to get network ID attribute for Openstack many compute instances ([GH-584](https://github.com/ystia/yorc/issues/584))
 
 ## 4.0.0-M7 (November 29, 2019)
 

deployments/capabilities.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"path"
 	"strings"
+	"time"
 
 	"github.com/pkg/errors"
 
@@ -356,6 +357,35 @@ func GetInstanceCapabilityAttributeValue(ctx context.Context, deploymentID, node
 	return GetCapabilityPropertyValue(ctx, deploymentID, nodeName, capabilityName, attributeName, nestedKeys...)
 }
 
+// LookupInstanceCapabilityAttributeValue executes a lookup to retrieve instance capability attribute value when attribute can be long to retrieve
+func LookupInstanceCapabilityAttributeValue(ctx context.Context, deploymentID, nodeName, instanceName, capabilityName, attributeName string, nestedKeys ...string) (string, error) {
+	log.Debugf("Capability attribute:%q lookup for deploymentID:%q, node name:%q, instance:%q, capability:%q", attributeName, deploymentID, nodeName, instanceName, capabilityName)
+	res := make(chan string, 1)
+	go func() {
+		for {
+			if attr, _ := GetInstanceCapabilityAttributeValue(ctx, deploymentID, nodeName, instanceName, capabilityName, attributeName, nestedKeys...); attr != nil && attr.RawString() != "" {
+				if attr != nil && attr.RawString() != "" {
+					res <- attr.RawString()
+					return
+				}
+			}
+
+			select {
+			case <-time.After(1 * time.Second):
+			case <-ctx.Done():
+				return
+			}
+		}
+	}()
+
+	select {
+	case val := <-res:
+		return val, nil
+	case <-ctx.Done():
+		return "", ctx.Err()
+	}
+}
+
 func getEndpointCapabilitityHostIPAttributeNameAndNetName(ctx context.Context, deploymentID, nodeName, capabilityName string) (string, *TOSCAValue, error) {
 	// First check the network name in the capability property to find the right
 	// IP address attribute (default: private address)

deployments/instances.go

@@ -28,6 +28,9 @@ import (
 	"github.com/ystia/yorc/v4/tosca"
 )
 
+// DefaultInstanceName is the default instance name for non scalable or single instance nodes.
+const DefaultInstanceName = "0"
+
 // SetInstanceStateStringWithContextualLogs stores the state of a given node instance and publishes a status change event
 // context is used to carry contextual information for logging (see events package)
 func SetInstanceStateStringWithContextualLogs(ctx context.Context, deploymentID, nodeName, instanceName, state string) error {

prov/monitoring/monitoring_mgr.go

@@ -115,7 +115,6 @@ func (mgr *monitoringMgr) startMonitoring() {
 
 			q := &api.QueryOptions{WaitIndex: waitIndex}
 			checks, rMeta, err := mgr.cc.KV().Keys(path.Join(consulutil.MonitoringKVPrefix, "checks")+"/", "/", q)
-			log.Debugf("Found %d checks", len(checks))
 			if err != nil {
 				handleError(err)
 				continue
@@ -126,7 +125,6 @@ func (mgr *monitoringMgr) startMonitoring() {
 				continue
 			}
 			waitIndex = rMeta.LastIndex
-			log.Debugf("Monitoring Wait index: %d", waitIndex)
 			for _, key := range checks {
 				id := path.Base(key)
 				check, err := NewCheckFromID(id)

prov/terraform/openstack/consul_test.go

@@ -47,6 +47,9 @@ func TestRunConsulOpenstackPackageTests(t *testing.T) {
 		t.Run("fipOSInstance", func(t *testing.T) {
 			testFipOSInstance(t, srv)
 		})
+		t.Run("fipMissingOSInstance", func(t *testing.T) {
+			testFipMissingOSInstance(t, srv)
+		})
 		t.Run("fipOSInstanceNotAllowed", func(t *testing.T) {
 			testFipOSInstanceNotAllowed(t, srv)
 		})

prov/terraform/openstack/osinstance.go

@@ -17,12 +17,10 @@ package openstack
 import (
 	"context"
 	"fmt"
+	"github.com/pkg/errors"
 	"path"
 	"strconv"
 	"strings"
-	"time"
-
-	"github.com/pkg/errors"
 
 	"github.com/ystia/yorc/v4/config"
 	"github.com/ystia/yorc/v4/deployments"
@@ -93,7 +91,7 @@ func addServerGroupMembership(ctx context.Context, deploymentID, nodeName string
 	}
 
 	if len(reqs) == 1 {
-		id, err := deployments.LookupInstanceAttributeValue(ctx, deploymentID, reqs[0].Node, "0", "id")
+		id, err := deployments.LookupInstanceAttributeValue(ctx, deploymentID, reqs[0].Node, deployments.DefaultInstanceName, "id")
 		if err != nil {
 			return err
 		}
@@ -246,40 +244,11 @@ func getVolumeID(ctx context.Context, deploymentID, volumeNodeName, instanceName
 	var volumeID string
 	log.Debugf("Looking for volume_id")
 	volumeIDValue, err := deployments.GetNodePropertyValue(ctx, deploymentID, volumeNodeName, "volume_id")
-	if err != nil {
+	if err != nil || volumeIDValue != nil && volumeIDValue.RawString() != "" {
 		return volumeID, err
 	}
-	if volumeIDValue == nil || volumeIDValue.RawString() == "" {
-		resultChan := make(chan string, 1)
-		go func() {
-			for {
-				// ignore errors and retry
-				volID, _ := deployments.GetInstanceAttributeValue(ctx, deploymentID, volumeNodeName, instanceName, "volume_id")
-				// As volumeID is an optional property GetInstanceAttribute then GetProperty
-				// may return an empty volumeID so keep checking as long as we have it
-				if volID != nil && volID.RawString() != "" {
-					resultChan <- volID.RawString()
-					return
-				}
-				select {
-				case <-time.After(1 * time.Second):
-				case <-ctx.Done():
-					// context cancelled, give up!
-					return
-				}
-			}
-		}()
-		select {
-		case volumeID = <-resultChan:
-		case <-ctx.Done():
-			return volumeID, ctx.Err()
-
-		}
-	} else {
-		volumeID = volumeIDValue.RawString()
-	}
 
-	return volumeID, err
+	return deployments.LookupInstanceAttributeValue(ctx, deploymentID, volumeNodeName, instanceName, "volume_id")
 }
 
 func getComputeInstanceNetworks(ctx context.Context, opts osInstanceOptions) ([]ComputeNetwork, error) {
@@ -414,28 +383,12 @@ func computeFloatingIPAddress(ctx context.Context, opts osInstanceOptions,
 	instanceName := opts.instanceName
 
 	log.Debugf("Looking for Floating IP")
-	var floatingIP string
-	resultChan := make(chan string, 1)
-	go func() {
-		for {
-			if fip, _ := deployments.GetInstanceCapabilityAttributeValue(ctx, deploymentID, networkNodeName, "0", "endpoint", "floating_ip_address"); fip != nil && fip.RawString() != "" {
-				resultChan <- fip.RawString()
-				return
-			}
 
-			select {
-			case <-time.After(1 * time.Second):
-			case <-ctx.Done():
-				// context cancelled, give up!
-				return
-			}
-		}
-	}()
-	select {
-	case floatingIP = <-resultChan:
-	case <-ctx.Done():
-		return ctx.Err()
+	floatingIP, err := deployments.LookupInstanceCapabilityAttributeValue(ctx, deploymentID, networkNodeName, opts.instanceName, "endpoint", "floating_ip_address")
+	if err != nil {
+		return err
 	}
+
 	floatingIPAssociate := ComputeFloatingIPAssociate{
 		Region:     instance.Region,
 		FloatingIP: floatingIP,
@@ -460,33 +413,11 @@ func computeNetworkAttributes(ctx context.Context, opts osInstanceOptions,
 	instance *ComputeInstance, outputs map[string]string) error {
 
 	log.Debugf("Looking for Network id for %q", networkNodeName)
-	var networkID string
-	resultChan := make(chan string, 1)
-	go func() {
-		for {
-			nID, err := deployments.GetInstanceAttributeValue(ctx, opts.deploymentID, networkNodeName, opts.instanceName, "network_id")
-			if err != nil {
-				log.Printf("[Warning] bypassing error while waiting for a network id: %v", err)
-			}
-			// As networkID is an optional property GetInstanceAttribute then GetProperty
-			// may return an empty networkID so keep checking as long as we have it
-			if nID != nil && nID.RawString() != "" {
-				resultChan <- nID.RawString()
-				return
-			}
-			select {
-			case <-time.After(1 * time.Second):
-			case <-ctx.Done():
-				// context cancelled, give up!
-				return
-			}
-		}
-	}()
-	select {
-	case networkID = <-resultChan:
-	case <-ctx.Done():
-		return ctx.Err()
+	networkID, err := deployments.LookupInstanceAttributeValue(ctx, opts.deploymentID, networkNodeName, deployments.DefaultInstanceName, "network_id")
+	if err != nil {
+		return err
 	}
+
 	cn := ComputeNetwork{UUID: networkID, AccessNetwork: false}
 	i := len(instance.Networks)
 	if instance.Networks == nil {

prov/terraform/openstack/osinstance_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/ystia/yorc/v4/tosca"
 	"path"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/consul/testutil"
 	"github.com/stretchr/testify/assert"
@@ -231,6 +232,42 @@ func testFipOSInstance(t *testing.T, srv *testutil.TestServer) {
 
 }
 
+func testFipMissingOSInstance(t *testing.T, srv *testutil.TestServer) {
+	t.Parallel()
+	deploymentID := loadTestYaml(t)
+
+	locationProps := config.DynamicMap{
+		"provisioning_over_fip_allowed": true,
+		"private_network_name":          "test",
+	}
+	var cfg config.Configuration
+
+	g := osGenerator{}
+	infrastructure := commons.Infrastructure{}
+	env := make([]string, 0)
+	outputs := make(map[string]string, 0)
+	resourceTypes := getOpenstackResourceTypes(locationProps)
+	ctx, cancel := context.WithCancel(context.Background())
+	go func() {
+		time.Sleep(3 * time.Second)
+		cancel()
+	}()
+
+	err := g.generateOSInstance(
+		ctx,
+		osInstanceOptions{
+			cfg:            cfg,
+			infrastructure: &infrastructure,
+			locationProps:  locationProps,
+			deploymentID:   deploymentID,
+			nodeName:       "Compute",
+			instanceName:   "0",
+			resourceTypes:  resourceTypes,
+		},
+		outputs, &env)
+	require.Errorf(t, err, "expected context canceled")
+}
+
 func testFipOSInstanceNotAllowed(t *testing.T, srv *testutil.TestServer) {
 	t.Parallel()
 	deploymentID := loadTestYaml(t)
@@ -370,7 +407,7 @@ func testComputeNetworkAttributes(t *testing.T, srv *testutil.TestServer) {
 	infrastructure := commons.Infrastructure{}
 	outputs := make(map[string]string, 0)
 	resourceTypes := getOpenstackResourceTypes(locationProps)
-	opts := osInstanceOptions{
+	optsInstance1 := osInstanceOptions{
 		cfg:            cfg,
 		infrastructure: &infrastructure,
 		locationProps:  locationProps,
@@ -385,15 +422,35 @@ func testComputeNetworkAttributes(t *testing.T, srv *testutil.TestServer) {
 	instKey := "instKey"
 	srv.PopulateKV(t, map[string][]byte{
 		path.Join(consulutil.DeploymentKVPrefix, deploymentID,
-			"/topology/instances", networkNodeName, opts.instanceName, "attributes", "network_id"): []byte(networkID),
+			"/topology/instances", networkNodeName, "0", "attributes", "network_id"): []byte(networkID),
 	})
 
-	instance := ComputeInstance{
+	instance1 := ComputeInstance{
 		Name: "instanceName",
 	}
-	err := computeNetworkAttributes(context.Background(), opts, networkNodeName, instKey,
-		&instance, outputs)
+	err := computeNetworkAttributes(context.Background(), optsInstance1, networkNodeName, instKey,
+		&instance1, outputs)
+	require.NoError(t, err, "Failed to compute network attributes")
+	require.Equal(t, 1, len(instance1.Networks), "Expected to have one compute instance network")
+	assert.Equal(t, networkID, instance1.Networks[0].UUID, "Wrong network ID")
+
+	// Check another instance
+	optsInstance2 := osInstanceOptions{
+		cfg:            cfg,
+		infrastructure: &infrastructure,
+		locationProps:  locationProps,
+		deploymentID:   deploymentID,
+		nodeName:       "ComputeA",
+		instanceName:   "1",
+		resourceTypes:  resourceTypes,
+	}
+
+	instance2 := ComputeInstance{
+		Name: "instanceName2",
+	}
+	err = computeNetworkAttributes(context.Background(), optsInstance2, networkNodeName, instKey,
+		&instance2, outputs)
 	require.NoError(t, err, "Failed to compute network attributes")
-	require.Equal(t, 1, len(instance.Networks), "Expected to have one compute instance network")
-	assert.Equal(t, networkID, instance.Networks[0].UUID, "Wrong network ID")
+	require.Equal(t, 1, len(instance2.Networks), "Expected to have one compute instance network")
+	assert.Equal(t, networkID, instance2.Networks[0].UUID, "Wrong network ID")
 }

prov/terraform/openstack/testdata/fipMissingOSInstance.yaml

@@ -0,0 +1,41 @@
+tosca_definitions_version: tosca_simple_yaml_1_0_0_wd03
+description: Alien4Cloud generated service template
+metadata:
+  template_name: Test
+  template_version: 0.1.0-SNAPSHOT
+  template_author: admin
+
+imports:
+  - openstack-types: <yorc-openstack-types.yml>
+
+topology_template:
+  node_templates:
+    Compute:
+      type: yorc.nodes.openstack.Compute
+      properties:
+        flavor: 2
+        image: 4bde6002-649d-4868-a5cb-fcd36d5ffa63
+        availability_zone: nova
+        key_pair: yorc
+        security_groups: openbar,default
+        region: Region3
+      requirements:
+        - network:
+            node: Network
+            capability: yorc.capabilities.openstack.FIPConnectivity
+      capabilities:
+        endpoint:
+          properties:
+            protocol: tcp
+            initiator: source
+            secure: true
+            network_name: PRIVATE
+            credentials: {user: cloud-user}
+        scalable:
+          properties:
+            max_instances: 1
+            min_instances: 1
+            default_instances: 1
+    Network:
+      type: yorc.nodes.openstack.FloatingIP
+

storage/store_mgr.go

@@ -18,9 +18,10 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	uuid "github.com/satori/go.uuid"
+	"math/rand"
 	"path"
 	"strings"
+	"time"
 
 	"github.com/matryer/resync"
 	"github.com/pkg/errors"
@@ -243,8 +244,9 @@ func checkAndBuildConfigStores(cfg config.Configuration) ([]config.Store, error)
 		}
 
 		if cfgStore.Name == "" {
-			extra := uuid.NewV4()
-			cfgStore.Name = fmt.Sprintf("%s%s-%s", cfgStore.Implementation, strings.Join(cfgStore.Types, ""), extra[0:3])
+			rand.Seed(time.Now().UnixNano())
+			extra := rand.Intn(100)
+			cfgStore.Name = fmt.Sprintf("%s%s-%d", cfgStore.Implementation, strings.Join(cfgStore.Types, ""), extra)
 		}
 
 		// Check store name is unique