From 5f091035e4802f7c011538e7a633361836019bd8 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Fri, 29 Oct 2021 11:42:48 +0200 Subject: [PATCH 01/56] Fix file formatting with gofumpt -w -s Signed-off-by: Mateusz Gozdek --- cmd/lock.go | 30 ++++++++++++++---------------- cmd/unlock.go | 30 ++++++++++++++---------------- pkg/client/client_test.go | 2 -- 3 files changed, 28 insertions(+), 34 deletions(-) diff --git a/cmd/lock.go b/cmd/lock.go index bc20f32..e769a60 100644 --- a/cmd/lock.go +++ b/cmd/lock.go @@ -9,22 +9,20 @@ import ( "github.com/flatcar-linux/fleetlock/pkg/client" ) -var ( - lock = &cobra.Command{ - Use: "recursive-lock", - RunE: func(cmd *cobra.Command, args []string) error { - httpClient := http.DefaultClient +var lock = &cobra.Command{ + Use: "recursive-lock", + RunE: func(cmd *cobra.Command, args []string) error { + httpClient := http.DefaultClient - c, err := client.New(url, group, id, httpClient) - if err != nil { - return fmt.Errorf("building the client: %w", err) - } + c, err := client.New(url, group, id, httpClient) + if err != nil { + return fmt.Errorf("building the client: %w", err) + } - if err := c.RecursiveLock(); err != nil { - return fmt.Errorf("locking: %w", err) - } + if err := c.RecursiveLock(); err != nil { + return fmt.Errorf("locking: %w", err) + } - return nil - }, - } -) + return nil + }, +} diff --git a/cmd/unlock.go b/cmd/unlock.go index 19656c3..b0abfd8 100644 --- a/cmd/unlock.go +++ b/cmd/unlock.go @@ -9,22 +9,20 @@ import ( "github.com/flatcar-linux/fleetlock/pkg/client" ) -var ( - unlock = &cobra.Command{ - Use: "unlock-if-held", - RunE: func(cmd *cobra.Command, args []string) error { - httpClient := http.DefaultClient +var unlock = &cobra.Command{ + Use: "unlock-if-held", + RunE: func(cmd *cobra.Command, args []string) error { + httpClient := http.DefaultClient - c, err := client.New(url, group, id, httpClient) - if err != nil { - return fmt.Errorf("building the client: %w", err) - } + c, err := client.New(url, group, id, httpClient) + if err != nil { + return fmt.Errorf("building the client: %w", err) + } - if err := c.UnlockIfHeld(); err != nil { - return fmt.Errorf("unlocking: %w", err) - } + if err := c.UnlockIfHeld(); err != nil { + return fmt.Errorf("unlocking: %w", err) + } - return nil - }, - } -) + return nil + }, +} diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index ae48200..587eb3d 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -34,7 +34,6 @@ func TestBadURL(t *testing.T) { } func TestRecursiveLock(t *testing.T) { - expURL := "http://1.2.3.4/v1/pre-reboot" for _, test := range []struct { @@ -87,7 +86,6 @@ func TestRecursiveLock(t *testing.T) { } func TestUnlockIfHeld(t *testing.T) { - expURL := "http://1.2.3.4/v1/steady-state" for _, test := range []struct { From d8684ec2f5dde3aa53b1d492d5f45b1adbc296e1 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Fri, 29 Oct 2021 11:43:08 +0200 Subject: [PATCH 02/56] .golangci.yml: initial commit Add basic configuration which currently does not report any issues, so we can gradually re-enable remaining linters. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..00446e9 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,93 @@ +output: + sort-results: true + +issues: + exclude-use-default: false + max-same-issues: 0 + max-issues-per-linter: 0 + +linters-settings: + gci: + local-prefixes: github.com/flatcar-linux/fleetlock + +linters: + disable-all: false + disable: + # We do not have clearly defined error types yet. + - goerr113 + # We do not require all used structs to have all fields initialized. + - exhaustivestruct + # Temporarily disable reporting linters. + - errcheck + - govet + # This linters has been deprecated. + - interfacer + - maligned + - golint + enable: + - asciicheck + - bodyclose + - cyclop + - deadcode + - depguard + - dogsled + #- dupl + - durationcheck + #- errcheck + - errname + #- errorlint + - exhaustive + - exportloopref + - forbidigo + - forcetypeassert + - funlen + - gci + #- gochecknoglobals + #- gochecknoinits + - gocognit + - goconst + #- gocritic + - gocyclo + #- godot + - gofmt + - gofumpt + - goheader + - goimports + #- gomnd + - gomoddirectives + - gomodguard + - goprintffuncname + - ifshort + - importas + - ineffassign + #- lll + - makezero + #- misspell + - nakedret + - nestif + - nilerr + #- nlreturn + #- noctx + - nolintlint + #- paralleltest + - prealloc + - predeclared + - promlinter + #- revive + - rowserrcheck + - scopelint + - sqlclosecheck + - structcheck + #- stylecheck + #- tagliatelle + - testpackage + - thelper + - tparallel + - typecheck + - unconvert + - unparam + - varcheck + - wastedassign + - whitespace + - wrapcheck + #- wsl From 51eb3a78a79d2541d0be97264d93f9773c81d1d2 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Fri, 29 Oct 2021 11:45:40 +0200 Subject: [PATCH 03/56] .github/workflows/go.yml: rename build job to test As it only runs unit tests right now. Signed-off-by: Mateusz Gozdek --- .github/workflows/go.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index adbb7d1..ce013a3 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -8,7 +8,8 @@ on: jobs: - build: + test: + name: Test runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 From 649d5d436bba03bbf848f79fcce8cd7546e4ae52 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Fri, 29 Oct 2021 11:45:56 +0200 Subject: [PATCH 04/56] .github/workflows/go.yml: add lint job Running golangci-lint. Signed-off-by: Mateusz Gozdek --- .github/workflows/go.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index ce013a3..a5e004a 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -21,3 +21,14 @@ jobs: - name: Test run: go test -v ./... + + lint: + name: Lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + + - name: golangci-lint + uses: golangci/golangci-lint-action@v2 + with: + version: v1.42 From bfea7cfe005e4b6bfd3f96a8e98747e43422a73d Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Fri, 29 Oct 2021 11:48:49 +0200 Subject: [PATCH 05/56] pkg/client: fix error wrapping on error unmarshalling To re-enable errorlint linter. Signed-off-by: Mateusz Gozdek --- pkg/client/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 7568648..a1c4074 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -87,7 +87,7 @@ func handleResponse(resp *http.Response) error { e := &Error{} if err := json.Unmarshal(body, &e); err != nil { - return fmt.Errorf("unmarshalling error: %v", err) + return fmt.Errorf("unmarshalling error: %w", err) } return fmt.Errorf("fleetlock error: %s", e.String()) From 10f3403b8cf3f0307b71375ddf64dcf9d4ea3e72 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Fri, 29 Oct 2021 11:49:15 +0200 Subject: [PATCH 06/56] .golangci.yml: re-enable errorlint linter Signed-off-by: Mateusz Gozdek --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 00446e9..3a053f5 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -35,7 +35,7 @@ linters: - durationcheck #- errcheck - errname - #- errorlint + - errorlint - exhaustive - exportloopref - forbidigo From 157b7844d16e3d03dba80a0f976016bd4288d45e Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Fri, 29 Oct 2021 13:36:23 +0200 Subject: [PATCH 07/56] pkg/client: improve name of URL variable and struct field Also so we don't have a mix of exported struct fields and capitalized variable names. Signed-off-by: Mateusz Gozdek --- pkg/client/client.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index a1c4074..83515c3 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -35,12 +35,12 @@ type ClientParams struct { } // Client holds the params related to the host -// in order to interact with the Fleet-Lock URL. +// in order to interact with the Fleet-Lock server URL. type Client struct { - URL string - group string - id string - http HTTPClient + baseServerURL string + group string + id string + http HTTPClient } func (c *Client) generateRequest(endpoint string) (*http.Request, error) { @@ -57,7 +57,7 @@ func (c *Client) generateRequest(endpoint string) (*http.Request, error) { } j := bytes.NewReader(body) - req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("%s/%s", c.URL, endpoint), j) + req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("%s/%s", c.baseServerURL, endpoint), j) if err != nil { return nil, fmt.Errorf("building request: %w", err) } @@ -129,15 +129,15 @@ func (c *Client) UnlockIfHeld() error { } // New builds a Fleet-Lock client. -func New(URL, group, ID string, c HTTPClient) (*Client, error) { - if _, err := url.ParseRequestURI(URL); err != nil { +func New(baseServerURL, group, ID string, c HTTPClient) (*Client, error) { + if _, err := url.ParseRequestURI(baseServerURL); err != nil { return nil, fmt.Errorf("parsing URL: %w", err) } return &Client{ - URL: URL, - http: c, - group: group, - id: ID, + baseServerURL: baseServerURL, + http: c, + group: group, + id: ID, }, nil } From 828296775039ac89b03ecc160b2fe4adcfbb8938 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 15:29:20 +0100 Subject: [PATCH 08/56] pkg/client: rename variable ID -> id To avoid capitalized local variables and to enable gocritic linter. Signed-off-by: Mateusz Gozdek --- pkg/client/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 83515c3..f67e443 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -129,7 +129,7 @@ func (c *Client) UnlockIfHeld() error { } // New builds a Fleet-Lock client. -func New(baseServerURL, group, ID string, c HTTPClient) (*Client, error) { +func New(baseServerURL, group, id string, c HTTPClient) (*Client, error) { if _, err := url.ParseRequestURI(baseServerURL); err != nil { return nil, fmt.Errorf("parsing URL: %w", err) } @@ -138,6 +138,6 @@ func New(baseServerURL, group, ID string, c HTTPClient) (*Client, error) { baseServerURL: baseServerURL, http: c, group: group, - id: ID, + id: id, }, nil } From a674d0d23eec01145b504f9af5c38c43cd6e016e Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 15:30:20 +0100 Subject: [PATCH 09/56] .golangci.yml: enable gocritic linter Now that all issues reported by it has been resolved. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 3a053f5..1ee4073 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -46,7 +46,7 @@ linters: #- gochecknoinits - gocognit - goconst - #- gocritic + - gocritic - gocyclo #- godot - gofmt From 8985817cf2e4b318c0615835bb28eff345d33b9a Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 15:31:21 +0100 Subject: [PATCH 10/56] pkg/client: run all tests in parallel To enable paralleltest linter. Signed-off-by: Mateusz Gozdek --- pkg/client/client_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 587eb3d..85a3253 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -23,6 +23,8 @@ func (h *httpClient) Do(req *http.Request) (*http.Response, error) { } func TestBadURL(t *testing.T) { + t.Parallel() + _, err := client.New("this is not an URL", "default", "1234", nil) if err == nil { t.Fatalf("should get an error") @@ -34,6 +36,8 @@ func TestBadURL(t *testing.T) { } func TestRecursiveLock(t *testing.T) { + t.Parallel() + expURL := "http://1.2.3.4/v1/pre-reboot" for _, test := range []struct { @@ -86,6 +90,8 @@ func TestRecursiveLock(t *testing.T) { } func TestUnlockIfHeld(t *testing.T) { + t.Parallel() + expURL := "http://1.2.3.4/v1/steady-state" for _, test := range []struct { From d8c467dac01f8726adfa7cdd393f590c32c6e415 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 15:31:36 +0100 Subject: [PATCH 11/56] .golangci.yml: enable paralleltest linter As it reports no issues anymore. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 1ee4073..b419f37 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -69,7 +69,7 @@ linters: #- nlreturn #- noctx - nolintlint - #- paralleltest + - paralleltest - prealloc - predeclared - promlinter From 0bfa3bbebc50430976dacec557e8a5bc0e69536d Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 15:33:39 +0100 Subject: [PATCH 12/56] pkg/client: add missing dots in comments Signed-off-by: Mateusz Gozdek --- pkg/client/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index f67e443..f82dcc3 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -98,7 +98,7 @@ func handleResponse(resp *http.Response) error { return nil } -// RecursiveLock tries to reserve (lock) a slot for rebooting +// RecursiveLock tries to reserve (lock) a slot for rebooting. func (c *Client) RecursiveLock() error { req, err := c.generateRequest("v1/pre-reboot") if err != nil { @@ -113,7 +113,7 @@ func (c *Client) RecursiveLock() error { return handleResponse(resp) } -// UnlockIfHeld tries to release (unlock) a slot that it was previously holding +// UnlockIfHeld tries to release (unlock) a slot that it was previously holding. func (c *Client) UnlockIfHeld() error { req, err := c.generateRequest("v1/steady-state") if err != nil { From 3bf6988abac21e9cbb55c2c1e06d6343a058722d Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 15:33:50 +0100 Subject: [PATCH 13/56] .golangci.yml: enable godot linter So all comments ends with dots. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index b419f37..b61ecfa 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -48,7 +48,7 @@ linters: - goconst - gocritic - gocyclo - #- godot + - godot - gofmt - gofumpt - goheader From 01e471978cfadcb7a786a5886b497862095e7ab2 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 15:36:40 +0100 Subject: [PATCH 14/56] pkg/client: improve formatting To be able to re-enable wsl linter. Signed-off-by: Mateusz Gozdek --- pkg/client/client.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/client/client.go b/pkg/client/client.go index f82dcc3..4123bd0 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -57,6 +57,7 @@ func (c *Client) generateRequest(endpoint string) (*http.Request, error) { } j := bytes.NewReader(body) + req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("%s/%s", c.baseServerURL, endpoint), j) if err != nil { return nil, fmt.Errorf("building request: %w", err) @@ -78,6 +79,7 @@ func handleResponse(resp *http.Response) error { case 3, 4, 5: // We try to extract an eventual error. r := bufio.NewReader(resp.Body) + body, err := ioutil.ReadAll(r) if err != nil { return fmt.Errorf("reading body: %w", err) From 24c0c4af1b5a671aeff7087b9992844df32b9c61 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 15:39:50 +0100 Subject: [PATCH 15/56] .golangci.yml: enable wsl linter Now that it does not report any issues. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index b61ecfa..50d7fcf 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -90,4 +90,4 @@ linters: - wastedassign - whitespace - wrapcheck - #- wsl + - wsl From 860d78c2485218fc51deffb8361405ba53055402 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 15:57:24 +0100 Subject: [PATCH 16/56] Consistently write FleetLock as "FleetLock" when possible This is the formatting used in https://coreos.github.io/zincati/development/fleetlock/protocol/ so I think this is the correct one which should be used when possible. Signed-off-by: Mateusz Gozdek --- README.md | 2 +- cmd/root.go | 6 +++--- pkg/client/client.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 4841d9c..cc6de15 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -## Fleetlock client +## FleetLock client [![Go Reference](https://pkg.go.dev/badge/github.com/flatcar-linux/fleetlock.svg)](https://pkg.go.dev/github.com/flatcar-linux/fleetlock) [![Go](https://github.com/flatcar-linux/fleetlock/actions/workflows/go.yml/badge.svg?branch=main)](https://github.com/flatcar-linux/fleetlock/actions/workflows/go.yml) diff --git a/cmd/root.go b/cmd/root.go index dd65678..51a0ca6 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -11,9 +11,9 @@ var ( ) func init() { - Root.PersistentFlags().StringVarP(&group, "group", "g", "default", "Fleet-Lock group") - Root.PersistentFlags().StringVarP(&id, "id", "i", "", "Fleet-Lock instance ID (/etc/machine-id for example)") - Root.PersistentFlags().StringVarP(&url, "url", "u", "", "Fleet-Lock endpoint URL") + Root.PersistentFlags().StringVarP(&group, "group", "g", "default", "FleetLock group") + Root.PersistentFlags().StringVarP(&id, "id", "i", "", "FleetLock instance ID (/etc/machine-id for example)") + Root.PersistentFlags().StringVarP(&url, "url", "u", "", "FleetLock endpoint URL") Root.AddCommand(lock) Root.AddCommand(unlock) diff --git a/pkg/client/client.go b/pkg/client/client.go index 4123bd0..e042e60 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -35,7 +35,7 @@ type ClientParams struct { } // Client holds the params related to the host -// in order to interact with the Fleet-Lock server URL. +// in order to interact with the FleetLock server URL. type Client struct { baseServerURL string group string @@ -130,7 +130,7 @@ func (c *Client) UnlockIfHeld() error { return handleResponse(resp) } -// New builds a Fleet-Lock client. +// New builds a FleetLock client. func New(baseServerURL, group, id string, c HTTPClient) (*Client, error) { if _, err := url.ParseRequestURI(baseServerURL); err != nil { return nil, fmt.Errorf("parsing URL: %w", err) From 5360680768fcdec2cea053a26542ca736ccb45a2 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 15:58:17 +0100 Subject: [PATCH 17/56] pkg/client: fix ClientParams documentation styling So stylecheck linter does not complain. Signed-off-by: Mateusz Gozdek --- pkg/client/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index e042e60..0bb97c2 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -25,7 +25,7 @@ type Payload struct { ClientParams *ClientParams `json:"client_params"` } -// Client params is the object holding the +// ClientParams is the object holding the // ID and the group for each client. type ClientParams struct { // ID is the client identifer. (e.g node name or UUID) From 2206f52a7846f3bffdad86b8cbf3f3ab1e4341e0 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 15:59:30 +0100 Subject: [PATCH 18/56] Add basic package comments So stylecheck linter does not complain. Signed-off-by: Mateusz Gozdek --- cmd/root.go | 1 + pkg/client/client.go | 1 + 2 files changed, 2 insertions(+) diff --git a/cmd/root.go b/cmd/root.go index 51a0ca6..b9f1d98 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -1,3 +1,4 @@ +// Package cmd implements fleetlockctl CLI. package cmd import ( diff --git a/pkg/client/client.go b/pkg/client/client.go index 0bb97c2..965bd09 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -1,3 +1,4 @@ +// Package client implements FleetLock client. package client import ( From b71ff231afb5c250099ad53aedca8b47d00482bb Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 15:59:46 +0100 Subject: [PATCH 19/56] .golangci.yml: enable stylecheck linter As all reported issues has been fixed. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 50d7fcf..6dffe77 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -78,7 +78,7 @@ linters: - scopelint - sqlclosecheck - structcheck - #- stylecheck + - stylecheck #- tagliatelle - testpackage - thelper From 2614160896cb10a553d03f99f17757d40e8e5d61 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 16:00:30 +0100 Subject: [PATCH 20/56] pkg/client: fix typo identifer -> identifier Signed-off-by: Mateusz Gozdek --- pkg/client/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 965bd09..48cfcd6 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -29,7 +29,7 @@ type Payload struct { // ClientParams is the object holding the // ID and the group for each client. type ClientParams struct { - // ID is the client identifer. (e.g node name or UUID) + // ID is the client identifier. (e.g node name or UUID) ID string `json:"id"` // Group is the reboot-group of the client. Group string `json:"group"` From f96890071853e0a48a6903fd00691516f20aa7c6 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 16:00:46 +0100 Subject: [PATCH 21/56] .golangci.yml: enable misspell linter As it does not find any more typos. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 6dffe77..b09f2c0 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -62,7 +62,7 @@ linters: - ineffassign #- lll - makezero - #- misspell + - misspell - nakedret - nestif - nilerr From 470441377a4ecfe19675259a3fe2c96258672b0a Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 16:02:07 +0100 Subject: [PATCH 22/56] pkg/client: fix too long line and duplicated error string in tests So lll linter can be enabled. Signed-off-by: Mateusz Gozdek --- pkg/client/client_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 85a3253..f275ebc 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -30,8 +30,9 @@ func TestBadURL(t *testing.T) { t.Fatalf("should get an error") } - if err != nil && err.Error() != "parsing URL: parse \"this is not an URL\": invalid URI for request" { - t.Fatalf("should have %s for the error, got: %s", "parsing URL: parse \"this is not an URL\": invalid URI for request", err.Error()) + expectedError := "parsing URL: parse \"this is not an URL\": invalid URI for request" + if err != nil && err.Error() != expectedError { + t.Fatalf("should have %s for the error, got: %s", expectedError, err.Error()) } } From eb6647057f08d1232ec2bbf3c668e867671b20ca Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 16:02:33 +0100 Subject: [PATCH 23/56] .golangci.yml: enable lll linter It does not report any issues anymore. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index b09f2c0..942550b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -60,7 +60,7 @@ linters: - ifshort - importas - ineffassign - #- lll + - lll - makezero - misspell - nakedret From e87e1b21735616130f21b25e84ce79a1dff002f8 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 16:03:32 +0100 Subject: [PATCH 24/56] pkg/client/client_test.go: improve formatting So nlreturn linter does not complain. Signed-off-by: Mateusz Gozdek --- pkg/client/client_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index f275ebc..72577b6 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -19,6 +19,7 @@ type httpClient struct { func (h *httpClient) Do(req *http.Request) (*http.Response, error) { h.r = req + return h.resp, h.doErr } From 465076d4d9ccf3b728f117d53dd0de575d36d830 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 16:03:54 +0100 Subject: [PATCH 25/56] .golangci.yml: enable nlreturn linter It does not report any issues anymore. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 942550b..051d89b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -66,7 +66,7 @@ linters: - nakedret - nestif - nilerr - #- nlreturn + - nlreturn #- noctx - nolintlint - paralleltest From 988ef1cf428cdb5133217c24ac5e66759af07298 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 16:10:33 +0100 Subject: [PATCH 26/56] pkg/client: rename ClientParams struct to just Params Otherwise revive linter complains with the following message: exported: type name will be used as client.ClientParams by other packages, and that stutters; consider calling this Params Signed-off-by: Mateusz Gozdek --- pkg/client/client.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 48cfcd6..8f8ccc7 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -23,12 +23,12 @@ type HTTPClient interface { type Payload struct { // ClientParams holds the parameters specific to the // FleetLock client. - ClientParams *ClientParams `json:"client_params"` + ClientParams *Params `json:"client_params"` } -// ClientParams is the object holding the +// Params is the object holding the // ID and the group for each client. -type ClientParams struct { +type Params struct { // ID is the client identifier. (e.g node name or UUID) ID string `json:"id"` // Group is the reboot-group of the client. @@ -46,7 +46,7 @@ type Client struct { func (c *Client) generateRequest(endpoint string) (*http.Request, error) { payload := &Payload{ - ClientParams: &ClientParams{ + ClientParams: &Params{ ID: c.id, Group: c.group, }, From a79a6a0017af9ea5a8398657c9af54317a364312 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 16:12:18 +0100 Subject: [PATCH 27/56] cmd: add comment for exported variable Root Even though it shouldn't really be exported as it makes it globally writable, but this fixes revive linter. Signed-off-by: Mateusz Gozdek --- cmd/root.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index b9f1d98..344aac3 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -6,9 +6,10 @@ import ( ) var ( - group, id, url string - + // Root implements fleetlockctl. Root = &cobra.Command{Use: "fleetlockctl"} + + group, id, url string ) func init() { From 3ed8192dbf0d0ddf1121d9f6ceef2943812bacb3 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 16:13:02 +0100 Subject: [PATCH 28/56] .golangci.yml: enable revive linter As it does not report any more issues. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 051d89b..0e43076 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -73,7 +73,7 @@ linters: - prealloc - predeclared - promlinter - #- revive + - revive - rowserrcheck - scopelint - sqlclosecheck From e5fbfdaf5d23b79e3027cfdfd22c555ceabbc93b Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 16:14:21 +0100 Subject: [PATCH 29/56] pkg/client: add nolint tag for client_params JSON field As it is required by FleetLock protocol but it is not a standard case for JSON format, so tagliatelle linter complains. Signed-off-by: Mateusz Gozdek --- pkg/client/client.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/client/client.go b/pkg/client/client.go index 8f8ccc7..a597e0b 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -23,6 +23,8 @@ type HTTPClient interface { type Payload struct { // ClientParams holds the parameters specific to the // FleetLock client. + // + //nolint:tagliatelle // FleetLock protocol requires exactly 'client_params' field. ClientParams *Params `json:"client_params"` } From 8d9945c36cce409397f38e0add98b3fc20e6cf22 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 16:15:10 +0100 Subject: [PATCH 30/56] .golangci.yml: enable tagliatelle linter It does not report any more issues. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 0e43076..3b5dfac 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -79,7 +79,7 @@ linters: - sqlclosecheck - structcheck - stylecheck - #- tagliatelle + - tagliatelle - testpackage - thelper - tparallel From aad369ae7d6dc275e785fec675ee9f47ae2663b0 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 16:18:59 +0100 Subject: [PATCH 31/56] cmd: don't use global variables and init functions As it is a bad practice. Signed-off-by: Mateusz Gozdek --- cmd/lock.go | 28 +++++++++++++++------------- cmd/root.go | 22 +++++++++++----------- cmd/unlock.go | 28 +++++++++++++++------------- main.go | 2 +- 4 files changed, 42 insertions(+), 38 deletions(-) diff --git a/cmd/lock.go b/cmd/lock.go index e769a60..e6590be 100644 --- a/cmd/lock.go +++ b/cmd/lock.go @@ -9,20 +9,22 @@ import ( "github.com/flatcar-linux/fleetlock/pkg/client" ) -var lock = &cobra.Command{ - Use: "recursive-lock", - RunE: func(cmd *cobra.Command, args []string) error { - httpClient := http.DefaultClient +func lock(group, id, url *string) *cobra.Command { + return &cobra.Command{ + Use: "recursive-lock", + RunE: func(cmd *cobra.Command, args []string) error { + httpClient := http.DefaultClient - c, err := client.New(url, group, id, httpClient) - if err != nil { - return fmt.Errorf("building the client: %w", err) - } + c, err := client.New(*url, *group, *id, httpClient) + if err != nil { + return fmt.Errorf("building the client: %w", err) + } - if err := c.RecursiveLock(); err != nil { - return fmt.Errorf("locking: %w", err) - } + if err := c.RecursiveLock(); err != nil { + return fmt.Errorf("locking: %w", err) + } - return nil - }, + return nil + }, + } } diff --git a/cmd/root.go b/cmd/root.go index 344aac3..8f324dd 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -5,18 +5,18 @@ import ( "github.com/spf13/cobra" ) -var ( - // Root implements fleetlockctl. - Root = &cobra.Command{Use: "fleetlockctl"} +// Command returns CLI command to be executed. +func Command() *cobra.Command { + cli := &cobra.Command{Use: "fleetlockctl"} - group, id, url string -) + var group, id, url string + + cli.PersistentFlags().StringVarP(&group, "group", "g", "default", "FleetLock group") + cli.PersistentFlags().StringVarP(&id, "id", "i", "", "FleetLock instance ID (/etc/machine-id for example)") + cli.PersistentFlags().StringVarP(&url, "url", "u", "", "FleetLock endpoint URL") -func init() { - Root.PersistentFlags().StringVarP(&group, "group", "g", "default", "FleetLock group") - Root.PersistentFlags().StringVarP(&id, "id", "i", "", "FleetLock instance ID (/etc/machine-id for example)") - Root.PersistentFlags().StringVarP(&url, "url", "u", "", "FleetLock endpoint URL") + cli.AddCommand(lock(&group, &id, &url)) + cli.AddCommand(unlock(&group, &id, &url)) - Root.AddCommand(lock) - Root.AddCommand(unlock) + return cli } diff --git a/cmd/unlock.go b/cmd/unlock.go index b0abfd8..b4f3693 100644 --- a/cmd/unlock.go +++ b/cmd/unlock.go @@ -9,20 +9,22 @@ import ( "github.com/flatcar-linux/fleetlock/pkg/client" ) -var unlock = &cobra.Command{ - Use: "unlock-if-held", - RunE: func(cmd *cobra.Command, args []string) error { - httpClient := http.DefaultClient +func unlock(group, id, url *string) *cobra.Command { + return &cobra.Command{ + Use: "unlock-if-held", + RunE: func(cmd *cobra.Command, args []string) error { + httpClient := http.DefaultClient - c, err := client.New(url, group, id, httpClient) - if err != nil { - return fmt.Errorf("building the client: %w", err) - } + c, err := client.New(*url, *group, *id, httpClient) + if err != nil { + return fmt.Errorf("building the client: %w", err) + } - if err := c.UnlockIfHeld(); err != nil { - return fmt.Errorf("unlocking: %w", err) - } + if err := c.UnlockIfHeld(); err != nil { + return fmt.Errorf("unlocking: %w", err) + } - return nil - }, + return nil + }, + } } diff --git a/main.go b/main.go index 28fd09e..18278b7 100644 --- a/main.go +++ b/main.go @@ -8,7 +8,7 @@ import ( ) func main() { - if err := cmd.Root.Execute(); err != nil { + if err := cmd.Command().Execute(); err != nil { fmt.Fprintf(os.Stderr, "unable to execute command: %v", err) } } From 3a3b997e75892572d96618b82d91c518d4b6f400 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 16:22:16 +0100 Subject: [PATCH 32/56] .golangci.yml: enable gochecknoglobals and gochecknoinits variables As we no longer use init or global variables. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 3b5dfac..e6af74d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -42,8 +42,8 @@ linters: - forcetypeassert - funlen - gci - #- gochecknoglobals - #- gochecknoinits + - gochecknoglobals + - gochecknoinits - gocognit - goconst - gocritic From 4fa57ee8d4f61c270807c3f11937c262f5c1e542 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 17:31:04 +0100 Subject: [PATCH 33/56] .golangci.yml: exclude false-positives from paralleltest linter Signed-off-by: Mateusz Gozdek --- .golangci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index e6af74d..29fee1b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -5,6 +5,11 @@ issues: exclude-use-default: false max-same-issues: 0 max-issues-per-linter: 0 + exclude-rules: + # False positive: https://github.com/kunwardeep/paralleltest/issues/8. + - linters: + - paralleltest + text: "does not use range value in test Run" linters-settings: gci: From 16a7d15fd41fa4e17fff97b2be7ab2f3deb20aff Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 17:31:26 +0100 Subject: [PATCH 34/56] pkg/client: refactor client tests to avoid cases duplication So dupl linter does not complain. Signed-off-by: Mateusz Gozdek --- pkg/client/client_test.go | 114 +++++++++++++++----------------------- 1 file changed, 45 insertions(+), 69 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 72577b6..79000a1 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -3,7 +3,7 @@ package client_test import ( "bytes" "errors" - "io" + "fmt" "io/ioutil" "net/http" "testing" @@ -37,15 +37,14 @@ func TestBadURL(t *testing.T) { } } -func TestRecursiveLock(t *testing.T) { +//nolint:funlen // Just many test cases. +func TestClient(t *testing.T) { t.Parallel() - expURL := "http://1.2.3.4/v1/pre-reboot" - for _, test := range []struct { statusCode int expErr error - body io.ReadCloser + body []byte doErr error }{ { @@ -54,12 +53,12 @@ func TestRecursiveLock(t *testing.T) { { statusCode: 500, expErr: errors.New("fleetlock error: this is an error (error_kind)"), - body: ioutil.NopCloser(bytes.NewReader([]byte(`{"kind":"error_kind","value":"this is an error"}`))), + body: []byte(`{"kind":"error_kind","value":"this is an error"}`), }, { statusCode: 500, expErr: errors.New("unmarshalling error: invalid character '\"' after object key:value pair"), - body: ioutil.NopCloser(bytes.NewReader([]byte(`{"kind":"error_kind" "value":"this is an error"}`))), + body: []byte(`{"kind":"error_kind" "value":"this is an error"}`), }, { statusCode: 100, @@ -70,77 +69,54 @@ func TestRecursiveLock(t *testing.T) { doErr: errors.New("connection refused"), }, } { - h := &httpClient{ - resp: &http.Response{ - StatusCode: test.statusCode, - Body: test.body, - }, - doErr: test.doErr, - } + test := test - c, _ := client.New("http://1.2.3.4", "default", "1234", h) + newClient := func(statusCode int, body []byte, doErr error) (*httpClient, *client.Client) { + h := &httpClient{ + resp: &http.Response{ + StatusCode: statusCode, + Body: ioutil.NopCloser(bytes.NewReader(body)), + }, + doErr: doErr, + } - err := c.RecursiveLock() - if err != nil && err.Error() != test.expErr.Error() { - t.Fatalf("should have %v for err, got: %v", test.expErr, err) - } + c, _ := client.New("http://1.2.3.4", "default", "1234", h) - if h.r.URL.String() != expURL { - t.Fatalf("should have %s for URL, got: %s", expURL, h.r.URL.String()) + return h, c } - } -} -func TestUnlockIfHeld(t *testing.T) { - t.Parallel() + t.Run(fmt.Sprintf("UnlockIfHeld_%d", test.statusCode), func(t *testing.T) { + t.Parallel() - expURL := "http://1.2.3.4/v1/steady-state" + h, c := newClient(test.statusCode, test.body, test.doErr) - for _, test := range []struct { - statusCode int - expErr error - body io.ReadCloser - doErr error - }{ - { - statusCode: 200, - }, - { - statusCode: 500, - expErr: errors.New("fleetlock error: this is an error (error_kind)"), - body: ioutil.NopCloser(bytes.NewReader([]byte(`{"kind":"error_kind","value":"this is an error"}`))), - }, - { - statusCode: 500, - expErr: errors.New("unmarshalling error: invalid character '\"' after object key:value pair"), - body: ioutil.NopCloser(bytes.NewReader([]byte(`{"kind":"error_kind" "value":"this is an error"}`))), - }, - { - statusCode: 100, - expErr: errors.New("unexpected status code: 100"), - }, - { - expErr: errors.New("doing the request: connection refused"), - doErr: errors.New("connection refused"), - }, - } { - h := &httpClient{ - resp: &http.Response{ - StatusCode: test.statusCode, - Body: test.body, - }, - doErr: test.doErr, - } + err := c.UnlockIfHeld() + if err != nil && err.Error() != test.expErr.Error() { + t.Fatalf("should have %v for err, got: %v", test.expErr, err) + } - c, _ := client.New("http://1.2.3.4", "default", "1234", h) + expURL := "http://1.2.3.4/v1/steady-state" - err := c.UnlockIfHeld() - if err != nil && err.Error() != test.expErr.Error() { - t.Fatalf("should have %v for err, got: %v", test.expErr, err) - } + if h.r.URL.String() != expURL { + t.Fatalf("should have %s for URL, got: %s", expURL, h.r.URL.String()) + } + }) - if h.r.URL.String() != expURL { - t.Fatalf("should have %s for URL, got: %s", expURL, h.r.URL.String()) - } + t.Run(fmt.Sprintf("RecursiveLock_%d", test.statusCode), func(t *testing.T) { + t.Parallel() + + h, c := newClient(test.statusCode, test.body, test.doErr) + + err := c.RecursiveLock() + if err != nil && err.Error() != test.expErr.Error() { + t.Fatalf("should have %v for err, got: %v", test.expErr, err) + } + + expURL := "http://1.2.3.4/v1/pre-reboot" + + if h.r.URL.String() != expURL { + t.Fatalf("should have %s for URL, got: %s", expURL, h.r.URL.String()) + } + }) } } From 02529f51f63919af7980f0075af6b23be8190d4e Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 17:31:47 +0100 Subject: [PATCH 35/56] .golangci.yml: enable dupl linter As it reports no issues right now. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 29fee1b..a0e8c09 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -36,7 +36,7 @@ linters: - deadcode - depguard - dogsled - #- dupl + - dupl - durationcheck #- errcheck - errname From bc5e63765ad74f0cf2da48f447b9d151214f67f3 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 17:32:58 +0100 Subject: [PATCH 36/56] pkg/client: remove unreachable code from handling response So govet linter can be enabled. Signed-off-by: Mateusz Gozdek --- pkg/client/client.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index a597e0b..2d6780d 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -99,8 +99,6 @@ func handleResponse(resp *http.Response) error { default: return fmt.Errorf("unexpected status code: %d", resp.StatusCode) } - - return nil } // RecursiveLock tries to reserve (lock) a slot for rebooting. From 92a646fa784806b8aee50c8b32cd73e681fa9d95 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 17:33:37 +0100 Subject: [PATCH 37/56] .golangci.yml: enable govet linter It does not report any issues anymore. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index a0e8c09..550e9b6 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -24,7 +24,6 @@ linters: - exhaustivestruct # Temporarily disable reporting linters. - errcheck - - govet # This linters has been deprecated. - interfacer - maligned @@ -62,6 +61,7 @@ linters: - gomoddirectives - gomodguard - goprintffuncname + - govet - ifshort - importas - ineffassign From d51ff0f01458ff3137d619d67fcac23d2c07fc5c Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 17:39:07 +0100 Subject: [PATCH 38/56] pkg/client: add nolint comment on closing response body So errcheck linter can be enabled. Signed-off-by: Mateusz Gozdek --- pkg/client/client.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/client/client.go b/pkg/client/client.go index 2d6780d..2438dc3 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -88,6 +88,7 @@ func handleResponse(resp *http.Response) error { return fmt.Errorf("reading body: %w", err) } + //nolint:errcheck // We do it best effort and at least stdlib client never returns error here. resp.Body.Close() e := &Error{} From 655b49d20415ab16edb6baabab8ee2fc0d39ec6b Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 17:41:08 +0100 Subject: [PATCH 39/56] pkg/client: add error handling in tests So errcheck linter can have extra checks enabled. Signed-off-by: Mateusz Gozdek --- pkg/client/client_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 79000a1..51425cd 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -80,7 +80,10 @@ func TestClient(t *testing.T) { doErr: doErr, } - c, _ := client.New("http://1.2.3.4", "default", "1234", h) + c, err := client.New("http://1.2.3.4", "default", "1234", h) + if err != nil { + t.Fatalf("Unexpected error creating client: %v", err) + } return h, c } From 2c3dc975d598195d2d0d84cd3b1501298b052fa7 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 17:41:37 +0100 Subject: [PATCH 40/56] .golangci.yml: enable errcheck linter Now that it does not report any more issues. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 550e9b6..15e6e76 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -22,8 +22,6 @@ linters: - goerr113 # We do not require all used structs to have all fields initialized. - exhaustivestruct - # Temporarily disable reporting linters. - - errcheck # This linters has been deprecated. - interfacer - maligned @@ -37,7 +35,6 @@ linters: - dogsled - dupl - durationcheck - #- errcheck - errname - errorlint - exhaustive From 298a8f8748d0c208c84f9571e8d640f47b09646b Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 17:41:57 +0100 Subject: [PATCH 41/56] .golangci.yml: enable extra linter settings Signed-off-by: Mateusz Gozdek --- .golangci.yml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index 15e6e76..22fb128 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -12,8 +12,28 @@ issues: text: "does not use range value in test Run" linters-settings: + errcheck: + check-type-assertions: true + check-blank: true gci: local-prefixes: github.com/flatcar-linux/fleetlock + godot: + capital: true + gofumpt: + extra-rules: true + govet: + enable-all: true + disable: + - fieldalignment + - shadow + makezero: + always: true + nolintlint: + allow-leading-space: false + require-explanation: true + require-specific: true + wsl: + force-err-cuddling: true linters: disable-all: false From c46224202f7f8444dd1b375f66ad63ba61409e1e Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 18:13:18 +0100 Subject: [PATCH 42/56] pkg/client: accept context for operations So they can be cancelled when take too much time. For now fleetlockcli has no timeout defined by default. Signed-off-by: Mateusz Gozdek --- cmd/lock.go | 3 +- cmd/unlock.go | 3 +- pkg/client/client.go | 15 ++++++---- pkg/client/client_test.go | 61 +++++++++++++++++++++++++++++++-------- 4 files changed, 62 insertions(+), 20 deletions(-) diff --git a/cmd/lock.go b/cmd/lock.go index e6590be..bdce9f0 100644 --- a/cmd/lock.go +++ b/cmd/lock.go @@ -1,6 +1,7 @@ package cmd import ( + "context" "fmt" "net/http" @@ -20,7 +21,7 @@ func lock(group, id, url *string) *cobra.Command { return fmt.Errorf("building the client: %w", err) } - if err := c.RecursiveLock(); err != nil { + if err := c.RecursiveLock(context.Background()); err != nil { return fmt.Errorf("locking: %w", err) } diff --git a/cmd/unlock.go b/cmd/unlock.go index b4f3693..139e2e6 100644 --- a/cmd/unlock.go +++ b/cmd/unlock.go @@ -1,6 +1,7 @@ package cmd import ( + "context" "fmt" "net/http" @@ -20,7 +21,7 @@ func unlock(group, id, url *string) *cobra.Command { return fmt.Errorf("building the client: %w", err) } - if err := c.UnlockIfHeld(); err != nil { + if err := c.UnlockIfHeld(context.Background()); err != nil { return fmt.Errorf("unlocking: %w", err) } diff --git a/pkg/client/client.go b/pkg/client/client.go index 2438dc3..13735d5 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -4,6 +4,7 @@ package client import ( "bufio" "bytes" + "context" "encoding/json" "fmt" "io/ioutil" @@ -46,7 +47,7 @@ type Client struct { http HTTPClient } -func (c *Client) generateRequest(endpoint string) (*http.Request, error) { +func (c *Client) generateRequest(ctx context.Context, endpoint string) (*http.Request, error) { payload := &Payload{ ClientParams: &Params{ ID: c.id, @@ -61,7 +62,9 @@ func (c *Client) generateRequest(endpoint string) (*http.Request, error) { j := bytes.NewReader(body) - req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("%s/%s", c.baseServerURL, endpoint), j) + targetURL := fmt.Sprintf("%s/%s", c.baseServerURL, endpoint) + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, targetURL, j) if err != nil { return nil, fmt.Errorf("building request: %w", err) } @@ -103,8 +106,8 @@ func handleResponse(resp *http.Response) error { } // RecursiveLock tries to reserve (lock) a slot for rebooting. -func (c *Client) RecursiveLock() error { - req, err := c.generateRequest("v1/pre-reboot") +func (c *Client) RecursiveLock(ctx context.Context) error { + req, err := c.generateRequest(ctx, "v1/pre-reboot") if err != nil { return fmt.Errorf("generating request: %w", err) } @@ -118,8 +121,8 @@ func (c *Client) RecursiveLock() error { } // UnlockIfHeld tries to release (unlock) a slot that it was previously holding. -func (c *Client) UnlockIfHeld() error { - req, err := c.generateRequest("v1/steady-state") +func (c *Client) UnlockIfHeld(ctx context.Context) error { + req, err := c.generateRequest(ctx, "v1/steady-state") if err != nil { return fmt.Errorf("generating request: %w", err) } diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 51425cd..ede89cc 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -2,6 +2,7 @@ package client_test import ( "bytes" + "context" "errors" "fmt" "io/ioutil" @@ -12,15 +13,14 @@ import ( ) type httpClient struct { - resp *http.Response - r *http.Request - doErr error + do func(req *http.Request) (*http.Response, error) + r *http.Request } -func (h *httpClient) Do(req *http.Request) (*http.Response, error) { - h.r = req +func (m *httpClient) Do(req *http.Request) (*http.Response, error) { + m.r = req - return h.resp, h.doErr + return m.do(req) } func TestBadURL(t *testing.T) { @@ -41,6 +41,8 @@ func TestBadURL(t *testing.T) { func TestClient(t *testing.T) { t.Parallel() + ctx := context.Background() + for _, test := range []struct { statusCode int expErr error @@ -73,11 +75,12 @@ func TestClient(t *testing.T) { newClient := func(statusCode int, body []byte, doErr error) (*httpClient, *client.Client) { h := &httpClient{ - resp: &http.Response{ - StatusCode: statusCode, - Body: ioutil.NopCloser(bytes.NewReader(body)), + do: func(req *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: statusCode, + Body: ioutil.NopCloser(bytes.NewReader(body)), + }, doErr }, - doErr: doErr, } c, err := client.New("http://1.2.3.4", "default", "1234", h) @@ -93,7 +96,7 @@ func TestClient(t *testing.T) { h, c := newClient(test.statusCode, test.body, test.doErr) - err := c.UnlockIfHeld() + err := c.UnlockIfHeld(ctx) if err != nil && err.Error() != test.expErr.Error() { t.Fatalf("should have %v for err, got: %v", test.expErr, err) } @@ -110,7 +113,7 @@ func TestClient(t *testing.T) { h, c := newClient(test.statusCode, test.body, test.doErr) - err := c.RecursiveLock() + err := c.RecursiveLock(ctx) if err != nil && err.Error() != test.expErr.Error() { t.Fatalf("should have %v for err, got: %v", test.expErr, err) } @@ -123,3 +126,37 @@ func TestClient(t *testing.T) { }) } } + +func Test_Client_use_given_context_for_requests(t *testing.T) { + t.Parallel() + + key := struct{}{} + value := "bar" + + h := &httpClient{ + do: func(req *http.Request) (*http.Response, error) { + if req.Context().Value(key) == nil { + t.Fatalf("Expected request to use given context") + } + + return &http.Response{ + StatusCode: 200, + }, nil + }, + } + + c, err := client.New("http://1.2.3.4", "default", "1234", h) + if err != nil { + t.Fatalf("Unexpected error creating client: %v", err) + } + + ctx := context.WithValue(context.Background(), key, value) + + if err := c.RecursiveLock(ctx); err != nil { + t.Fatalf("Unexpected error while doing recursive lock: %v", err) + } + + if err := c.UnlockIfHeld(ctx); err != nil { + t.Fatalf("Unexpected error while unlocking: %v", err) + } +} From 3b472bef32b6bb901fa270d6c91b5f547fc42bd7 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 18:22:26 +0100 Subject: [PATCH 43/56] .golangci.yml: enable noctx linter It does not report any issues anymore. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 22fb128..ff180ba 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -89,7 +89,7 @@ linters: - nestif - nilerr - nlreturn - #- noctx + - noctx - nolintlint - paralleltest - prealloc From b167c5e76ed6c77645ed201f6cbabb82e73464d9 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 18:31:40 +0100 Subject: [PATCH 44/56] pkg/client: get rid of magic numbers for HTTP response codes Replace it with more convinient ranges to satisfy gomnd linter. Signed-off-by: Mateusz Gozdek --- pkg/client/client.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 13735d5..b9b65cb 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -77,12 +77,12 @@ func (c *Client) generateRequest(ctx context.Context, endpoint string) (*http.Re } func handleResponse(resp *http.Response) error { - statusType := resp.StatusCode / 100 + maxHTTPErrorCode := 600 - switch statusType { - case 2: + switch code := resp.StatusCode; { + case code >= 200 && code < 300: return nil - case 3, 4, 5: + case code >= 300 && code < maxHTTPErrorCode: // We try to extract an eventual error. r := bufio.NewReader(resp.Body) From 507dc95f9d93248845df2cac1d4afd132cd76c89 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 18:32:22 +0100 Subject: [PATCH 45/56] .golangci.yml: enable gomnd linter It does not report any more issues. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index ff180ba..4645b50 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -74,7 +74,7 @@ linters: - gofumpt - goheader - goimports - #- gomnd + - gomnd - gomoddirectives - gomodguard - goprintffuncname From 3f2ab41535fc49d332850a84bd4ccf1c0fefee3c Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 18:34:52 +0100 Subject: [PATCH 46/56] .golangci.yml: update list of linters So it matches auto-generated one. Signed-off-by: Mateusz Gozdek --- .golangci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 4645b50..6587bdd 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -55,6 +55,7 @@ linters: - dogsled - dupl - durationcheck + - errcheck - errname - errorlint - exhaustive @@ -70,6 +71,7 @@ linters: - gocritic - gocyclo - godot + - godox - gofmt - gofumpt - goheader @@ -78,7 +80,6 @@ linters: - gomoddirectives - gomodguard - goprintffuncname - - govet - ifshort - importas - ineffassign From f170b88bf03a4911b148f1351a7aba2c7e5db9e0 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 18:35:21 +0100 Subject: [PATCH 47/56] Makefile: add targets for validating golangci-lint configuration Signed-off-by: Mateusz Gozdek --- Makefile | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Makefile b/Makefile index 39d13f8..ec2193d 100644 --- a/Makefile +++ b/Makefile @@ -1,2 +1,19 @@ +GOLANGCILINT := go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.42.1 + build: @go build -o ./fleetlockctl main.go + +.PHONY: test-working-tree-clean +test-working-tree-clean: + @test -z "$$(git status --porcelain)" || (echo "Commit all changes before running this target"; exit 1) + +.PHONY: update-linters +update-linters: + # Remove all enabled linters. + sed -i '/^ enable:/q0' .golangci.yml + # Then add all possible linters to config. + $(GOLANGCILINT) linters | grep -E '^\S+:' | cut -d: -f1 | sort | sed 's/^/ - /g' | grep -v -E "($$(grep '^ disable:' -A 100 .golangci.yml | grep -E ' - \S+$$' | awk '{print $$2}' | tr \\n '|' | sed 's/|$$//g'))" >> .golangci.yml + +.PHONY: test-update-linters +test-update-linters: test-working-tree-clean update-linters + @test -z "$$(git status --porcelain)" || (echo "Linter configuration outdated. Run 'make update-linters' and commit generated changes to fix."; exit 1) From 42eb7971d53515f5080de757fe71c2363054185f Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 18:53:56 +0100 Subject: [PATCH 48/56] .github/workflows/go.yml: check if list of linters is up to date So new linters are either explicitly enabled or disabled while updating the linter. Signed-off-by: Mateusz Gozdek --- .github/workflows/go.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index a5e004a..28f5056 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -25,9 +25,13 @@ jobs: lint: name: Lint runs-on: ubuntu-latest + container: golangci/golangci-lint:v1.42 steps: - uses: actions/checkout@v2 + - name: Check linters are up to date + run: make test-update-linters + - name: golangci-lint uses: golangci/golangci-lint-action@v2 with: From 86a59b3ee4a855da7442498da570767eee302474 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 18:54:36 +0100 Subject: [PATCH 49/56] Makefile: don't run build in parallel As with targets without associated source files it is usually not desired to run things in parallel. Signed-off-by: Mateusz Gozdek --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index ec2193d..8258d18 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,6 @@ GOLANGCILINT := go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.42.1 +.PHONY: build build: @go build -o ./fleetlockctl main.go From 2ce5c530a67eb33dcccf8dfb6c03f1538b890e7b Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Sun, 31 Oct 2021 18:56:58 +0100 Subject: [PATCH 50/56] Makefile: add help target So each target has some helpful description. Signed-off-by: Mateusz Gozdek --- Makefile | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 8258d18..128d336 100644 --- a/Makefile +++ b/Makefile @@ -1,20 +1,24 @@ GOLANGCILINT := go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.42.1 .PHONY: build -build: +build: ## Compiles CLI binary. @go build -o ./fleetlockctl main.go .PHONY: test-working-tree-clean -test-working-tree-clean: +test-working-tree-clean: ## Checks if working directory is clean. @test -z "$$(git status --porcelain)" || (echo "Commit all changes before running this target"; exit 1) .PHONY: update-linters -update-linters: +update-linters: ## Updates list of linters in .golangci.yml file based on currently installed golangci-lint binary. # Remove all enabled linters. sed -i '/^ enable:/q0' .golangci.yml # Then add all possible linters to config. $(GOLANGCILINT) linters | grep -E '^\S+:' | cut -d: -f1 | sort | sed 's/^/ - /g' | grep -v -E "($$(grep '^ disable:' -A 100 .golangci.yml | grep -E ' - \S+$$' | awk '{print $$2}' | tr \\n '|' | sed 's/|$$//g'))" >> .golangci.yml .PHONY: test-update-linters -test-update-linters: test-working-tree-clean update-linters +test-update-linters: test-working-tree-clean update-linters ## Verifies that list of linters in .golangci.yml file is up to date. @test -z "$$(git status --porcelain)" || (echo "Linter configuration outdated. Run 'make update-linters' and commit generated changes to fix."; exit 1) + +.PHONY: help +help: ## Prints help message. + @grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' From 2ff0ffcf3ac1daa3bb13a463faa833567b9405fa Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Mon, 1 Nov 2021 15:09:44 +0100 Subject: [PATCH 51/56] .github/workflows/go.yml: add spell checking job To catch more potential typos. Signed-off-by: Mateusz Gozdek --- .github/workflows/go.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 28f5056..0aa8aa1 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -36,3 +36,14 @@ jobs: uses: golangci/golangci-lint-action@v2 with: version: v1.42 + + codespell: + name: Spell Check + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: codespell-project/actions-codespell@master + with: + skip: .git + check_filenames: true + check_hidden: true From 421638cc6ded103914fcfa92413e27298f3700dd Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Mon, 1 Nov 2021 15:20:32 +0100 Subject: [PATCH 52/56] pkg/client: move exported functions to the top of the file As if one looks at the source code, exported functions take precedence in importance than unexported functions. Also the order is that the New() is first, then the client methods, as usually you need New() first to do other things. Signed-off-by: Mateusz Gozdek --- pkg/client/client.go | 88 ++++++++++++++++++++++---------------------- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index b9b65cb..3b35b6e 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -47,6 +47,50 @@ type Client struct { http HTTPClient } +// New builds a FleetLock client. +func New(baseServerURL, group, id string, c HTTPClient) (*Client, error) { + if _, err := url.ParseRequestURI(baseServerURL); err != nil { + return nil, fmt.Errorf("parsing URL: %w", err) + } + + return &Client{ + baseServerURL: baseServerURL, + http: c, + group: group, + id: id, + }, nil +} + +// RecursiveLock tries to reserve (lock) a slot for rebooting. +func (c *Client) RecursiveLock(ctx context.Context) error { + req, err := c.generateRequest(ctx, "v1/pre-reboot") + if err != nil { + return fmt.Errorf("generating request: %w", err) + } + + resp, err := c.http.Do(req) + if err != nil { + return fmt.Errorf("doing the request: %w", err) + } + + return handleResponse(resp) +} + +// UnlockIfHeld tries to release (unlock) a slot that it was previously holding. +func (c *Client) UnlockIfHeld(ctx context.Context) error { + req, err := c.generateRequest(ctx, "v1/steady-state") + if err != nil { + return fmt.Errorf("generating request: %w", err) + } + + resp, err := c.http.Do(req) + if err != nil { + return fmt.Errorf("doing the request: %w", err) + } + + return handleResponse(resp) +} + func (c *Client) generateRequest(ctx context.Context, endpoint string) (*http.Request, error) { payload := &Payload{ ClientParams: &Params{ @@ -104,47 +148,3 @@ func handleResponse(resp *http.Response) error { return fmt.Errorf("unexpected status code: %d", resp.StatusCode) } } - -// RecursiveLock tries to reserve (lock) a slot for rebooting. -func (c *Client) RecursiveLock(ctx context.Context) error { - req, err := c.generateRequest(ctx, "v1/pre-reboot") - if err != nil { - return fmt.Errorf("generating request: %w", err) - } - - resp, err := c.http.Do(req) - if err != nil { - return fmt.Errorf("doing the request: %w", err) - } - - return handleResponse(resp) -} - -// UnlockIfHeld tries to release (unlock) a slot that it was previously holding. -func (c *Client) UnlockIfHeld(ctx context.Context) error { - req, err := c.generateRequest(ctx, "v1/steady-state") - if err != nil { - return fmt.Errorf("generating request: %w", err) - } - - resp, err := c.http.Do(req) - if err != nil { - return fmt.Errorf("doing the request: %w", err) - } - - return handleResponse(resp) -} - -// New builds a FleetLock client. -func New(baseServerURL, group, id string, c HTTPClient) (*Client, error) { - if _, err := url.ParseRequestURI(baseServerURL); err != nil { - return nil, fmt.Errorf("parsing URL: %w", err) - } - - return &Client{ - baseServerURL: baseServerURL, - http: c, - group: group, - id: id, - }, nil -} From fc4b82464dfbab988b744b1e31b940bf8085e00c Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Tue, 2 Nov 2021 10:08:53 +0100 Subject: [PATCH 53/56] cmd: improve description of id flag Signed-off-by: Mateusz Gozdek --- cmd/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/root.go b/cmd/root.go index 8f324dd..37b9ec7 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -12,7 +12,7 @@ func Command() *cobra.Command { var group, id, url string cli.PersistentFlags().StringVarP(&group, "group", "g", "default", "FleetLock group") - cli.PersistentFlags().StringVarP(&id, "id", "i", "", "FleetLock instance ID (/etc/machine-id for example)") + cli.PersistentFlags().StringVarP(&id, "id", "i", "", "FleetLock instance ID (e.g. content of /etc/machine-id file)") cli.PersistentFlags().StringVarP(&url, "url", "u", "", "FleetLock endpoint URL") cli.AddCommand(lock(&group, &id, &url)) From 60bf0cbcd00a22e3c8c00dd5f71db2513ce1267d Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Tue, 2 Nov 2021 10:10:15 +0100 Subject: [PATCH 54/56] main.go: exit with error exit code on execution error Signed-off-by: Mateusz Gozdek --- main.go | 1 + 1 file changed, 1 insertion(+) diff --git a/main.go b/main.go index 18278b7..75211ef 100644 --- a/main.go +++ b/main.go @@ -10,5 +10,6 @@ import ( func main() { if err := cmd.Command().Execute(); err != nil { fmt.Fprintf(os.Stderr, "unable to execute command: %v", err) + os.Exit(1) } } From a1262c875f94643b83eba2b3a07194be74a11355 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Tue, 2 Nov 2021 10:11:45 +0100 Subject: [PATCH 55/56] main.go: don't print error message twice cobra already takes care of printing the execution error to stderr. This also prevents printing the error message without newline at the end. Signed-off-by: Mateusz Gozdek --- main.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/main.go b/main.go index 75211ef..909182a 100644 --- a/main.go +++ b/main.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "os" "github.com/flatcar-linux/fleetlock/cmd" @@ -9,7 +8,6 @@ import ( func main() { if err := cmd.Command().Execute(); err != nil { - fmt.Fprintf(os.Stderr, "unable to execute command: %v", err) os.Exit(1) } } From 0e1c5d3b381fee75084921f26314e09bd272082d Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Tue, 2 Nov 2021 10:21:06 +0100 Subject: [PATCH 56/56] cmd: add description for subcommands So when help is invoked, every available subcommand has some description. Signed-off-by: Mateusz Gozdek --- cmd/lock.go | 3 ++- cmd/unlock.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/lock.go b/cmd/lock.go index bdce9f0..5b920b8 100644 --- a/cmd/lock.go +++ b/cmd/lock.go @@ -12,7 +12,8 @@ import ( func lock(group, id, url *string) *cobra.Command { return &cobra.Command{ - Use: "recursive-lock", + Use: "recursive-lock", + Short: "Try to reserve (lock) a slot for rebooting", RunE: func(cmd *cobra.Command, args []string) error { httpClient := http.DefaultClient diff --git a/cmd/unlock.go b/cmd/unlock.go index 139e2e6..7e82ef9 100644 --- a/cmd/unlock.go +++ b/cmd/unlock.go @@ -12,7 +12,8 @@ import ( func unlock(group, id, url *string) *cobra.Command { return &cobra.Command{ - Use: "unlock-if-held", + Use: "unlock-if-held", + Short: "Try to release (unlock) a slot that it was previously holding", RunE: func(cmd *cobra.Command, args []string) error { httpClient := http.DefaultClient