add clienterr package

- add tests for loki client errors

internals/overlord/logstate/clienterr/doc.go

@@ -0,0 +1,20 @@
+// Copyright (c) 2023 Canonical Ltd
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 3 as
+// published by the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+/*
+Package clienterr contains common error types which are recognised by the log
+gatherer. logstate.logClient implementations should return these error types
+to communicate with the gatherer.
+*/
+package clienterr

internals/overlord/logstate/clienterr/err.go

@@ -0,0 +1,37 @@
+// Copyright (c) 2023 Canonical Ltd
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 3 as
+// published by the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package clienterr
+
+import (
+	"fmt"
+	"time"
+)
+
+// Backoff should be returned if the server indicates we are sending too many
+// requests (e.g. an HTTP 429 response).
+type Backoff struct {
+	RetryAfter time.Time
+}
+
+func (b Backoff) Error() string {
+	return fmt.Sprintf("too many requests, retry after %v", b.RetryAfter)
+}
+
+// Timeout should be returned if the client's Flush times out.
+type Timeout struct{}
+
+func (Timeout) Error() string {
+	return "client flush timed out"
+}

internals/overlord/logstate/gatherer.go

@@ -19,10 +19,10 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/canonical/pebble/internals/overlord/logstate/loki"
 	"gopkg.in/tomb.v2"
 
 	"github.com/canonical/pebble/internals/logger"
+	"github.com/canonical/pebble/internals/overlord/logstate/loki"
 	"github.com/canonical/pebble/internals/plan"
 	"github.com/canonical/pebble/internals/servicelog"
 )

internals/overlord/logstate/loki/export_test.go

@@ -0,0 +1,25 @@
+// Copyright (c) 2023 Canonical Ltd
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 3 as
+// published by the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package loki
+
+import "time"
+
+func SetRequestTimeout(new time.Duration) (restore func()) {
+	oldRequestTimeout := requestTimeout
+	requestTimeout = new
+	return func() {
+		requestTimeout = oldRequestTimeout
+	}
+}

internals/overlord/logstate/loki/loki.go

@@ -30,10 +30,9 @@ import (
 	"github.com/canonical/pebble/internals/servicelog"
 )
 
-const (
-	maxRequestSize = 10
-	requestTimeout = 10 * time.Second
-)
+const maxRequestSize = 10
+
+var requestTimeout = 10 * time.Second
 
 type Client struct {
 	target    *plan.LogTarget
@@ -90,8 +89,18 @@ func (c *Client) Flush(ctx context.Context) error {
 
 	resp, err := c.httpClient.Do(httpReq)
 	if err != nil {
-		return err
+		return handleHTTPClientErr(err)
 	}
+	return handleServerResponse(resp)
+}
+
+func handleHTTPClientErr(err error) error {
+	// req timeout -> timeout
+	// context cancelled -> timeout
+	return err
+}
+
+func handleServerResponse(resp *http.Response) error {
 	defer func() {
 		// Drain request body to allow connection reuse
 		// see https://pkg.go.dev/net/http#Response.Body
@@ -100,6 +109,8 @@ func (c *Client) Flush(ctx context.Context) error {
 	}()
 
 	// Check response status code to see if it was successful
+	// TODO: handle 429
+	// TODO: rebase, add logic to gatherer to recognise clienterrs
 	if code := resp.StatusCode; code < 200 || code >= 300 {
 		// Request to Loki failed
 		b, err := io.ReadAll(io.LimitReader(resp.Body, 1024))

internals/overlord/logstate/loki/loki_test.go

@@ -1,24 +1,46 @@
-package loki
+// Copyright (c) 2023 Canonical Ltd
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 3 as
+// published by the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package loki_test
 
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"testing"
 	"time"
 
 	. "gopkg.in/check.v1"
 
+	"github.com/canonical/pebble/internals/overlord/logstate/clienterr"
+	"github.com/canonical/pebble/internals/overlord/logstate/loki"
 	"github.com/canonical/pebble/internals/plan"
 	"github.com/canonical/pebble/internals/servicelog"
 )
 
-type lokiSuite struct{}
+type suite struct{}
 
-var _ = Suite(&lokiSuite{})
+var _ = Suite(&suite{})
 
-func (*lokiSuite) TestLokiRequest(c *C) {
+func Test(t *testing.T) {
+	TestingT(t)
+}
+
+func (*suite) TestRequest(c *C) {
 	input := []servicelog.Entry{{
 		Time:    time.Date(2023, 12, 31, 12, 34, 50, 0, time.UTC),
 		Service: "svc1",
@@ -113,7 +135,7 @@ func (*lokiSuite) TestLokiRequest(c *C) {
 	}))
 	defer server.Close()
 
-	client := NewClient(&plan.LogTarget{Location: server.URL})
+	client := loki.NewClient(&plan.LogTarget{Location: server.URL})
 	for _, entry := range input {
 		err := client.Write(context.Background(), entry)
 		c.Assert(err, IsNil)
@@ -123,7 +145,7 @@ func (*lokiSuite) TestLokiRequest(c *C) {
 	c.Assert(err, IsNil)
 }
 
-func (*lokiSuite) TestLokiFlushCancelContext(c *C) {
+func (*suite) TestFlushCancelContext(c *C) {
 	serverCtx, killServer := context.WithCancel(context.Background())
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		select {
@@ -135,7 +157,7 @@ func (*lokiSuite) TestLokiFlushCancelContext(c *C) {
 	defer server.Close()
 	defer killServer()
 
-	client := NewClient(&plan.LogTarget{Location: server.URL})
+	client := loki.NewClient(&plan.LogTarget{Location: server.URL})
 	err := client.Write(context.Background(), servicelog.Entry{
 		Time:    time.Now(),
 		Service: "svc1",
@@ -150,7 +172,7 @@ func (*lokiSuite) TestLokiFlushCancelContext(c *C) {
 		defer cancel()
 
 		err := client.Flush(ctx)
-		c.Assert(err, ErrorMatches, ".*context deadline exceeded")
+		c.Assert(errors.Is(err, clienterr.Timeout{}), Equals, true)
 		close(flushReturned)
 	}()
 
@@ -162,6 +184,49 @@ func (*lokiSuite) TestLokiFlushCancelContext(c *C) {
 	}
 }
 
+func (*suite) TestServerTimeout(c *C) {
+	restore := loki.SetRequestTimeout(1 * time.Microsecond)
+	defer restore()
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		time.Sleep(2 * time.Microsecond)
+	}))
+	defer server.Close()
+
+	client := loki.NewClient(&plan.LogTarget{Location: server.URL})
+	err := client.Write(context.Background(), servicelog.Entry{
+		Time:    time.Now(),
+		Service: "svc1",
+		Message: "this is a log line\n",
+	})
+	c.Assert(err, IsNil)
+
+	err = client.Flush(context.Background())
+	c.Assert(errors.Is(err, clienterr.Timeout{}), Equals, true)
+}
+
+func (*suite) TestTooManyRequests(c *C) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusTooManyRequests)
+		w.Header().Add("Retry-After", "Tue, 15 Aug 2023 08:49:37 GMT")
+	}))
+	defer server.Close()
+
+	client := loki.NewClient(&plan.LogTarget{Location: server.URL})
+	err := client.Write(context.Background(), servicelog.Entry{
+		Time:    time.Now(),
+		Service: "svc1",
+		Message: "this is a log line\n",
+	})
+	c.Assert(err, IsNil)
+
+	expectedErr := clienterr.Backoff{
+		RetryAfter: time.Date(2023, 8, 15, 8, 49, 37, 0, time.UTC),
+	}
+	err = client.Flush(context.Background())
+	c.Assert(errors.Is(err, expectedErr), Equals, true)
+}
+
 // Strips all extraneous whitespace from JSON
 func flattenJSON(s string) (string, error) {
 	var v any