From 84fa7fa0df124b9e49d394411dac87bcdbcd9386 Mon Sep 17 00:00:00 2001 From: Alex Peters Date: Tue, 13 Feb 2024 10:25:09 +0100 Subject: [PATCH] Review feedback --- .github/workflows/linter.yaml | 36 --------------------------- .golangci.yaml | 46 ----------------------------------- Makefile | 29 ---------------------- cmd/lingo/main.go | 10 +++++--- pkg/autoscaler/autoscaler.go | 38 ++++++++++++++--------------- 5 files changed, 25 insertions(+), 134 deletions(-) delete mode 100644 .github/workflows/linter.yaml delete mode 100644 .golangci.yaml diff --git a/.github/workflows/linter.yaml b/.github/workflows/linter.yaml deleted file mode 100644 index 72655638..00000000 --- a/.github/workflows/linter.yaml +++ /dev/null @@ -1,36 +0,0 @@ -name: lint - -on: - push: - branches: - - main - paths-ignore: - - '**.md' - pull_request: - branches: - - main - paths-ignore: - - '**.md' - -permissions: read-all - -jobs: - lint: - runs-on: ubuntu-latest - steps: - - name: Harden Runner - uses: step-security/harden-runner@eb238b55efaa70779f274895e782ed17c84f2895 # v2.6.1 - with: - egress-policy: audit - - - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 - - - uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0 - with: - go-version: "1.21" - check-latest: true - - - name: lint - uses: golangci/golangci-lint-action@3a919529898de77ec3da873e3063ca4b10e7f5cc # v3.7.0 - with: - version: v1.55.2 \ No newline at end of file diff --git a/.golangci.yaml b/.golangci.yaml deleted file mode 100644 index 8ffedeea..00000000 --- a/.golangci.yaml +++ /dev/null @@ -1,46 +0,0 @@ -run: - timeout: 5m - tests: false - sort-results: true - allow-parallel-runners: true - -linters-settings: - gci: - custom-order: true - sections: - - standard # Standard section: captures all standard packages. - - default # Default section: contains all imports that could not be matched to another section type. - - prefix(*.k8s.io) - - prefix(github.com/substratusai/lingo) - lll: - line-length: 200 - misspell: - locale: US - staticcheck: - go: "1.21" - -linters: - disable-all: true - enable: - - errcheck - - errorlint - - exportloopref - - forcetypeassert - - gci - - gocritic - - goconst - - godot - - gofmt - - gofumpt - - goimports - - gosec - - gosimple - - govet - - ineffassign - - misspell - - revive - - staticcheck - - typecheck - - unconvert - - unused - - whitespace \ No newline at end of file diff --git a/Makefile b/Makefile index 5e46a4d9..639c369a 100644 --- a/Makefile +++ b/Makefile @@ -26,32 +26,3 @@ envtest: $(ENVTEST) ## Download envtest-setup locally if necessary. $(ENVTEST): $(LOCALBIN) test -s $(LOCALBIN)/setup-envtest || GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest -GOLANGCI ?= $(LOCALBIN)/golangci-lint - -.PHONY: golangci -golangci: $(GOLANGCI) ## Download golangci-lint locally if necessary. -$(GOLANGCI): $(LOCALBIN) - test -s $(LOCALBIN)golangci-lint || GOBIN=$(LOCALBIN) go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.55.2 - - -.PHONY: lint -lint: golangci - golangci-lint run ./... --timeout 5m - -GOLANGCI ?= $(LOCALBIN)/golangci-lint - -.PHONY: golangci -golangci: $(GOLANGCI) ## Download golangci-lint locally if necessary. -$(GOLANGCI): $(LOCALBIN) - test -s $(LOCALBIN)golangci-lint || GOBIN=$(LOCALBIN) go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.55.2 - -GCI ?= $(LOCALBIN)/gci - -.PHONY: formatter -formatter: $(GCI) ## Download gci locally if necessary. -$(GCI): $(LOCALBIN) - test -s $(LOCALBIN)gci || GOBIN=$(LOCALBIN) go install github.com/daixiang0/gci@v0.12.1 - -.PHONY: format -format: formatter - find . -name '*.go' -type f -not -path "./vendor*" -not -path "*.git*" | xargs gci write --skip-generated -s standard -s default -s "prefix(*.k8s.io)" -s "prefix(github.com/substratusai/lingo)" --custom-order \ No newline at end of file diff --git a/cmd/lingo/main.go b/cmd/lingo/main.go index c366f02f..59d9e2b2 100644 --- a/cmd/lingo/main.go +++ b/cmd/lingo/main.go @@ -66,10 +66,12 @@ func run() error { concurrency := getEnvInt("CONCURRENCY", 100) scaleDownDelay := getEnvInt("SCALE_DOWN_DELAY", 30) - var metricsAddr string - var probeAddr string - var concurrencyPerReplica int - var requestHeaderTimeout time.Duration + var ( + metricsAddr string + probeAddr string + concurrencyPerReplica int + requestHeaderTimeout time.Duration // setting to prevent slowloris attack on http server + ) flag.StringVar(&metricsAddr, "metrics-bind-address", ":8082", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") diff --git a/pkg/autoscaler/autoscaler.go b/pkg/autoscaler/autoscaler.go index c903c505..f49ab63c 100644 --- a/pkg/autoscaler/autoscaler.go +++ b/pkg/autoscaler/autoscaler.go @@ -56,24 +56,24 @@ type Autoscaler struct { movingAvgQueueSize map[string]*movingaverage.Simple } -func (r *Autoscaler) SetupWithManager(mgr ctrl.Manager) error { +func (a *Autoscaler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&corev1.ConfigMap{}). - Complete(r) + Complete(a) } -func (r *Autoscaler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +func (a *Autoscaler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { var cm corev1.ConfigMap - if err := r.Get(ctx, req.NamespacedName, &cm); err != nil { + if err := a.Get(ctx, req.NamespacedName, &cm); err != nil { return ctrl.Result{}, fmt.Errorf("get: %w", err) } return ctrl.Result{}, nil } -func (r *Autoscaler) Start() { - for range time.Tick(r.Interval) { - if !r.LeaderElection.IsLeader.Load() { +func (a *Autoscaler) Start() { + for range time.Tick(a.Interval) { + if !a.LeaderElection.IsLeader.Load() { log.Println("Not leader, doing nothing") continue } @@ -81,11 +81,11 @@ func (r *Autoscaler) Start() { log.Println("Calculating scales for all") // TODO: Remove hardcoded Service lookup by name "lingo". - otherLingoEndpoints := r.Endpoints.GetAllHosts("lingo", "stats") + otherLingoEndpoints := a.Endpoints.GetAllHosts("lingo", "stats") stats, errs := aggregateStats(stats.Stats{ - ActiveRequests: r.Queues.TotalCounts(), - }, r.HTTPClient, otherLingoEndpoints) + ActiveRequests: a.Queues.TotalCounts(), + }, a.HTTPClient, otherLingoEndpoints) if len(errs) != 0 { for _, err := range errs { log.Printf("Failed to aggregate stats: %v", err) @@ -95,25 +95,25 @@ func (r *Autoscaler) Start() { for deploymentName, waitCount := range stats.ActiveRequests { log.Println("Is leader, autoscaling") - avg := r.getMovingAvgQueueSize(deploymentName) + avg := a.getMovingAvgQueueSize(deploymentName) avg.Next(float64(waitCount)) flt := avg.Calculate() - normalized := flt / float64(r.ConcurrencyPerReplica) + normalized := flt / float64(a.ConcurrencyPerReplica) ceil := math.Ceil(normalized) log.Printf("Average for deployment: %s: %v (ceil: %v), current wait count: %v", deploymentName, flt, ceil, waitCount) - r.Deployments.SetDesiredScale(deploymentName, int32(ceil)) + a.Deployments.SetDesiredScale(deploymentName, int32(ceil)) } } } -func (r *Autoscaler) getMovingAvgQueueSize(deploymentName string) *movingaverage.Simple { - r.movingAvgQueueSizeMtx.Lock() - a, ok := r.movingAvgQueueSize[deploymentName] +func (a *Autoscaler) getMovingAvgQueueSize(deploymentName string) *movingaverage.Simple { + a.movingAvgQueueSizeMtx.Lock() + a, ok := a.movingAvgQueueSize[deploymentName] if !ok { - a = movingaverage.NewSimple(make([]float64, r.AverageCount)) - r.movingAvgQueueSize[deploymentName] = a + a = movingaverage.NewSimple(make([]float64, a.AverageCount)) + a.movingAvgQueueSize[deploymentName] = a } - r.movingAvgQueueSizeMtx.Unlock() + a.movingAvgQueueSizeMtx.Unlock() return a }