diff --git a/pkg/controller/endpointmonitor/endpointmonitor_controller.go b/pkg/controller/endpointmonitor/endpointmonitor_controller.go index a405bbbd..e210bef2 100644 --- a/pkg/controller/endpointmonitor/endpointmonitor_controller.go +++ b/pkg/controller/endpointmonitor/endpointmonitor_controller.go @@ -117,10 +117,17 @@ func (r *ReconcileEndpointMonitor) Reconcile(request reconcile.Request) (reconci delay := time.Until(createTime.Add(config.GetControllerConfig().CreationDelay)) for index := 0; index < len(r.monitorServices); index++ { - monitor := findMonitorByName(r.monitorServices[index], monitorName) + monitor, err := findMonitorByName(r.monitorServices[index], monitorName) + // if there was some error while getting a monitor, re-queue it to try on the next reconcile iteration + if err != nil { + return reconcile.Result{RequeueAfter: RequeueTime}, err + } if monitor != nil { // Monitor already exists, update if required err = r.handleUpdate(request, instance, *monitor, r.monitorServices[index]) + if err != nil { + log.Errorf("Error while handling update: %s", err) + } } else { // Monitor doesn't exist, create monitor if delay.Nanoseconds() > 0 { @@ -129,18 +136,16 @@ func (r *ReconcileEndpointMonitor) Reconcile(request reconcile.Request) (reconci return reconcile.Result{RequeueAfter: delay}, nil } err = r.handleCreate(request, instance, monitorName, r.monitorServices[index]) + if err != nil { + log.Errorf("Error while handling create: %s", err) + } } } return reconcile.Result{RequeueAfter: RequeueTime}, err } -func findMonitorByName(monitorService monitors.MonitorServiceProxy, monitorName string) *models.Monitor { - - monitor, _ := monitorService.GetByName(monitorName) - // Monitor Exists - if monitor != nil { - return monitor - } - return nil +func findMonitorByName(monitorService monitors.MonitorServiceProxy, monitorName string) (*models.Monitor, error) { + monitor, err := monitorService.GetByName(monitorName) + return monitor, err } diff --git a/pkg/controller/endpointmonitor/endpointmonitor_controller_test.go b/pkg/controller/endpointmonitor/endpointmonitor_controller_test.go index 8327c6d8..d8685dd5 100644 --- a/pkg/controller/endpointmonitor/endpointmonitor_controller_test.go +++ b/pkg/controller/endpointmonitor/endpointmonitor_controller_test.go @@ -123,8 +123,8 @@ func TestEndpointMonitorReconcileUpdate(t *testing.T) { monitorCount = 0 for index := 0; index < len(r.monitorServices); index++ { - monitor := findMonitorByName(r.monitorServices[index], monitorName) - if monitor != nil && monitor.URL == testURLFacebook { + monitor, err := findMonitorByName(r.monitorServices[index], monitorName) + if monitor != nil && err == nil && monitor.URL == testURLFacebook { log.Info("Found Updated Monitor for Provider: " + r.monitorServices[index].GetType()) monitorCount++ } @@ -185,7 +185,10 @@ func TestEndpointMonitorReconcileDelete(t *testing.T) { monitorCount = 0 for index := 0; index < len(r.monitorServices); index++ { - monitor := findMonitorByName(r.monitorServices[index], monitorName) + monitor, err := findMonitorByName(r.monitorServices[index], monitorName) + if err != nil { + t.Error(err, "Could not findMonitorByName") + } if monitor == nil { monitorCount++ } @@ -232,7 +235,10 @@ func setupReconcilerAndCreateResource() (client.Client, *ReconcileEndpointMonito func getMonitorCount(monitorName string, monitorServices []monitors.MonitorServiceProxy) int { monitorCount := 0 for index := 0; index < len(monitorServices); index++ { - monitor := findMonitorByName(monitorServices[index], monitorName) + monitor, err := findMonitorByName(monitorServices[index], monitorName) + if err != nil { + log.Error(err, "Could not findMonitorByName") + } if monitor != nil { log.Info("Found Monitor for Provider: " + monitorServices[index].GetType()) monitorCount++ diff --git a/pkg/monitors/appinsights/appinsights-monitor.go b/pkg/monitors/appinsights/appinsights-monitor.go index 53b1cf02..493f43bf 100644 --- a/pkg/monitors/appinsights/appinsights-monitor.go +++ b/pkg/monitors/appinsights/appinsights-monitor.go @@ -189,8 +189,10 @@ func (aiService *AppinsightsMonitorService) GetByName(monitorName string) (*mode log.Println("AppInsights Monitor's GetByName method has been called") webtest, err := aiService.insightsClient.Get(aiService.ctx, aiService.resourceGroup, monitorName) if err != nil { + // if not found let's mark it as non-error if webtest.Response.StatusCode == http.StatusNotFound { - return nil, fmt.Errorf("Application Insights WebTest %s was not found in Resource Group %s", monitorName, aiService.resourceGroup) + log.Printf("Application Insights WebTest %s was not found in Resource Group %s", monitorName, aiService.resourceGroup) + return nil, nil } return nil, fmt.Errorf("Error retrieving Application Insights WebTests %s (Resource Group %s): %v", monitorName, aiService.resourceGroup, err) } diff --git a/pkg/monitors/gcloud/gcloud-monitor.go b/pkg/monitors/gcloud/gcloud-monitor.go index 13e78460..38ca2a17 100644 --- a/pkg/monitors/gcloud/gcloud-monitor.go +++ b/pkg/monitors/gcloud/gcloud-monitor.go @@ -60,8 +60,9 @@ func (service *MonitorService) GetByName(name string) (monitor *models.Monitor, return &localMonitor, nil } } - - return nil, fmt.Errorf("Unable to locate monitor with name %v", name) + // monitor not found and no errors raised + log.Printf("There is no gcloud MonitorService with name %s found.", name) + return nil, nil } func (service *MonitorService) GetAll() (monitors []models.Monitor) { diff --git a/pkg/monitors/pingdom/pingdom-monitor.go b/pkg/monitors/pingdom/pingdom-monitor.go index 54fe202d..43bd8a7c 100644 --- a/pkg/monitors/pingdom/pingdom-monitor.go +++ b/pkg/monitors/pingdom/pingdom-monitor.go @@ -27,7 +27,7 @@ type PingdomMonitorService struct { client *pingdom.Client } -func (monitor *PingdomMonitorService) Equal(oldMonitor models.Monitor, newMonitor models.Monitor) bool { +func (service *PingdomMonitorService) Equal(oldMonitor models.Monitor, newMonitor models.Monitor) bool { // TODO: Retrieve oldMonitor config and compare it here return false } @@ -45,21 +45,23 @@ func (service *PingdomMonitorService) Setup(p config.Provider) { BaseURL: service.url, }) if err != nil { - log.Println("Error Seting Up Monitor Service: ", err.Error()) + log.Println("Error Setting Up Monitor Service: ", err.Error()) } } func (service *PingdomMonitorService) GetByName(name string) (*models.Monitor, error) { - var match *models.Monitor - - monitors := service.GetAll() + monitors, err := service.GetAllWithAPIErrorHandler() + if err != nil { + return nil, fmt.Errorf("error received while getting all Pingdom monitors: %s", err.Error()) + } for _, mon := range monitors { if mon.Name == name { return &mon, nil } } - - return match, fmt.Errorf("Unable to locate monitor with name %v", name) + // neither error and any matched monitor + log.Printf("There is no PingdomMonitorService with name %s found.", name) + return nil, nil } func (service *PingdomMonitorService) GetAll() []models.Monitor { @@ -82,6 +84,26 @@ func (service *PingdomMonitorService) GetAll() []models.Monitor { return monitors } +// Function that gets all monitors with an error handling to avoid duplicate checks creation +func (service *PingdomMonitorService) GetAllWithAPIErrorHandler() ([]models.Monitor, error) { + var monitors []models.Monitor + + checks, err := service.client.Checks.List() + if err != nil { + return nil, err + } + for _, mon := range checks { + newMon := models.Monitor{ + URL: mon.Hostname, + ID: fmt.Sprintf("%v", mon.ID), + Name: mon.Name, + } + monitors = append(monitors, newMon) + } + + return monitors, nil +} + func (service *PingdomMonitorService) Add(m models.Monitor) { httpCheck := service.createHttpCheck(m) diff --git a/pkg/monitors/pingdom/pingdom-monitor_test.go b/pkg/monitors/pingdom/pingdom-monitor_test.go index 48915b86..5b3ee3c7 100644 --- a/pkg/monitors/pingdom/pingdom-monitor_test.go +++ b/pkg/monitors/pingdom/pingdom-monitor_test.go @@ -2,7 +2,9 @@ package pingdom import ( log "github.com/sirupsen/logrus" + "net/url" "testing" + "time" "github.com/stakater/IngressMonitorController/pkg/config" "github.com/stakater/IngressMonitorController/pkg/models" @@ -21,22 +23,33 @@ func TestAddMonitorWithCorrectValues(t *testing.T) { return } service.Setup(*provider) - m := models.Monitor{Name: "google-test", URL: "https://google1.com"} + urlToTest := "https://google1.com" + parsedUrl, err := url.Parse(urlToTest) + if err != nil { + t.Errorf("Error: %s", err) + } + m := models.Monitor{Name: "google-test", URL: urlToTest} service.Add(m) - + //Creation delay + time.Sleep(5 * time.Second) mRes, err := service.GetByName("google-test") if err != nil { t.Error("Error: " + err.Error()) } - if mRes.Name != m.Name || mRes.URL != m.URL { - t.Error("URL and name should be the same") + // mRes.URL is a domain name, without scheme, so we parsed URL previously + if mRes.Name != m.Name || mRes.URL != parsedUrl.Host { + t.Errorf("URL and name should be the same %+v", mRes) } service.Remove(*mRes) + //Deletion delay + time.Sleep(5 * time.Second) monitor, err := service.GetByName(mRes.Name) - + if err != nil { + t.Error("Error: " + err.Error()) + } if monitor != nil { - t.Error("Monitor should've been deleted ", monitor, err) + t.Error("Monitor should've been deleted ", monitor) } } @@ -53,16 +66,21 @@ func TestUpdateMonitorWithCorrectValues(t *testing.T) { return } service.Setup(*provider) - - m := models.Monitor{Name: "google-test", URL: "https://google.com"} + urlToTest := "https://google.com" + parsedUrl, err := url.Parse(urlToTest) + if err != nil { + t.Errorf("Error: %s", err) + } + m := models.Monitor{Name: "google-test", URL: urlToTest} service.Add(m) - + //Creation delay + time.Sleep(5 * time.Second) mRes, err := service.GetByName("google-test") if err != nil { t.Error("Error: " + err.Error()) } - if mRes.Name != m.Name || mRes.URL != m.URL { + if mRes.Name != m.Name || mRes.URL != parsedUrl.Host { t.Error("URL and name should be the same") } @@ -75,15 +93,17 @@ func TestUpdateMonitorWithCorrectValues(t *testing.T) { if err != nil { t.Error("Error: " + err.Error()) } - if mRes.URL != "https://facebook.com" { + if mRes.URL != "facebook.com" { t.Error("URL and name should be the same") } service.Remove(*mRes) monitor, err := service.GetByName(mRes.Name) - + if err != nil { + t.Error("Error: " + err.Error()) + } if monitor != nil { - t.Error("Monitor should've been deleted ", monitor, err) + t.Error("Monitor should've been deleted ", monitor) } } diff --git a/pkg/monitors/statuscake/statuscake-monitor.go b/pkg/monitors/statuscake/statuscake-monitor.go index baf60600..906622db 100644 --- a/pkg/monitors/statuscake/statuscake-monitor.go +++ b/pkg/monitors/statuscake/statuscake-monitor.go @@ -3,7 +3,6 @@ package statuscake import ( "bytes" "encoding/json" - "errors" "io/ioutil" "net/http" "net/url" @@ -202,8 +201,8 @@ func (service *StatusCakeMonitorService) GetByName(name string) (*models.Monitor return &monitor, nil } } - errorString := "GetByName Request failed for name: " + name - return nil, errors.New(errorString) + log.Printf("There is no StatusCakeMonitorService with name %s found.", name) + return nil, nil } // GetAll function will fetch all monitors diff --git a/pkg/monitors/statuscake/statuscake-monitor_test.go b/pkg/monitors/statuscake/statuscake-monitor_test.go index 6220f2ca..ac131a1e 100644 --- a/pkg/monitors/statuscake/statuscake-monitor_test.go +++ b/pkg/monitors/statuscake/statuscake-monitor_test.go @@ -38,8 +38,11 @@ func TestAddMonitorWithCorrectValues(t *testing.T) { monitor, err := service.GetByName(mRes.Name) + if err != nil { + t.Error("Error: " + err.Error()) + } if monitor != nil { - t.Error("Monitor should've been deleted ", monitor, err) + t.Error("Monitor should've been deleted ", monitor) } } @@ -83,9 +86,11 @@ func TestUpdateMonitorWithCorrectValues(t *testing.T) { service.Remove(*mRes) monitor, err := service.GetByName(mRes.Name) - + if err != nil { + t.Error("Error: " + err.Error()) + } if monitor != nil { - t.Error("Monitor should've been deleted ", monitor, err) + t.Error("Monitor should've been deleted ", monitor) } } diff --git a/pkg/monitors/updown/updown-monitor.go b/pkg/monitors/updown/updown-monitor.go index ce9673cb..a52eb670 100644 --- a/pkg/monitors/updown/updown-monitor.go +++ b/pkg/monitors/updown/updown-monitor.go @@ -2,7 +2,6 @@ package updown import ( - "fmt" "net/http" "net/url" @@ -94,8 +93,8 @@ func (updownService *UpdownMonitorService) GetByName(monitorName string) (*model return &updownMonitor, nil } } - - return nil, fmt.Errorf("Unable to locate %v monitor", monitorName) + log.Printf("There is no UpdownMonitorService with name %s found.", monitorName) + return nil, nil } // Add function method will add a monitor (updown check) diff --git a/pkg/monitors/uptime/uptime-monitor.go b/pkg/monitors/uptime/uptime-monitor.go index df83e4b6..a1b7c194 100644 --- a/pkg/monitors/uptime/uptime-monitor.go +++ b/pkg/monitors/uptime/uptime-monitor.go @@ -2,7 +2,6 @@ package uptime import ( "encoding/json" - "errors" "strconv" "strings" @@ -44,9 +43,8 @@ func (monitor *UpTimeMonitorService) GetByName(name string) (*models.Monitor, er } } - errorString := name + " not found" - log.Println(errorString) - return nil, errors.New(errorString) + log.Printf("There is no UpTimeMonitorService with name %s found.", name) + return nil, nil } func (monitor *UpTimeMonitorService) GetAll() []models.Monitor { diff --git a/pkg/monitors/uptime/uptime-monitor_test.go b/pkg/monitors/uptime/uptime-monitor_test.go index 175a332b..bd5d571f 100644 --- a/pkg/monitors/uptime/uptime-monitor_test.go +++ b/pkg/monitors/uptime/uptime-monitor_test.go @@ -163,9 +163,12 @@ func TestAddMonitorWithIncorrectValues(t *testing.T) { service.Add(m) - _, err := service.GetByName("google-test") + monitor, err := service.GetByName("google-test") - if err == nil { + if err != nil { + t.Error("Error: " + err.Error()) + } + if monitor != nil { t.Error("google-test should not have existed") } }