Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

For grafana monitor, fixes bug with invalid tenant ID #567

Merged
merged 2 commits into from
Jan 17, 2024
+74 −34
Merged

For grafana monitor, fixes bug with invalid tenant ID #567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4fd67d
For grafana monitor, check update now uses tenant ID from existing check
thunef Jan 15, 2024
2ae2004
Fixes test to allow checks to already exist
thunef Jan 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 30 additions & 16 deletions pkg/monitors/grafana/grafana-monitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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{
Expand All @@ -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
Expand All @@ -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
Expand Down
62 changes: 44 additions & 18 deletions pkg/monitors/grafana/grafana-monitor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand Down
Loading