Merge pull request #135 from markylaing/lint

Adds go linters and lint workflow

.github/workflows/tests.yml

@@ -8,6 +8,32 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
+  code-tests:
+    name: Code tests
+    runs-on: ubuntu-22.04
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v3
+
+      - name: Install Go
+        uses: actions/setup-go@v4
+        with:
+          go-version: 1.18.x
+
+      - name: Install dependencies
+        run: |
+          sudo add-apt-repository ppa:dqlite/dev -y --no-update
+          sudo apt-get update
+          sudo apt-get install --no-install-recommends -y libdqlite-dev pkg-config
+
+      - name: Update gomod
+        working-directory: microcloud
+        run: make update-gomod
+
+      - name: Run static analysis
+        working-directory: microcloud
+        run: make check-static
+
   system-tests:
     name: System tests
     runs-on: ubuntu-22.04

microcloud/.golangci.yml

@@ -0,0 +1,13 @@
+linters:
+  enable:
+    - gofmt
+    - misspell
+    - godot
+    - whitespace
+    - gci
+linters-settings:
+  gci:
+    sections:
+      - standard
+      - default
+      - prefix(github.com/canonical/microcloud/microcloud)

microcloud/Makefile

@@ -22,17 +22,14 @@ check-system:
 .PHONY: check-static
 check-static:
 ifeq ($(shell command -v golangci-lint 2> /dev/null),)
-	go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
-endif
-ifeq ($(shell command -v shellcheck 2> /dev/null),)
-	echo "Please install shellcheck"
-	exit 1
+	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $$HOME/go/bin
 endif
 ifeq ($(shell command -v revive 2> /dev/null),)
 	go install github.com/mgechev/revive@latest
 endif
 	golangci-lint run --timeout 5m
-	revive -set_exit_status ./...
+	revive -config revive.toml -exclude ./cmd/... -set_exit_status ./...
+	run-parts --exit-on-error --regex '.sh' test/lint
 
 # Update targets.
 .PHONY: update-gomod

microcloud/api/services.go

@@ -19,7 +19,7 @@ import (
 type endpointHandler func(*state.State, *http.Request) response.Response
 
 // authHandler ensures a request has been authenticated with the mDNS broadcast secret.
-func authHandler(sh *service.ServiceHandler, f endpointHandler) endpointHandler {
+func authHandler(sh *service.Handler, f endpointHandler) endpointHandler {
 	return func(s *state.State, r *http.Request) response.Response {
 		if r.RemoteAddr == "@" {
 			logger.Debug("Allowing unauthenticated request through unix socket")
@@ -45,7 +45,7 @@ func authHandler(sh *service.ServiceHandler, f endpointHandler) endpointHandler
 }
 
 // ServicesCmd represents the /1.0/services API on MicroCloud.
-var ServicesCmd = func(sh *service.ServiceHandler) rest.Endpoint {
+var ServicesCmd = func(sh *service.Handler) rest.Endpoint {
 	return rest.Endpoint{
 		AllowedBeforeInit: true,
 		Name:              "services",
@@ -78,7 +78,7 @@ func servicesPut(s *state.State, r *http.Request) response.Response {
 		addr = req.Address
 	}
 
-	sh, err := service.NewServiceHandler(s.Name(), addr, s.OS.StateDir, false, false, services...)
+	sh, err := service.NewHandler(s.Name(), addr, s.OS.StateDir, false, false, services...)
 	if err != nil {
 		return response.SmartError(err)
 	}

microcloud/api/services_proxy.go

@@ -17,17 +17,17 @@ import (
 )
 
 // LXDProxy proxies all requests from MicroCloud to LXD.
-func LXDProxy(sh *service.ServiceHandler) rest.Endpoint {
+func LXDProxy(sh *service.Handler) rest.Endpoint {
 	return proxy(sh, "lxd", "services/lxd/{rest:.*}", lxdHandler)
 }
 
 // CephProxy proxies all requests from MicroCloud to MicroCeph.
-func CephProxy(sh *service.ServiceHandler) rest.Endpoint {
+func CephProxy(sh *service.Handler) rest.Endpoint {
 	return proxy(sh, "microceph", "services/microceph/{rest:.*}", microHandler("microceph", MicroCephDir))
 }
 
 // OVNProxy proxies all requests from MicroCloud to MicroOVN.
-func OVNProxy(sh *service.ServiceHandler) rest.Endpoint {
+func OVNProxy(sh *service.Handler) rest.Endpoint {
 	return proxy(sh, "microovn", "services/microovn/{rest:.*}", microHandler("microovn", MicroOVNDir))
 }
 
@@ -41,7 +41,7 @@ const MicroCephDir = "/var/snap/microceph/common/state"
 const MicroOVNDir = "/var/snap/microovn/common/state"
 
 // proxy returns a proxy endpoint with the given handler and access applied to all REST methods.
-func proxy(sh *service.ServiceHandler, name, path string, handler endpointHandler) rest.Endpoint {
+func proxy(sh *service.Handler, name, path string, handler endpointHandler) rest.Endpoint {
 	return rest.Endpoint{
 		AllowedBeforeInit: true,
 		Name:              name,

microcloud/cmd/microcloud/add.go

@@ -67,7 +67,7 @@ func (c *cmdAdd) Run(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	s, err := service.NewServiceHandler(status.Name, addr, c.common.FlagMicroCloudDir, c.common.FlagLogDebug, c.common.FlagLogVerbose, services...)
+	s, err := service.NewHandler(status.Name, addr, c.common.FlagMicroCloudDir, c.common.FlagLogDebug, c.common.FlagLogVerbose, services...)
 	if err != nil {
 		return err
 	}

microcloud/cmd/microcloud/ask.go

@@ -44,7 +44,7 @@ func askRetry(question string, autoSetup bool, f func() error) {
 func askMissingServices(services []types.ServiceType, stateDirs map[types.ServiceType]string, autoSetup bool) ([]types.ServiceType, error) {
 	missingServices := []string{}
 	for serviceType, stateDir := range stateDirs {
-		if service.ServiceExists(serviceType, stateDir) {
+		if service.Exists(serviceType, stateDir) {
 			services = append(services, serviceType)
 		} else {
 			missingServices = append(missingServices, string(serviceType))
@@ -140,7 +140,7 @@ func askAddress(autoSetup bool, listenAddr string) (string, *net.IPNet, error) {
 	return listenAddr, subnet, nil
 }
 
-func askDisks(sh *service.ServiceHandler, peers map[string]mdns.ServerInfo, bootstrap bool, autoSetup bool, wipeAllDisks bool) (map[string][]lxdAPI.ClusterMemberConfigKey, map[string][]cephTypes.DisksPost, error) {
+func askDisks(sh *service.Handler, peers map[string]mdns.ServerInfo, bootstrap bool, autoSetup bool, wipeAllDisks bool) (map[string][]lxdAPI.ClusterMemberConfigKey, map[string][]cephTypes.DisksPost, error) {
 	if bootstrap {
 		// Add the local system to the list of peers so we can select disks.
 		peers[sh.Name] = mdns.ServerInfo{Name: sh.Name}
@@ -481,7 +481,7 @@ func askRemotePool(peerDisks map[string][]lxdAPI.ResourcesStorageDisk, autoSetup
 	return nil, fmt.Errorf("Unable to add remote storage pool: Each peer (minimum 3) must have allocated disks")
 }
 
-func askNetwork(sh *service.ServiceHandler, peers map[string]mdns.ServerInfo, lxdConfig map[string][]api.ClusterMemberConfigKey, bootstrap bool, autoSetup bool) (map[string]string, map[string]string, error) {
+func askNetwork(sh *service.Handler, peers map[string]mdns.ServerInfo, lxdConfig map[string][]api.ClusterMemberConfigKey, bootstrap bool, autoSetup bool) (map[string]string, map[string]string, error) {
 	// Automatic setup gets a basic fan setup.
 	if autoSetup {
 		return nil, nil, nil

microcloud/cmd/microcloud/main_init.go

@@ -61,7 +61,7 @@ func (c *cmdInit) Run(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("Failed to retrieve system hostname: %w", err)
 	}
 
-	if !c.flagAutoSetup {
+	if !c.flagAutoSetup { //nolint:staticcheck
 		// FIXME: MicroCeph does not currently support non-hostname cluster names.
 		// name, err = cli.AskString(fmt.Sprintf("Specify a name for this system [default=%s]: ", name), name, nil)
 		// if err != nil {
@@ -80,7 +80,7 @@ func (c *cmdInit) Run(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	s, err := service.NewServiceHandler(name, addr, c.common.FlagMicroCloudDir, c.common.FlagLogDebug, c.common.FlagLogVerbose, services...)
+	s, err := service.NewHandler(name, addr, c.common.FlagMicroCloudDir, c.common.FlagLogDebug, c.common.FlagLogVerbose, services...)
 	if err != nil {
 		return err
 	}
@@ -144,7 +144,7 @@ func (c *cmdInit) Run(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
-func lookupPeers(s *service.ServiceHandler, autoSetup bool, subnet *net.IPNet) (map[string]mdns.ServerInfo, error) {
+func lookupPeers(s *service.Handler, autoSetup bool, subnet *net.IPNet) (map[string]mdns.ServerInfo, error) {
 	header := []string{"NAME", "IFACE", "ADDR"}
 	var table *SelectableTable
 	var answers []string
@@ -161,7 +161,6 @@ func lookupPeers(s *service.ServiceHandler, autoSetup bool, subnet *net.IPNet) (
 
 			answers, err = table.GetSelections()
 			selectionCh <- err
-			return
 		}()
 	}
 
@@ -264,7 +263,7 @@ func lookupPeers(s *service.ServiceHandler, autoSetup bool, subnet *net.IPNet) (
 	return selectedPeers, nil
 }
 
-func AddPeers(sh *service.ServiceHandler, peers map[string]mdns.ServerInfo, localDisks map[string][]lxdAPI.ClusterMemberConfigKey, cephDisks map[string][]cephTypes.DisksPost) error {
+func AddPeers(sh *service.Handler, peers map[string]mdns.ServerInfo, localDisks map[string][]lxdAPI.ClusterMemberConfigKey, cephDisks map[string][]cephTypes.DisksPost) error {
 	joinConfig := make(map[string]types.ServicesPut, len(peers))
 	secrets := make(map[string]string, len(peers))
 	for peer, info := range peers {
@@ -370,7 +369,7 @@ func AddPeers(sh *service.ServiceHandler, peers map[string]mdns.ServerInfo, loca
 
 // waitForCluster will loop until the timeout has passed, or all cluster members for all services have reported that
 // their join process is complete.
-func waitForCluster(sh *service.ServiceHandler, secrets map[string]string, peers map[string]types.ServicesPut) error {
+func waitForCluster(sh *service.Handler, secrets map[string]string, peers map[string]types.ServicesPut) error {
 	cloud := sh.Services[types.MicroCloud].(*service.CloudService)
 	joinedChan := cloud.RequestJoin(context.Background(), secrets, peers)
 	timeAfter := time.After(5 * time.Minute)
@@ -411,7 +410,7 @@ func waitForCluster(sh *service.ServiceHandler, secrets map[string]string, peers
 	}
 }
 
-func postClusterSetup(bootstrap bool, sh *service.ServiceHandler, peers map[string]mdns.ServerInfo, lxdDisks map[string][]lxdAPI.ClusterMemberConfigKey, cephDisks map[string][]cephTypes.DisksPost, uplinkNetworks map[string]string, networkConfig map[string]string) error {
+func postClusterSetup(bootstrap bool, sh *service.Handler, peers map[string]mdns.ServerInfo, lxdDisks map[string][]lxdAPI.ClusterMemberConfigKey, cephDisks map[string][]cephTypes.DisksPost, uplinkNetworks map[string]string, networkConfig map[string]string) error {
 	cephTargets := map[string]string{}
 	for target := range cephDisks {
 		cephTargets[target] = peers[target].AuthSecret

microcloud/cmd/microcloud/selection_table.go

@@ -233,6 +233,7 @@ func (t *SelectableTable) prepareTemplate() {
 
 		return line
 	}
+
 	core.TemplateFuncsNoColor["add"] = core.TemplateFuncsWithColor["add"]
 	core.TemplateFuncsNoColor["scroll_hint_bot"] = core.TemplateFuncsWithColor["scroll_hint_bot"]
 	core.TemplateFuncsNoColor["scroll_hint_top"] = core.TemplateFuncsWithColor["scroll_hint_top"]
@@ -253,7 +254,6 @@ Up/down to move; right to select all; left to select none.`,
 	go func() {
 		err := survey.AskOne(t.prompt, &t.answers, survey.WithKeepFilter(true))
 		if err != nil && err.Error() != "please provide options to select from" {
-
 			t.askChan <- fmt.Errorf("Failed to confirm selection: %w", err)
 			return
 		}

microcloud/cmd/microcloud/sql.go

@@ -49,7 +49,7 @@ func (c *cmdSQL) Run(cmd *cobra.Command, args []string) error {
 	}
 
 	if dump != "" {
-		fmt.Printf(dump)
+		fmt.Print(dump)
 		return nil
 	}
 

microcloud/cmd/microcloudd/main.go

@@ -70,14 +70,14 @@ func (c *cmdDaemon) Run(cmd *cobra.Command, args []string) error {
 	}
 
 	for serviceType, stateDir := range optionalServices {
-		if service.ServiceExists(serviceType, stateDir) {
+		if service.Exists(serviceType, stateDir) {
 			services = append(services, serviceType)
 		} else {
 			logger.Infof("Skipping %s service, could not detect state directory", serviceType)
 		}
 	}
 
-	s, err := service.NewServiceHandler(name, addr, c.flagMicroCloudDir, c.global.flagLogDebug, c.global.flagLogVerbose, services...)
+	s, err := service.NewHandler(name, addr, c.flagMicroCloudDir, c.global.flagLogDebug, c.global.flagLogVerbose, services...)
 	if err != nil {
 		return err
 	}
@@ -98,8 +98,8 @@ func (c *cmdDaemon) Run(cmd *cobra.Command, args []string) error {
 					continue
 				}
 
-				if service.ServiceExists(serviceName, stateDir) {
-					newService, err := service.NewServiceHandler(name, addr, c.flagMicroCloudDir, false, false, serviceName)
+				if service.Exists(serviceName, stateDir) {
+					newService, err := service.NewHandler(name, addr, c.flagMicroCloudDir, false, false, serviceName)
 					if err != nil {
 						logger.Error("Failed to create servie handler for service", logger.Ctx{"service": serviceName, "error": err})
 						break

microcloud/mdns/lookup.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"io"
 	"log"
 	"net"
 	"strconv"
@@ -40,9 +39,7 @@ func (s ServerInfo) LookupKey() string {
 }
 
 // forwardingWriter forwards the mdns log message to LXD's logger package.
-type forwardingWriter struct {
-	w io.Writer
-}
+type forwardingWriter struct{}
 
 func (f forwardingWriter) Write(p []byte) (int, error) {
 	logMsg := string(p)
@@ -127,6 +124,7 @@ func LookupPeers(ctx context.Context, version string, localPeer string) (map[str
 	return peers, nil
 }
 
+// Lookup searches for the given service name over mdns.
 func Lookup(ctx context.Context, service string, size int) ([]*mdns.ServiceEntry, error) {
 	log.SetOutput(forwardingWriter{})
 	ctx, cancel := context.WithCancel(ctx)

microcloud/mdns/mdns.go

@@ -10,12 +10,13 @@ import (
 // ClusterService is the service name used for broadcasting willingness to join a cluster.
 const ClusterService = "_microcloud"
 
-// serviceSize is the maximum number of simultaneous broadcasts of the same mDNS service.
+// ServiceSize is the maximum number of simultaneous broadcasts of the same mDNS service.
 const ServiceSize = 10
 
 // clusterSize is the maximum number of cluster members we can find.
 const clusterSize = 1000
 
+// NewBroadcast returns a running mdns.Server which broadcasts the service at the given name and address.
 func NewBroadcast(name string, addr string, port int, service string, txt []byte) (*mdns.Server, error) {
 	var sendTXT []string
 	if txt != nil {

microcloud/revive.toml

@@ -0,0 +1,2 @@
+[rule.exported]
+    Arguments = ["checkPrivateReceivers"]

microcloud/service/lxd.go

@@ -235,7 +235,7 @@ func (s LXDService) Port() int {
 	return s.port
 }
 
-// AddLocalPools adds local zfs storage pools on the target peers, with the given source disks.
+// AddLocalPool adds local zfs storage pool on the target peers, with the given source disks.
 func (s *LXDService) AddLocalPool(source string, wipe bool) error {
 	c, err := s.client("")
 	if err != nil {
@@ -363,6 +363,7 @@ func (s *LXDService) GetResources(target string, address string, secret string)
 	return client.GetServerResources()
 }
 
+// GetUplinkInterfaces returns a map of peer name to slice of api.Network that may be used with OVN.
 func (s LXDService) GetUplinkInterfaces(bootstrap bool, peers map[string]mdns.ServerInfo) (map[string][]api.Network, error) {
 	clients := map[string]lxd.InstanceServer{}
 	networks := map[string][]api.Network{}
@@ -446,6 +447,7 @@ func (s LXDService) GetUplinkInterfaces(bootstrap bool, peers map[string]mdns.Se
 	return candidates, nil
 }
 
+// SetupNetwork configures LXD to use the OVN network uplink or to use a fan overlay if this is not available.
 func (s LXDService) SetupNetwork(uplinkNetworks map[string]string, networkConfig map[string]string) error {
 	client, err := s.client("")
 	if err != nil {

microcloud/service/lxd_join.go

@@ -16,6 +16,7 @@ func (s *LXDService) configFromToken(token string) (*api.ClusterPut, error) {
 	if err != nil {
 		return nil, fmt.Errorf("Invalid cluster join token: %w", err)
 	}
+
 	config := &api.ClusterPut{
 		Cluster:         api.Cluster{ServerName: s.name, Enabled: true},
 		ServerAddress:   util.CanonicalNetworkAddress(s.address, s.port),

microcloud/service/microcloud.go

@@ -60,7 +60,7 @@ func NewCloudService(ctx context.Context, name string, addr string, dir string,
 }
 
 // StartCloud launches the MicroCloud daemon with the appropriate hooks.
-func (s *CloudService) StartCloud(service *ServiceHandler, endpoints []rest.Endpoint) error {
+func (s *CloudService) StartCloud(service *Handler, endpoints []rest.Endpoint) error {
 	return s.client.Start(endpoints, nil, &config.Hooks{
 		OnBootstrap: func(s *state.State) error { return service.StopBroadcast() },
 		PostJoin:    func(s *state.State) error { return service.StopBroadcast() },

microcloud/service/microovn.go

@@ -47,7 +47,7 @@ func NewOVNService(ctx context.Context, name string, addr string, cloudDir strin
 	}, nil
 }
 
-// client returns a client to the OVN unix socket.
+// Client returns a client to the OVN unix socket.
 func (s OVNService) Client() (*client.Client, error) {
 	return s.m.LocalClient()
 }