From 002c9985d3862e10a41a37aa4675038892f81f44 Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Wed, 17 May 2017 00:18:43 -0400 Subject: [PATCH 1/9] Enable CRUD of metrics --- .../librato/resource_librato_metric.go | 411 ++++++++++++++++++ .../librato/resource_librato_metric_test.go | 122 ++++++ 2 files changed, 533 insertions(+) create mode 100644 builtin/providers/librato/resource_librato_metric.go create mode 100644 builtin/providers/librato/resource_librato_metric_test.go diff --git a/builtin/providers/librato/resource_librato_metric.go b/builtin/providers/librato/resource_librato_metric.go new file mode 100644 index 000000000000..ad447c124c3f --- /dev/null +++ b/builtin/providers/librato/resource_librato_metric.go @@ -0,0 +1,411 @@ +package librato + +import ( + "fmt" + "log" + "reflect" + "time" + + "github.com/hashicorp/terraform/helper/resource" + "github.com/hashicorp/terraform/helper/schema" + "github.com/henrikhodne/go-librato/librato" +) + +const ( + metricTypeGauge = "gauge" + metricTypeCounter = "counter" + metricTypeComposite = "composite" +) + +var metricTypes = []string{metricTypeGauge, metricTypeCounter, metricTypeComposite} + +func resourceLibratoMetric() *schema.Resource { + return &schema.Resource{ + Create: resourceLibratoMetricCreate, + Read: resourceLibratoMetricRead, + Update: resourceLibratoMetricUpdate, + Delete: resourceLibratoMetricDelete, + + Schema: map[string]*schema.Schema{ + "name": &schema.Schema{ + Type: schema.TypeString, + Required: true, + ForceNew: false, + }, + "type": &schema.Schema{ + Type: schema.TypeString, + Required: true, + }, + "display_name": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + "description": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + "period": &schema.Schema{ + Type: schema.TypeInt, + Optional: true, + }, + "composite": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + "attributes": &schema.Schema{ + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "color": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + "display_max": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + "display_min": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + "display_units_long": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + "display_units_short": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + "display_stacked": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + }, + "created_by_ua": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + "gap_detection": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + }, + "aggregate": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + }, + }, + }, + }, + }, + } +} + +func resourceLibratoMetricCreate(d *schema.ResourceData, meta interface{}) error { + client := meta.(*librato.Client) + + metric := new(librato.Metric) + if a, ok := d.GetOk("name"); ok { + metric.Name = librato.String(a.(string)) + } + if a, ok := d.GetOk("display_name"); ok { + metric.DisplayName = librato.String(a.(string)) + } + if a, ok := d.GetOk("type"); ok { + metric.Type = librato.String(a.(string)) + } + if a, ok := d.GetOk("description"); ok { + metric.Description = librato.String(a.(string)) + } + if a, ok := d.GetOk("period"); ok { + metric.Period = librato.Uint(a.(uint)) + } + + if a, ok := d.GetOk("attributes"); ok { + attributeData := a.([]interface{}) + if len(attributeData) > 1 { + return fmt.Errorf("Only one set of attributes per alert is supported") + } + + if len(attributeData) == 1 && attributeData[0] == nil { + return fmt.Errorf("No attributes found in attributes block") + } + + attributeDataMap := attributeData[0].(map[string]interface{}) + attributes := new(librato.MetricAttributes) + + if v, ok := attributeDataMap["color"].(string); ok && v != "" { + attributes.Color = librato.String(v) + } + if v, ok := attributeDataMap["display_max"].(string); ok && v != "" { + attributes.DisplayMax = librato.String(v) + } + if v, ok := attributeDataMap["display_min"].(string); ok && v != "" { + attributes.DisplayMin = librato.String(v) + } + if v, ok := attributeDataMap["display_units_long"].(string); ok && v != "" { + attributes.DisplayUnitsLong = *librato.String(v) + } + if v, ok := attributeDataMap["display_units_short"].(string); ok && v != "" { + attributes.DisplayUnitsShort = *librato.String(v) + } + if v, ok := attributeDataMap["created_by_ua"].(string); ok && v != "" { + attributes.CreatedByUA = *librato.String(v) + } + if v, ok := attributeDataMap["display_stacked"].(bool); ok { + attributes.DisplayStacked = *librato.Bool(v) + } + if v, ok := attributeDataMap["gap_detection"].(bool); ok { + attributes.GapDetection = *librato.Bool(v) + } + if v, ok := attributeDataMap["aggregate"].(bool); ok { + attributes.Aggregate = *librato.Bool(v) + } + + metric.Attributes = attributes + } + + _, err := client.Metrics.Edit(metric) + if err != nil { + return fmt.Errorf("Error creating Librato service: %s", err) + } + + resource.Retry(1*time.Minute, func() *resource.RetryError { + _, _, err := client.Metrics.Get(*metric.Name) + if err != nil { + if errResp, ok := err.(*librato.ErrorResponse); ok && errResp.Response.StatusCode == 404 { + return resource.RetryableError(err) + } + return resource.NonRetryableError(err) + } + return nil + }) + + return resourceLibratoMetricReadResult(d, metric) +} + +func resourceLibratoMetricReadResult(d *schema.ResourceData, metric *librato.Metric) error { + d.SetId(*metric.Name) + d.Set("id", *metric.Name) + d.Set("name", *metric.Name) + d.Set("type", *metric.Type) + + if metric.Description != nil { + d.Set("description", *metric.Description) + } + + if metric.DisplayName != nil { + d.Set("display_name", *metric.DisplayName) + } + + if metric.Period != nil { + d.Set("period", *metric.Period) + } + + if metric.Composite != nil { + d.Set("composite", *metric.Composite) + } + + attributes := resourceLibratoMetricAttributesGather(d, metric.Attributes) + d.Set("attributes", attributes) + + return nil +} + +// Flattens an attributes hash into something that flatmap.Flatten() can handle +func resourceLibratoMetricAttributesGather(d *schema.ResourceData, attributes *librato.MetricAttributes) []map[string]interface{} { + result := make([]map[string]interface{}, 0, 1) + + if attributes != nil { + retAttributes := make(map[string]interface{}) + if attributes.Color != nil { + retAttributes["color"] = *attributes.Color + } + if attributes.DisplayMax != nil { + retAttributes["display_max"] = attributes.DisplayMax + } + if attributes.DisplayMin != nil { + retAttributes["display_min"] = attributes.DisplayMin + } + if attributes.DisplayUnitsLong != "" { + retAttributes["display_units_long"] = attributes.DisplayUnitsLong + } + if attributes.DisplayUnitsShort != "" { + retAttributes["display_units_short"] = attributes.DisplayUnitsShort + } + if attributes.CreatedByUA != "" { + retAttributes["created_by_ua"] = attributes.CreatedByUA + } + retAttributes["display_stacked"] = attributes.DisplayStacked || false + retAttributes["gap_detection"] = attributes.GapDetection || false + retAttributes["aggregate"] = attributes.Aggregate || false + + result = append(result, retAttributes) + } + + return result +} + +func resourceLibratoMetricRead(d *schema.ResourceData, meta interface{}) error { + client := meta.(*librato.Client) + + id := d.Id() + + log.Printf("[INFO] Reading Librato Metric: %s", id) + metric, _, err := client.Metrics.Get(id) + if err != nil { + if errResp, ok := err.(*librato.ErrorResponse); ok && errResp.Response.StatusCode == 404 { + d.SetId("") + return nil + } + return fmt.Errorf("Error reading Librato Metric %s: %s", id, err) + } + log.Printf("[INFO] Received Librato Metric: %+v", *metric) + + return resourceLibratoMetricReadResult(d, metric) +} + +func resourceLibratoMetricUpdate(d *schema.ResourceData, meta interface{}) error { + client := meta.(*librato.Client) + + metricID := d.Id() + + // Just to have whole object for comparison before/after update + fullMetric, _, err := client.Metrics.Get(metricID) + if err != nil { + return err + } + + metric := new(librato.Metric) + metric.Name = librato.String(d.Get("name").(string)) + + if d.HasChange("type") { + metric.Type = librato.String(d.Get("type").(string)) + fullMetric.Type = metric.Type + } + + if d.HasChange("description") { + metric.Description = librato.String(d.Get("description").(string)) + fullMetric.Description = metric.Description + } + + if d.HasChange("display_name") { + metric.DisplayName = librato.String(d.Get("display_name").(string)) + fullMetric.DisplayName = metric.DisplayName + } + + if d.HasChange("period") { + metric.Period = librato.Uint(d.Get("period").(uint)) + fullMetric.Period = metric.Period + } + + if d.HasChange("composite") { + metric.Composite = librato.String(d.Get("composite").(string)) + fullMetric.Composite = metric.Composite + } + + if d.HasChange("attributes") { + attributeData := d.Get("attributes").([]interface{}) + if len(attributeData) > 1 { + return fmt.Errorf("Only one set of attributes per alert is supported") + } + + if len(attributeData) == 1 && attributeData[0] == nil { + return fmt.Errorf("No attributes found in attributes block") + } + + attributeDataMap := attributeData[0].(map[string]interface{}) + attributes := new(librato.MetricAttributes) + + if v, ok := attributeDataMap["color"].(string); ok && v != "" { + attributes.Color = librato.String(v) + } + if v, ok := attributeDataMap["display_max"].(string); ok && v != "" { + attributes.DisplayMax = librato.String(v) + } + if v, ok := attributeDataMap["display_min"].(string); ok && v != "" { + attributes.DisplayMin = librato.String(v) + } + if v, ok := attributeDataMap["display_units_long"].(string); ok && v != "" { + attributes.DisplayUnitsLong = *librato.String(v) + } + if v, ok := attributeDataMap["display_units_short"].(string); ok && v != "" { + attributes.DisplayUnitsShort = *librato.String(v) + } + if v, ok := attributeDataMap["created_by_ua"].(string); ok && v != "" { + attributes.CreatedByUA = *librato.String(v) + } + if v, ok := attributeDataMap["display_stacked"].(bool); ok { + attributes.DisplayStacked = *librato.Bool(v) + } + if v, ok := attributeDataMap["gap_detection"].(bool); ok { + attributes.GapDetection = *librato.Bool(v) + } + if v, ok := attributeDataMap["aggregate"].(bool); ok { + attributes.Aggregate = *librato.Bool(v) + } + + metric.Attributes = attributes + fullMetric.Attributes = attributes + } + + log.Printf("[INFO] Updating Librato metric: %v", metric) + _, err = client.Metrics.Edit(metric) + if err != nil { + return fmt.Errorf("Error updating Librato metric: %s", err) + } + + log.Printf("[INFO] Updated Librato metric %s", metricID) + + // Wait for propagation since Librato updates are eventually consistent + wait := resource.StateChangeConf{ + Pending: []string{fmt.Sprintf("%t", false)}, + Target: []string{fmt.Sprintf("%t", true)}, + Timeout: 5 * time.Minute, + MinTimeout: 2 * time.Second, + ContinuousTargetOccurence: 5, + Refresh: func() (interface{}, string, error) { + log.Printf("[DEBUG] Checking if Librato Metric %s was updated yet", metricID) + changedMetric, _, getErr := client.Metrics.Get(metricID) + if getErr != nil { + return changedMetric, "", getErr + } + isEqual := reflect.DeepEqual(*fullMetric, *changedMetric) + log.Printf("[DEBUG] Updated Librato Metric %s match: %t", metricID, isEqual) + return changedMetric, fmt.Sprintf("%t", isEqual), nil + }, + } + + _, err = wait.WaitForState() + if err != nil { + return fmt.Errorf("Failed updating Librato Metric %s: %s", metricID, err) + } + + return resourceLibratoMetricRead(d, meta) +} + +func resourceLibratoMetricDelete(d *schema.ResourceData, meta interface{}) error { + client := meta.(*librato.Client) + + id := d.Id() + + log.Printf("[INFO] Deleting Metric: %s", id) + _, err := client.Metrics.Delete(id) + if err != nil { + return fmt.Errorf("Error deleting Metric: %s", err) + } + + resource.Retry(1*time.Minute, func() *resource.RetryError { + _, _, err := client.Metrics.Get(id) + if err != nil { + if errResp, ok := err.(*librato.ErrorResponse); ok && errResp.Response.StatusCode == 404 { + return nil + } + log.Printf("[DEBUG] unknown error attempting to Get metric: %s", err) + return resource.NonRetryableError(err) + } + return resource.RetryableError(fmt.Errorf("metric still exists")) + }) + + d.SetId("") + return nil +} diff --git a/builtin/providers/librato/resource_librato_metric_test.go b/builtin/providers/librato/resource_librato_metric_test.go new file mode 100644 index 000000000000..a5227153f4ce --- /dev/null +++ b/builtin/providers/librato/resource_librato_metric_test.go @@ -0,0 +1,122 @@ +package librato + +import ( + "fmt" + "strings" + "testing" + + "github.com/hashicorp/terraform/helper/acctest" + "github.com/hashicorp/terraform/helper/resource" + "github.com/hashicorp/terraform/terraform" + "github.com/henrikhodne/go-librato/librato" +) + +func TestAccLibratoMetric_Basic(t *testing.T) { + var metric librato.Metric + name := fmt.Sprintf("tftest-metric-%s", acctest.RandString(10)) + typ := "counter" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckLibratoMetricDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCheckLibratoMetricConfig(name, typ), + Check: resource.ComposeTestCheckFunc( + testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), + testAccCheckLibratoMetricName(&metric, name), + testAccCheckLibratoMetricType(&metric, []string{"gauge", "counter", "composite"}), + resource.TestCheckResourceAttr( + "librato_metric.foobar", "name", name), + ), + }, + }, + }) +} + +func testAccCheckLibratoMetricDestroy(s *terraform.State) error { + client := testAccProvider.Meta().(*librato.Client) + + for _, rs := range s.RootModule().Resources { + if rs.Type != "librato_metric" { + continue + } + + _, _, err := client.Metrics.Get(rs.Primary.ID) + if err == nil { + return fmt.Errorf("Metric still exists") + } + } + + return nil +} + +func testAccCheckLibratoMetricConfig(name, typ string) string { + return strings.TrimSpace(fmt.Sprintf(` + resource "librato_metric" "foobar" { + name = "%s" + type = "%s" + description = "A test composite metric" + composite = "s(\"librato.cpu.percent.user\", {\"environment\" : \"prod\", \"service\": \"api\"})" + attributes { + display_stacked = true, + created_by_ua = "go-librato/0.1" + } + }`, name, typ)) +} + +func testAccCheckLibratoMetricName(metric *librato.Metric, name string) resource.TestCheckFunc { + return func(s *terraform.State) error { + if metric.Name == nil || *metric.Name != name { + return fmt.Errorf("Bad name: %s", *metric.Name) + } + + return nil + } +} + +func testAccCheckLibratoMetricType(metric *librato.Metric, validTypes []string) resource.TestCheckFunc { + return func(s *terraform.State) error { + m := make(map[string]bool) + for _, v := range validTypes { + m[v] = true + } + + if !m[*metric.Type] { + return fmt.Errorf("Bad metric type: %s", *metric.Type) + } + + return nil + } +} + +func testAccCheckLibratoMetricExists(n string, metric *librato.Metric) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("No Metric ID is set") + } + + client := testAccProvider.Meta().(*librato.Client) + + foundMetric, _, err := client.Metrics.Get(rs.Primary.ID) + + if err != nil { + return err + } + + if foundMetric.Name == nil || *foundMetric.Name != rs.Primary.ID { + return fmt.Errorf("Metric not found") + } + + *metric = *foundMetric + + return nil + } +} From 52040c69a4637d06c443500ab09d5b645d97cc34 Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Wed, 17 May 2017 11:33:44 -0400 Subject: [PATCH 2/9] Fixup bad rebase --- builtin/providers/librato/provider.go | 1 + .../librato/resource_librato_metric.go | 12 +++++++-- .../henrikhodne/go-librato/librato/metrics.go | 25 ++++++++++++++----- vendor/vendor.json | 6 ++--- 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/builtin/providers/librato/provider.go b/builtin/providers/librato/provider.go index 7b1a64016cf7..203e06facfe0 100644 --- a/builtin/providers/librato/provider.go +++ b/builtin/providers/librato/provider.go @@ -28,6 +28,7 @@ func Provider() terraform.ResourceProvider { ResourcesMap: map[string]*schema.Resource{ "librato_space": resourceLibratoSpace(), "librato_space_chart": resourceLibratoSpaceChart(), + "librato_metric": resourceLibratoMetric(), "librato_alert": resourceLibratoAlert(), "librato_service": resourceLibratoService(), }, diff --git a/builtin/providers/librato/resource_librato_metric.go b/builtin/providers/librato/resource_librato_metric.go index ad447c124c3f..f1d7aa7f8fa4 100644 --- a/builtin/providers/librato/resource_librato_metric.go +++ b/builtin/providers/librato/resource_librato_metric.go @@ -1,6 +1,7 @@ package librato import ( + "encoding/json" "fmt" "log" "reflect" @@ -166,6 +167,7 @@ func resourceLibratoMetricCreate(d *schema.ResourceData, meta interface{}) error _, err := client.Metrics.Edit(metric) if err != nil { + log.Printf("[INFO] ERROR creating Metric: %s", err) return fmt.Errorf("Error creating Librato service: %s", err) } @@ -259,7 +261,8 @@ func resourceLibratoMetricRead(d *schema.ResourceData, meta interface{}) error { } return fmt.Errorf("Error reading Librato Metric %s: %s", id, err) } - log.Printf("[INFO] Received Librato Metric: %+v", *metric) + + log.Printf("[INFO] Read Librato Metric: %s", structToString(metric)) return resourceLibratoMetricReadResult(d, metric) } @@ -400,7 +403,7 @@ func resourceLibratoMetricDelete(d *schema.ResourceData, meta interface{}) error if errResp, ok := err.(*librato.ErrorResponse); ok && errResp.Response.StatusCode == 404 { return nil } - log.Printf("[DEBUG] unknown error attempting to Get metric: %s", err) + log.Printf("[INFO] non-retryable error attempting to Get metric: %s", err) return resource.NonRetryableError(err) } return resource.RetryableError(fmt.Errorf("metric still exists")) @@ -409,3 +412,8 @@ func resourceLibratoMetricDelete(d *schema.ResourceData, meta interface{}) error d.SetId("") return nil } + +func structToString(i interface{}) string { + s, _ := json.Marshal(i) + return string(s) +} diff --git a/vendor/github.com/henrikhodne/go-librato/librato/metrics.go b/vendor/github.com/henrikhodne/go-librato/librato/metrics.go index 974204a29b43..3754547c470d 100644 --- a/vendor/github.com/henrikhodne/go-librato/librato/metrics.go +++ b/vendor/github.com/henrikhodne/go-librato/librato/metrics.go @@ -14,28 +14,37 @@ type MetricsService struct { // Metric represents a Librato Metric. type Metric struct { Name *string `json:"name"` + Description *string `json:"description,omitempty"` + Type *string `json:"type"` Period *uint `json:"period,omitempty"` DisplayName *string `json:"display_name,omitempty"` + Composite *string `json:"composite,omitempty"` Attributes *MetricAttributes `json:"attributes,omitempty"` } +// MetricAttributes are named attributes as key:value pairs. type MetricAttributes struct { Color *string `json:"color"` // These are interface{} because sometimes the Librato API // returns strings, and sometimes it returns integers DisplayMax interface{} `json:"display_max"` DisplayMin interface{} `json:"display_min"` + DisplayUnitsLong string `json:"display_units_long"` DisplayUnitsShort string `json:"display_units_short"` DisplayStacked bool `json:"display_stacked"` DisplayTransform string `json:"display_transform"` + CreatedByUA string `json:"created_by_ua,omitempty"` + GapDetection bool `json:"gap_detection,omitempty"` + Aggregate bool `json:"aggregate,omitempty"` } +// ListMetricsOptions are used to coordinate paging of metrics. type ListMetricsOptions struct { *PaginationMeta Name string `url:"name,omitempty"` } -// Advance to the specified page in result set, while retaining +// AdvancePage advances to the specified page in result set, while retaining // the filtering options. func (l *ListMetricsOptions) AdvancePage(next *PaginationMeta) ListMetricsOptions { return ListMetricsOptions{ @@ -44,6 +53,7 @@ func (l *ListMetricsOptions) AdvancePage(next *PaginationMeta) ListMetricsOption } } +// ListMetricsResponse represents the response of a List call against the metrics service. type ListMetricsResponse struct { ThisPage *PaginationResponseMeta NextPage *PaginationMeta @@ -51,7 +61,7 @@ type ListMetricsResponse struct { // List metrics using the provided options. // -// Librato API docs: https://www.librato.com/docs/api/#retrieve-metrics +// Librato API docs: https://www.librato.com/docs/api/#list-a-subset-of-metrics func (m *MetricsService) List(opts *ListMetricsOptions) ([]Metric, *ListMetricsResponse, error) { u, err := urlWithOptions("metrics", opts) if err != nil { @@ -83,7 +93,7 @@ func (m *MetricsService) List(opts *ListMetricsOptions) ([]Metric, *ListMetricsR // Get a metric by name // -// Librato API docs: https://www.librato.com/docs/api/#retrieve-metric-by-name +// Librato API docs: https://www.librato.com/docs/api/#retrieve-a-metric-by-name func (m *MetricsService) Get(name string) (*Metric, *http.Response, error) { u := fmt.Sprintf("metrics/%s", name) req, err := m.client.NewRequest("GET", u, nil) @@ -100,6 +110,7 @@ func (m *MetricsService) Get(name string) (*Metric, *http.Response, error) { return metric, resp, err } +// MeasurementSubmission represents the payload to submit/create a metric. type MeasurementSubmission struct { MeasureTime *uint `json:"measure_time,omitempty"` Source *string `json:"source,omitempty"` @@ -107,6 +118,7 @@ type MeasurementSubmission struct { Counters []*Measurement `json:"counters,omitempty"` } +// Measurement represents a Librato Measurement. type Measurement struct { Name string `json:"name"` Value *float64 `json:"value,omitempty"` @@ -114,6 +126,7 @@ type Measurement struct { Source *string `json:"source,omitempty"` } +// GaugeMeasurement represents a Librato measurement gauge. type GaugeMeasurement struct { *Measurement Count *uint `json:"count,omitempty"` @@ -125,7 +138,7 @@ type GaugeMeasurement struct { // Submit metrics // -// Librato API docs: https://www.librato.com/docs/api/#submit-metrics +// Librato API docs: https://www.librato.com/docs/api/#create-a-measurement func (m *MetricsService) Submit(measurements *MeasurementSubmission) (*http.Response, error) { req, err := m.client.NewRequest("POST", "/metrics", measurements) if err != nil { @@ -137,7 +150,7 @@ func (m *MetricsService) Submit(measurements *MeasurementSubmission) (*http.Resp // Edit a metric. // -// Librato API docs: https://www.librato.com/docs/api/#update-metric-by-name +// Librato API docs: https://www.librato.com/docs/api/#update-a-metric-by-name func (m *MetricsService) Edit(metric *Metric) (*http.Response, error) { u := fmt.Sprintf("metrics/%s", *metric.Name) @@ -151,7 +164,7 @@ func (m *MetricsService) Edit(metric *Metric) (*http.Response, error) { // Delete a metric. // -// Librato API docs: https://www.librato.com/docs/api/#delete-metric-by-name +// Librato API docs: https://www.librato.com/docs/api/#delete-a-metric-by-name func (m *MetricsService) Delete(name string) (*http.Response, error) { u := fmt.Sprintf("metrics/%s", name) req, err := m.client.NewRequest("DELETE", u, nil) diff --git a/vendor/vendor.json b/vendor/vendor.json index e628c7b42a4c..e1f7296f34f2 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -2248,10 +2248,10 @@ "revisionTime": "2016-07-20T23:31:40Z" }, { - "checksumSHA1": "jq2E42bB0kwKaerHXwJslUea4eM=", + "checksumSHA1": "HtxHX0u0oN+aiTN1Pd67Y4ilMdI=", "path": "github.com/henrikhodne/go-librato/librato", - "revision": "6e9aa4b1a8a8b735ad14b4f1c9542ef183e82dc2", - "revisionTime": "2016-08-11T07:26:26Z" + "revision": "1bca649ee479cdfcf2e19f30ecb74b6f23345e5a", + "revisionTime": "2017-05-13T14:06:44Z" }, { "checksumSHA1": "K6exl2ouL7d8cR2i378EzZOdRVI=", From 45c93cfb0724d049fe96f0b8ae557f5906bc1445 Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Thu, 18 May 2017 09:37:16 -0400 Subject: [PATCH 3/9] Separate metric test by type --- builtin/providers/librato/common.go | 40 +++++ .../providers/librato/common_helpers_test.go | 12 ++ .../librato/resource_librato_metric.go | 163 ++++++++++-------- .../librato/resource_librato_metric_test.go | 142 +++++++++++++-- .../librato/resource_librato_service.go | 56 +----- 5 files changed, 279 insertions(+), 134 deletions(-) create mode 100644 builtin/providers/librato/common.go create mode 100644 builtin/providers/librato/common_helpers_test.go diff --git a/builtin/providers/librato/common.go b/builtin/providers/librato/common.go new file mode 100644 index 000000000000..79056fab7ea2 --- /dev/null +++ b/builtin/providers/librato/common.go @@ -0,0 +1,40 @@ +package librato + +import ( + "encoding/json" + "fmt" +) + +// Encodes a hash into a JSON string +func attributesFlatten(attrs map[string]string) (string, error) { + byteArray, err := json.Marshal(attrs) + if err != nil { + return "", fmt.Errorf("Error encoding to JSON: %s", err) + } + + return string(byteArray), nil +} + +// Takes JSON in a string & decodes into a hash +func attributesExpand(raw string) (map[string]string, error) { + attrs := make(map[string]string) + err := json.Unmarshal([]byte(raw), &attrs) + if err != nil { + return nil, fmt.Errorf("Error decoding JSON: %s", err) + } + + return attrs, err +} + +func normalizeJSON(jsonString interface{}) string { + if jsonString == nil || jsonString == "" { + return "" + } + var j interface{} + err := json.Unmarshal([]byte(jsonString.(string)), &j) + if err != nil { + return fmt.Sprintf("Error parsing JSON: %s", err) + } + b, _ := json.Marshal(j) + return string(b[:]) +} diff --git a/builtin/providers/librato/common_helpers_test.go b/builtin/providers/librato/common_helpers_test.go new file mode 100644 index 000000000000..c2744589f62b --- /dev/null +++ b/builtin/providers/librato/common_helpers_test.go @@ -0,0 +1,12 @@ +package librato + +import ( + "testing" + "time" +) + +func sleep(t *testing.T, amount time.Duration) func() { + return func() { + time.Sleep(amount * time.Second) + } +} diff --git a/builtin/providers/librato/resource_librato_metric.go b/builtin/providers/librato/resource_librato_metric.go index f1d7aa7f8fa4..45410543127b 100644 --- a/builtin/providers/librato/resource_librato_metric.go +++ b/builtin/providers/librato/resource_librato_metric.go @@ -102,6 +102,8 @@ func resourceLibratoMetric() *schema.Resource { } func resourceLibratoMetricCreate(d *schema.ResourceData, meta interface{}) error { + log.Println("[INFO] Creating Librato metric") + client := meta.(*librato.Client) metric := new(librato.Metric) @@ -120,11 +122,15 @@ func resourceLibratoMetricCreate(d *schema.ResourceData, meta interface{}) error if a, ok := d.GetOk("period"); ok { metric.Period = librato.Uint(a.(uint)) } + if a, ok := d.GetOk("composite"); ok { + metric.Composite = librato.String(a.(string)) + } if a, ok := d.GetOk("attributes"); ok { + attributeData := a.([]interface{}) if len(attributeData) > 1 { - return fmt.Errorf("Only one set of attributes per alert is supported") + return fmt.Errorf("Only one set of attributes per metric is supported") } if len(attributeData) == 1 && attributeData[0] == nil { @@ -168,7 +174,7 @@ func resourceLibratoMetricCreate(d *schema.ResourceData, meta interface{}) error _, err := client.Metrics.Edit(metric) if err != nil { log.Printf("[INFO] ERROR creating Metric: %s", err) - return fmt.Errorf("Error creating Librato service: %s", err) + return fmt.Errorf("Error creating Librato metric: %s", err) } resource.Retry(1*time.Minute, func() *resource.RetryError { @@ -182,72 +188,12 @@ func resourceLibratoMetricCreate(d *schema.ResourceData, meta interface{}) error return nil }) - return resourceLibratoMetricReadResult(d, metric) -} - -func resourceLibratoMetricReadResult(d *schema.ResourceData, metric *librato.Metric) error { - d.SetId(*metric.Name) - d.Set("id", *metric.Name) - d.Set("name", *metric.Name) - d.Set("type", *metric.Type) - - if metric.Description != nil { - d.Set("description", *metric.Description) - } - - if metric.DisplayName != nil { - d.Set("display_name", *metric.DisplayName) - } - - if metric.Period != nil { - d.Set("period", *metric.Period) - } - - if metric.Composite != nil { - d.Set("composite", *metric.Composite) - } - - attributes := resourceLibratoMetricAttributesGather(d, metric.Attributes) - d.Set("attributes", attributes) - - return nil -} - -// Flattens an attributes hash into something that flatmap.Flatten() can handle -func resourceLibratoMetricAttributesGather(d *schema.ResourceData, attributes *librato.MetricAttributes) []map[string]interface{} { - result := make([]map[string]interface{}, 0, 1) - - if attributes != nil { - retAttributes := make(map[string]interface{}) - if attributes.Color != nil { - retAttributes["color"] = *attributes.Color - } - if attributes.DisplayMax != nil { - retAttributes["display_max"] = attributes.DisplayMax - } - if attributes.DisplayMin != nil { - retAttributes["display_min"] = attributes.DisplayMin - } - if attributes.DisplayUnitsLong != "" { - retAttributes["display_units_long"] = attributes.DisplayUnitsLong - } - if attributes.DisplayUnitsShort != "" { - retAttributes["display_units_short"] = attributes.DisplayUnitsShort - } - if attributes.CreatedByUA != "" { - retAttributes["created_by_ua"] = attributes.CreatedByUA - } - retAttributes["display_stacked"] = attributes.DisplayStacked || false - retAttributes["gap_detection"] = attributes.GapDetection || false - retAttributes["aggregate"] = attributes.Aggregate || false - - result = append(result, retAttributes) - } - - return result + return metricReadResult(d, metric) } func resourceLibratoMetricRead(d *schema.ResourceData, meta interface{}) error { + log.Println("[INFO] Reading Librato metric") + client := meta.(*librato.Client) id := d.Id() @@ -264,10 +210,12 @@ func resourceLibratoMetricRead(d *schema.ResourceData, meta interface{}) error { log.Printf("[INFO] Read Librato Metric: %s", structToString(metric)) - return resourceLibratoMetricReadResult(d, metric) + return metricReadResult(d, metric) } func resourceLibratoMetricUpdate(d *schema.ResourceData, meta interface{}) error { + log.Println("[INFO] Updating Librato metric") + client := meta.(*librato.Client) metricID := d.Id() @@ -309,7 +257,7 @@ func resourceLibratoMetricUpdate(d *schema.ResourceData, meta interface{}) error if d.HasChange("attributes") { attributeData := d.Get("attributes").([]interface{}) if len(attributeData) > 1 { - return fmt.Errorf("Only one set of attributes per alert is supported") + return fmt.Errorf("Only one set of attributes per metric is supported") } if len(attributeData) == 1 && attributeData[0] == nil { @@ -351,7 +299,9 @@ func resourceLibratoMetricUpdate(d *schema.ResourceData, meta interface{}) error fullMetric.Attributes = attributes } - log.Printf("[INFO] Updating Librato metric: %v", metric) + log.Printf("[INFO] Updating Librato metric: %v", structToString(metric)) + log.Printf("[INFO] Librato fullMetric: %v", structToString(fullMetric)) + _, err = client.Metrics.Edit(metric) if err != nil { return fmt.Errorf("Error updating Librato metric: %s", err) @@ -367,19 +317,20 @@ func resourceLibratoMetricUpdate(d *schema.ResourceData, meta interface{}) error MinTimeout: 2 * time.Second, ContinuousTargetOccurence: 5, Refresh: func() (interface{}, string, error) { - log.Printf("[DEBUG] Checking if Librato Metric %s was updated yet", metricID) + log.Printf("[INFO] Checking if Librato Metric %s was updated yet", metricID) changedMetric, _, getErr := client.Metrics.Get(metricID) if getErr != nil { return changedMetric, "", getErr } isEqual := reflect.DeepEqual(*fullMetric, *changedMetric) - log.Printf("[DEBUG] Updated Librato Metric %s match: %t", metricID, isEqual) + log.Printf("[INFO] Updated Librato Metric %s match: %t", metricID, isEqual) return changedMetric, fmt.Sprintf("%t", isEqual), nil }, } _, err = wait.WaitForState() if err != nil { + log.Printf("[INFO] ERROR - Failed updating Librato Metric %s: %s", metricID, err) return fmt.Errorf("Failed updating Librato Metric %s: %s", metricID, err) } @@ -387,6 +338,8 @@ func resourceLibratoMetricUpdate(d *schema.ResourceData, meta interface{}) error } func resourceLibratoMetricDelete(d *schema.ResourceData, meta interface{}) error { + log.Println("[INFO] Deleting Librato metric") + client := meta.(*librato.Client) id := d.Id() @@ -397,22 +350,92 @@ func resourceLibratoMetricDelete(d *schema.ResourceData, meta interface{}) error return fmt.Errorf("Error deleting Metric: %s", err) } + log.Printf("[INFO] Verifying Metric %s deleted", id) resource.Retry(1*time.Minute, func() *resource.RetryError { + + log.Printf("[INFO] GETing Metric %s", id) _, _, err := client.Metrics.Get(id) if err != nil { if errResp, ok := err.(*librato.ErrorResponse); ok && errResp.Response.StatusCode == 404 { + log.Printf("[INFO] GET returned a 404 for Metric %s\n", id) return nil } log.Printf("[INFO] non-retryable error attempting to Get metric: %s", err) return resource.NonRetryableError(err) } + + log.Printf("[INFO] retryable error attempting to Get metric: %s", id) return resource.RetryableError(fmt.Errorf("metric still exists")) }) + log.Println("[INFO] I think Metric is deleted") + d.SetId("") return nil } +func metricReadResult(d *schema.ResourceData, metric *librato.Metric) error { + d.SetId(*metric.Name) + d.Set("id", *metric.Name) + d.Set("name", *metric.Name) + d.Set("type", *metric.Type) + + if metric.Description != nil { + d.Set("description", *metric.Description) + } + + if metric.DisplayName != nil { + d.Set("display_name", *metric.DisplayName) + } + + if metric.Period != nil { + d.Set("period", *metric.Period) + } + + if metric.Composite != nil { + d.Set("composite", *metric.Composite) + } + + attributes := metricAttributesGather(d, metric.Attributes) + d.Set("attributes", attributes) + + return nil +} + +// Flattens an attributes hash into something that flatmap.Flatten() can handle +func metricAttributesGather(d *schema.ResourceData, attributes *librato.MetricAttributes) []map[string]interface{} { + result := make([]map[string]interface{}, 0, 1) + + if attributes != nil { + retAttributes := make(map[string]interface{}) + if attributes.Color != nil { + retAttributes["color"] = *attributes.Color + } + if attributes.DisplayMax != nil { + retAttributes["display_max"] = attributes.DisplayMax + } + if attributes.DisplayMin != nil { + retAttributes["display_min"] = attributes.DisplayMin + } + if attributes.DisplayUnitsLong != "" { + retAttributes["display_units_long"] = attributes.DisplayUnitsLong + } + if attributes.DisplayUnitsShort != "" { + retAttributes["display_units_short"] = attributes.DisplayUnitsShort + } + if attributes.CreatedByUA != "" { + retAttributes["created_by_ua"] = attributes.CreatedByUA + } + retAttributes["display_stacked"] = attributes.DisplayStacked || false + retAttributes["gap_detection"] = attributes.GapDetection || false + retAttributes["aggregate"] = attributes.Aggregate || false + + result = append(result, retAttributes) + } + + return result +} + func structToString(i interface{}) string { s, _ := json.Marshal(i) return string(s) diff --git a/builtin/providers/librato/resource_librato_metric_test.go b/builtin/providers/librato/resource_librato_metric_test.go index a5227153f4ce..4202599516ec 100644 --- a/builtin/providers/librato/resource_librato_metric_test.go +++ b/builtin/providers/librato/resource_librato_metric_test.go @@ -11,10 +11,10 @@ import ( "github.com/henrikhodne/go-librato/librato" ) -func TestAccLibratoMetric_Basic(t *testing.T) { +func TestAccLibratoCounterMetric(t *testing.T) { var metric librato.Metric name := fmt.Sprintf("tftest-metric-%s", acctest.RandString(10)) - typ := "counter" + counter := "counter" resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -22,7 +22,88 @@ func TestAccLibratoMetric_Basic(t *testing.T) { CheckDestroy: testAccCheckLibratoMetricDestroy, Steps: []resource.TestStep{ { - Config: testAccCheckLibratoMetricConfig(name, typ), + Config: counterMetricConfig(name, counter, fmt.Sprintf("A test %s metric", counter)), + Check: resource.ComposeTestCheckFunc( + testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), + testAccCheckLibratoMetricName(&metric, name), + testAccCheckLibratoMetricType(&metric, []string{"gauge", "counter", "composite"}), + resource.TestCheckResourceAttr( + "librato_metric.foobar", "name", name), + ), + }, + { + PreConfig: sleep(t, 15), + Config: counterMetricConfig(name, counter, fmt.Sprintf("An updated test %s metric", counter)), + Check: resource.ComposeTestCheckFunc( + testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), + testAccCheckLibratoMetricName(&metric, name), + testAccCheckLibratoMetricType(&metric, []string{"gauge", "counter", "composite"}), + resource.TestCheckResourceAttr( + "librato_metric.foobar", "name", name), + ), + }, + }, + }) +} + +func TestAccLibratoGaugeMetric(t *testing.T) { + var metric librato.Metric + name := fmt.Sprintf("tftest-metric-%s", acctest.RandString(10)) + gauge := "gauge" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckLibratoMetricDestroy, + Steps: []resource.TestStep{ + { + Config: gaugeMetricConfig(name, gauge, fmt.Sprintf("A test %s metric", gauge)), + Check: resource.ComposeTestCheckFunc( + testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), + testAccCheckLibratoMetricName(&metric, name), + testAccCheckLibratoMetricType(&metric, []string{"gauge", "counter", "composite"}), + resource.TestCheckResourceAttr( + "librato_metric.foobar", "name", name), + ), + }, + { + PreConfig: sleep(t, 15), + Config: gaugeMetricConfig(name, gauge, fmt.Sprintf("An updated test %s metric", gauge)), + Check: resource.ComposeTestCheckFunc( + testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), + testAccCheckLibratoMetricName(&metric, name), + testAccCheckLibratoMetricType(&metric, []string{"gauge", "counter", "composite"}), + resource.TestCheckResourceAttr( + "librato_metric.foobar", "name", name), + ), + }, + }, + }) +} + +func TestAccLibratoCompositeMetric(t *testing.T) { + var metric librato.Metric + name := fmt.Sprintf("tftest-metric-%s", acctest.RandString(10)) + composite := "composite" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckLibratoMetricDestroy, + Steps: []resource.TestStep{ + { + Config: compositeMetricConfig(name, composite, fmt.Sprintf("A test %s metric", composite)), + Check: resource.ComposeTestCheckFunc( + testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), + testAccCheckLibratoMetricName(&metric, name), + testAccCheckLibratoMetricType(&metric, []string{"gauge", "counter", "composite"}), + resource.TestCheckResourceAttr( + "librato_metric.foobar", "name", name), + ), + }, + { + PreConfig: sleep(t, 15), + Config: compositeMetricConfig(name, composite, fmt.Sprintf("An updated test %s metric", composite)), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), testAccCheckLibratoMetricName(&metric, name), @@ -43,6 +124,7 @@ func testAccCheckLibratoMetricDestroy(s *terraform.State) error { continue } + fmt.Printf("*** testAccCheckLibratoMetricDestroy ID: %s\n", rs.Primary.ID) _, _, err := client.Metrics.Get(rs.Primary.ID) if err == nil { return fmt.Errorf("Metric still exists") @@ -52,20 +134,6 @@ func testAccCheckLibratoMetricDestroy(s *terraform.State) error { return nil } -func testAccCheckLibratoMetricConfig(name, typ string) string { - return strings.TrimSpace(fmt.Sprintf(` - resource "librato_metric" "foobar" { - name = "%s" - type = "%s" - description = "A test composite metric" - composite = "s(\"librato.cpu.percent.user\", {\"environment\" : \"prod\", \"service\": \"api\"})" - attributes { - display_stacked = true, - created_by_ua = "go-librato/0.1" - } - }`, name, typ)) -} - func testAccCheckLibratoMetricName(metric *librato.Metric, name string) resource.TestCheckFunc { return func(s *terraform.State) error { if metric.Name == nil || *metric.Name != name { @@ -120,3 +188,43 @@ func testAccCheckLibratoMetricExists(n string, metric *librato.Metric) resource. return nil } } + +func counterMetricConfig(name, typ, desc string) string { + return strings.TrimSpace(fmt.Sprintf(` + resource "librato_metric" "foobar" { + name = "%s" + type = "%s" + description = "%s" + attributes { + display_stacked = true, + created_by_ua = "go-librato/0.1" + } + }`, name, typ, desc)) +} + +func gaugeMetricConfig(name, typ, desc string) string { + return strings.TrimSpace(fmt.Sprintf(` + resource "librato_metric" "foobar" { + name = "%s" + type = "%s" + description = "%s" + attributes { + display_stacked = true, + created_by_ua = "go-librato/0.1" + } + }`, name, typ, desc)) +} + +func compositeMetricConfig(name, typ, desc string) string { + return strings.TrimSpace(fmt.Sprintf(` + resource "librato_metric" "foobar" { + name = "%s" + type = "%s" + description = "%s" + composite = "s(\"librato.cpu.percent.user\", {\"environment\" : \"prod\", \"service\": \"api\"})" + attributes { + display_stacked = true, + created_by_ua = "go-librato/0.1" + } + }`, name, typ, desc)) +} diff --git a/builtin/providers/librato/resource_librato_service.go b/builtin/providers/librato/resource_librato_service.go index e289fee0d7fc..855fc40410c3 100644 --- a/builtin/providers/librato/resource_librato_service.go +++ b/builtin/providers/librato/resource_librato_service.go @@ -1,7 +1,6 @@ package librato import ( - "encoding/json" "fmt" "log" "reflect" @@ -37,49 +36,12 @@ func resourceLibratoService() *schema.Resource { "settings": &schema.Schema{ Type: schema.TypeString, Required: true, - StateFunc: normalizeJson, + StateFunc: normalizeJSON, }, }, } } -// Takes JSON in a string. Decodes JSON into -// settings hash -func resourceLibratoServicesExpandSettings(rawSettings string) (map[string]string, error) { - var settings map[string]string - - settings = make(map[string]string) - err := json.Unmarshal([]byte(rawSettings), &settings) - if err != nil { - return nil, fmt.Errorf("Error decoding JSON: %s", err) - } - - return settings, err -} - -// Encodes a settings hash into a JSON string -func resourceLibratoServicesFlatten(settings map[string]string) (string, error) { - byteArray, err := json.Marshal(settings) - if err != nil { - return "", fmt.Errorf("Error encoding to JSON: %s", err) - } - - return string(byteArray), nil -} - -func normalizeJson(jsonString interface{}) string { - if jsonString == nil || jsonString == "" { - return "" - } - var j interface{} - err := json.Unmarshal([]byte(jsonString.(string)), &j) - if err != nil { - return fmt.Sprintf("Error parsing JSON: %s", err) - } - b, _ := json.Marshal(j) - return string(b[:]) -} - func resourceLibratoServiceCreate(d *schema.ResourceData, meta interface{}) error { client := meta.(*librato.Client) @@ -91,7 +53,7 @@ func resourceLibratoServiceCreate(d *schema.ResourceData, meta interface{}) erro service.Title = librato.String(v.(string)) } if v, ok := d.GetOk("settings"); ok { - res, err := resourceLibratoServicesExpandSettings(normalizeJson(v.(string))) + res, err := attributesExpand(normalizeJSON(v.(string))) if err != nil { return fmt.Errorf("Error expanding Librato service settings: %s", err) } @@ -144,7 +106,7 @@ func resourceLibratoServiceReadResult(d *schema.ResourceData, service *librato.S d.Set("id", *service.ID) d.Set("type", *service.Type) d.Set("title", *service.Title) - settings, _ := resourceLibratoServicesFlatten(service.Settings) + settings, _ := attributesFlatten(service.Settings) d.Set("settings", settings) return nil @@ -174,9 +136,9 @@ func resourceLibratoServiceUpdate(d *schema.ResourceData, meta interface{}) erro fullService.Title = service.Title } if d.HasChange("settings") { - res, err := resourceLibratoServicesExpandSettings(normalizeJson(d.Get("settings").(string))) - if err != nil { - return fmt.Errorf("Error expanding Librato service settings: %s", err) + res, getErr := attributesExpand(normalizeJSON(d.Get("settings").(string))) + if getErr != nil { + return fmt.Errorf("Error expanding Librato service settings: %s", getErr) } service.Settings = res fullService.Settings = res @@ -198,9 +160,9 @@ func resourceLibratoServiceUpdate(d *schema.ResourceData, meta interface{}) erro ContinuousTargetOccurence: 5, Refresh: func() (interface{}, string, error) { log.Printf("[DEBUG] Checking if Librato Service %d was updated yet", serviceID) - changedService, _, err := client.Services.Get(uint(serviceID)) - if err != nil { - return changedService, "", err + changedService, _, scErr := client.Services.Get(uint(serviceID)) + if scErr != nil { + return changedService, "", scErr } isEqual := reflect.DeepEqual(*fullService, *changedService) log.Printf("[DEBUG] Updated Librato Service %d match: %t", serviceID, isEqual) From 78b61ad5f854d4be94ff44e6e76eed02b1ae2a86 Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Thu, 18 May 2017 16:13:11 -0400 Subject: [PATCH 4/9] Cleanup and slight refactors --- .../librato/resource_librato_metric.go | 12 +---- .../librato/resource_librato_metric_test.go | 48 ++++++++----------- 2 files changed, 21 insertions(+), 39 deletions(-) diff --git a/builtin/providers/librato/resource_librato_metric.go b/builtin/providers/librato/resource_librato_metric.go index 45410543127b..14ffe0f70b40 100644 --- a/builtin/providers/librato/resource_librato_metric.go +++ b/builtin/providers/librato/resource_librato_metric.go @@ -18,8 +18,6 @@ const ( metricTypeComposite = "composite" ) -var metricTypes = []string{metricTypeGauge, metricTypeCounter, metricTypeComposite} - func resourceLibratoMetric() *schema.Resource { return &schema.Resource{ Create: resourceLibratoMetricCreate, @@ -84,7 +82,7 @@ func resourceLibratoMetric() *schema.Resource { }, "created_by_ua": &schema.Schema{ Type: schema.TypeString, - Optional: true, + Computed: true, }, "gap_detection": &schema.Schema{ Type: schema.TypeBool, @@ -102,8 +100,6 @@ func resourceLibratoMetric() *schema.Resource { } func resourceLibratoMetricCreate(d *schema.ResourceData, meta interface{}) error { - log.Println("[INFO] Creating Librato metric") - client := meta.(*librato.Client) metric := new(librato.Metric) @@ -192,8 +188,6 @@ func resourceLibratoMetricCreate(d *schema.ResourceData, meta interface{}) error } func resourceLibratoMetricRead(d *schema.ResourceData, meta interface{}) error { - log.Println("[INFO] Reading Librato metric") - client := meta.(*librato.Client) id := d.Id() @@ -214,8 +208,6 @@ func resourceLibratoMetricRead(d *schema.ResourceData, meta interface{}) error { } func resourceLibratoMetricUpdate(d *schema.ResourceData, meta interface{}) error { - log.Println("[INFO] Updating Librato metric") - client := meta.(*librato.Client) metricID := d.Id() @@ -338,8 +330,6 @@ func resourceLibratoMetricUpdate(d *schema.ResourceData, meta interface{}) error } func resourceLibratoMetricDelete(d *schema.ResourceData, meta interface{}) error { - log.Println("[INFO] Deleting Librato metric") - client := meta.(*librato.Client) id := d.Id() diff --git a/builtin/providers/librato/resource_librato_metric_test.go b/builtin/providers/librato/resource_librato_metric_test.go index 4202599516ec..a40e7972adf3 100644 --- a/builtin/providers/librato/resource_librato_metric_test.go +++ b/builtin/providers/librato/resource_librato_metric_test.go @@ -14,7 +14,7 @@ import ( func TestAccLibratoCounterMetric(t *testing.T) { var metric librato.Metric name := fmt.Sprintf("tftest-metric-%s", acctest.RandString(10)) - counter := "counter" + typ := "counter" resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -22,22 +22,22 @@ func TestAccLibratoCounterMetric(t *testing.T) { CheckDestroy: testAccCheckLibratoMetricDestroy, Steps: []resource.TestStep{ { - Config: counterMetricConfig(name, counter, fmt.Sprintf("A test %s metric", counter)), + Config: counterMetricConfig(name, typ, fmt.Sprintf("A test %s metric", typ)), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), testAccCheckLibratoMetricName(&metric, name), - testAccCheckLibratoMetricType(&metric, []string{"gauge", "counter", "composite"}), + testAccCheckLibratoMetricType(&metric, typ), resource.TestCheckResourceAttr( "librato_metric.foobar", "name", name), ), }, { PreConfig: sleep(t, 15), - Config: counterMetricConfig(name, counter, fmt.Sprintf("An updated test %s metric", counter)), + Config: counterMetricConfig(name, typ, fmt.Sprintf("An updated test %s metric", typ)), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), testAccCheckLibratoMetricName(&metric, name), - testAccCheckLibratoMetricType(&metric, []string{"gauge", "counter", "composite"}), + testAccCheckLibratoMetricType(&metric, typ), resource.TestCheckResourceAttr( "librato_metric.foobar", "name", name), ), @@ -49,7 +49,7 @@ func TestAccLibratoCounterMetric(t *testing.T) { func TestAccLibratoGaugeMetric(t *testing.T) { var metric librato.Metric name := fmt.Sprintf("tftest-metric-%s", acctest.RandString(10)) - gauge := "gauge" + typ := "gauge" resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -57,22 +57,22 @@ func TestAccLibratoGaugeMetric(t *testing.T) { CheckDestroy: testAccCheckLibratoMetricDestroy, Steps: []resource.TestStep{ { - Config: gaugeMetricConfig(name, gauge, fmt.Sprintf("A test %s metric", gauge)), + Config: gaugeMetricConfig(name, typ, fmt.Sprintf("A test %s metric", typ)), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), testAccCheckLibratoMetricName(&metric, name), - testAccCheckLibratoMetricType(&metric, []string{"gauge", "counter", "composite"}), + testAccCheckLibratoMetricType(&metric, typ), resource.TestCheckResourceAttr( "librato_metric.foobar", "name", name), ), }, { PreConfig: sleep(t, 15), - Config: gaugeMetricConfig(name, gauge, fmt.Sprintf("An updated test %s metric", gauge)), + Config: gaugeMetricConfig(name, typ, fmt.Sprintf("An updated test %s metric", typ)), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), testAccCheckLibratoMetricName(&metric, name), - testAccCheckLibratoMetricType(&metric, []string{"gauge", "counter", "composite"}), + testAccCheckLibratoMetricType(&metric, typ), resource.TestCheckResourceAttr( "librato_metric.foobar", "name", name), ), @@ -84,7 +84,7 @@ func TestAccLibratoGaugeMetric(t *testing.T) { func TestAccLibratoCompositeMetric(t *testing.T) { var metric librato.Metric name := fmt.Sprintf("tftest-metric-%s", acctest.RandString(10)) - composite := "composite" + typ := "composite" resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -92,22 +92,22 @@ func TestAccLibratoCompositeMetric(t *testing.T) { CheckDestroy: testAccCheckLibratoMetricDestroy, Steps: []resource.TestStep{ { - Config: compositeMetricConfig(name, composite, fmt.Sprintf("A test %s metric", composite)), + Config: compositeMetricConfig(name, typ, fmt.Sprintf("A test %s metric", typ)), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), testAccCheckLibratoMetricName(&metric, name), - testAccCheckLibratoMetricType(&metric, []string{"gauge", "counter", "composite"}), + testAccCheckLibratoMetricType(&metric, typ), resource.TestCheckResourceAttr( "librato_metric.foobar", "name", name), ), }, { PreConfig: sleep(t, 15), - Config: compositeMetricConfig(name, composite, fmt.Sprintf("An updated test %s metric", composite)), + Config: compositeMetricConfig(name, typ, fmt.Sprintf("An updated test %s metric", typ)), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), testAccCheckLibratoMetricName(&metric, name), - testAccCheckLibratoMetricType(&metric, []string{"gauge", "counter", "composite"}), + testAccCheckLibratoMetricType(&metric, typ), resource.TestCheckResourceAttr( "librato_metric.foobar", "name", name), ), @@ -144,14 +144,9 @@ func testAccCheckLibratoMetricName(metric *librato.Metric, name string) resource } } -func testAccCheckLibratoMetricType(metric *librato.Metric, validTypes []string) resource.TestCheckFunc { +func testAccCheckLibratoMetricType(metric *librato.Metric, wantType string) resource.TestCheckFunc { return func(s *terraform.State) error { - m := make(map[string]bool) - for _, v := range validTypes { - m[v] = true - } - - if !m[*metric.Type] { + if metric.Type == nil || *metric.Type != wantType { return fmt.Errorf("Bad metric type: %s", *metric.Type) } @@ -196,8 +191,7 @@ func counterMetricConfig(name, typ, desc string) string { type = "%s" description = "%s" attributes { - display_stacked = true, - created_by_ua = "go-librato/0.1" + display_stacked = true } }`, name, typ, desc)) } @@ -209,8 +203,7 @@ func gaugeMetricConfig(name, typ, desc string) string { type = "%s" description = "%s" attributes { - display_stacked = true, - created_by_ua = "go-librato/0.1" + display_stacked = true } }`, name, typ, desc)) } @@ -223,8 +216,7 @@ func compositeMetricConfig(name, typ, desc string) string { description = "%s" composite = "s(\"librato.cpu.percent.user\", {\"environment\" : \"prod\", \"service\": \"api\"})" attributes { - display_stacked = true, - created_by_ua = "go-librato/0.1" + display_stacked = true } }`, name, typ, desc)) } From 53a1a4b73ba5a24015962fceb61f32d54e0f4676 Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Thu, 25 May 2017 10:22:00 -0400 Subject: [PATCH 5/9] Add documentation for metrics --- .../providers/librato/r/metric.html.markdown | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 website/source/docs/providers/librato/r/metric.html.markdown diff --git a/website/source/docs/providers/librato/r/metric.html.markdown b/website/source/docs/providers/librato/r/metric.html.markdown new file mode 100644 index 000000000000..2f2ba9f7fc94 --- /dev/null +++ b/website/source/docs/providers/librato/r/metric.html.markdown @@ -0,0 +1,71 @@ +--- +layout: "librato" +page_title: "Librato: librato_metric" +sidebar_current: "docs-librato-resource-metric" +description: |- + Provides a Librato Metric resource. This can be used to create and manage metrics on Librato. +--- + +# librato\_metric + +Provides a Librato Metric resource. This can be used to create and manage metrics on Librato. + +## Example Usage + +```hcl +# Create a new Librato metric +resource "librato_metric" "mymetric" { + name = "MyMetric" + type = "counter" + description = "A Test Metric" + attributes { + display_stacked = true + } +} +``` + +## Argument Reference + +The following arguments are supported: + +* `type` - (Required) The type of metric to create (gauge, counter, or composite). +* `name` - (Required) The unique identifier of the metric. +* `display_name` - The name which will be used for the metric when viewing the Metrics website. +* `description` - Text that can be used to explain precisely what the metric is measuring. +* `period` - Number of seconds that is the standard reporting period of the metric. +* `attributes` - The attributes hash configures specific components of a metric’s visualization. +* `composite` - The definition of the composite metric. + +## Attributes Reference + +The following attributes are exported: + +* `name` - The identifier for the metric. +* `display_name` - The name which will be used for the metric when viewing the Metrics website. +* `type` - The type of metric to create (gauge, counter, or composite). +* `description` - Text that describes precisely what the metric is measuring. +* `period` - Number of seconds that is the standard reporting period of the metric. Setting the period enables Metrics to detect abnormal interruptions in reporting and aids in analytics. For gauge metrics that have service-side aggregation enabled, this option will define the period that aggregation occurs on. +* `source_lag` - +* `composite` - The composite definition. Only used when type is composite. + +Attributes (`attributes`) support the following: + +* `color` - Sets a default color to prefer when visually rendering the metric. Must be a seven character string that represents the hex code of the color e.g. #52D74C. +* `display_max` - If a metric has a known theoretical maximum value, set display_max so that visualizations can provide perspective of the current values relative to the maximum value. +* `display_min` - If a metric has a known theoretical minimum value, set display_min so that visualizations can provide perspective of the current values relative to the minimum value. +* `display_units_long` - A string that identifies the unit of measurement e.g. Microseconds. Typically the long form of display_units_short and used in visualizations e.g. the Y-axis label on a graph. +* `display_units_short` - A terse (usually abbreviated) string that identifies the unit of measurement e.g. uS (Microseconds). Typically the short form of display_units_long and used in visualizations e.g. the tooltip for a point on a graph. +* `display_stacked` - A boolean value indicating whether or not multiple metric streams should be aggregated in a visualization (e.g. stacked graphs). By default counters have display_stacked enabled while gauges have it disabled. +* `summarize_function` - Determines how to calculate values when rolling up from raw values to higher resolution intervals. Must be one of: ‘average’, 'sum’, 'count’, 'min’, 'max’. If summarize_function is not set the behavior defaults to average. + +If the values of the measurements to be rolled up are: 2, 10, 5: + +* average: 5.67 +* sum: 17 +* count: 3 +* min: 2 +* max: 10 + +* `aggregate` - Enable service-side aggregation for this metric. When enabled, measurements sent using the same tag set will be aggregated into single measurements on an interval defined by the period of the metric. If there is no period defined for the metric then all measurements will be aggregated on a 60-second interval. + +This option takes a value of true or false. If this option is not set for a metric it will default to false. From 6a0dca93afb3f66127a78dcc90b259eb5d1ce417 Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Thu, 25 May 2017 10:27:22 -0400 Subject: [PATCH 6/9] Restore previous version for now --- builtin/providers/librato/common.go | 40 ------------- .../librato/resource_librato_service.go | 56 ++++++++++++++++--- 2 files changed, 47 insertions(+), 49 deletions(-) delete mode 100644 builtin/providers/librato/common.go diff --git a/builtin/providers/librato/common.go b/builtin/providers/librato/common.go deleted file mode 100644 index 79056fab7ea2..000000000000 --- a/builtin/providers/librato/common.go +++ /dev/null @@ -1,40 +0,0 @@ -package librato - -import ( - "encoding/json" - "fmt" -) - -// Encodes a hash into a JSON string -func attributesFlatten(attrs map[string]string) (string, error) { - byteArray, err := json.Marshal(attrs) - if err != nil { - return "", fmt.Errorf("Error encoding to JSON: %s", err) - } - - return string(byteArray), nil -} - -// Takes JSON in a string & decodes into a hash -func attributesExpand(raw string) (map[string]string, error) { - attrs := make(map[string]string) - err := json.Unmarshal([]byte(raw), &attrs) - if err != nil { - return nil, fmt.Errorf("Error decoding JSON: %s", err) - } - - return attrs, err -} - -func normalizeJSON(jsonString interface{}) string { - if jsonString == nil || jsonString == "" { - return "" - } - var j interface{} - err := json.Unmarshal([]byte(jsonString.(string)), &j) - if err != nil { - return fmt.Sprintf("Error parsing JSON: %s", err) - } - b, _ := json.Marshal(j) - return string(b[:]) -} diff --git a/builtin/providers/librato/resource_librato_service.go b/builtin/providers/librato/resource_librato_service.go index 855fc40410c3..e289fee0d7fc 100644 --- a/builtin/providers/librato/resource_librato_service.go +++ b/builtin/providers/librato/resource_librato_service.go @@ -1,6 +1,7 @@ package librato import ( + "encoding/json" "fmt" "log" "reflect" @@ -36,12 +37,49 @@ func resourceLibratoService() *schema.Resource { "settings": &schema.Schema{ Type: schema.TypeString, Required: true, - StateFunc: normalizeJSON, + StateFunc: normalizeJson, }, }, } } +// Takes JSON in a string. Decodes JSON into +// settings hash +func resourceLibratoServicesExpandSettings(rawSettings string) (map[string]string, error) { + var settings map[string]string + + settings = make(map[string]string) + err := json.Unmarshal([]byte(rawSettings), &settings) + if err != nil { + return nil, fmt.Errorf("Error decoding JSON: %s", err) + } + + return settings, err +} + +// Encodes a settings hash into a JSON string +func resourceLibratoServicesFlatten(settings map[string]string) (string, error) { + byteArray, err := json.Marshal(settings) + if err != nil { + return "", fmt.Errorf("Error encoding to JSON: %s", err) + } + + return string(byteArray), nil +} + +func normalizeJson(jsonString interface{}) string { + if jsonString == nil || jsonString == "" { + return "" + } + var j interface{} + err := json.Unmarshal([]byte(jsonString.(string)), &j) + if err != nil { + return fmt.Sprintf("Error parsing JSON: %s", err) + } + b, _ := json.Marshal(j) + return string(b[:]) +} + func resourceLibratoServiceCreate(d *schema.ResourceData, meta interface{}) error { client := meta.(*librato.Client) @@ -53,7 +91,7 @@ func resourceLibratoServiceCreate(d *schema.ResourceData, meta interface{}) erro service.Title = librato.String(v.(string)) } if v, ok := d.GetOk("settings"); ok { - res, err := attributesExpand(normalizeJSON(v.(string))) + res, err := resourceLibratoServicesExpandSettings(normalizeJson(v.(string))) if err != nil { return fmt.Errorf("Error expanding Librato service settings: %s", err) } @@ -106,7 +144,7 @@ func resourceLibratoServiceReadResult(d *schema.ResourceData, service *librato.S d.Set("id", *service.ID) d.Set("type", *service.Type) d.Set("title", *service.Title) - settings, _ := attributesFlatten(service.Settings) + settings, _ := resourceLibratoServicesFlatten(service.Settings) d.Set("settings", settings) return nil @@ -136,9 +174,9 @@ func resourceLibratoServiceUpdate(d *schema.ResourceData, meta interface{}) erro fullService.Title = service.Title } if d.HasChange("settings") { - res, getErr := attributesExpand(normalizeJSON(d.Get("settings").(string))) - if getErr != nil { - return fmt.Errorf("Error expanding Librato service settings: %s", getErr) + res, err := resourceLibratoServicesExpandSettings(normalizeJson(d.Get("settings").(string))) + if err != nil { + return fmt.Errorf("Error expanding Librato service settings: %s", err) } service.Settings = res fullService.Settings = res @@ -160,9 +198,9 @@ func resourceLibratoServiceUpdate(d *schema.ResourceData, meta interface{}) erro ContinuousTargetOccurence: 5, Refresh: func() (interface{}, string, error) { log.Printf("[DEBUG] Checking if Librato Service %d was updated yet", serviceID) - changedService, _, scErr := client.Services.Get(uint(serviceID)) - if scErr != nil { - return changedService, "", scErr + changedService, _, err := client.Services.Get(uint(serviceID)) + if err != nil { + return changedService, "", err } isEqual := reflect.DeepEqual(*fullService, *changedService) log.Printf("[DEBUG] Updated Librato Service %d match: %t", serviceID, isEqual) From 1c76194197e23109c3480cab1c730ed4d0b6522a Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Thu, 25 May 2017 14:09:04 -0400 Subject: [PATCH 7/9] Respond to review feedback --- .../providers/librato/common_helpers_test.go | 2 + .../librato/resource_librato_metric.go | 190 +++++++----------- .../librato/resource_librato_metric_test.go | 52 +++-- 3 files changed, 109 insertions(+), 135 deletions(-) diff --git a/builtin/providers/librato/common_helpers_test.go b/builtin/providers/librato/common_helpers_test.go index c2744589f62b..dde279781510 100644 --- a/builtin/providers/librato/common_helpers_test.go +++ b/builtin/providers/librato/common_helpers_test.go @@ -1,12 +1,14 @@ package librato import ( + "log" "testing" "time" ) func sleep(t *testing.T, amount time.Duration) func() { return func() { + log.Printf("[INFO] Sleeping for %d seconds...", amount) time.Sleep(amount * time.Second) } } diff --git a/builtin/providers/librato/resource_librato_metric.go b/builtin/providers/librato/resource_librato_metric.go index 14ffe0f70b40..d4da199bafd2 100644 --- a/builtin/providers/librato/resource_librato_metric.go +++ b/builtin/providers/librato/resource_librato_metric.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "log" - "reflect" "time" "github.com/hashicorp/terraform/helper/resource" @@ -12,11 +11,11 @@ import ( "github.com/henrikhodne/go-librato/librato" ) -const ( - metricTypeGauge = "gauge" - metricTypeCounter = "counter" - metricTypeComposite = "composite" -) +// const ( +// metricTypeCounter = "counter" +// metricTypeGauge = "gauge" +// metricTypeComposite = "composite" +// ) func resourceLibratoMetric() *schema.Resource { return &schema.Resource{ @@ -26,69 +25,71 @@ func resourceLibratoMetric() *schema.Resource { Delete: resourceLibratoMetricDelete, Schema: map[string]*schema.Schema{ - "name": &schema.Schema{ + "name": { Type: schema.TypeString, Required: true, ForceNew: false, }, - "type": &schema.Schema{ + "type": { Type: schema.TypeString, Required: true, }, - "display_name": &schema.Schema{ + "display_name": { Type: schema.TypeString, Optional: true, }, - "description": &schema.Schema{ + "description": { Type: schema.TypeString, Optional: true, }, - "period": &schema.Schema{ + "period": { Type: schema.TypeInt, Optional: true, }, - "composite": &schema.Schema{ + "composite": { Type: schema.TypeString, Optional: true, }, - "attributes": &schema.Schema{ + "attributes": { Type: schema.TypeList, Optional: true, + MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "color": &schema.Schema{ + "color": { Type: schema.TypeString, Optional: true, }, - "display_max": &schema.Schema{ + "display_max": { Type: schema.TypeString, Optional: true, }, - "display_min": &schema.Schema{ + "display_min": { Type: schema.TypeString, Optional: true, }, - "display_units_long": &schema.Schema{ + "display_units_long": { Type: schema.TypeString, Optional: true, }, - "display_units_short": &schema.Schema{ + "display_units_short": { Type: schema.TypeString, Optional: true, }, - "display_stacked": &schema.Schema{ + "display_stacked": { Type: schema.TypeBool, Optional: true, + Default: false, }, - "created_by_ua": &schema.Schema{ + "created_by_ua": { Type: schema.TypeString, Computed: true, }, - "gap_detection": &schema.Schema{ + "gap_detection": { Type: schema.TypeBool, Optional: true, }, - "aggregate": &schema.Schema{ + "aggregate": { Type: schema.TypeBool, Optional: true, }, @@ -102,21 +103,18 @@ func resourceLibratoMetric() *schema.Resource { func resourceLibratoMetricCreate(d *schema.ResourceData, meta interface{}) error { client := meta.(*librato.Client) - metric := new(librato.Metric) - if a, ok := d.GetOk("name"); ok { - metric.Name = librato.String(a.(string)) + metric := librato.Metric{ + Name: librato.String(d.Get("name").(string)), + Type: librato.String(d.Get("type").(string)), } if a, ok := d.GetOk("display_name"); ok { metric.DisplayName = librato.String(a.(string)) } - if a, ok := d.GetOk("type"); ok { - metric.Type = librato.String(a.(string)) - } if a, ok := d.GetOk("description"); ok { metric.Description = librato.String(a.(string)) } if a, ok := d.GetOk("period"); ok { - metric.Period = librato.Uint(a.(uint)) + metric.Period = librato.Uint(uint(a.(int))) } if a, ok := d.GetOk("composite"); ok { metric.Composite = librato.String(a.(string)) @@ -125,14 +123,6 @@ func resourceLibratoMetricCreate(d *schema.ResourceData, meta interface{}) error if a, ok := d.GetOk("attributes"); ok { attributeData := a.([]interface{}) - if len(attributeData) > 1 { - return fmt.Errorf("Only one set of attributes per metric is supported") - } - - if len(attributeData) == 1 && attributeData[0] == nil { - return fmt.Errorf("No attributes found in attributes block") - } - attributeDataMap := attributeData[0].(map[string]interface{}) attributes := new(librato.MetricAttributes) @@ -167,13 +157,13 @@ func resourceLibratoMetricCreate(d *schema.ResourceData, meta interface{}) error metric.Attributes = attributes } - _, err := client.Metrics.Edit(metric) + _, err := client.Metrics.Edit(&metric) if err != nil { log.Printf("[INFO] ERROR creating Metric: %s", err) return fmt.Errorf("Error creating Librato metric: %s", err) } - resource.Retry(1*time.Minute, func() *resource.RetryError { + retryErr := resource.Retry(1*time.Minute, func() *resource.RetryError { _, _, err := client.Metrics.Get(*metric.Name) if err != nil { if errResp, ok := err.(*librato.ErrorResponse); ok && errResp.Response.StatusCode == 404 { @@ -183,8 +173,12 @@ func resourceLibratoMetricCreate(d *schema.ResourceData, meta interface{}) error } return nil }) + if retryErr != nil { + return fmt.Errorf("Error creating Librato metric: %s", retryErr) + } - return metricReadResult(d, metric) + d.SetId(*metric.Name) + return resourceLibratoMetricRead(d, meta) } func resourceLibratoMetricRead(d *schema.ResourceData, meta interface{}) error { @@ -202,60 +196,61 @@ func resourceLibratoMetricRead(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("Error reading Librato Metric %s: %s", id, err) } - log.Printf("[INFO] Read Librato Metric: %s", structToString(metric)) + d.Set("name", metric.Name) + d.Set("type", metric.Type) - return metricReadResult(d, metric) -} + if metric.Description != nil { + d.Set("description", metric.Description) + } -func resourceLibratoMetricUpdate(d *schema.ResourceData, meta interface{}) error { - client := meta.(*librato.Client) + if metric.DisplayName != nil { + d.Set("display_name", metric.DisplayName) + } + + if metric.Period != nil { + d.Set("period", metric.Period) + } - metricID := d.Id() + if metric.Composite != nil { + d.Set("composite", metric.Composite) + } - // Just to have whole object for comparison before/after update - fullMetric, _, err := client.Metrics.Get(metricID) - if err != nil { + attributes := metricAttributesGather(d, metric.Attributes) + + // Since attributes isn't a simple terraform type (TypeList), it's best to + // catch the error returned from the d.Set() function, and handle accordingly. + if err := d.Set("attributes", attributes); err != nil { return err } + return nil +} + +func resourceLibratoMetricUpdate(d *schema.ResourceData, meta interface{}) error { + client := meta.(*librato.Client) + + id := d.Id() + metric := new(librato.Metric) - metric.Name = librato.String(d.Get("name").(string)) + metric.Name = librato.String(id) if d.HasChange("type") { metric.Type = librato.String(d.Get("type").(string)) - fullMetric.Type = metric.Type } - if d.HasChange("description") { metric.Description = librato.String(d.Get("description").(string)) - fullMetric.Description = metric.Description } - if d.HasChange("display_name") { metric.DisplayName = librato.String(d.Get("display_name").(string)) - fullMetric.DisplayName = metric.DisplayName } - if d.HasChange("period") { - metric.Period = librato.Uint(d.Get("period").(uint)) - fullMetric.Period = metric.Period + metric.Period = librato.Uint(uint(d.Get("period").(int))) } - if d.HasChange("composite") { metric.Composite = librato.String(d.Get("composite").(string)) - fullMetric.Composite = metric.Composite } - if d.HasChange("attributes") { attributeData := d.Get("attributes").([]interface{}) - if len(attributeData) > 1 { - return fmt.Errorf("Only one set of attributes per metric is supported") - } - - if len(attributeData) == 1 && attributeData[0] == nil { - return fmt.Errorf("No attributes found in attributes block") - } - attributeDataMap := attributeData[0].(map[string]interface{}) attributes := new(librato.MetricAttributes) @@ -286,20 +281,17 @@ func resourceLibratoMetricUpdate(d *schema.ResourceData, meta interface{}) error if v, ok := attributeDataMap["aggregate"].(bool); ok { attributes.Aggregate = *librato.Bool(v) } - metric.Attributes = attributes - fullMetric.Attributes = attributes } log.Printf("[INFO] Updating Librato metric: %v", structToString(metric)) - log.Printf("[INFO] Librato fullMetric: %v", structToString(fullMetric)) - _, err = client.Metrics.Edit(metric) + _, err := client.Metrics.Edit(metric) if err != nil { return fmt.Errorf("Error updating Librato metric: %s", err) } - log.Printf("[INFO] Updated Librato metric %s", metricID) + log.Printf("[INFO] Updated Librato metric %s", id) // Wait for propagation since Librato updates are eventually consistent wait := resource.StateChangeConf{ @@ -309,21 +301,19 @@ func resourceLibratoMetricUpdate(d *schema.ResourceData, meta interface{}) error MinTimeout: 2 * time.Second, ContinuousTargetOccurence: 5, Refresh: func() (interface{}, string, error) { - log.Printf("[INFO] Checking if Librato Metric %s was updated yet", metricID) - changedMetric, _, getErr := client.Metrics.Get(metricID) + log.Printf("[INFO] Checking if Librato Metric %s was updated yet", id) + changedMetric, _, getErr := client.Metrics.Get(id) if getErr != nil { return changedMetric, "", getErr } - isEqual := reflect.DeepEqual(*fullMetric, *changedMetric) - log.Printf("[INFO] Updated Librato Metric %s match: %t", metricID, isEqual) - return changedMetric, fmt.Sprintf("%t", isEqual), nil + return changedMetric, "true", nil }, } _, err = wait.WaitForState() if err != nil { - log.Printf("[INFO] ERROR - Failed updating Librato Metric %s: %s", metricID, err) - return fmt.Errorf("Failed updating Librato Metric %s: %s", metricID, err) + log.Printf("[INFO] ERROR - Failed updating Librato Metric %s: %s", id, err) + return fmt.Errorf("Failed updating Librato Metric %s: %s", id, err) } return resourceLibratoMetricRead(d, meta) @@ -341,13 +331,13 @@ func resourceLibratoMetricDelete(d *schema.ResourceData, meta interface{}) error } log.Printf("[INFO] Verifying Metric %s deleted", id) - resource.Retry(1*time.Minute, func() *resource.RetryError { + retryErr := resource.Retry(1*time.Minute, func() *resource.RetryError { - log.Printf("[INFO] GETing Metric %s", id) + log.Printf("[INFO] Getting Metric %s", id) _, _, err := client.Metrics.Get(id) if err != nil { if errResp, ok := err.(*librato.ErrorResponse); ok && errResp.Response.StatusCode == 404 { - log.Printf("[INFO] GET returned a 404 for Metric %s\n", id) + log.Printf("[INFO] Metric %s not found, removing from state", id) return nil } log.Printf("[INFO] non-retryable error attempting to Get metric: %s", err) @@ -357,38 +347,10 @@ func resourceLibratoMetricDelete(d *schema.ResourceData, meta interface{}) error log.Printf("[INFO] retryable error attempting to Get metric: %s", id) return resource.RetryableError(fmt.Errorf("metric still exists")) }) - - log.Println("[INFO] I think Metric is deleted") - - d.SetId("") - return nil -} - -func metricReadResult(d *schema.ResourceData, metric *librato.Metric) error { - d.SetId(*metric.Name) - d.Set("id", *metric.Name) - d.Set("name", *metric.Name) - d.Set("type", *metric.Type) - - if metric.Description != nil { - d.Set("description", *metric.Description) - } - - if metric.DisplayName != nil { - d.Set("display_name", *metric.DisplayName) + if retryErr != nil { + return fmt.Errorf("Error deleting librato metric: %s", retryErr) } - if metric.Period != nil { - d.Set("period", *metric.Period) - } - - if metric.Composite != nil { - d.Set("composite", *metric.Composite) - } - - attributes := metricAttributesGather(d, metric.Attributes) - d.Set("attributes", attributes) - return nil } diff --git a/builtin/providers/librato/resource_librato_metric_test.go b/builtin/providers/librato/resource_librato_metric_test.go index a40e7972adf3..a7e47d9a81f6 100644 --- a/builtin/providers/librato/resource_librato_metric_test.go +++ b/builtin/providers/librato/resource_librato_metric_test.go @@ -11,18 +11,20 @@ import ( "github.com/henrikhodne/go-librato/librato" ) -func TestAccLibratoCounterMetric(t *testing.T) { +func TestAccLibratoMetrics(t *testing.T) { var metric librato.Metric + name := fmt.Sprintf("tftest-metric-%s", acctest.RandString(10)) typ := "counter" - + desc1 := fmt.Sprintf("A test %s metric", typ) + desc2 := fmt.Sprintf("An updated test %s metric", typ) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckLibratoMetricDestroy, Steps: []resource.TestStep{ { - Config: counterMetricConfig(name, typ, fmt.Sprintf("A test %s metric", typ)), + Config: counterMetricConfig(name, typ, desc1), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), testAccCheckLibratoMetricName(&metric, name), @@ -33,31 +35,30 @@ func TestAccLibratoCounterMetric(t *testing.T) { }, { PreConfig: sleep(t, 15), - Config: counterMetricConfig(name, typ, fmt.Sprintf("An updated test %s metric", typ)), + Config: counterMetricConfig(name, typ, desc2), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), testAccCheckLibratoMetricName(&metric, name), testAccCheckLibratoMetricType(&metric, typ), + testAccCheckLibratoMetricDescription(&metric, desc2), resource.TestCheckResourceAttr( "librato_metric.foobar", "name", name), ), }, }, }) -} - -func TestAccLibratoGaugeMetric(t *testing.T) { - var metric librato.Metric - name := fmt.Sprintf("tftest-metric-%s", acctest.RandString(10)) - typ := "gauge" + name = fmt.Sprintf("tftest-metric-%s", acctest.RandString(10)) + typ = "gauge" + desc1 = fmt.Sprintf("A test %s metric", typ) + desc2 = fmt.Sprintf("An updated test %s metric", typ) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckLibratoMetricDestroy, Steps: []resource.TestStep{ { - Config: gaugeMetricConfig(name, typ, fmt.Sprintf("A test %s metric", typ)), + Config: gaugeMetricConfig(name, typ, desc1), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), testAccCheckLibratoMetricName(&metric, name), @@ -68,31 +69,30 @@ func TestAccLibratoGaugeMetric(t *testing.T) { }, { PreConfig: sleep(t, 15), - Config: gaugeMetricConfig(name, typ, fmt.Sprintf("An updated test %s metric", typ)), + Config: gaugeMetricConfig(name, typ, desc2), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), testAccCheckLibratoMetricName(&metric, name), testAccCheckLibratoMetricType(&metric, typ), + testAccCheckLibratoMetricDescription(&metric, desc2), resource.TestCheckResourceAttr( "librato_metric.foobar", "name", name), ), }, }, }) -} - -func TestAccLibratoCompositeMetric(t *testing.T) { - var metric librato.Metric - name := fmt.Sprintf("tftest-metric-%s", acctest.RandString(10)) - typ := "composite" + name = fmt.Sprintf("tftest-metric-%s", acctest.RandString(10)) + typ = "composite" + desc1 = fmt.Sprintf("A test %s metric", typ) + desc2 = fmt.Sprintf("An updated test %s metric", typ) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckLibratoMetricDestroy, Steps: []resource.TestStep{ { - Config: compositeMetricConfig(name, typ, fmt.Sprintf("A test %s metric", typ)), + Config: compositeMetricConfig(name, typ, desc1), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), testAccCheckLibratoMetricName(&metric, name), @@ -103,11 +103,12 @@ func TestAccLibratoCompositeMetric(t *testing.T) { }, { PreConfig: sleep(t, 15), - Config: compositeMetricConfig(name, typ, fmt.Sprintf("An updated test %s metric", typ)), + Config: compositeMetricConfig(name, typ, desc2), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), testAccCheckLibratoMetricName(&metric, name), testAccCheckLibratoMetricType(&metric, typ), + testAccCheckLibratoMetricDescription(&metric, desc2), resource.TestCheckResourceAttr( "librato_metric.foobar", "name", name), ), @@ -124,7 +125,6 @@ func testAccCheckLibratoMetricDestroy(s *terraform.State) error { continue } - fmt.Printf("*** testAccCheckLibratoMetricDestroy ID: %s\n", rs.Primary.ID) _, _, err := client.Metrics.Get(rs.Primary.ID) if err == nil { return fmt.Errorf("Metric still exists") @@ -144,6 +144,16 @@ func testAccCheckLibratoMetricName(metric *librato.Metric, name string) resource } } +func testAccCheckLibratoMetricDescription(metric *librato.Metric, desc string) resource.TestCheckFunc { + return func(s *terraform.State) error { + if metric.Description == nil || *metric.Description != desc { + return fmt.Errorf("Bad description: %s", *metric.Description) + } + + return nil + } +} + func testAccCheckLibratoMetricType(metric *librato.Metric, wantType string) resource.TestCheckFunc { return func(s *terraform.State) error { if metric.Type == nil || *metric.Type != wantType { From e6b4dc9b4249adf524f0febf1f30122a26ae94d3 Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Thu, 25 May 2017 14:22:51 -0400 Subject: [PATCH 8/9] reduce sleep time --- builtin/providers/librato/resource_librato_metric_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/providers/librato/resource_librato_metric_test.go b/builtin/providers/librato/resource_librato_metric_test.go index a7e47d9a81f6..63ea41432232 100644 --- a/builtin/providers/librato/resource_librato_metric_test.go +++ b/builtin/providers/librato/resource_librato_metric_test.go @@ -34,7 +34,7 @@ func TestAccLibratoMetrics(t *testing.T) { ), }, { - PreConfig: sleep(t, 15), + PreConfig: sleep(t, 5), Config: counterMetricConfig(name, typ, desc2), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), @@ -68,7 +68,7 @@ func TestAccLibratoMetrics(t *testing.T) { ), }, { - PreConfig: sleep(t, 15), + PreConfig: sleep(t, 5), Config: gaugeMetricConfig(name, typ, desc2), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), @@ -102,7 +102,7 @@ func TestAccLibratoMetrics(t *testing.T) { ), }, { - PreConfig: sleep(t, 15), + PreConfig: sleep(t, 5), Config: compositeMetricConfig(name, typ, desc2), Check: resource.ComposeTestCheckFunc( testAccCheckLibratoMetricExists("librato_metric.foobar", &metric), From 527d79aa3c463681069db23393f09d48c66013b5 Mon Sep 17 00:00:00 2001 From: Chris Johnson Date: Thu, 25 May 2017 14:26:55 -0400 Subject: [PATCH 9/9] removed unused constants --- builtin/providers/librato/resource_librato_metric.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/builtin/providers/librato/resource_librato_metric.go b/builtin/providers/librato/resource_librato_metric.go index d4da199bafd2..abbeca17c7a1 100644 --- a/builtin/providers/librato/resource_librato_metric.go +++ b/builtin/providers/librato/resource_librato_metric.go @@ -11,12 +11,6 @@ import ( "github.com/henrikhodne/go-librato/librato" ) -// const ( -// metricTypeCounter = "counter" -// metricTypeGauge = "gauge" -// metricTypeComposite = "composite" -// ) - func resourceLibratoMetric() *schema.Resource { return &schema.Resource{ Create: resourceLibratoMetricCreate,