Merge pull request #13 from oscp/notReadyChecks

Add minor check for buildnodes and implement better output

daemon/client/checks/certificates.go

@@ -1,29 +1,29 @@
 package checks
 
 import (
+	"crypto/tls"
+	"crypto/x509"
+	"encoding/base64"
 	"encoding/pem"
+	"errors"
+	"fmt"
+	"gopkg.in/yaml.v2"
+	"io/ioutil"
 	"log"
+	"net/http"
 	"os"
-	"io/ioutil"
-	"fmt"
 	"path/filepath"
-	"crypto/x509"
 	"time"
-	"gopkg.in/yaml.v2"
-	"encoding/base64"
-	"errors"
-	"net/http"
-	"crypto/tls"
 )
 
 type Cert struct {
-	File string
+	File     string
 	DaysLeft int
 }
 
 type KubeConfig struct {
 	APIVersion string `yaml:"apiVersion"`
-	Clusters []struct {
+	Clusters   []struct {
 		Cluster struct {
 			CertificateAuthorityData string `yaml:"certificate-authority-data"`
 			Server                   string `yaml:"server"`
@@ -40,7 +40,7 @@ type KubeConfig struct {
 	} `yaml:"contexts"`
 	CurrentContext string `yaml:"current-context"`
 	Kind           string `yaml:"kind"`
-	Preferences struct {
+	Preferences    struct {
 	} `yaml:"preferences"`
 	Users []struct {
 		Name string `yaml:"name"`
@@ -122,7 +122,7 @@ func CheckKubeSslCertificates(kubePaths []string, days int) error {
 
 		err = yaml.Unmarshal(data, &kubeConfig)
 		if err != nil {
-			msg := fmt.Sprintf("unmarshalling %s failed (%s)", kubeFile,  err.Error())
+			msg := fmt.Sprintf("unmarshalling %s failed (%s)", kubeFile, err.Error())
 			log.Println(msg)
 			return errors.New(msg)
 		}
@@ -310,7 +310,7 @@ func getExpiredCerts(filePaths []string, days int) (error, []Cert) {
 
 			if int(daysLeft) <= days {
 				log.Println(fmt.Sprintf("%s expires in %d days", file, int(daysLeft)))
-				expiredCerts = append(expiredCerts, Cert { File: file, DaysLeft: int(daysLeft) })
+				expiredCerts = append(expiredCerts, Cert{File: file, DaysLeft: int(daysLeft)})
 			}
 		}
 	}

daemon/client/checks/openshift.go

@@ -33,41 +33,51 @@ func CheckMasterApis(urls string) error {
 	}
 }
 
-func CheckOcGetNodes() error {
+func CheckOcGetNodes(buildNodes bool) error {
 	log.Println("Checking oc get nodes output")
 
-	out, err := runOcGetNodes()
-	if err != nil {
-		return err
-	}
-
-	if strings.Contains(out, "NotReady") {
-		// Wait a few seconds and see if still NotReady
-		// to avoid wrong alerts
-		time.Sleep(10 * time.Second)
-
-		out2, err := runOcGetNodes()
+	var out string
+	var err error
+	for i := 0; i < 5; i++ {
+		out, err = runOcGetNodes(buildNodes)
 		if err != nil {
 			return err
 		}
-		if strings.Contains(out2, "NotReady") {
+		if strings.Contains(out, "NotReady") {
+			// Wait a few seconds and see if still NotReady
+			// to avoid wrong alerts
 			time.Sleep(10 * time.Second)
-
-			out3, err := runOcGetNodes()
-			if err != nil {
-				return err
-			}
-			if strings.Contains(out3, "NotReady") {
-				return errors.New("Some node is not ready! 'oc get nodes' output contained NotReady. Output: " + out3)
-			}
+			continue
 		}
+		return nil
+	}
+	var purpose string
+	if buildNodes {
+		purpose = "Buildnode "
+	} else {
+		purpose = "Workernode "
 	}
+	return errors.New(purpose + getNotReadyNodeNames(out) + " is not ready! 'oc get nodes' output contained NotReady. Output: " + out)
+}
 
-	return nil
+func getNotReadyNodeNames(out string) string {
+	lines := strings.Split(out, "\n")
+	var notReadyNodes []string
+	for _, line := range lines {
+		if strings.Contains(line, "NotReady") {
+			s := strings.Fields(line)[0]
+			notReadyNodes = append(notReadyNodes, s)
+		}
+	}
+	return strings.Join(notReadyNodes, ", ")
 }
 
-func runOcGetNodes() (string, error) {
-	out, err := exec.Command("bash", "-c", "oc get nodes --show-labels | grep -v monitoring=false | grep -v purpose=buildnode | grep -v SchedulingDisabled").Output()
+func runOcGetNodes(buildNodes bool) (string, error) {
+	buildNodes_grep_params := "-v"
+	if buildNodes {
+		buildNodes_grep_params = ""
+	}
+	out, err := exec.Command("bash", "-c", fmt.Sprintf("oc get nodes --show-labels | grep -v monitoring=false | grep %s purpose=buildnode | grep -v SchedulingDisabled", buildNodes_grep_params)).Output()
 	if err != nil {
 		msg := "Could not parse oc get nodes output: " + err.Error()
 		log.Println(msg)

daemon/client/checks/storage.go

@@ -5,12 +5,12 @@ import (
 	"errors"
 	"fmt"
 	"log"
+	"os"
 	"os/exec"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
-	"os"
-	"regexp"
 )
 
 func CheckOpenFileCount() error {

daemon/client/handlers/general.go

@@ -47,4 +47,4 @@ func generateResponse(w http.ResponseWriter, errors []string) {
 
 	w.Header().Set("Content-Type", "application/json")
 	w.Write(json)
-}
+}

daemon/client/handlers/major.go

@@ -51,7 +51,9 @@ func HandleMajorChecks(daemonType string, w http.ResponseWriter, r *http.Request
 			log.Fatal("env variables 'ETCD_IPS', 'REGISTRY_SVC_IP', 'ROUTER_IPS', 'CHECK_CERTIFICATE_URLS' must be specified on type 'MASTER'")
 		}
 
-		if err := checks.CheckOcGetNodes(); err != nil {
+		// boolean false means exclude buildnodes
+		// boolean true means only buildnodes
+		if err := checks.CheckOcGetNodes(false); err != nil {
 			errors = append(errors, err.Error())
 		}
 
@@ -109,4 +111,4 @@ func HandleMajorChecks(daemonType string, w http.ResponseWriter, r *http.Request
 	}
 
 	generateResponse(w, errors)
-}
+}

daemon/client/handlers/minor.go

@@ -58,6 +58,12 @@ func HandleMinorChecks(daemonType string, w http.ResponseWriter, r *http.Request
 			log.Fatal("allowedWithout seems not to be an integer", allowedWithout)
 		}
 
+		// boolean false means exclude buildnodes
+		// boolean true means only buildnodes
+		if err := checks.CheckOcGetNodes(true); err != nil {
+			errors = append(errors, err.Error())
+		}
+
 		if err := checks.CheckExternalSystem(externalSystem); err != nil {
 			errors = append(errors, err.Error())
 		}

daemon/client/webserver.go

@@ -1,10 +1,10 @@
 package client
 
 import (
+	"github.com/oscp/openshift-monitoring/daemon/client/handlers"
 	"log"
 	"net/http"
 	"os"
-	"github.com/oscp/openshift-monitoring/daemon/client/handlers"
 )
 
 func RunWebserver(daemonType string) {