Skip to content

Commit

Permalink
Format with gofumpt and check it in CI (#1076)
Browse files Browse the repository at this point in the history
This PR:

1. Formats the codebase with gofumpt
2. Changes the makefile fmt target to use gofumpt instead of go fmt
3. Adds a formatting job in CI to check that the codebase is formatted
   according to gofumpt

For more on gofumpt, see: https://github.com/mvdan/gofumpt

ref neondatabase/cloud#15747
  • Loading branch information
sharnoff authored Nov 4, 2024
1 parent 233360e commit e5b6faf
Show file tree
Hide file tree
Showing 28 changed files with 139 additions and 99 deletions.
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ linters:
- staticcheck # some rules from staticcheck.io
- typecheck # typechecks code, like the compiler
- unused # checks for unused constants/variables/functions/types
- gofumpt # Formatter.

# explicitly enabled:
- asciicheck # all identifiers are ASCII
Expand Down
10 changes: 6 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ endif
# https://github.com/neondatabase/autoscaling/pull/130#issuecomment-1496276620
export GOFLAGS=-buildvcs=false

GOFUMPT_VERSION ?= v0.7.0

# Setting SHELL to bash allows bash commands to be executed by recipes.
# Options are set to exit when a recipe line exits non-zero or a piped command fails.
SHELL = /usr/bin/env bash -o pipefail
Expand Down Expand Up @@ -108,7 +110,7 @@ generate: ## Generate boilerplate DeepCopy methods, manifests, and Go client

.PHONY: fmt
fmt: ## Run go fmt against code.
go fmt ./...
go run mvdan.cc/gofumpt@${GOFUMPT_VERSION} -w .

.PHONY: vet
vet: ## Run go vet against code.
Expand All @@ -119,7 +121,7 @@ vet: ## Run go vet against code.

TESTARGS ?= ./...
.PHONY: test
test: fmt vet envtest ## Run tests.
test: vet envtest ## Run tests.
# chmodding KUBEBUILDER_ASSETS dir to make it deletable by owner,
# otherwise it fails with actions/checkout on self-hosted GitHub runners
# ref: https://github.com/kubernetes-sigs/controller-runtime/pull/2245
Expand All @@ -132,7 +134,7 @@ test: fmt vet envtest ## Run tests.
##@ Build

.PHONY: build
build: fmt vet bin/vm-builder ## Build all neonvm binaries.
build: vet bin/vm-builder ## Build all neonvm binaries.
GOOS=linux go build -o bin/controller neonvm/main.go
GOOS=linux go build -o bin/vxlan-controller neonvm/tools/vxlan/controller/main.go
GOOS=linux go build -o bin/runner neonvm/runner/*.go
Expand All @@ -141,7 +143,7 @@ build: fmt vet bin/vm-builder ## Build all neonvm binaries.
bin/vm-builder: ## Build vm-builder binary.
GOOS=linux CGO_ENABLED=0 go build -o bin/vm-builder -ldflags "-X main.Version=${GIT_INFO} -X main.NeonvmDaemonImage=${IMG_DAEMON}" vm-builder/main.go
.PHONY: run
run: fmt vet ## Run a controller from your host.
run: vet ## Run a controller from your host.
go run ./neonvm/main.go

.PHONY: lint
Expand Down
8 changes: 4 additions & 4 deletions neonvm-runner/cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func getLines(input []byte, commentMarker []byte) [][]byte {
lines := bytes.Split(input, []byte("\n"))
var output [][]byte
for _, currentLine := range lines {
var commentIndex = bytes.Index(currentLine, commentMarker)
commentIndex := bytes.Index(currentLine, commentMarker)
if commentIndex == -1 {
output = append(output, currentLine)
} else {
Expand Down Expand Up @@ -335,7 +335,7 @@ func createISO9660runtime(
}
}

outputFile, err := os.OpenFile(diskPath, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0644)
outputFile, err := os.OpenFile(diskPath, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0o644)
if err != nil {
return err
}
Expand Down Expand Up @@ -534,7 +534,7 @@ func createISO9660FromPath(logger *zap.Logger, diskName string, diskPath string,
}
}

outputFile, err := os.OpenFile(diskPath, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0644)
outputFile, err := os.OpenFile(diskPath, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0o644)
if err != nil {
return err
}
Expand Down Expand Up @@ -1733,7 +1733,7 @@ func defaultNetwork(logger *zap.Logger, cidr string, ports []vmv1.Port) (mac.MAC
// Adding VM's IP address to the /etc/hosts, so we can access it easily from
// the pod. This is particularly useful for ssh into the VM from the runner
// pod.
f, err := os.OpenFile("/etc/hosts", os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
f, err := os.OpenFile("/etc/hosts", os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644)
if err != nil {
return nil, err
}
Expand Down
6 changes: 1 addition & 5 deletions neonvm-vxlan-controller/cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ const (
extraNetCidr = "10.100.0.0/16"
)

var (
deleteIfaces = flag.Bool("delete", false, `delete VXLAN interfaces`)
)
var deleteIfaces = flag.Bool("delete", false, `delete VXLAN interfaces`)

func main() {
flag.Parse()
Expand Down Expand Up @@ -192,7 +190,6 @@ func createVxlanInterface(name string, vxlanID int, ownIP string, bridgeName str
}

func updateFDB(vxlanName string, nodeIPs []string, ownIP string) error {

broadcastFdbMac, _ := net.ParseMAC("00:00:00:00:00:00")

// get vxlan interface details
Expand Down Expand Up @@ -248,7 +245,6 @@ func deleteLink(name string) error {
}

func upsertIptablesRules() error {

// manage iptables
ipt, err := iptables.New(iptables.IPFamily(iptables.ProtocolIPv4), iptables.Timeout(5))
if err != nil {
Expand Down
13 changes: 7 additions & 6 deletions neonvm/apis/neonvm/v1/webhook_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,13 @@ import (
// These tests use Ginkgo (BDD-style Go testing framework). Refer to
// http://onsi.github.io/ginkgo/ to learn more about Ginkgo.

var cfg *rest.Config
var k8sClient client.Client
var testEnv *envtest.Environment
var ctx context.Context
var cancel context.CancelFunc
var (
cfg *rest.Config
k8sClient client.Client
testEnv *envtest.Environment
ctx context.Context
cancel context.CancelFunc
)

func TestAPIs(t *testing.T) {
RegisterFailHandler(Fail)
Expand Down Expand Up @@ -127,7 +129,6 @@ var _ = BeforeSuite(func() {
conn.Close()
return nil
}).Should(Succeed())

})

var _ = AfterSuite(func() {
Expand Down
1 change: 0 additions & 1 deletion pkg/agent/billing/billing.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ func (mc *MetricsCollector) Run(
store VMStoreForNode,
metrics PromMetrics,
) error {

collectTicker := time.NewTicker(time.Second * time.Duration(mc.conf.CollectEverySeconds))
defer collectTicker.Stop()
// Offset by half a second, so it's a bit more deterministic.
Expand Down
2 changes: 2 additions & 0 deletions pkg/agent/billing/indexedstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,12 @@ func (i *VMNodeIndex) Add(vm *vmapi.VirtualMachine) {
i.forNode[vm.UID] = vm
}
}

func (i *VMNodeIndex) Update(oldVM, newVM *vmapi.VirtualMachine) {
i.Delete(oldVM)
i.Add(newVM)
}

func (i *VMNodeIndex) Delete(vm *vmapi.VirtualMachine) {
// note: delete is a no-op if the key isn't present.
delete(i.forNode, vm.UID)
Expand Down
6 changes: 4 additions & 2 deletions pkg/agent/billing/prommetrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,10 @@ func (m PromMetrics) forBatch() batchMetrics {
}
}

type isEndpointFlag bool
type autoscalingEnabledFlag bool
type (
isEndpointFlag bool
autoscalingEnabledFlag bool
)

func (b batchMetrics) inc(isEndpoint isEndpointFlag, autoscalingEnabled autoscalingEnabledFlag, phase vmapi.VmPhase) {
key := batchMetricsLabels{
Expand Down
1 change: 0 additions & 1 deletion pkg/agent/core/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1495,7 +1495,6 @@ func TestDownscalePivotBack(t *testing.T) {
a.Do(state.UpdateSystemMetrics, newMetrics)
a.Call(getDesiredResources, state, clock.Now()).
Equals(resForCU(2))

}
}

Expand Down
8 changes: 5 additions & 3 deletions pkg/agent/core/testhelpers/construct.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,11 @@ func CreateVmInfo(config InitialVmInfoConfig, opts ...VmInfoOpt) api.VmInfo {
return vm
}

type coreConfigModifier func(*core.Config)
type vmInfoConfigModifier func(*InitialVmInfoConfig)
type vmInfoModifier func(InitialVmInfoConfig, *api.VmInfo)
type (
coreConfigModifier func(*core.Config)
vmInfoConfigModifier func(*InitialVmInfoConfig)
vmInfoModifier func(InitialVmInfoConfig, *api.VmInfo)
)

var (
_ VmInfoOpt = vmInfoConfigModifier(nil)
Expand Down
6 changes: 4 additions & 2 deletions pkg/agent/prommetrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,10 @@ const (

// Copied bucket values from controller runtime latency metric. We can
// adjust them in the future if needed.
var buckets = []float64{0.005, 0.01, 0.025, 0.05, 0.1, 0.15, 0.2, 0.25, 0.3, 0.35, 0.4, 0.45, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0,
1.25, 1.5, 1.75, 2.0, 2.5, 3.0, 3.5, 4.0, 4.5, 5, 6, 7, 8, 9, 10, 15, 20, 25, 30, 40, 50, 60}
var buckets = []float64{
0.005, 0.01, 0.025, 0.05, 0.1, 0.15, 0.2, 0.25, 0.3, 0.35, 0.4, 0.45, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0,
1.25, 1.5, 1.75, 2.0, 2.5, 3.0, 3.5, 4.0, 4.5, 5, 6, 7, 8, 9, 10, 15, 20, 25, 30, 40, 50, 60,
}

func makeGlobalMetrics() (GlobalMetrics, *prometheus.Registry) {
reg := prometheus.NewRegistry()
Expand Down
1 change: 0 additions & 1 deletion pkg/agent/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,6 @@ func (r *Runner) doNeonVMRequest(
// Also relevant: <https://github.com/neondatabase/autoscaling/issues/23>
_, err = r.global.vmClient.NeonvmV1().VirtualMachines(r.vmName.Namespace).
Patch(requestCtx, r.vmName.Name, ktypes.JSONPatchType, patchPayload, metav1.PatchOptions{})

if err != nil {
errMsg := util.RootError(err).Error()
// Some error messages contain the object name. We could try to filter them all out, but
Expand Down
1 change: 0 additions & 1 deletion pkg/api/vminfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ func NewVmMemInfo(memSlots vmapi.MemorySlots, memSlotSize resource.Quantity) VmM
Use: uint16(memSlots.Use),
SlotSize: Bytes(memSlotSize.Value()),
}

}

// VmConfig stores the autoscaling-specific "extra" configuration derived from labels and
Expand Down
2 changes: 0 additions & 2 deletions pkg/billing/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ func NewS3Client(ctx context.Context, cfg S3ClientConfig) (*S3Client, error) {
defer cancel()

s3Config, err := awsconfig.LoadDefaultConfig(ctx, awsconfig.WithRegion(cfg.Region))

if err != nil {
return nil, S3Error{Err: err}
}
Expand Down Expand Up @@ -189,7 +188,6 @@ func (c S3Client) send(ctx context.Context, payload []byte, _ TraceID) error {
Key: &key,
Body: r,
})

if err != nil {
return S3Error{Err: err}
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/neonvm/controllers/functests/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ import (
// These tests use Ginkgo (BDD-style Go testing framework). Refer to
// http://onsi.github.io/ginkgo/ to learn more about Ginkgo.

var cfg *rest.Config
var k8sClient client.Client
var testEnv *envtest.Environment
var (
cfg *rest.Config
k8sClient client.Client
testEnv *envtest.Environment
)

func TestAPIs(t *testing.T) {
RegisterFailHandler(Fail)
Expand Down Expand Up @@ -69,7 +71,6 @@ var _ = BeforeSuite(func() {
k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
Expect(err).NotTo(HaveOccurred())
Expect(k8sClient).NotTo(BeNil())

})

var _ = AfterSuite(func() {
Expand Down
1 change: 0 additions & 1 deletion pkg/neonvm/controllers/functests/vm_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import (

var _ = Describe("VirtualMachine controller", func() {
Context("VirtualMachine controller test", func() {

const VirtualMachineName = "test-virtualmachine"

ctx := context.Background()
Expand Down
6 changes: 4 additions & 2 deletions pkg/neonvm/controllers/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@ const OutcomeLabel = "outcome"
func MakeReconcilerMetrics() ReconcilerMetrics {
// Copied bucket values from controller runtime latency metric. We can
// adjust them in the future if needed.
buckets := []float64{0.005, 0.01, 0.025, 0.05, 0.1, 0.15, 0.2, 0.25, 0.3, 0.35, 0.4, 0.45, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0,
1.25, 1.5, 1.75, 2.0, 2.5, 3.0, 3.5, 4.0, 4.5, 5, 6, 7, 8, 9, 10, 15, 20, 25, 30, 40, 50, 60}
buckets := []float64{
0.005, 0.01, 0.025, 0.05, 0.1, 0.15, 0.2, 0.25, 0.3, 0.35, 0.4, 0.45, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0,
1.25, 1.5, 1.75, 2.0, 2.5, 3.0, 3.5, 4.0, 4.5, 5, 6, 7, 8, 9, 10, 15, 20, 25, 30, 40, 50, 60,
}

m := ReconcilerMetrics{
failing: util.RegisterMetric(metrics.Registry, prometheus.NewGaugeVec(
Expand Down
Loading

0 comments on commit e5b6faf

Please sign in to comment.