Skip to content

Commit 08c3f1c

Browse files
leonardocearmrumnencia
authored
feat: return proper gRPC error codes for expected conditions (#549)
The plugin now returns a 404 status code when a requested WAL file does not exist in the object store. This prevents misleading log entries such as "Error while handling gRPC request" for expected missing-file scenarios. The `ErrEndOfWALStreamReached` now returns `OutOfRange` error. The `ErrMissingPermissions` now returns `FailedPrecondition` error. Signed-off-by: Leonardo Cecchi <[email protected]> Signed-off-by: Armando Ruocco <[email protected]> Signed-off-by: Marco Nenciarini <[email protected]> Co-authored-by: Armando Ruocco <[email protected]> Co-authored-by: Marco Nenciarini <[email protected]>
1 parent 65a0d11 commit 08c3f1c

File tree

2 files changed

+27
-24
lines changed

2 files changed

+27
-24
lines changed

internal/cnpgi/common/errors.go

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,25 @@
11
package common
22

3-
// walNotFoundError is raised when a WAL file has not been found in the object store
4-
type walNotFoundError struct{}
3+
import (
4+
"google.golang.org/grpc/codes"
5+
"google.golang.org/grpc/status"
6+
)
57

6-
func newWALNotFoundError() *walNotFoundError { return &walNotFoundError{} }
8+
// ErrEndOfWALStreamReached is returned when end of WAL is detected in the cloud archive.
9+
var ErrEndOfWALStreamReached = status.Errorf(codes.OutOfRange, "end of WAL reached")
710

8-
// ShouldPrintStackTrace tells whether the sidecar log stream should contain the stack trace
9-
func (e walNotFoundError) ShouldPrintStackTrace() bool {
10-
return false
11-
}
11+
// ErrMissingPermissions is raised when the sidecar has no
12+
// permission to download the credentials needed to reach
13+
// the object storage.
14+
// This will be fixed by the reconciliation loop in the
15+
// operator plugin.
16+
var ErrMissingPermissions = status.Errorf(codes.FailedPrecondition,
17+
"no permission to download the backup credentials, retrying")
1218

13-
// Error implements the error interface
14-
func (e walNotFoundError) Error() string {
15-
return "WAL file not found"
19+
// newWALNotFoundError returns a error that states that a
20+
// certain WAL file has not been found. This error is
21+
// compatible with GRPC status codes, resulting in a 404
22+
// being used as a response code.
23+
func newWALNotFoundError(walName string) error {
24+
return status.Errorf(codes.NotFound, "wal %q not found", walName)
1625
}

internal/cnpgi/common/wal.go

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,6 @@ import (
2727
"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/config"
2828
)
2929

30-
// ErrMissingPermissions is raised when the sidecar has no
31-
// permission to download the credentials needed to reach
32-
// the object storage.
33-
// This will be fixed by the reconciliation loop in the
34-
// operator plugin.
35-
var ErrMissingPermissions = fmt.Errorf("no permission to download the backup credentials, retrying")
36-
3730
// SpoolManagementError is raised when a spool management
3831
// error has been detected
3932
type SpoolManagementError struct {
@@ -95,11 +88,14 @@ func (w WALServiceImplementation) Archive(
9588
ctx context.Context,
9689
request *wal.WALArchiveRequest,
9790
) (*wal.WALArchiveResult, error) {
98-
contextLogger := log.FromContext(ctx)
99-
contextLogger.Debug("starting wal archive")
100-
10191
baseWalName := path.Base(request.GetSourceFileName())
10292

93+
contextLogger := log.FromContext(ctx)
94+
contextLogger.Debug("wal archive start", "walName", baseWalName)
95+
defer func() {
96+
contextLogger.Debug("wal archive end", "walName", baseWalName)
97+
}()
98+
10399
// Step 1: parse the configuration and get the environment variables needed
104100
// for barman-cloud-wal-archive
105101
configuration, err := config.NewFromClusterJSON(request.ClusterDefinition)
@@ -122,6 +118,7 @@ func (w WALServiceImplementation) Archive(
122118
)
123119
if err != nil {
124120
if apierrors.IsForbidden(err) {
121+
contextLogger.Info(ErrMissingPermissions.Error())
125122
return nil, ErrMissingPermissions
126123
}
127124
return nil, err
@@ -350,7 +347,7 @@ func (w WALServiceImplementation) restoreFromBarmanObjectStore(
350347
// The failure has already been logged in walRestorer.RestoreList method
351348
if walStatus[0].Err != nil {
352349
if errors.Is(walStatus[0].Err, barmanRestorer.ErrWALNotFound) {
353-
return newWALNotFoundError()
350+
return newWALNotFoundError(walStatus[0].WalName)
354351
}
355352

356353
return walStatus[0].Err
@@ -457,9 +454,6 @@ func gatherWALFilesToRestore(walName string, parallel int) (walList []string, er
457454
return walList, err
458455
}
459456

460-
// ErrEndOfWALStreamReached is returned when end of WAL is detected in the cloud archive.
461-
var ErrEndOfWALStreamReached = errors.New("end of WAL reached")
462-
463457
// checkEndOfWALStreamFlag returns ErrEndOfWALStreamReached if the flag is set in the restorer.
464458
func checkEndOfWALStreamFlag(walRestorer *barmanRestorer.WALRestorer) error {
465459
contain, err := walRestorer.IsEndOfWALStream()

0 commit comments

Comments
 (0)