Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DB: Remove ErrAlreadyDefined sentinel error. #13252

Merged
merged 7 commits into from
Apr 3, 2024
12 changes: 2 additions & 10 deletions lxd/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,7 @@ func backupCreate(s *state.State, args db.InstanceBackup, sourceInst instance.In
return tx.CreateInstanceBackup(ctx, args)
})
if err != nil {
if err == db.ErrAlreadyDefined {
return fmt.Errorf("Backup %q already exists", args.Name)
}

return fmt.Errorf("Insert backup info into database: %w", err)
return fmt.Errorf("Failed creating instance backup record: %w", err)
}

revert.Add(func() {
Expand Down Expand Up @@ -410,11 +406,7 @@ func volumeBackupCreate(s *state.State, args db.StoragePoolVolumeBackup, project
return tx.CreateStoragePoolVolumeBackup(ctx, args)
})
if err != nil {
if err == db.ErrAlreadyDefined {
return fmt.Errorf("Backup %q already exists", args.Name)
}

return fmt.Errorf("Failed creating backup record: %w", err)
return fmt.Errorf("Failed creating storage volume backup record: %w", err)
}

revert.Add(func() {
Expand Down
4 changes: 2 additions & 2 deletions lxd/db/backups.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ WHERE projects.name=? AND instances.name=?`
func (c *ClusterTx) CreateInstanceBackup(ctx context.Context, args InstanceBackup) error {
_, err := c.getInstanceBackupID(ctx, args.Name)
if err == nil {
return ErrAlreadyDefined
return api.StatusErrorf(http.StatusConflict, "Backup for instance %q already exists", args.Name)
}

instanceOnlyInt := 0
Expand Down Expand Up @@ -388,7 +388,7 @@ ORDER BY storage_volumes_backups.id`
func (c *ClusterTx) CreateStoragePoolVolumeBackup(ctx context.Context, args StoragePoolVolumeBackup) error {
_, err := c.getStoragePoolVolumeBackupID(ctx, args.Name)
if err == nil {
return ErrAlreadyDefined
return api.StatusErrorf(http.StatusConflict, "Backup for storage volume %q already exists", args.Name)
}

volumeOnlyInt := 0
Expand Down
4 changes: 0 additions & 4 deletions lxd/db/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ import (
)

var (
// ErrAlreadyDefined hapens when the given entry already exists,
// for example a container.
ErrAlreadyDefined = fmt.Errorf("The record already exists")

// ErrNoClusterMember is used to indicate no cluster member has been found for a resource.
ErrNoClusterMember = fmt.Errorf("No cluster member found")
)
2 changes: 1 addition & 1 deletion lxd/db/networks.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func (c *ClusterTx) CreatePendingNetwork(ctx context.Context, node string, proje
}

if count != 0 {
return ErrAlreadyDefined
return api.StatusErrorf(http.StatusConflict, "Network %q already exists", name)
}

// Insert the node-specific configuration with state networkPending.
Expand Down
3 changes: 2 additions & 1 deletion lxd/db/networks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package db_test

import (
"context"
"net/http"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -94,7 +95,7 @@ func TestNetworksCreatePending_AlreadyDefined(t *testing.T) {
require.NoError(t, err)

err = tx.CreatePendingNetwork(context.Background(), "buzz", api.ProjectDefaultName, "network1", db.NetworkTypeBridge, map[string]string{})
require.Equal(t, db.ErrAlreadyDefined, err)
assert.True(t, api.StatusErrorCheck(err, http.StatusConflict))
}

// If no node with the given name is found, an error is returned.
Expand Down
2 changes: 1 addition & 1 deletion lxd/db/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ func (c *ClusterTx) RenameNode(ctx context.Context, old string, new string) erro
}

if count != 0 {
return ErrAlreadyDefined
return api.StatusErrorf(http.StatusConflict, "A cluster member already exists with name %q", new)
}

stmt := `UPDATE nodes SET name=? WHERE name=?`
Expand Down
4 changes: 3 additions & 1 deletion lxd/db/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package db_test

import (
"context"
"net/http"
"testing"
"time"

Expand All @@ -14,6 +15,7 @@ import (
"github.com/canonical/lxd/lxd/db/cluster"
"github.com/canonical/lxd/lxd/db/operationtype"
"github.com/canonical/lxd/lxd/response"
"github.com/canonical/lxd/shared/api"
"github.com/canonical/lxd/shared/osarch"
"github.com/canonical/lxd/shared/version"
)
Expand Down Expand Up @@ -145,7 +147,7 @@ func TestRenameNode(t *testing.T) {
_, err = tx.CreateNode("buzz", "5.6.7.8:666")
require.NoError(t, err)
err = tx.RenameNode(context.Background(), "rusp", "buzz")
assert.Equal(t, db.ErrAlreadyDefined, err)
assert.True(t, api.StatusErrorCheck(err, http.StatusConflict))
}

// Remove a new raft node.
Expand Down
2 changes: 1 addition & 1 deletion lxd/db/storage_pools.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func (c *ClusterTx) CreatePendingStoragePool(ctx context.Context, node string, n
}

if count != 0 {
return ErrAlreadyDefined
return api.StatusErrorf(http.StatusConflict, "A storage pool already exists with name %q", name)
}

// Insert a node-specific entry pointing to ourselves with state storagePoolPending.
Expand Down
4 changes: 3 additions & 1 deletion lxd/db/storage_pools_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package db_test

import (
"context"
"net/http"
"testing"
"time"

Expand All @@ -13,6 +14,7 @@ import (
"github.com/canonical/lxd/lxd/db"
"github.com/canonical/lxd/lxd/db/cluster"
"github.com/canonical/lxd/lxd/response"
"github.com/canonical/lxd/shared/api"
)

func init() {
Expand Down Expand Up @@ -151,7 +153,7 @@ func TestStoragePoolsCreatePending_AlreadyDefined(t *testing.T) {
require.NoError(t, err)

err = tx.CreatePendingStoragePool(context.Background(), "buzz", "pool1", "dir", map[string]string{})
require.Equal(t, db.ErrAlreadyDefined, err)
assert.True(t, api.StatusErrorCheck(err, http.StatusConflict))
}

// If no node with the given name is found, an error is returned.
Expand Down
2 changes: 1 addition & 1 deletion lxd/instance/instance_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ func CreateInternal(s *state.State, args db.InstanceArgs, clearLogDir bool) (Ins
return nil
})
if err != nil {
if err == db.ErrAlreadyDefined {
if api.StatusErrorCheck(err, http.StatusConflict) {
thing := "Instance"
if shared.IsSnapshot(args.Name) {
thing = "Snapshot"
Expand Down
5 changes: 2 additions & 3 deletions lxd/networks.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package main
import (
"context"
"encoding/json"
"errors"
"fmt"
"net"
"net/http"
Expand Down Expand Up @@ -403,7 +402,7 @@ func networksPost(d *Daemon, r *http.Request) response.Response {
return tx.CreatePendingNetwork(ctx, targetNode, projectName, req.Name, netType.DBType(), req.Config)
})
if err != nil {
if err == db.ErrAlreadyDefined {
if api.StatusErrorCheck(err, http.StatusConflict) {
return response.BadRequest(fmt.Errorf("The network is already defined on member %q", targetNode))
}

Expand Down Expand Up @@ -447,7 +446,7 @@ func networksPost(d *Daemon, r *http.Request) response.Response {
// Don't pass in any config, as these nodes don't have any node-specific
// config and we don't want to create duplicate global config.
err = tx.CreatePendingNetwork(ctx, member.Name, projectName, req.Name, netType.DBType(), nil)
if err != nil && !errors.Is(err, db.ErrAlreadyDefined) {
if err != nil && !api.StatusErrorCheck(err, http.StatusConflict) {
return fmt.Errorf("Failed creating pending network for member %q: %w", member.Name, err)
}
}
Expand Down
2 changes: 0 additions & 2 deletions lxd/response/smart.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@ import (
"net/http"
"os"

"github.com/canonical/lxd/lxd/db"
"github.com/canonical/lxd/shared/api"
)

var httpResponseErrors = map[int][]error{
http.StatusNotFound: {os.ErrNotExist, sql.ErrNoRows},
http.StatusForbidden: {os.ErrPermission},
http.StatusConflict: {db.ErrAlreadyDefined},
}

// SmartError returns the right error message based on err.
Expand Down
2 changes: 1 addition & 1 deletion lxd/storage_pools.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ func storagePoolsPost(d *Daemon, r *http.Request) response.Response {
return tx.CreatePendingStoragePool(ctx, targetNode, req.Name, req.Driver, req.Config)
})
if err != nil {
if err == db.ErrAlreadyDefined {
if api.StatusErrorCheck(err, http.StatusConflict) {
return response.BadRequest(fmt.Errorf("The storage pool already defined on member %q", targetNode))
}

Expand Down
3 changes: 2 additions & 1 deletion lxd/storage_pools_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"context"
"fmt"
"net/http"

"github.com/canonical/lxd/lxd/cluster/request"
"github.com/canonical/lxd/lxd/db"
Expand All @@ -22,7 +23,7 @@ func storagePoolDBCreate(s *state.State, poolName string, poolDescription string
return err
})
if err == nil {
return -1, fmt.Errorf("The storage pool already exists: %w", db.ErrAlreadyDefined)
return -1, api.StatusErrorf(http.StatusConflict, "Storage pool %q already exists", poolName)
}

// Make sure that we don't pass a nil to the next function.
Expand Down
Loading