Send the error message to user when enabling canary feature fails. (#990)

* Send the error message to user when enabling canary feature fails.
* Fix controller doesn’t reply canary error when promClient tries connect to invalid prometheus server

canaryconfigmgr/canaryConfigMgr.go

@@ -62,11 +62,16 @@ func MakeCanaryConfigMgr(fissionClient *crd.FissionClient, kubeClient *kubernete
 		}
 	}
 
+	promClient, err := MakePrometheusClient(prometheusSvc)
+	if err != nil {
+		return nil, err
+	}
+
 	configMgr := &canaryConfigMgr{
 		fissionClient:          fissionClient,
 		kubeClient:             kubeClient,
 		crdClient:              crdClient,
-		promClient:             MakePrometheusClient(prometheusSvc),
+		promClient:             promClient,
 		canaryCfgCancelFuncMap: makecanaryConfigCancelFuncMap(),
 	}
 

canaryconfigmgr/prometheusClient.go

@@ -18,9 +18,10 @@ package canaryconfigmgr
 
 import (
 	"fmt"
-	"golang.org/x/net/context"
 	"time"
 
+	"golang.org/x/net/context"
+
 	promClient "github.com/prometheus/client_golang/api/prometheus"
 	"github.com/prometheus/common/model"
 	log "github.com/sirupsen/logrus"
@@ -30,7 +31,7 @@ type PrometheusApiClient struct {
 	client promClient.QueryAPI
 }
 
-func MakePrometheusClient(prometheusSvc string) *PrometheusApiClient {
+func MakePrometheusClient(prometheusSvc string) (*PrometheusApiClient, error) {
 	promApiConfig := promClient.Config{
 		Address: prometheusSvc,
 	}
@@ -38,14 +39,31 @@ func MakePrometheusClient(prometheusSvc string) *PrometheusApiClient {
 	promApiClient, err := promClient.New(promApiConfig)
 	if err != nil {
 		log.Errorf("Error creating prometheus api client for svc : %s, err : %v", prometheusSvc, err)
+		return nil, err
 	}
 
 	apiQueryClient := promClient.NewQueryAPI(promApiClient)
 
+	// By default, the prometheus client library doesn't test server connectivity when creating
+	// prometheus client. As a workaround, here we send out a test query string to ensure that
+	// prometheus server is running.
+	for i := 0; i < 15; i++ {
+		_, err = apiQueryClient.Query(context.Background(), "http_requests_total", time.Now())
+		if err == nil {
+			break
+		}
+		time.Sleep(time.Second)
+	}
+
+	if err != nil {
+		log.Printf("Error sending test query to prometheus server: %v", err)
+		return nil, err
+	}
+
 	log.Printf("Successfully made prometheus client with service : %s", prometheusSvc)
 	return &PrometheusApiClient{
 		client: apiQueryClient,
-	}
+	}, nil
 }
 
 func (promApiClient *PrometheusApiClient) GetFunctionFailurePercentage(path, method, funcName, funcNs string, window string) (float64, error) {

controller/api.go

@@ -32,7 +32,6 @@ import (
 
 	"github.com/fission/fission"
 	"github.com/fission/fission/crd"
-	config "github.com/fission/fission/featureconfig"
 	"github.com/fission/fission/fission/logdb"
 )
 
@@ -54,7 +53,7 @@ type (
 		workflowApiUrl    string
 		functionNamespace string
 		useIstio          bool
-		featureConfig     *config.FeatureConfig
+		featureStatus     map[string]string
 	}
 
 	logDBConfig struct {
@@ -64,7 +63,7 @@ type (
 	}
 )
 
-func MakeAPI(featureConfig *config.FeatureConfig) (*API, error) {
+func MakeAPI(featureStatus map[string]string) (*API, error) {
 	api, err := makeCRDBackedAPI()
 
 	u := os.Getenv("STORAGE_SERVICE_URL")
@@ -95,7 +94,7 @@ func MakeAPI(featureConfig *config.FeatureConfig) (*API, error) {
 		api.functionNamespace = "fission-function"
 	}
 
-	api.featureConfig = featureConfig
+	api.featureStatus = featureStatus
 
 	return api, err
 }

controller/canaryConfigApi.go

@@ -18,6 +18,7 @@ package controller
 
 import (
 	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"net/http"
 
@@ -27,11 +28,13 @@ import (
 
 	"github.com/fission/fission"
 	"github.com/fission/fission/crd"
+	config "github.com/fission/fission/featureconfig"
 )
 
 func (a *API) CanaryConfigApiCreate(w http.ResponseWriter, r *http.Request) {
-	if !a.featureConfig.CanaryConfig.IsEnabled {
+	featureErr := a.featureStatus[config.CanaryFeature]
-		a.respondWithError(w, fission.MakeError(http.StatusBadRequest, "Please enable canary feature while installing fission"))
+	if len(featureErr) > 0 {
+		a.respondWithError(w, fission.MakeError(http.StatusInternalServerError, fmt.Sprintf("Error enabling canary feature: %v", featureErr)))
 		return
 	}
 
@@ -66,8 +69,9 @@ func (a *API) CanaryConfigApiCreate(w http.ResponseWriter, r *http.Request) {
 }
 
 func (a *API) CanaryConfigApiGet(w http.ResponseWriter, r *http.Request) {
-	if !a.featureConfig.CanaryConfig.IsEnabled {
+	featureErr := a.featureStatus[config.CanaryFeature]
-		a.respondWithError(w, fission.MakeError(http.StatusBadRequest, "Please enable canary feature while installing fission"))
+	if len(featureErr) > 0 {
+		a.respondWithError(w, fission.MakeError(http.StatusInternalServerError, fmt.Sprintf("Error enabling canary feature: %v", featureErr)))
 		return
 	}
 
@@ -95,8 +99,9 @@ func (a *API) CanaryConfigApiGet(w http.ResponseWriter, r *http.Request) {
 }
 
 func (a *API) CanaryConfigApiList(w http.ResponseWriter, r *http.Request) {
-	if !a.featureConfig.CanaryConfig.IsEnabled {
+	featureErr := a.featureStatus[config.CanaryFeature]
-		a.respondWithError(w, fission.MakeError(http.StatusBadRequest, "Please enable canary feature while installing fission"))
+	if len(featureErr) > 0 {
+		a.respondWithError(w, fission.MakeError(http.StatusInternalServerError, fmt.Sprintf("Error enabling canary feature: %v", featureErr)))
 		return
 	}
 
@@ -121,8 +126,9 @@ func (a *API) CanaryConfigApiList(w http.ResponseWriter, r *http.Request) {
 }
 
 func (a *API) CanaryConfigApiUpdate(w http.ResponseWriter, r *http.Request) {
-	if !a.featureConfig.CanaryConfig.IsEnabled {
+	featureErr := a.featureStatus[config.CanaryFeature]
-		a.respondWithError(w, fission.MakeError(http.StatusBadRequest, "Please enable canary feature while installing fission"))
+	if len(featureErr) > 0 {
+		a.respondWithError(w, fission.MakeError(http.StatusInternalServerError, fmt.Sprintf("Error enabling canary feature: %v", featureErr)))
 		return
 	}
 
@@ -155,8 +161,9 @@ func (a *API) CanaryConfigApiUpdate(w http.ResponseWriter, r *http.Request) {
 }
 
 func (a *API) CanaryConfigApiDelete(w http.ResponseWriter, r *http.Request) {
-	if !a.featureConfig.CanaryConfig.IsEnabled {
+	featureErr := a.featureStatus[config.CanaryFeature]
-		a.respondWithError(w, fission.MakeError(http.StatusBadRequest, "Please enable canary feature while installing fission"))
+	if len(featureErr) > 0 {
+		a.respondWithError(w, fission.MakeError(http.StatusInternalServerError, fmt.Sprintf("Error enabling canary feature: %v", featureErr)))
 		return
 	}
 

controller/config.go

@@ -28,12 +28,13 @@ import (
 	config "github.com/fission/fission/featureconfig"
 )
 
-func ConfigCanaryFeature(context context.Context, fissionClient *crd.FissionClient, kubeClient *kubernetes.Clientset, featureConfig *config.FeatureConfig) error {
+func ConfigCanaryFeature(context context.Context, fissionClient *crd.FissionClient, kubeClient *kubernetes.Clientset, featureConfig *config.FeatureConfig, featureStatus map[string]string) error {
 	// start the appropriate controller
 	if featureConfig.CanaryConfig.IsEnabled {
 		canaryCfgMgr, err := canaryconfigmgr.MakeCanaryConfigMgr(fissionClient, kubeClient, fissionClient.GetCrdClient(),
 			featureConfig.CanaryConfig.PrometheusSvc)
 		if err != nil {
+			featureStatus[config.CanaryFeature] = err.Error()
 			return fmt.Errorf("failed to start canary config manager: %v", err)
 		}
 		canaryCfgMgr.Run(context)
@@ -44,22 +45,23 @@ func ConfigCanaryFeature(context context.Context, fissionClient *crd.FissionClie
 }
 
 // ConfigureFeatures gets the feature config and configures the features that are enabled
-func ConfigureFeatures(context context.Context, unitTestMode bool, fissionClient *crd.FissionClient, kubeClient *kubernetes.Clientset) (*config.FeatureConfig, error) {
+func ConfigureFeatures(context context.Context, unitTestMode bool, fissionClient *crd.FissionClient, kubeClient *kubernetes.Clientset) (map[string]string, error) {
 	// set feature enabled to false if unitTestMode
 	if unitTestMode {
-		featureConfig := &config.FeatureConfig{}
+		return nil, nil
-		return featureConfig, nil
 	}
 
 	// get the featureConfig from config map mounted onto the file system
 	featureConfig, err := config.GetFeatureConfig()
 	if err != nil {
 		log.Printf("Error getting feature config : %v", err)
-		return featureConfig, err
+		return nil, err
 	}
 
+	featureStatus := make(map[string]string)
+
 	// configure respective features
 	// in the future when new optional features are added, we need to add corresponding feature handlers and invoke them here
-	err = ConfigCanaryFeature(context, fissionClient, kubeClient, featureConfig)
+	err = ConfigCanaryFeature(context, fissionClient, kubeClient, featureConfig, featureStatus)
-	return featureConfig, err
+	return featureStatus, err
 }

controller/controller.go

@@ -44,15 +44,13 @@ func Start(port int, unitTestFlag bool) {
 	}
 
 	ctx, cancel := context.WithCancel(context.Background())
-	featureConfig, err := ConfigureFeatures(ctx, unitTestFlag, fc, kc)
+	featureStatus, err := ConfigureFeatures(ctx, unitTestFlag, fc, kc)
 	if err != nil {
 		log.Printf("Error configuring features : %v. Proceeding without optional features", err.Error())
-		// set all features to false for the MakeApi call below
-		featureConfig.CanaryConfig.IsEnabled = false
 	}
 	defer cancel()
 
-	api, err := MakeAPI(featureConfig)
+	api, err := MakeAPI(featureStatus)
 	if err != nil {
 		log.Fatalf("Failed to start controller: %v", err)
 	}

featureconfig/types.go

@@ -18,6 +18,7 @@ package featureconfig
 
 const (
 	FeatureConfigFile = "/etc/config/config.yaml"
+	CanaryFeature     = "canary"
 )
 
 type (

fission/canaryconfig.go

@@ -60,7 +60,7 @@ func canaryConfigCreate(c *cli.Context) error {
 
 	htTrigger, err := client.HTTPTriggerGet(m)
 	if err != nil {
-		util.CheckErr(err, "Trigger referenced in the canary config is not created")
+		util.CheckErr(err, "find trigger referenced in the canary config")
 	}
 
 	// check that the trigger has function reference type function weights