Skip to content

Commit

Permalink
Merge pull request #567 from stakater/thunef/grafana-check-update-ten…
Browse files Browse the repository at this point in the history 
…ant-fix

For grafana monitor, fixes bug with invalid tenant ID
  • Loading branch information
@thunef
thunef authored Jan 17, 2024
2 parents e6d10d1 + 2ae2004 commit 5a01726
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 34 deletions.
46 changes: 30 additions & 16 deletions pkg/monitors/grafana/grafana-monitor.go
View file
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
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

0 comments on commit 5a01726

Please sign in to comment.