From f4fd67de56e23c1d0b98c740b1b4497b346b398d Mon Sep 17 00:00:00 2001 From: Fredrik Thune Date: Mon, 15 Jan 2024 17:05:54 +0100 Subject: [PATCH 1/2] For grafana monitor, check update now uses tenant ID from existing check --- pkg/monitors/grafana/grafana-monitor.go | 46 ++++++++++++++++--------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/pkg/monitors/grafana/grafana-monitor.go b/pkg/monitors/grafana/grafana-monitor.go index 533fd9d1..840c6e8a 100644 --- a/pkg/monitors/grafana/grafana-monitor.go +++ b/pkg/monitors/grafana/grafana-monitor.go @@ -14,7 +14,7 @@ import ( "strconv" ) -var log = logf.Log.WithName("gcloud-monitor") +var log = logf.Log.WithName("grafana-monitor") const ( // Default value for monitor configuration @@ -31,6 +31,18 @@ type GrafanaMonitorService struct { frequency int64 } +func getID(monitor models.Monitor) (int64, error) { + var checkId int64 + if len(monitor.ID) > 0 { + idResult, err := strconv.ParseInt(monitor.ID, 10, 64) + if err != nil { + return 0, fmt.Errorf("Error converting ID %v %v", monitor.ID, err) + } + checkId = idResult + } + return checkId, nil +} + func (service *GrafanaMonitorService) Setup(provider config.Provider) { service.ctx = context.Background() service.apiKey = provider.ApiKey @@ -73,7 +85,7 @@ func (service *GrafanaMonitorService) GetAll() (monitors []models.Monitor) { return monitors } -func (service *GrafanaMonitorService) CreateSyntheticCheck(monitor models.Monitor) (*synthetic_monitoring.Check, error) { +func (service *GrafanaMonitorService) CreateSyntheticCheck(monitor models.Monitor, tenantID int64) (*synthetic_monitoring.Check, error) { probes, err := service.smClient.ListProbes(service.ctx) if err != nil { return nil, fmt.Errorf("Error listing probes %v", err) @@ -84,18 +96,9 @@ func (service *GrafanaMonitorService) CreateSyntheticCheck(monitor models.Monito probeIDs[i] = p.Id } - var checkId int64 - if len(monitor.ID) > 0 { - idResult, err := strconv.ParseInt(monitor.ID, 10, 64) - if err != nil { - return nil, fmt.Errorf("Error converting ID %v %v", monitor.ID, err) - } - checkId = idResult - } - var tenantID int64 - grafanaConfig, _ := monitor.Config.(*endpointmonitorv1alpha1.GrafanaConfig) - if grafanaConfig != nil { - tenantID = grafanaConfig.TenantId + checkId, err := getID(monitor) + if err != nil { + return nil, fmt.Errorf("Error converting ID %v %v", monitor.ID, err) } // Creating a new Check object return &synthetic_monitoring.Check{ @@ -118,7 +121,8 @@ func (service *GrafanaMonitorService) CreateSyntheticCheck(monitor models.Monito // Add adds a new monitor to Grafana Synthetic Monitoring service func (service *GrafanaMonitorService) Add(monitor models.Monitor) { - newCheck, err := service.CreateSyntheticCheck(monitor) + var tenantID int64 + newCheck, err := service.CreateSyntheticCheck(monitor, tenantID) if err != nil { log.Error(err, "Failed to create synthetic check") return @@ -135,7 +139,17 @@ func (service *GrafanaMonitorService) Add(monitor models.Monitor) { } func (service *GrafanaMonitorService) Update(monitor models.Monitor) { - newCheck, err := service.CreateSyntheticCheck(monitor) + checkID, err := getID(monitor) + if err != nil { + log.Error(err, "Failed to get ID") + return + } + check, err := service.smClient.GetCheck(service.ctx, checkID) + if err != nil { + log.Error(err, "Failed to get check") + return + } + newCheck, err := service.CreateSyntheticCheck(monitor, check.TenantId) if err != nil { log.Error(err, "Failed to create synthetic check") return From 2ae200431624e761843f44cfc1aed0dc6adee57a Mon Sep 17 00:00:00 2001 From: Fredrik Thune Date: Tue, 16 Jan 2024 15:05:31 +0100 Subject: [PATCH 2/2] Fixes test to allow checks to already exist --- pkg/monitors/grafana/grafana-monitor_test.go | 62 ++++++++++++++------ 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/pkg/monitors/grafana/grafana-monitor_test.go b/pkg/monitors/grafana/grafana-monitor_test.go index ec048d7d..d78cd8f9 100644 --- a/pkg/monitors/grafana/grafana-monitor_test.go +++ b/pkg/monitors/grafana/grafana-monitor_test.go @@ -25,27 +25,35 @@ func TestAddMonitorWithCorrectValues(t *testing.T) { return } service.Setup(*provider) + m := models.Monitor{Name: "google-test", URL: "https://google.com"} + preExistingMonitor, _ := service.GetByName(m.Name) + + if preExistingMonitor != nil { + service.Remove(*preExistingMonitor) + } + + previousResources := len(service.GetAll()) service.Add(m) mRes := service.GetAll() - if len(mRes) == 0 { + if len(mRes) == previousResources { t.Errorf("Found empty response for Monitor. Name: %s and URL: %s", m.Name, m.URL) } - if len(mRes) > 1 { + if len(mRes) > previousResources+1 { t.Errorf("Found too many response for Monitor, %v, after add.", len(mRes)) } - if mRes[0].Name != m.Name || mRes[0].URL != m.URL { - t.Error("URL and name should be the same", mRes[0], m) - } monitor, err := service.GetByName(m.Name) if err != nil { t.Error("Monitor should've been found", monitor, err) } - service.Remove(mRes[0]) + if monitor.Name != m.Name || monitor.URL != m.URL { + t.Error("URL and name should be the same", monitor, m) + } + service.Remove(*monitor) monitor, err = service.GetByName(m.Name) @@ -62,38 +70,56 @@ func TestUpdateMonitorWithCorrectValues(t *testing.T) { return } service.Setup(*provider) + m := models.Monitor{Name: "google-test", URL: "https://google.com"} + preExistingMonitor, _ := service.GetByName(m.Name) + + if preExistingMonitor != nil { + service.Remove(*preExistingMonitor) + } + + previousResources := len(service.GetAll()) service.Add(m) mRes := service.GetAll() - if len(mRes) == 0 { + if len(mRes) == previousResources { t.Errorf("Found empty response for Monitor. Name: %s and URL: %s", m.Name, m.URL) } - if len(mRes) > 1 { + if len(mRes) > previousResources+1 { t.Errorf("Found too many response for Monitor, %v, after add.", len(mRes)) } - m2 := models.Monitor{Name: "stakater-test", URL: "https://stakater.com", ID: mRes[0].ID, Config: mRes[0].Config} + monitor, err := service.GetByName(m.Name) + if err != nil || monitor == nil { + t.Error("Monitor should've been found", monitor, err) + } + if monitor.Name != m.Name || monitor.URL != m.URL { + t.Error("URL and name should be the same", monitor, m) + } + m2 := models.Monitor{Name: "stakater-test", URL: "https://stakater.com", ID: monitor.ID, Config: monitor.Config} service.Update(m2) mRes2 := service.GetAll() - if len(mRes2) == 0 { + if len(mRes2) == previousResources { t.Errorf("Found empty response for Monitor. Name: %s and URL: %s", m2.Name, m2.URL) } - if len(mRes2) > 1 { + if len(mRes2) > previousResources+1 { t.Errorf("Found too many response for Monitor, %v, after update.", len(mRes2)) } - if mRes2[0].Name != m2.Name || mRes2[0].URL != m2.URL { - t.Error("URL and name should be the same", mRes2[0], m2) - } - - monitor, err := service.GetByName(m.Name) - if monitor != nil { + monitor1, _ := service.GetByName(m.Name) + if monitor1 != nil { t.Error("Monitor should not exist since it was updated", monitor, err) } - service.Remove(mRes2[0]) + monitor2, err := service.GetByName(m2.Name) + if err != nil { + t.Error("Monitor should've been found", monitor, err) + } + if monitor2.Name != m2.Name || monitor2.URL != m2.URL { + t.Error("URL and name should be the same", monitor2, m2) + } + service.Remove(*monitor2) monitor, err = service.GetByName(m2.Name)