Use golangci-lint (#15)

This commit is contained in:
Nikolay Edigaryev 2023-01-31 22:22:28 +04:00 committed by GitHub
parent 92e8732d46
commit 6bcc02d815
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 243 additions and 80 deletions

4
.cirrus.star Normal file
View File

@ -0,0 +1,4 @@
load("github.com/cirrus-modules/golang@main", "lint_task")
def main(ctx):
return [lint_task()]

125
.golangci.yml Normal file
View File

@ -0,0 +1,125 @@
run:
timeout: 5m
linters-settings:
# Even in Rust you can get away with partial matching,
# so make sure that the linter respects the programmer's
# intent expressed in the form of "default" case.
exhaustive:
default-signifies-exhaustive: true
linters:
enable:
- asciicheck
- bodyclose
- deadcode
- depguard
- dupl
- errcheck
- exhaustive
- exportloopref
- gochecknoinits
- gocognit
- goconst
- gocritic
- gocyclo
- godot
- godox
- goerr113
- gofmt
- goheader
- golint
- gomodguard
- goprintffuncname
- gosec
- gosimple
- govet
- ifshort
- ineffassign
- interfacer
- lll
- makezero
- misspell
- nakedret
- nestif
- noctx
- nolintlint
- predeclared
- rowserrcheck
- scopelint
- sqlclosecheck
- staticcheck
- structcheck
- stylecheck
- testpackage
- tparallel
- typecheck
- unconvert
- unparam
- unused
- varcheck
- whitespace
disable:
# Messages like "struct of size 104 bytes could be of size 96 bytes" from a package
# that was last updated 2 years ago[1] are barely helpful.
#
# After all, we're writing the code for other people, so let's trust the compiler here (that's
# constantly evolving compared to this linter) and revisit this if memory usage becomes a problem.
#
# [1]: https://github.com/mdempsky/maligned/commit/6e39bd26a8c8b58c5a22129593044655a9e25959
- maligned
# We don't have high-performance requirements at this moment, so sacrificing
# the code readability for marginal performance gains is not worth it.
- prealloc
# New linters that require a lot of codebase churn and noise, but perhaps we can enable them in the future.
- nlreturn
- wrapcheck
- errorlint
# Unfortunately, we use globals due to how spf13/cobra works.
- gochecknoglobals
# That's fine that some Proto objects don't have all fields initialized
- exhaustivestruct
# Style linters that are total nuts.
- wsl
- gofumpt
- goimports
- funlen
# This conflicts with the Protocol Buffers Version 3 design,
# which is largely based on default values for struct fields.
- exhaustivestruct
# Enough parallelism for now.
- paralleltest
# Ill-based assumptions about identifiers like fmt.Println without taking context into account.
- forbidigo
# Advantages of using t.Helper() are too small to waste developer's cognitive stamina on it.
- thelper
# Too restrictive defaults, plus there's already a gocyclo linter in place.
- cyclop
# Gives false positives for textbook examples[1][2]
# [1]: https://github.com/charithe/durationcheck/issues/7
# [2]: https://golang.org/pkg/time/ (see "To convert an integer number of units to a Duration, multiply:")
- durationcheck
# No way to disable the "exported" check for the whole project[1]
# [1]: https://github.com/mgechev/revive/issues/244#issuecomment-560512162
- revive
# Unfortunately too much false-positives, e.g. for a 0700 umask or number 10 when using strconv.FormatInt()
- gomnd
issues:
# Don't hide multiple issues that belong to one class since GitHub annotations can handle them all nicely.
max-issues-per-linter: 0
max-same-issues: 0

View File

@ -1,4 +1,4 @@
package delete
package deletecmd
import (
"github.com/spf13/cobra"

View File

@ -1,4 +1,4 @@
package delete
package deletecmd
import (
"github.com/cirruslabs/orchard/pkg/client"

View File

@ -3,7 +3,7 @@ package command
import (
"github.com/cirruslabs/orchard/internal/command/controller"
"github.com/cirruslabs/orchard/internal/command/create"
deletepkg "github.com/cirruslabs/orchard/internal/command/delete"
deletepkg "github.com/cirruslabs/orchard/internal/command/deletecmd"
"github.com/cirruslabs/orchard/internal/command/dev"
"github.com/cirruslabs/orchard/internal/command/list"
"github.com/cirruslabs/orchard/internal/command/worker"

View File

@ -92,7 +92,7 @@ func (controller *Controller) getVM(ctx *gin.Context) responder.Responder {
})
}
func (controller *Controller) listVMs(ctx *gin.Context) responder.Responder {
func (controller *Controller) listVMs(_ *gin.Context) responder.Responder {
return controller.storeView(func(txn *storepkg.Txn) responder.Responder {
vms, err := txn.ListVMs()
if err != nil {

View File

@ -91,7 +91,7 @@ func (controller *Controller) getWorker(ctx *gin.Context) responder.Responder {
})
}
func (controller *Controller) listWorkers(ctx *gin.Context) responder.Responder {
func (controller *Controller) listWorkers(_ *gin.Context) responder.Responder {
return controller.storeView(func(txn *storepkg.Txn) responder.Responder {
workers, err := txn.ListWorkers()
if err != nil {

View File

@ -3,13 +3,17 @@ package controller
import (
"context"
"crypto/tls"
"errors"
"fmt"
storepkg "github.com/cirruslabs/orchard/internal/controller/store"
"go.uber.org/zap"
"net"
"net/http"
"time"
)
var ErrInitFailed = errors.New("controller initialization failed")
type Controller struct {
dataDir string
listenAddr string
@ -30,7 +34,8 @@ func New(opts ...Option) (*Controller, error) {
// Apply defaults
if controller.dataDir == "" {
return nil, fmt.Errorf("%w: please specify the data directory path with WithDataDir()")
return nil, fmt.Errorf("%w: please specify the data directory path with WithDataDir()",
ErrInitFailed)
}
if controller.listenAddr == "" {
controller.listenAddr = ":6120"
@ -57,7 +62,8 @@ func New(opts ...Option) (*Controller, error) {
}
controller.httpServer = &http.Server{
Handler: controller.initAPI(),
Handler: controller.initAPI(),
ReadTimeout: 5 * time.Second,
}
return controller, nil
@ -85,7 +91,10 @@ func (controller *Controller) Run(ctx context.Context) error {
// A helper function to shut down the HTTP server on context cancellation
go func() {
<-ctx.Done()
controller.httpServer.Shutdown(ctx)
if err := controller.httpServer.Shutdown(ctx); err != nil {
controller.logger.Errorf("failed to cleanly shutdown the HTTP server: %v", err)
}
}()
if err := controller.httpServer.Serve(controller.listener); err != nil {

View File

@ -5,11 +5,3 @@ import "path/filepath"
func (controller *Controller) dbPath() string {
return filepath.Join(controller.dataDir, "db")
}
func (controller *Controller) caCertPath() string {
return filepath.Join(controller.dataDir, "ca.crt")
}
func (controller *Controller) caKeyPath() string {
return filepath.Join(controller.dataDir, "ca.key")
}

View File

@ -9,14 +9,14 @@ import (
const janitorInterval = 5 * time.Second
func (controller *Controller) runJanitor(store *storepkg.Store) error {
ticker := time.Tick(janitorInterval)
ticker := time.NewTicker(janitorInterval)
for {
if err := controller.runJanitorInner(store); err != nil {
return err
}
<-ticker
<-ticker.C
}
}
@ -34,7 +34,9 @@ func (controller *Controller) runJanitorInner(store *storepkg.Store) error {
}
for _, worker := range workers {
if time.Now().Sub(worker.LastSeen).Minutes() > 1 {
worker := worker
if time.Since(worker.LastSeen).Minutes() > 1 {
controller.logger.Debugf("removing outdated worker %s", worker.Name)
err := store.Update(func(txn *storepkg.Txn) error {

View File

@ -10,14 +10,14 @@ import (
const schedulerInterval = 5 * time.Second
func runScheduler(store *storepkg.Store) error {
ticker := time.Tick(schedulerInterval)
ticker := time.NewTicker(schedulerInterval)
for {
if err := runSchedulerInner(store); err != nil {
return err
}
<-ticker
<-ticker.C
}
}
@ -49,12 +49,16 @@ func runSchedulerInner(store *storepkg.Store) error {
})
for _, vm := range vms {
vm := vm
if vm.Worker != "" {
continue
}
// Find an appropriate worker to run this VM on
for _, worker := range workers {
worker := worker
vm.Worker = worker.Name
err := store.Update(func(txn *storepkg.Txn) error {

View File

@ -1,3 +1,4 @@
//nolint:dupl // maybe we'll figure out how to make DB resource accessors generic in the future
package store
import (

View File

@ -1,3 +1,4 @@
//nolint:dupl // maybe we'll figure out how to make DB resource accessors generic in the future
package store
import (

View File

@ -66,7 +66,7 @@ func (vm *VM) Close() error {
_, _, err := Tart(context.Background(), "delete", vm.id)
if err != nil {
return fmt.Errorf("failed to delete VM %s: %v", vm.id, err)
return fmt.Errorf("%w: failed to delete VM %s: %v", ErrFailed, vm.id, err)
}
return nil

View File

@ -1,10 +1,13 @@
package vmmanager
import (
"errors"
"fmt"
v1 "github.com/cirruslabs/orchard/pkg/resource/v1"
)
var ErrFailed = errors.New("VM manager failed")
type VMManager struct {
vms map[string]*VM
}
@ -24,7 +27,7 @@ func (vmm *VMManager) Exists(vmResource *v1.VM) bool {
func (vmm *VMManager) Get(vmResource *v1.VM) (*VM, error) {
managedVM, ok := vmm.vms[vmResource.UID]
if !ok {
return nil, fmt.Errorf("VM does not exist")
return nil, fmt.Errorf("%w: VM does not exist", ErrFailed)
}
return managedVM, nil
@ -32,7 +35,7 @@ func (vmm *VMManager) Get(vmResource *v1.VM) (*VM, error) {
func (vmm *VMManager) Create(vmResource *v1.VM) (*VM, error) {
if _, ok := vmm.vms[vmResource.UID]; ok {
return nil, fmt.Errorf("VM already exists")
return nil, fmt.Errorf("%w: VM already exists", ErrFailed)
}
managedVM := NewVM(vmResource)
@ -45,7 +48,7 @@ func (vmm *VMManager) Create(vmResource *v1.VM) (*VM, error) {
func (vmm *VMManager) Delete(vmResource *v1.VM) error {
managedVM, ok := vmm.vms[vmResource.UID]
if !ok {
return fmt.Errorf("VM does not exist")
return fmt.Errorf("%w: VM does not exist", ErrFailed)
}
if err := managedVM.Close(); err != nil {

View File

@ -59,14 +59,14 @@ func New(opts ...Option) (*Worker, error) {
}
func (worker *Worker) Run(ctx context.Context) error {
tickCh := time.Tick(pollInterval)
tickCh := time.NewTicker(pollInterval)
for {
if err := worker.registerWorker(ctx); err != nil {
worker.logger.Warnf("failed to register worker: %v", err)
select {
case <-tickCh:
case <-tickCh.C:
// continue
case <-ctx.Done():
return ctx.Err()
@ -80,7 +80,7 @@ func (worker *Worker) Run(ctx context.Context) error {
worker.logger.Errorf("failed to update worker resource: %v", err)
select {
case <-tickCh:
case <-tickCh.C:
// continue
case <-ctx.Done():
return ctx.Err()
@ -93,7 +93,7 @@ func (worker *Worker) Run(ctx context.Context) error {
worker.logger.Warnf("failed to sync VMs: %v", err)
select {
case <-tickCh:
case <-tickCh.C:
// continue
case <-ctx.Done():
return ctx.Err()
@ -103,7 +103,7 @@ func (worker *Worker) Run(ctx context.Context) error {
}
select {
case <-tickCh:
case <-tickCh.C:
// continue
case <-ctx.Done():
return ctx.Err()
@ -139,7 +139,8 @@ func (worker *Worker) updateWorker(ctx context.Context) error {
}
if workerResource.UID != worker.uid {
return fmt.Errorf("%w: our UID is %s, controller's ID is %s", ErrPollFailed)
return fmt.Errorf("%w: our UID is %s, controller's UID is %s",
ErrPollFailed, worker.uid, workerResource.UID)
}
worker.logger.Debugf("got worker from the API")
@ -147,7 +148,7 @@ func (worker *Worker) updateWorker(ctx context.Context) error {
workerResource.LastSeen = time.Now()
if err := worker.client.Workers().Update(ctx, workerResource); err != nil {
return fmt.Errorf("%w: failed to update worker in the API: %v", err)
return fmt.Errorf("%w: failed to update worker in the API: %v", ErrPollFailed, err)
}
worker.logger.Debugf("updated worker in the API")
@ -164,45 +165,63 @@ func (worker *Worker) syncVMs(ctx context.Context) error {
worker.logger.Infof("syncing %d VMs...", len(vms))
for _, vmResource := range vms {
vmResource := vmResource
if vmResource.Worker != worker.name {
continue
}
if !vmResource.DeletedAt.IsZero() {
worker.logger.Debugf("deleting VM %s (%s)", vmResource.Name, vmResource.UID)
// Delete VM locally, report to the controller
if worker.vmm.Exists(&vmResource) {
if err := worker.vmm.Delete(&vmResource); err != nil {
return err
}
}
if err := worker.client.VMs().Delete(ctx, vmResource.Name, true); err != nil {
return fmt.Errorf("%w: failed to delete VM %s (%s) from the API: %v",
ErrPollFailed, vmResource.Name, vmResource.UID, err)
}
worker.logger.Infof("deleted VM %s (%s)", vmResource.Name, vmResource.UID)
} else if !worker.vmm.Exists(&vmResource) {
worker.logger.Debugf("creating VM %s (%s)", vmResource.Name, vmResource.UID)
// Create or update VM locally, report to controller
_, err := worker.vmm.Create(&vmResource)
if err != nil {
if err := worker.deleteVM(ctx, vmResource); err != nil {
return err
}
vmResource.Status = v1.VMStatusRunning
if err := worker.client.VMs().Update(ctx, &vmResource); err != nil {
return fmt.Errorf("%w: failed to update VM %s (%s) in the API: %v",
ErrPollFailed, vmResource.Name, vmResource.UID, err)
} else if !worker.vmm.Exists(&vmResource) {
if err := worker.createVM(ctx, vmResource); err != nil {
return err
}
worker.logger.Infof("spawned VM %s (%s)", vmResource.Name, vmResource.UID)
}
}
return nil
}
func (worker *Worker) deleteVM(ctx context.Context, vmResource v1.VM) error {
worker.logger.Debugf("deleting VM %s (%s)", vmResource.Name, vmResource.UID)
// Delete VM locally, report to the controller
if worker.vmm.Exists(&vmResource) {
if err := worker.vmm.Delete(&vmResource); err != nil {
return err
}
}
if err := worker.client.VMs().Delete(ctx, vmResource.Name, true); err != nil {
return fmt.Errorf("%w: failed to delete VM %s (%s) from the API: %v",
ErrPollFailed, vmResource.Name, vmResource.UID, err)
}
worker.logger.Infof("deleted VM %s (%s)", vmResource.Name, vmResource.UID)
return nil
}
func (worker *Worker) createVM(ctx context.Context, vmResource v1.VM) error {
worker.logger.Debugf("creating VM %s (%s)", vmResource.Name, vmResource.UID)
// Create or update VM locally, report to controller
_, err := worker.vmm.Create(&vmResource)
if err != nil {
return err
}
vmResource.Status = v1.VMStatusRunning
if err := worker.client.VMs().Update(ctx, &vmResource); err != nil {
return fmt.Errorf("%w: failed to update VM %s (%s) in the API: %v",
ErrPollFailed, vmResource.Name, vmResource.UID, err)
}
worker.logger.Infof("spawned VM %s (%s)", vmResource.Name, vmResource.UID)
return nil
}

View File

@ -67,13 +67,13 @@ func (client *Client) request(
in interface{},
out interface{},
params map[string]string,
) (*http.Response, error) {
) error {
var body io.Reader
if in != nil {
jsonBytes, err := json.Marshal(in)
if err != nil {
return nil, fmt.Errorf("%w to marshal request body: %v", ErrFailed, err)
return fmt.Errorf("%w to marshal request body: %v", ErrFailed, err)
}
body = bytes.NewBuffer(jsonBytes)
@ -81,7 +81,7 @@ func (client *Client) request(
endpointURL, err := url.Parse("v1/" + path)
if err != nil {
return nil, fmt.Errorf("%w to parse API endpoint path: %v", ErrFailed, err)
return fmt.Errorf("%w to parse API endpoint path: %v", ErrFailed, err)
}
endpointURL = &url.URL{
@ -100,31 +100,34 @@ func (client *Client) request(
request, err := http.NewRequestWithContext(ctx, method, endpointURL.String(), body)
if err != nil {
return nil, fmt.Errorf("%w instantiate a request: %v", ErrFailed, err)
return fmt.Errorf("%w instantiate a request: %v", ErrFailed, err)
}
response, err := client.httpClient.Do(request)
if err != nil {
return nil, fmt.Errorf("%w to make a request: %v", ErrFailed, err)
return fmt.Errorf("%w to make a request: %v", ErrFailed, err)
}
defer func() {
_ = response.Body.Close()
}()
if response.StatusCode != http.StatusOK {
return nil, fmt.Errorf("%w to make a request: %d %s",
return fmt.Errorf("%w to make a request: %d %s",
ErrFailed, response.StatusCode, http.StatusText(response.StatusCode))
}
if out != nil {
bodyBytes, err := io.ReadAll(response.Body)
if err != nil {
return nil, fmt.Errorf("%w to read response body: %v", ErrFailed, err)
return fmt.Errorf("%w to read response body: %v", ErrFailed, err)
}
if err := json.Unmarshal(bodyBytes, out); err != nil {
return nil, fmt.Errorf("%w to unmarshal response body: %v", ErrFailed, err)
return fmt.Errorf("%w to unmarshal response body: %v", ErrFailed, err)
}
}
return response, nil
return nil
}
func (client *Client) Workers() *WorkersService {

View File

@ -12,7 +12,7 @@ type VMsService struct {
}
func (service *VMsService) Create(ctx context.Context, vm *v1.VM) error {
_, err := service.client.request(ctx, http.MethodPost, "vms",
err := service.client.request(ctx, http.MethodPost, "vms",
vm, nil, nil)
if err != nil {
return err
@ -24,7 +24,7 @@ func (service *VMsService) Create(ctx context.Context, vm *v1.VM) error {
func (service *VMsService) List(ctx context.Context) ([]v1.VM, error) {
var vms []v1.VM
_, err := service.client.request(ctx, http.MethodGet, "vms",
err := service.client.request(ctx, http.MethodGet, "vms",
nil, &vms, nil)
if err != nil {
return nil, err
@ -36,7 +36,7 @@ func (service *VMsService) List(ctx context.Context) ([]v1.VM, error) {
func (service *VMsService) Get(ctx context.Context, name string) (*v1.VM, error) {
var vm v1.VM
_, err := service.client.request(ctx, http.MethodGet, fmt.Sprintf("vms/%s", name),
err := service.client.request(ctx, http.MethodGet, fmt.Sprintf("vms/%s", name),
nil, &vm, nil)
if err != nil {
return nil, err
@ -46,7 +46,7 @@ func (service *VMsService) Get(ctx context.Context, name string) (*v1.VM, error)
}
func (service *VMsService) Update(ctx context.Context, vm *v1.VM) error {
_, err := service.client.request(ctx, http.MethodPut, fmt.Sprintf("vms/%s", vm.Name),
err := service.client.request(ctx, http.MethodPut, fmt.Sprintf("vms/%s", vm.Name),
vm, nil, nil)
if err != nil {
return err
@ -62,7 +62,7 @@ func (service *VMsService) Delete(ctx context.Context, name string, force bool)
params["force"] = "true"
}
_, err := service.client.request(ctx, http.MethodDelete, fmt.Sprintf("vms/%s", name),
err := service.client.request(ctx, http.MethodDelete, fmt.Sprintf("vms/%s", name),
nil, nil, params)
if err != nil {
return err

View File

@ -12,7 +12,7 @@ type WorkersService struct {
}
func (service *WorkersService) Create(ctx context.Context, worker *v1.Worker) (*v1.Worker, error) {
_, err := service.client.request(ctx, http.MethodPost, "workers",
err := service.client.request(ctx, http.MethodPost, "workers",
worker, &worker, nil)
if err != nil {
return nil, err
@ -24,7 +24,7 @@ func (service *WorkersService) Create(ctx context.Context, worker *v1.Worker) (*
func (service *WorkersService) List(ctx context.Context) ([]v1.Worker, error) {
var workers []v1.Worker
_, err := service.client.request(ctx, http.MethodGet, "workers",
err := service.client.request(ctx, http.MethodGet, "workers",
nil, &workers, nil)
if err != nil {
return nil, err
@ -36,7 +36,7 @@ func (service *WorkersService) List(ctx context.Context) ([]v1.Worker, error) {
func (service *WorkersService) Get(ctx context.Context, name string) (*v1.Worker, error) {
var worker v1.Worker
_, err := service.client.request(ctx, http.MethodGet, fmt.Sprintf("workers/%s", name),
err := service.client.request(ctx, http.MethodGet, fmt.Sprintf("workers/%s", name),
nil, &worker, nil)
if err != nil {
return nil, err
@ -46,7 +46,7 @@ func (service *WorkersService) Get(ctx context.Context, name string) (*v1.Worker
}
func (service *WorkersService) Update(ctx context.Context, worker *v1.Worker) error {
_, err := service.client.request(ctx, http.MethodPut, fmt.Sprintf("workers/%s", worker.Name),
err := service.client.request(ctx, http.MethodPut, fmt.Sprintf("workers/%s", worker.Name),
worker, nil, nil)
if err != nil {
return err
@ -56,7 +56,7 @@ func (service *WorkersService) Update(ctx context.Context, worker *v1.Worker) er
}
func (service *WorkersService) Delete(ctx context.Context, name string) error {
_, err := service.client.request(ctx, http.MethodDelete, fmt.Sprintf("workers/%s", name),
err := service.client.request(ctx, http.MethodDelete, fmt.Sprintf("workers/%s", name),
nil, nil, nil)
if err != nil {
return err