From fb9a94d450e09791231f21674bb01d376e24fc98 Mon Sep 17 00:00:00 2001 From: Artem Miroshnychenko Date: Mon, 2 Nov 2020 23:26:33 +0200 Subject: [PATCH 01/12] feat: findMonitorByName with error handling, better error handling system and pingdom monitor with method that prevents duplicated check creation --- .../endpointmonitor_controller.go | 17 +++++---- .../appinsights/appinsights-monitor.go | 4 ++- pkg/monitors/gcloud/gcloud-monitor.go | 5 +-- pkg/monitors/gcloud/gcloud-monitor_test.go | 11 ++++-- pkg/monitors/pingdom/pingdom-monitor.go | 36 +++++++++++++++---- pkg/monitors/pingdom/pingdom-monitor_test.go | 12 ++++--- pkg/monitors/statuscake/statuscake-monitor.go | 5 ++- .../statuscake/statuscake-monitor_test.go | 11 ++++-- pkg/monitors/updown/updown-monitor.go | 5 ++- pkg/monitors/updown/updown-monitor_test.go | 12 ++++--- pkg/monitors/uptime/uptime-monitor.go | 6 ++-- pkg/monitors/uptime/uptime-monitor_test.go | 7 ++-- 12 files changed, 86 insertions(+), 45 deletions(-) diff --git a/pkg/controller/endpointmonitor/endpointmonitor_controller.go b/pkg/controller/endpointmonitor/endpointmonitor_controller.go index a405bbbd..81e2ad28 100644 --- a/pkg/controller/endpointmonitor/endpointmonitor_controller.go +++ b/pkg/controller/endpointmonitor/endpointmonitor_controller.go @@ -117,7 +117,11 @@ 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]) @@ -135,12 +139,7 @@ func (r *ReconcileEndpointMonitor) Reconcile(request reconcile.Request) (reconci 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/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/gcloud/gcloud-monitor_test.go b/pkg/monitors/gcloud/gcloud-monitor_test.go index a3ee7fa2..d38e5d31 100644 --- a/pkg/monitors/gcloud/gcloud-monitor_test.go +++ b/pkg/monitors/gcloud/gcloud-monitor_test.go @@ -49,8 +49,11 @@ func TestAddMonitorWithCorrectValues(t *testing.T) { } service.Remove(*mRes) - monitor, err := service.GetByName(mRes.Name) + 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) } @@ -102,8 +105,10 @@ 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/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..d3e38ddd 100644 --- a/pkg/monitors/pingdom/pingdom-monitor_test.go +++ b/pkg/monitors/pingdom/pingdom-monitor_test.go @@ -34,9 +34,11 @@ func TestAddMonitorWithCorrectValues(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) } } @@ -82,8 +84,10 @@ 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/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 5e1637be..b4bb87b3 100644 --- a/pkg/monitors/statuscake/statuscake-monitor_test.go +++ b/pkg/monitors/statuscake/statuscake-monitor_test.go @@ -36,8 +36,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) } } @@ -80,9 +83,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/updown/updown-monitor_test.go b/pkg/monitors/updown/updown-monitor_test.go index c2f1a44c..4a618068 100644 --- a/pkg/monitors/updown/updown-monitor_test.go +++ b/pkg/monitors/updown/updown-monitor_test.go @@ -110,8 +110,10 @@ func TestGetByNameMonitorWhileNoCheckExists(t *testing.T) { UpdownService.Setup(*provider) var nilMonitorModelObj *models.Monitor - monitorObject, _ := UpdownService.GetByName("NoExistingCheck") - + monitorObject, err := UpdownService.GetByName("NoExistingCheck") + if err != nil { + t.Error("Error: " + err.Error()) + } assert.Equal(t, monitorObject, nilMonitorModelObj) } @@ -175,8 +177,10 @@ func TestGetByNameMonitorWhileCheckExists(t *testing.T) { firstElement := 0 var nilMonitorModelObj *models.Monitor monitorSlice := UpdownService.GetAll() - monitorObject, _ := UpdownService.GetByName(monitorSlice[firstElement].ID) - + monitorObject, err := UpdownService.GetByName(monitorSlice[firstElement].ID) + if err != nil { + t.Error("Error: " + err.Error()) + } assert.NotEqual(t, &monitorObject, nilMonitorModelObj) } 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") } } From b1e3a10a35846f879328b414b16af7885e3e63dd Mon Sep 17 00:00:00 2001 From: Artem Miroshnychenko Date: Mon, 2 Nov 2020 23:41:42 +0200 Subject: [PATCH 02/12] fix: tests issues --- .../endpointmonitor_controller_test.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/pkg/controller/endpointmonitor/endpointmonitor_controller_test.go b/pkg/controller/endpointmonitor/endpointmonitor_controller_test.go index 8327c6d8..0f250f64 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++ From 39ec7781a1d5b7670f6ca1eefa8f0de2505c1fff Mon Sep 17 00:00:00 2001 From: Artem Miroshnychenko Date: Mon, 2 Nov 2020 23:42:58 +0200 Subject: [PATCH 03/12] fix: fomatting --- .../endpointmonitor/endpointmonitor_controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/endpointmonitor/endpointmonitor_controller_test.go b/pkg/controller/endpointmonitor/endpointmonitor_controller_test.go index 0f250f64..5e1e616a 100644 --- a/pkg/controller/endpointmonitor/endpointmonitor_controller_test.go +++ b/pkg/controller/endpointmonitor/endpointmonitor_controller_test.go @@ -124,7 +124,7 @@ func TestEndpointMonitorReconcileUpdate(t *testing.T) { monitorCount = 0 for index := 0; index < len(r.monitorServices); index++ { monitor, err := findMonitorByName(r.monitorServices[index], monitorName) - if monitor != nil && err != nil && monitor.URL == testURLFacebook { + if monitor != nil && err != nil && monitor.URL == testURLFacebook { log.Info("Found Updated Monitor for Provider: " + r.monitorServices[index].GetType()) monitorCount++ } From 8f0930eed32d6ada9b82b9f016db7040c2c14882 Mon Sep 17 00:00:00 2001 From: Artem Miroshnychenko Date: Tue, 3 Nov 2020 00:45:32 +0200 Subject: [PATCH 04/12] fix: test condition --- .../endpointmonitor/endpointmonitor_controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/endpointmonitor/endpointmonitor_controller_test.go b/pkg/controller/endpointmonitor/endpointmonitor_controller_test.go index 5e1e616a..d8685dd5 100644 --- a/pkg/controller/endpointmonitor/endpointmonitor_controller_test.go +++ b/pkg/controller/endpointmonitor/endpointmonitor_controller_test.go @@ -124,7 +124,7 @@ func TestEndpointMonitorReconcileUpdate(t *testing.T) { monitorCount = 0 for index := 0; index < len(r.monitorServices); index++ { monitor, err := findMonitorByName(r.monitorServices[index], monitorName) - if monitor != nil && err != nil && monitor.URL == testURLFacebook { + if monitor != nil && err == nil && monitor.URL == testURLFacebook { log.Info("Found Updated Monitor for Provider: " + r.monitorServices[index].GetType()) monitorCount++ } From 7cee7e2aefc867848b1d61398b3193f2724b6fb2 Mon Sep 17 00:00:00 2001 From: Artem Miroshnychenko Date: Tue, 3 Nov 2020 18:28:18 +0200 Subject: [PATCH 05/12] fix: pingdom tests fixed and updownmonitor tests skip if provider doesn't exist --- pkg/monitors/gcloud/gcloud-monitor_test.go | 8 +-- pkg/monitors/pingdom/pingdom-monitor_test.go | 32 ++++++--- pkg/monitors/updown/updown-monitor_test.go | 73 ++++++++++++++++++-- 3 files changed, 96 insertions(+), 17 deletions(-) diff --git a/pkg/monitors/gcloud/gcloud-monitor_test.go b/pkg/monitors/gcloud/gcloud-monitor_test.go index d38e5d31..eed92797 100644 --- a/pkg/monitors/gcloud/gcloud-monitor_test.go +++ b/pkg/monitors/gcloud/gcloud-monitor_test.go @@ -17,7 +17,7 @@ func TestAddMonitorWithCorrectValues(t *testing.T) { provider := util.GetProviderWithName(config, "gcloud") if provider == nil { // TODO: Currently forcing to pass the test as we dont have gcloud account to test - // Fail this case in future when have a valid gcloud account + // Fail this case in future when have a valid updown account log.Error("Failed to find provider") return } @@ -29,7 +29,7 @@ func TestAddMonitorWithCorrectValues(t *testing.T) { if err != nil { // TODO: Currently forcing to pass the test as we dont have gcloud account to test - // Fail this case in future when have a valid gcloud account + // Fail this case in future when have a valid updown account log.Error(err.Error()) return } @@ -67,7 +67,7 @@ func TestUpdateMonitorWithCorrectValues(t *testing.T) { provider := util.GetProviderWithName(config, "gcloud") if provider == nil { // TODO: Currently forcing to pass the test as we dont have gcloud account to test - // Fail this case in future when have a valid gcloud account + // Fail this case in future when have a valid updown account log.Error("Failed to find provider") return } @@ -80,7 +80,7 @@ func TestUpdateMonitorWithCorrectValues(t *testing.T) { if err != nil { // TODO: Currently forcing to pass the test as we dont have gcloud account to test - // Fail this case in future when have a valid gcloud account + // Fail this case in future when have a valid updown account log.Error(err.Error()) return } diff --git a/pkg/monitors/pingdom/pingdom-monitor_test.go b/pkg/monitors/pingdom/pingdom-monitor_test.go index d3e38ddd..78b51ca8 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,16 +23,23 @@ 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") } service.Remove(*mRes) monitor, err := service.GetByName(mRes.Name) @@ -55,16 +64,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") } @@ -77,7 +91,7 @@ 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") } diff --git a/pkg/monitors/updown/updown-monitor_test.go b/pkg/monitors/updown/updown-monitor_test.go index 4a618068..d934085b 100644 --- a/pkg/monitors/updown/updown-monitor_test.go +++ b/pkg/monitors/updown/updown-monitor_test.go @@ -1,14 +1,14 @@ package updown import ( - "testing" - "time" - + log "github.com/sirupsen/logrus" endpointmonitorv1alpha1 "github.com/stakater/IngressMonitorController/pkg/apis/endpointmonitor/v1alpha1" "github.com/stakater/IngressMonitorController/pkg/config" "github.com/stakater/IngressMonitorController/pkg/models" "github.com/stakater/IngressMonitorController/pkg/util" "github.com/stretchr/testify/assert" + "testing" + "time" ) type Block struct { @@ -45,7 +45,6 @@ func TestSetupMonitorWithCorrectValues(t *testing.T) { UpdownService := UpdownMonitorService{} provider := util.GetProviderWithName(config, "Updown") - Block{ Try: func() { UpdownService.Setup(*provider) @@ -74,6 +73,12 @@ func TestRemoveCleanUp(t *testing.T) { config := config.GetControllerConfigTest() UpdownService := UpdownMonitorService{} provider := util.GetProviderWithName(config, "Updown") + if provider == nil { + // TODO: Currently forcing to pass the test as we dont have gcloud account to test + // Fail this case in future when have a valid updown account + log.Error("Provider Updown not exists") + return + } UpdownService.Setup(*provider) monitorSlice := UpdownService.GetAll() @@ -95,6 +100,12 @@ func TestGetAllMonitorWhileNoCheckExists(t *testing.T) { config := config.GetControllerConfigTest() UpdownService := UpdownMonitorService{} provider := util.GetProviderWithName(config, "Updown") + if provider == nil { + // TODO: Currently forcing to pass the test as we dont have gcloud account to test + // Fail this case in future when have a valid updown account + log.Error("Provider Updown not exists") + return + } UpdownService.Setup(*provider) monitorSlice := UpdownService.GetAll() @@ -107,6 +118,12 @@ func TestGetByNameMonitorWhileNoCheckExists(t *testing.T) { config := config.GetControllerConfigTest() UpdownService := UpdownMonitorService{} provider := util.GetProviderWithName(config, "Updown") + if provider == nil { + // TODO: Currently forcing to pass the test as we dont have gcloud account to test + // Fail this case in future when have a valid updown account + log.Error("Provider Updown not exists") + return + } UpdownService.Setup(*provider) var nilMonitorModelObj *models.Monitor @@ -122,6 +139,12 @@ func TestAddMonitorWhileNoCheckExists(t *testing.T) { config := config.GetControllerConfigTest() UpdownService := UpdownMonitorService{} provider := util.GetProviderWithName(config, "Updown") + if provider == nil { + // TODO: Currently forcing to pass the test as we dont have gcloud account to test + // Fail this case in future when have a valid updown account + log.Error("Provider Updown not exists") + return + } UpdownService.Setup(*provider) monitorConfig := &endpointmonitorv1alpha1.UpdownConfig{ @@ -142,6 +165,12 @@ func TestAddMonitorWhileCheckExists(t *testing.T) { config := config.GetControllerConfigTest() UpdownService := UpdownMonitorService{} provider := util.GetProviderWithName(config, "Updown") + if provider == nil { + // TODO: Currently forcing to pass the test as we dont have gcloud account to test + // Fail this case in future when have a valid updown account + log.Error("Provider Updown not exists") + return + } UpdownService.Setup(*provider) newMonitor := models.Monitor{ @@ -155,6 +184,12 @@ func TestGetAllMonitorWhileCheckExists(t *testing.T) { config := config.GetControllerConfigTest() UpdownService := UpdownMonitorService{} provider := util.GetProviderWithName(config, "Updown") + if provider == nil { + // TODO: Currently forcing to pass the test as we dont have gcloud account to test + // Fail this case in future when have a valid updown account + log.Error("Provider Updown not exists") + return + } UpdownService.Setup(*provider) time.Sleep(30 * time.Second) @@ -172,6 +207,12 @@ func TestGetByNameMonitorWhileCheckExists(t *testing.T) { config := config.GetControllerConfigTest() UpdownService := UpdownMonitorService{} provider := util.GetProviderWithName(config, "Updown") + if provider == nil { + // TODO: Currently forcing to pass the test as we dont have gcloud account to test + // Fail this case in future when have a valid updown account + log.Error("Provider Updown not exists") + return + } UpdownService.Setup(*provider) firstElement := 0 @@ -189,6 +230,12 @@ func TestUpdateMonitorWhileCheckExists(t *testing.T) { config := config.GetControllerConfigTest() UpdownService := UpdownMonitorService{} provider := util.GetProviderWithName(config, "Updown") + if provider == nil { + // TODO: Currently forcing to pass the test as we dont have gcloud account to test + // Fail this case in future when have a valid updown account + log.Error("Provider Updown not exists") + return + } UpdownService.Setup(*provider) firstElement := 0 @@ -214,6 +261,12 @@ func TestGetAllMonitorWhileCheckUpdated(t *testing.T) { config := config.GetControllerConfigTest() UpdownService := UpdownMonitorService{} provider := util.GetProviderWithName(config, "Updown") + if provider == nil { + // TODO: Currently forcing to pass the test as we dont have gcloud account to test + // Fail this case in future when have a valid updown account + log.Error("Provider Updown not exists") + return + } UpdownService.Setup(*provider) time.Sleep(10 * time.Second) @@ -228,6 +281,12 @@ func TestRemoveMonitorWhileCheckExists(t *testing.T) { config := config.GetControllerConfigTest() UpdownService := UpdownMonitorService{} provider := util.GetProviderWithName(config, "Updown") + if provider == nil { + // TODO: Currently forcing to pass the test as we dont have gcloud account to test + // Fail this case in future when have a valid updown account + log.Error("Provider Updown not exists") + return + } UpdownService.Setup(*provider) firstElement := 0 @@ -245,6 +304,12 @@ func TestGetAllMonitorWhenCheckAreRemoved(t *testing.T) { config := config.GetControllerConfigTest() UpdownService := UpdownMonitorService{} provider := util.GetProviderWithName(config, "Updown") + if provider == nil { + // TODO: Currently forcing to pass the test as we dont have gcloud account to test + // Fail this case in future when have a valid updown account + log.Error("Provider Updown not exists") + return + } UpdownService.Setup(*provider) time.Sleep(30 * time.Second) From 093e16f104e99717f70ac1999b519c01767c8627 Mon Sep 17 00:00:00 2001 From: Artem Miroshnychenko Date: Tue, 3 Nov 2020 18:34:49 +0200 Subject: [PATCH 06/12] fix: comment --- pkg/monitors/gcloud/gcloud-monitor_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/monitors/gcloud/gcloud-monitor_test.go b/pkg/monitors/gcloud/gcloud-monitor_test.go index eed92797..223bb96c 100644 --- a/pkg/monitors/gcloud/gcloud-monitor_test.go +++ b/pkg/monitors/gcloud/gcloud-monitor_test.go @@ -17,7 +17,7 @@ func TestAddMonitorWithCorrectValues(t *testing.T) { provider := util.GetProviderWithName(config, "gcloud") if provider == nil { // TODO: Currently forcing to pass the test as we dont have gcloud account to test - // Fail this case in future when have a valid updown account + // Fail this case in future when have a valid gcloud account log.Error("Failed to find provider") return } @@ -29,7 +29,7 @@ func TestAddMonitorWithCorrectValues(t *testing.T) { if err != nil { // TODO: Currently forcing to pass the test as we dont have gcloud account to test - // Fail this case in future when have a valid updown account + // Fail this case in future when have a valid gcloud account log.Error(err.Error()) return } @@ -67,7 +67,7 @@ func TestUpdateMonitorWithCorrectValues(t *testing.T) { provider := util.GetProviderWithName(config, "gcloud") if provider == nil { // TODO: Currently forcing to pass the test as we dont have gcloud account to test - // Fail this case in future when have a valid updown account + // Fail this case in future when have a valid gcloud account log.Error("Failed to find provider") return } @@ -80,7 +80,7 @@ func TestUpdateMonitorWithCorrectValues(t *testing.T) { if err != nil { // TODO: Currently forcing to pass the test as we dont have gcloud account to test - // Fail this case in future when have a valid updown account + // Fail this case in future when have a valid gcloud account log.Error(err.Error()) return } From f6cc7e0a03af3c9de9a2aa0b7ffbe3cbb9214565 Mon Sep 17 00:00:00 2001 From: Artem Miroshnychenko Date: Tue, 3 Nov 2020 19:16:54 +0200 Subject: [PATCH 07/12] fix: deletion delay on pingdom --- pkg/monitors/pingdom/pingdom-monitor_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/monitors/pingdom/pingdom-monitor_test.go b/pkg/monitors/pingdom/pingdom-monitor_test.go index 78b51ca8..5b3ee3c7 100644 --- a/pkg/monitors/pingdom/pingdom-monitor_test.go +++ b/pkg/monitors/pingdom/pingdom-monitor_test.go @@ -39,9 +39,11 @@ func TestAddMonitorWithCorrectValues(t *testing.T) { } // 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") + 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()) From 468402884534bbb38eb048f1a98e4c79fda50d53 Mon Sep 17 00:00:00 2001 From: Artem Miroshnychenko Date: Tue, 3 Nov 2020 19:41:44 +0200 Subject: [PATCH 08/12] fix: try to fix linter errors --- .../endpointmonitor/endpointmonitor_controller.go | 8 ++++++++ pkg/monitors/uptimerobot/uptime-monitor_test.go | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/controller/endpointmonitor/endpointmonitor_controller.go b/pkg/controller/endpointmonitor/endpointmonitor_controller.go index 81e2ad28..9ccc9c07 100644 --- a/pkg/controller/endpointmonitor/endpointmonitor_controller.go +++ b/pkg/controller/endpointmonitor/endpointmonitor_controller.go @@ -125,6 +125,10 @@ func (r *ReconcileEndpointMonitor) Reconcile(request reconcile.Request) (reconci if monitor != nil { // Monitor already exists, update if required err = r.handleUpdate(request, instance, *monitor, r.monitorServices[index]) + if err != nil { + log.Error("Error while handling update: %s", err) + return reconcile.Result{RequeueAfter: RequeueTime}, err + } } else { // Monitor doesn't exist, create monitor if delay.Nanoseconds() > 0 { @@ -133,6 +137,10 @@ 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.Error("Error while handling create: %s", err) + return reconcile.Result{RequeueAfter: RequeueTime}, err + } } } diff --git a/pkg/monitors/uptimerobot/uptime-monitor_test.go b/pkg/monitors/uptimerobot/uptime-monitor_test.go index daa73bb0..f0c6c2d2 100644 --- a/pkg/monitors/uptimerobot/uptime-monitor_test.go +++ b/pkg/monitors/uptimerobot/uptime-monitor_test.go @@ -368,7 +368,7 @@ func TestAddMonitorWithAlertContacts(t *testing.T) { service.Setup(*provider) configAlertContacts := &endpointmonitorv1alpha1.UptimeRobotConfig{ - AlertContacts: "2628365_0_0", + AlertContacts: "2937190", } m := models.Monitor{Name: "google-test", URL: "https://google.com", Config: configAlertContacts} @@ -388,7 +388,7 @@ func TestAddMonitorWithAlertContacts(t *testing.T) { providerConfig, _ := mRes.Config.(*endpointmonitorv1alpha1.UptimeRobotConfig) - if "2628365_0_0" != providerConfig.AlertContacts { + if "2937190_0_0" != providerConfig.AlertContacts { t.Error("The alert-contacts is incorrect, expected: 2628365_0_0, but was: " + providerConfig.AlertContacts) } service.Remove(*mRes) From 6584bce9adb09e4c4ed06649e3f132a90617644f Mon Sep 17 00:00:00 2001 From: Artem Miroshnychenko Date: Tue, 3 Nov 2020 19:42:20 +0200 Subject: [PATCH 09/12] Revert "fix: try to fix linter errors" This reverts commit 468402884534bbb38eb048f1a98e4c79fda50d53. --- .../endpointmonitor/endpointmonitor_controller.go | 8 -------- pkg/monitors/uptimerobot/uptime-monitor_test.go | 4 ++-- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/pkg/controller/endpointmonitor/endpointmonitor_controller.go b/pkg/controller/endpointmonitor/endpointmonitor_controller.go index 9ccc9c07..81e2ad28 100644 --- a/pkg/controller/endpointmonitor/endpointmonitor_controller.go +++ b/pkg/controller/endpointmonitor/endpointmonitor_controller.go @@ -125,10 +125,6 @@ func (r *ReconcileEndpointMonitor) Reconcile(request reconcile.Request) (reconci if monitor != nil { // Monitor already exists, update if required err = r.handleUpdate(request, instance, *monitor, r.monitorServices[index]) - if err != nil { - log.Error("Error while handling update: %s", err) - return reconcile.Result{RequeueAfter: RequeueTime}, err - } } else { // Monitor doesn't exist, create monitor if delay.Nanoseconds() > 0 { @@ -137,10 +133,6 @@ 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.Error("Error while handling create: %s", err) - return reconcile.Result{RequeueAfter: RequeueTime}, err - } } } diff --git a/pkg/monitors/uptimerobot/uptime-monitor_test.go b/pkg/monitors/uptimerobot/uptime-monitor_test.go index f0c6c2d2..daa73bb0 100644 --- a/pkg/monitors/uptimerobot/uptime-monitor_test.go +++ b/pkg/monitors/uptimerobot/uptime-monitor_test.go @@ -368,7 +368,7 @@ func TestAddMonitorWithAlertContacts(t *testing.T) { service.Setup(*provider) configAlertContacts := &endpointmonitorv1alpha1.UptimeRobotConfig{ - AlertContacts: "2937190", + AlertContacts: "2628365_0_0", } m := models.Monitor{Name: "google-test", URL: "https://google.com", Config: configAlertContacts} @@ -388,7 +388,7 @@ func TestAddMonitorWithAlertContacts(t *testing.T) { providerConfig, _ := mRes.Config.(*endpointmonitorv1alpha1.UptimeRobotConfig) - if "2937190_0_0" != providerConfig.AlertContacts { + if "2628365_0_0" != providerConfig.AlertContacts { t.Error("The alert-contacts is incorrect, expected: 2628365_0_0, but was: " + providerConfig.AlertContacts) } service.Remove(*mRes) From 2d89e3f89b045e75c95ec287712cc40fa6ef0f09 Mon Sep 17 00:00:00 2001 From: Artem Miroshnychenko Date: Tue, 3 Nov 2020 19:43:30 +0200 Subject: [PATCH 10/12] fix: linter errors fix --- .../endpointmonitor/endpointmonitor_controller.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/controller/endpointmonitor/endpointmonitor_controller.go b/pkg/controller/endpointmonitor/endpointmonitor_controller.go index 81e2ad28..9ccc9c07 100644 --- a/pkg/controller/endpointmonitor/endpointmonitor_controller.go +++ b/pkg/controller/endpointmonitor/endpointmonitor_controller.go @@ -125,6 +125,10 @@ func (r *ReconcileEndpointMonitor) Reconcile(request reconcile.Request) (reconci if monitor != nil { // Monitor already exists, update if required err = r.handleUpdate(request, instance, *monitor, r.monitorServices[index]) + if err != nil { + log.Error("Error while handling update: %s", err) + return reconcile.Result{RequeueAfter: RequeueTime}, err + } } else { // Monitor doesn't exist, create monitor if delay.Nanoseconds() > 0 { @@ -133,6 +137,10 @@ 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.Error("Error while handling create: %s", err) + return reconcile.Result{RequeueAfter: RequeueTime}, err + } } } From d539a7a8ec1f31c1ff5be14f84a16a4bc6c2473f Mon Sep 17 00:00:00 2001 From: Artem Miroshnychenko Date: Tue, 3 Nov 2020 19:47:53 +0200 Subject: [PATCH 11/12] fix: errorf instead error --- pkg/controller/endpointmonitor/endpointmonitor_controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/controller/endpointmonitor/endpointmonitor_controller.go b/pkg/controller/endpointmonitor/endpointmonitor_controller.go index 9ccc9c07..aaed49f3 100644 --- a/pkg/controller/endpointmonitor/endpointmonitor_controller.go +++ b/pkg/controller/endpointmonitor/endpointmonitor_controller.go @@ -126,7 +126,7 @@ func (r *ReconcileEndpointMonitor) Reconcile(request reconcile.Request) (reconci // Monitor already exists, update if required err = r.handleUpdate(request, instance, *monitor, r.monitorServices[index]) if err != nil { - log.Error("Error while handling update: %s", err) + log.Errorf("Error while handling update: %s", err) return reconcile.Result{RequeueAfter: RequeueTime}, err } } else { @@ -138,7 +138,7 @@ func (r *ReconcileEndpointMonitor) Reconcile(request reconcile.Request) (reconci } err = r.handleCreate(request, instance, monitorName, r.monitorServices[index]) if err != nil { - log.Error("Error while handling create: %s", err) + log.Errorf("Error while handling create: %s", err) return reconcile.Result{RequeueAfter: RequeueTime}, err } } From 0f5d73a863130c274a8849c75894b6f0cbc582e5 Mon Sep 17 00:00:00 2001 From: Artem Miroshnychenko Date: Thu, 12 Nov 2020 13:58:28 +0200 Subject: [PATCH 12/12] fix: remove redundant return --- pkg/controller/endpointmonitor/endpointmonitor_controller.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/controller/endpointmonitor/endpointmonitor_controller.go b/pkg/controller/endpointmonitor/endpointmonitor_controller.go index aaed49f3..e210bef2 100644 --- a/pkg/controller/endpointmonitor/endpointmonitor_controller.go +++ b/pkg/controller/endpointmonitor/endpointmonitor_controller.go @@ -127,7 +127,6 @@ func (r *ReconcileEndpointMonitor) Reconcile(request reconcile.Request) (reconci err = r.handleUpdate(request, instance, *monitor, r.monitorServices[index]) if err != nil { log.Errorf("Error while handling update: %s", err) - return reconcile.Result{RequeueAfter: RequeueTime}, err } } else { // Monitor doesn't exist, create monitor @@ -139,7 +138,6 @@ func (r *ReconcileEndpointMonitor) Reconcile(request reconcile.Request) (reconci 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 } } }