From 5e21c7f74cc5243922312952663448ebd8538540 Mon Sep 17 00:00:00 2001 From: Ivan Genchev Date: Tue, 20 Jul 2021 17:36:30 +0100 Subject: [PATCH] Lint and gofmt checks (#24) * Added lint and gofmt build rules * Lint and gofmt fixes and some very minor refactoring * Added Circle job for linting and gofmt * Quickfix: get plz before running tests * Fix the working dir * Quickfix: remove creating symlink * Source ~/.profile before running plz * Fix the PATH when running plz * Fix the PATH when running plz - second attempt * Make a symlink for go * Removed the unused build_and_test job --- .circleci/config.yml | 25 ++++++++++++------- cmd/BUILD | 14 +++++++++++ pkg/service_exporter/BUILD | 14 +++++++++++ pkg/service_exporter/service_exporter.go | 16 +++++------- pkg/service_exporter/service_exporter_test.go | 6 ++--- pkg/service_quotas/BUILD | 14 +++++++++++ pkg/service_quotas/asg_limits.go | 2 +- pkg/service_quotas/ec2_limits.go | 8 +++--- pkg/service_quotas/tags.go | 5 ++-- 9 files changed, 75 insertions(+), 29 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index f9561b2..bd19852 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -3,14 +3,24 @@ # Check https://circleci.com/docs/2.0/ for more details version: 2 jobs: - build_and_test: - working_directory: ~/aws-service-quotas-exporter - machine: true + test: + docker: + - image: circleci/golang:1.14 + working_directory: /go/github.com/thought-machine/aws-service-quotas-exporter steps: - checkout + - run: sudo ln -s /usr/local/go/bin/go /usr/local/bin/go + - run: curl https://get.please.build | sh + - run: go get -u golang.org/x/lint/golint - run: - name: Build Docker Image - command: docker build -f build/Dockerfile-builder . --rm=false -t thoughtmachine/aws-service-quotas-exporter:"$CIRCLE_SHA1" + name: "Lint" + command: source ~/.profile && plz run parallel --include=lint --show_all_output + - run: + name: "gofmt check" + command: source ~/.profile && plz run parallel --include=gofmt --show_all_output + - run: + name: "Test" + command: source ~/.profile && plz test ./... --show_all_output build_and_release: working_directory: ~/ machine: true @@ -64,7 +74,4 @@ workflows: ignore: /.*/ test-buildable: jobs: - - build_and_test: - filters: - branches: - ignore: master + - test diff --git a/cmd/BUILD b/cmd/BUILD index c5004d6..7d501dd 100644 --- a/cmd/BUILD +++ b/cmd/BUILD @@ -9,3 +9,17 @@ go_binary( "//third_party/go:go-flags", ], ) + +sh_cmd( + name = "lint", + cmd = "golint -set_exit_status $SRCS", + srcs = glob(["*.go"]), + labels = ["lint"] +) + +sh_cmd( + name = "gofmt", + cmd = "[ -z \"$(gofmt -l $SRCS)\" ] && exit 0 || exit 1", + srcs = glob(["*.go"]), + labels = ["gofmt"], +) diff --git a/pkg/service_exporter/BUILD b/pkg/service_exporter/BUILD index e864706..1d2f0d8 100644 --- a/pkg/service_exporter/BUILD +++ b/pkg/service_exporter/BUILD @@ -21,3 +21,17 @@ go_test( "//third_party/go:testify", ], ) + +sh_cmd( + name = "lint", + cmd = "golint -set_exit_status $SRCS", + srcs = glob(["*.go"]), + labels = ["lint"] +) + +sh_cmd( + name = "gofmt", + cmd = "[ -z \"$(gofmt -l $SRCS)\" ] && exit 0 || exit 1", + srcs = glob(["*.go"]), + labels = ["gofmt"], +) diff --git a/pkg/service_exporter/service_exporter.go b/pkg/service_exporter/service_exporter.go index b498b38..904afde 100644 --- a/pkg/service_exporter/service_exporter.go +++ b/pkg/service_exporter/service_exporter.go @@ -52,7 +52,7 @@ func NewServiceQuotasExporter(region, profile string, refreshPeriod int, include waitForMetrics: ch, includedAWSTags: includedAWSTags, } - go exporter.createQuotasAndDescriptions(false) + go exporter.createOrUpdateQuotasAndDescriptions(false) go exporter.refreshMetrics() return exporter, nil @@ -63,15 +63,11 @@ func (e *ServiceQuotasExporter) refreshMetrics() { for { time.Sleep(time.Duration(e.refreshPeriod) * time.Second) - e.updateMetrics() + e.createOrUpdateQuotasAndDescriptions(true) } } -func (e *ServiceQuotasExporter) updateMetrics() { - e.createQuotasAndDescriptions(true) -} - -func (e *ServiceQuotasExporter) createQuotasAndDescriptions(refresh bool) { +func (e *ServiceQuotasExporter) createOrUpdateQuotasAndDescriptions(update bool) { quotas, err := e.quotasClient.QuotasAndUsage() if err != nil { log.Fatalf("Could not retrieve quotas and limits: %s", err) @@ -91,9 +87,9 @@ func (e *ServiceQuotasExporter) createQuotasAndDescriptions(refresh bool) { labelValues = append(labelValues, quota.Tags[prometheusFormatTag]) } - if refresh { + if update { if resourceMetric, ok := e.metrics[key]; ok { - log.Infof("Refreshing metrics for resource (%s)", resourceID) + log.Infof("Updating metrics for resource (%s)", resourceID) resourceMetric.usage = quota.Usage resourceMetric.limit = quota.Quota resourceMetric.labelValues = labelValues @@ -116,7 +112,7 @@ func (e *ServiceQuotasExporter) createQuotasAndDescriptions(refresh bool) { } } - if !refresh { + if !update { close(e.waitForMetrics) } } diff --git a/pkg/service_exporter/service_exporter_test.go b/pkg/service_exporter/service_exporter_test.go index 0beffd2..0153b38 100644 --- a/pkg/service_exporter/service_exporter_test.go +++ b/pkg/service_exporter/service_exporter_test.go @@ -41,7 +41,7 @@ func TestUpdateMetrics(t *testing.T) { refreshPeriod: 360, } - exporter.updateMetrics() + exporter.createOrUpdateQuotasAndDescriptions(true) expectedMetrics := map[string]Metric{ "i-asdasd1": Metric{usage: 5, limit: 10, labelValues: []string{"i-asdasd1", "dummy-value"}}, @@ -82,7 +82,7 @@ func TestCreateQuotasAndDescriptions(t *testing.T) { includedAWSTags: []string{"dummy-tag", "dummy-tag2"}, } - exporter.createQuotasAndDescriptions(false) + exporter.createOrUpdateQuotasAndDescriptions(false) firstUsageDesc := newDesc(region, firstQ.Name, "used_total", "Used amount of desc1", []string{"resource", "dummy_tag", "dummy_tag2"}) firstLimitDesc := newDesc(region, firstQ.Name, "limit_total", "Limit of desc1", []string{"resource", "dummy_tag", "dummy_tag2"}) @@ -135,7 +135,7 @@ func TestCreateQuotasAndDescriptionsRefresh(t *testing.T) { refreshPeriod: 360, } - exporter.updateMetrics() + exporter.createOrUpdateQuotasAndDescriptions(true) expectedMetrics := map[string]Metric{ "i-asdasd1": Metric{usage: 5, limit: 10, labelValues: []string{"i-asdasd1", "dummy-value"}, usageDesc: desc}, diff --git a/pkg/service_quotas/BUILD b/pkg/service_quotas/BUILD index 7e9b107..3203172 100644 --- a/pkg/service_quotas/BUILD +++ b/pkg/service_quotas/BUILD @@ -20,3 +20,17 @@ go_test( "//third_party/go:testify", ], ) + +sh_cmd( + name = "lint", + cmd = "golint -set_exit_status $SRCS", + srcs = glob(["*.go"]), + labels = ["lint"] +) + +sh_cmd( + name = "gofmt", + cmd = "[ -z \"$(gofmt -l $SRCS)\" ] && exit 0 || exit 1", + srcs = glob(["*.go"]), + labels = ["gofmt"], +) diff --git a/pkg/service_quotas/asg_limits.go b/pkg/service_quotas/asg_limits.go index 0ff7773..bb6dd7c 100644 --- a/pkg/service_quotas/asg_limits.go +++ b/pkg/service_quotas/asg_limits.go @@ -31,7 +31,7 @@ func (c *ASGUsageCheck) Usage() ([]QuotaUsage, error) { numRunningInstances := 0 for _, instance := range asg.Instances { if isRunning(instance) { - numRunningInstances += 1 + numRunningInstances++ } } diff --git a/pkg/service_quotas/ec2_limits.go b/pkg/service_quotas/ec2_limits.go index 961a95d..9b73cff 100644 --- a/pkg/service_quotas/ec2_limits.go +++ b/pkg/service_quotas/ec2_limits.go @@ -51,8 +51,8 @@ func (c *RulesPerSecurityGroupUsageCheck) Usage() ([]QuotaUsage, error) { func(page *ec2.DescribeSecurityGroupsOutput, lastPage bool) bool { if page != nil { for _, group := range page.SecurityGroups { - var inboundRules int = 0 - var outboundRules int = 0 + var inboundRules int + var outboundRules int tags := ec2TagsToQuotaUsageTags(group.Tags) @@ -66,7 +66,7 @@ func (c *RulesPerSecurityGroupUsageCheck) Usage() ([]QuotaUsage, error) { ResourceName: group.GroupId, Description: inboundRulesPerSecGrpDesc, Usage: float64(inboundRules), - Tags: tags, + Tags: tags, } for _, rule := range group.IpPermissionsEgress { @@ -79,7 +79,7 @@ func (c *RulesPerSecurityGroupUsageCheck) Usage() ([]QuotaUsage, error) { ResourceName: group.GroupId, Description: outboundRulesPerSecGrpDesc, Usage: float64(outboundRules), - Tags: tags, + Tags: tags, } quotaUsages = append(quotaUsages, []QuotaUsage{inboundUsage, outboundUsage}...) diff --git a/pkg/service_quotas/tags.go b/pkg/service_quotas/tags.go index 7db1398..9a847d8 100644 --- a/pkg/service_quotas/tags.go +++ b/pkg/service_quotas/tags.go @@ -5,10 +5,11 @@ import ( "strings" ) - var invalidLabelCharactersRE = regexp.MustCompile(`[^a-zA-Z0-9_]`) -var matchAllCap = regexp.MustCompile("([a-z0-9])([A-Z])") +var matchAllCap = regexp.MustCompile("([a-z0-9])([A-Z])") +// ToPrometheusNamingFormat modifies string `s` to conform with the Prom naming +// conventions func ToPrometheusNamingFormat(s string) string { return toSnakeCase(invalidLabelCharactersRE.ReplaceAllString(s, "_")) }