Merge pull request #633 from ystia/bugfix/GH-623_detect_consul_startu…

…p_on_bootstrap

Check Consul startup using the server leader api

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### BUG FIXES
 
 * Location is not listed if its properties are missing ([GH-625](https://github.com/ystia/yorc/issues/625))
+* Sometimes Yorc bootstrap fails to start local Yorc instance because Consul is not properly started ([GH-623](https://github.com/ystia/yorc/issues/623))
 
 ## 4.0.0-rc.1 (March 30, 2020)
 

commands/bootstrap/setup.go

@@ -15,9 +15,9 @@
 package bootstrap
 
 import (
-	"bufio"
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -360,51 +360,56 @@ func startConsul(workingDirectoryPath string) (*exec.Cmd, error) {
 		fmt.Println("Consul is already running")
 		return nil, nil
 	}
-
-	fmt.Println("Starting Consul...")
+	consulLogPath := filepath.Join(workingDirectoryPath, "consul.log")
+	consulLogFile, err := os.OpenFile(consulLogPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600)
+	if err != nil {
+		return nil, err
+	}
+	fmt.Printf("Starting Consul (logs: %s)", consulLogPath)
 	// Consul startup options
 	// Specifying a bind address or it would fail on a setup with multiple
 	// IPv4 addresses configured
 	cmdArgs := "agent -server -bootstrap-expect 1 -bind 127.0.0.1 -data-dir " + dataDir
 	cmd = exec.Command(executable, strings.Split(cmdArgs, " ")...)
-	output, _ := cmd.StdoutPipe()
+	cmd.Stdout = consulLogFile
 	err = cmd.Start()
 	if err != nil {
 		return nil, err
 	}
 
 	// Wait for a new leader to be elected, else Yorc could try to access Consul
 	// when it is not yet ready
-	err = waitForOutput(output, "New leader elected", 60*time.Second)
-	if err != nil {
-		cleanBootstrapSetup(workingDirectoryPath)
-		return nil, err
-	}
-
-	fmt.Println("...Consul started")
+	waitForConsulReadiness("http://127.0.0.1:8500")
+	fmt.Println(" Consul started!")
 	return cmd, err
 }
 
-func waitForOutput(output io.ReadCloser, expected string, timeout time.Duration) error {
-
-	reader := bufio.NewReader(output)
-	timer := time.NewTimer(timeout)
-	defer timer.Stop()
+func waitForConsulReadiness(consulHTTPEndpoint string) {
 	for {
-		select {
-		case <-timer.C:
-			return fmt.Errorf("Timeout waiting for %s", expected)
-		default:
-			line, err := reader.ReadString('\n')
-			if err != nil {
-				return err
-			}
-			if strings.Contains(line, expected) {
-				return nil
-			}
+		fmt.Print(".")
+		leader, _ := getConsulLeader(consulHTTPEndpoint)
+		if leader != "" {
+			return
 		}
+		<-time.After(2 * time.Second)
 	}
+}
 
+func getConsulLeader(consulHTTPEndpoint string) (string, error) {
+	resp, err := http.Get(fmt.Sprintf("%s/v1/status/leader", consulHTTPEndpoint))
+	if err != nil {
+		return "", err
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		return "", errors.New(resp.Status)
+	}
+	bodyBytes, err := ioutil.ReadAll(resp.Body)
+	matches := regexp.MustCompile(`"(.*)"`).FindStringSubmatch(string(bodyBytes))
+	if len(matches) < 2 {
+		return "", nil
+	}
+	return matches[1], nil
 }
 
 // downloadDependencies downloads Yorc Server dependencies

commands/bootstrap/setup_test.go

@@ -0,0 +1,111 @@
+// Copyright 2018 Bull S.A.S. Atos Technologies - Bull, Rue Jean Jaures, B.P.68, 78340, Les Clayes-sous-Bois, France.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package bootstrap
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"gotest.tools/v3/assert"
+)
+
+func Test_waitForConsulReadiness(t *testing.T) {
+	type endpoint struct {
+		started    bool
+		httpStatus int
+		text       string
+	}
+	tests := []struct {
+		name          string
+		endpoint      endpoint
+		shouldSucceed bool
+	}{
+		{"NormalStartup", endpoint{true, 200, `"127.0.0.1:8300"`}, true},
+		{"ConsulStartedButNotReady", endpoint{true, 200, `""`}, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+
+			ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(tt.endpoint.httpStatus)
+				w.Write([]byte(tt.endpoint.text))
+			}))
+			if !tt.endpoint.started {
+				// close now!
+				ts.Close()
+			} else {
+				defer ts.Close()
+			}
+			c := make(chan struct{}, 0)
+			go func() {
+				waitForConsulReadiness(ts.URL)
+				close(c)
+			}()
+			var timedOut bool
+			select {
+			case <-c:
+			case <-time.After(6 * time.Second):
+				timedOut = true
+			}
+			if tt.shouldSucceed == timedOut {
+				t.Errorf("expecting waitForConsulReadiness() to succeed but it timedout")
+			} else if !tt.shouldSucceed && !timedOut {
+				t.Errorf("expecting waitForConsulReadiness() to timeout but it succeeded")
+			}
+		})
+	}
+}
+
+func Test_getConsulLeader(t *testing.T) {
+	type endpoint struct {
+		started    bool
+		httpStatus int
+		text       string
+	}
+	tests := []struct {
+		name     string
+		endpoint endpoint
+		want     string
+		wantErr  bool
+	}{
+		{"NormalStartup", endpoint{true, 200, `"127.0.0.1:8300"`}, "127.0.0.1:8300", false},
+		{"ConsulStartedButNotReady", endpoint{true, 200, `""`}, "", false},
+		{"ConsulStartedButBadResponse", endpoint{true, 200, `qsdhjgy@àà*ùù$ugfbdnflv`}, "", false},
+		{"ConsulStartedButBadStatus", endpoint{true, 500, ``}, "", true},
+		{"ConsulNotStarted", endpoint{false, 0, ``}, "", true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(tt.endpoint.httpStatus)
+				w.Write([]byte(tt.endpoint.text))
+			}))
+			defer ts.Close()
+			got, err := getConsulLeader(ts.URL)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("getConsulLeader() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if err != nil && tt.endpoint.httpStatus == 500 {
+				assert.ErrorContains(t, err, "500")
+			}
+			if got != tt.want {
+				t.Errorf("getConsulLeader() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

go.mod

@@ -115,6 +115,7 @@ require (
 	gopkg.in/ory-am/dockertest.v3 v3.3.5 // indirect
 	gopkg.in/yaml.v2 v2.2.4
 	gotest.tools v2.2.0+incompatible // indirect
+	gotest.tools/v3 v3.0.0
 	k8s.io/api v0.0.0-20180628040859-072894a440bd
 	k8s.io/apimachinery v0.0.0-20180621070125-103fd098999d
 	k8s.io/client-go v8.0.0+incompatible

go.sum

@@ -619,6 +619,7 @@ gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo=
 gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw=
 gotest.tools/v3 v3.0.0 h1:d+tVGRu6X0ZBQ+kyAR8JKi6AXhTP2gmQaoIYaGFz634=
 gotest.tools/v3 v3.0.0/go.mod h1:TUP+/YtXl/dp++T+SZ5v2zUmLVBHmptSb/ajDLCJ+3c=
+gotest.tools/v3 v3.0.2 h1:kG1BFyqVHuQoVQiR1bWGnfz/fmHvvuiSPIV7rvl360E=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 k8s.io/api v0.0.0-20180628040859-072894a440bd h1:HzgYeLDS1jLxw8DGr68KJh9cdQ5iZJizG0HZWstIhfQ=
 k8s.io/api v0.0.0-20180628040859-072894a440bd/go.mod h1:iuAfoD4hCxJ8Onx9kaTIt30j7jUFS00AXQi6QMi99vA=