[lxc-devel] [lxd/master] lxd: Improve error messages
monstermunchkin on Github
lxc-bot at linuxcontainers.org
Tue Jun 19 12:42:39 UTC 2018
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 373 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20180619/1a2c6e71/attachment.bin>
-------------- next part --------------
From 14e3cb460aa31e43c17b14578a7aa081827eccba Mon Sep 17 00:00:00 2001
From: Thomas Hipp <thomas.hipp at canonical.com>
Date: Tue, 19 Jun 2018 12:55:26 +0200
Subject: [PATCH] lxd: Improve error messages
Resolves #4453
Signed-off-by: Thomas Hipp <thomas.hipp at canonical.com>
---
lxd/api.go | 2 +-
lxd/api_cluster.go | 2 +-
lxd/certificates.go | 4 ++--
lxd/container_file.go | 2 +-
lxd/container_metadata.go | 4 ++--
lxd/container_patch.go | 2 +-
lxd/container_post.go | 2 +-
lxd/container_put.go | 2 +-
lxd/container_snapshot.go | 4 ++--
lxd/containers_post.go | 2 +-
lxd/daemon.go | 6 +++---
lxd/images.go | 10 +++++-----
lxd/networks.go | 11 ++++++-----
lxd/operations.go | 4 ++--
lxd/profiles.go | 2 +-
lxd/response.go | 48 ++++++++++++++++++++++++++++++++++++-----------
lxd/storage_pools.go | 2 +-
lxd/storage_volumes.go | 10 ++++++----
18 files changed, 74 insertions(+), 45 deletions(-)
diff --git a/lxd/api.go b/lxd/api.go
index 825874f1b..f6509de1c 100644
--- a/lxd/api.go
+++ b/lxd/api.go
@@ -39,7 +39,7 @@ func RestServer(d *Daemon) *http.Server {
mux.NotFoundHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
logger.Info("Sending top level 404", log.Ctx{"url": r.URL})
w.Header().Set("Content-Type", "application/json")
- NotFound.Render(w)
+ NotFound(nil).Render(w)
})
return &http.Server{Handler: &lxdHttpServer{r: mux, d: d}}
diff --git a/lxd/api_cluster.go b/lxd/api_cluster.go
index a8cc8f978..1aa7b6011 100644
--- a/lxd/api_cluster.go
+++ b/lxd/api_cluster.go
@@ -408,7 +408,7 @@ func clusterNodeGet(d *Daemon, r *http.Request) Response {
}
}
- return NotFound
+ return NotFound(fmt.Errorf("node '%s' not found", name))
}
func clusterNodePost(d *Daemon, r *http.Request) Response {
diff --git a/lxd/certificates.go b/lxd/certificates.go
index 7c2a5a0c7..09416a192 100644
--- a/lxd/certificates.go
+++ b/lxd/certificates.go
@@ -106,7 +106,7 @@ func certificatesPost(d *Daemon, r *http.Request) Response {
return SmartError(err)
}
if d.checkTrustedClient(r) != nil && util.PasswordCheck(secret, req.Password) != nil {
- return Forbidden
+ return Forbidden(nil)
}
if req.Type != "client" {
@@ -295,7 +295,7 @@ func certificateFingerprintDelete(d *Daemon, r *http.Request) Response {
certInfo, err := d.cluster.CertificateGet(fingerprint)
if err != nil {
- return NotFound
+ return NotFound(err)
}
err = d.cluster.CertDelete(certInfo.Fingerprint)
diff --git a/lxd/container_file.go b/lxd/container_file.go
index c9ffbd9e6..e179fc79e 100644
--- a/lxd/container_file.go
+++ b/lxd/container_file.go
@@ -42,7 +42,7 @@ func containerFileHandler(d *Daemon, r *http.Request) Response {
case "DELETE":
return containerFileDelete(c, path, r)
default:
- return NotFound
+ return NotFound(fmt.Errorf("method '%s' not found", r.Method))
}
}
diff --git a/lxd/container_metadata.go b/lxd/container_metadata.go
index 2f3a790b4..e1400f663 100644
--- a/lxd/container_metadata.go
+++ b/lxd/container_metadata.go
@@ -170,7 +170,7 @@ func containerMetadataTemplatesGet(d *Daemon, r *http.Request) Response {
}
if !shared.PathExists(templatePath) {
- return NotFound
+ return NotFound(fmt.Errorf("path '%s' not found", templatePath))
}
// Create a temporary file with the template content (since the container
@@ -298,7 +298,7 @@ func containerMetadataTemplatesDelete(d *Daemon, r *http.Request) Response {
}
if !shared.PathExists(templatePath) {
- return NotFound
+ return NotFound(fmt.Errorf("path '%s' not found", templatePath))
}
// Delete the template
diff --git a/lxd/container_patch.go b/lxd/container_patch.go
index 2b5a386db..e39ee6083 100644
--- a/lxd/container_patch.go
+++ b/lxd/container_patch.go
@@ -31,7 +31,7 @@ func containerPatch(d *Daemon, r *http.Request) Response {
c, err := containerLoadByName(d.State(), name)
if err != nil {
- return NotFound
+ return NotFound(err)
}
// Validate the ETag
diff --git a/lxd/container_post.go b/lxd/container_post.go
index a3ae76620..41f3353de 100644
--- a/lxd/container_post.go
+++ b/lxd/container_post.go
@@ -180,7 +180,7 @@ func containerPost(d *Daemon, r *http.Request) Response {
// Check that the name isn't already in use
id, _ := d.cluster.ContainerID(req.Name)
if id > 0 {
- return Conflict
+ return Conflict(fmt.Errorf("name '%s' already in use", req.Name))
}
run := func(*operation) error {
diff --git a/lxd/container_put.go b/lxd/container_put.go
index 7c18fa287..b738b1937 100644
--- a/lxd/container_put.go
+++ b/lxd/container_put.go
@@ -35,7 +35,7 @@ func containerPut(d *Daemon, r *http.Request) Response {
c, err := containerLoadByName(d.State(), name)
if err != nil {
- return NotFound
+ return NotFound(err)
}
// Validate the ETag
diff --git a/lxd/container_snapshot.go b/lxd/container_snapshot.go
index fc39d03b8..5f885a508 100644
--- a/lxd/container_snapshot.go
+++ b/lxd/container_snapshot.go
@@ -183,7 +183,7 @@ func snapshotHandler(d *Daemon, r *http.Request) Response {
case "DELETE":
return snapshotDelete(sc, snapshotName)
default:
- return NotFound
+ return NotFound(fmt.Errorf("method '%s' not found", r.Method))
}
}
@@ -288,7 +288,7 @@ func snapshotPost(d *Daemon, r *http.Request, sc container, containerName string
// Check that the name isn't already in use
id, _ := d.cluster.ContainerID(fullName)
if id > 0 {
- return Conflict
+ return Conflict(fmt.Errorf("name '%s' already in use", fullName))
}
rename := func(op *operation) error {
diff --git a/lxd/containers_post.go b/lxd/containers_post.go
index 4ea62a10b..300698117 100644
--- a/lxd/containers_post.go
+++ b/lxd/containers_post.go
@@ -177,7 +177,7 @@ func createFromNone(d *Daemon, req *api.ContainersPost) Response {
func createFromMigration(d *Daemon, req *api.ContainersPost) Response {
// Validate migration mode
if req.Source.Mode != "pull" && req.Source.Mode != "push" {
- return NotImplemented
+ return NotImplemented(fmt.Errorf("mode '%s' not implemented", req.Source.Mode))
}
var c container
diff --git a/lxd/daemon.go b/lxd/daemon.go
index fef03f24f..b37ff701c 100644
--- a/lxd/daemon.go
+++ b/lxd/daemon.go
@@ -266,7 +266,7 @@ func (d *Daemon) createCmd(restAPI *mux.Router, version string, c Command) {
logger.Warn(
"rejecting request from untrusted client",
log.Ctx{"ip": r.RemoteAddr})
- Forbidden.Render(w)
+ Forbidden(nil).Render(w)
return
}
@@ -284,7 +284,7 @@ func (d *Daemon) createCmd(restAPI *mux.Router, version string, c Command) {
}
var resp Response
- resp = NotImplemented
+ resp = NotImplemented(nil)
switch r.Method {
case "GET":
@@ -308,7 +308,7 @@ func (d *Daemon) createCmd(restAPI *mux.Router, version string, c Command) {
resp = c.patch(d, r)
}
default:
- resp = NotFound
+ resp = NotFound(fmt.Errorf("method '%s' not found", r.Method))
}
if err := resp.Render(w); err != nil {
diff --git a/lxd/images.go b/lxd/images.go
index ac4128d58..371a1ec56 100644
--- a/lxd/images.go
+++ b/lxd/images.go
@@ -1208,7 +1208,7 @@ func imageGet(d *Daemon, r *http.Request) Response {
}
if !info.Public && public && !imageValidSecret(info.Fingerprint, secret) {
- return NotFound
+ return NotFound(fmt.Errorf("image '%s' not found", info.Fingerprint))
}
etag := []interface{}{info.Public, info.AutoUpdate, info.Properties}
@@ -1324,7 +1324,7 @@ func aliasesPost(d *Daemon, r *http.Request) Response {
// This is just to see if the alias name already exists.
_, _, err := d.cluster.ImageAliasGet(req.Name, true)
if err == nil {
- return Conflict
+ return Conflict(fmt.Errorf("alias '%s' already exists", req.Name))
}
id, _, err := d.cluster.ImageGet(req.Target, false, false)
@@ -1495,7 +1495,7 @@ func aliasPost(d *Daemon, r *http.Request) Response {
// Check that the name isn't already in use
id, _, _ := d.cluster.ImageAliasGet(req.Name, true)
if id > 0 {
- return Conflict
+ return Conflict(fmt.Errorf("alias '%s' already in use", req.Name))
}
id, _, err := d.cluster.ImageAliasGet(name, true)
@@ -1527,7 +1527,7 @@ func imageExport(d *Daemon, r *http.Request) Response {
}
if !imgInfo.Public && !imgInfo.Cached {
- return NotFound
+ return NotFound(fmt.Errorf("image '%s' not found", fingerprint))
}
} else {
_, imgInfo, err = d.cluster.ImageGet(fingerprint, false, false)
@@ -1536,7 +1536,7 @@ func imageExport(d *Daemon, r *http.Request) Response {
}
if !imgInfo.Public && public && !imageValidSecret(imgInfo.Fingerprint, secret) {
- return NotFound
+ return NotFound(fmt.Errorf("image '%s' not found", imgInfo.Fingerprint))
}
}
diff --git a/lxd/networks.go b/lxd/networks.go
index 23d4eaf5c..5b486fd64 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -14,6 +14,7 @@ import (
"github.com/gorilla/mux"
log "github.com/lxc/lxd/shared/log15"
+ "github.com/pkg/errors"
lxd "github.com/lxc/lxd/client"
"github.com/lxc/lxd/lxd/cluster"
@@ -435,7 +436,7 @@ func networkDelete(d *Daemon, r *http.Request) Response {
// Get the existing network
n, err := networkLoadByName(state, name)
if err != nil {
- return NotFound
+ return NotFound(err)
}
withDatabase := true
@@ -503,7 +504,7 @@ func networkPost(d *Daemon, r *http.Request) Response {
// Get the existing network
n, err := networkLoadByName(state, name)
if err != nil {
- return NotFound
+ return NotFound(err)
}
// Sanity checks
@@ -523,7 +524,7 @@ func networkPost(d *Daemon, r *http.Request) Response {
}
if shared.StringInSlice(req.Name, networks) {
- return Conflict
+ return Conflict(fmt.Errorf("network '%s' already exists", req.Name))
}
// Rename it
@@ -614,7 +615,7 @@ func doNetworkUpdate(d *Daemon, name string, oldConfig map[string]string, req ap
// Load the network
n, err := networkLoadByName(d.State(), name)
if err != nil {
- return NotFound
+ return NotFound(err)
}
err = n.Update(req)
@@ -639,7 +640,7 @@ func networkLeasesGet(d *Daemon, r *http.Request) Response {
// Validate that we do have leases for it
if !n.Managed || n.Type != "bridge" {
- return NotFound
+ return NotFound(errors.New("leases not found"))
}
if !shared.PathExists(leaseFile) {
diff --git a/lxd/operations.go b/lxd/operations.go
index 849256ed2..299ab8107 100644
--- a/lxd/operations.go
+++ b/lxd/operations.go
@@ -492,7 +492,7 @@ func operationAPIDelete(d *Daemon, r *http.Request) Response {
op, err := operationGet(id)
if err != nil {
- return NotFound
+ return NotFound(err)
}
_, err = op.Cancel()
@@ -554,7 +554,7 @@ func operationAPIWaitGet(d *Daemon, r *http.Request) Response {
id := mux.Vars(r)["id"]
op, err := operationGet(id)
if err != nil {
- return NotFound
+ return NotFound(err)
}
_, err = op.WaitFinal(timeout)
diff --git a/lxd/profiles.go b/lxd/profiles.go
index fee793eb2..c5c8b134b 100644
--- a/lxd/profiles.go
+++ b/lxd/profiles.go
@@ -294,7 +294,7 @@ func profilePost(d *Daemon, r *http.Request) Response {
// Check that the name isn't already in use
id, _, _ := d.cluster.ProfileGet(req.Name)
if id > 0 {
- return Conflict
+ return Conflict(fmt.Errorf("name '%s' already in use", req.Name))
}
if strings.Contains(req.Name, "/") {
diff --git a/lxd/response.go b/lxd/response.go
index 85ac1fd92..64321987d 100644
--- a/lxd/response.go
+++ b/lxd/response.go
@@ -492,11 +492,37 @@ func (r *errorResponse) Render(w http.ResponseWriter) error {
return nil
}
-/* Some standard responses */
-var NotImplemented = &errorResponse{http.StatusNotImplemented, "not implemented"}
-var NotFound = &errorResponse{http.StatusNotFound, "not found"}
-var Forbidden = &errorResponse{http.StatusForbidden, "not authorized"}
-var Conflict = &errorResponse{http.StatusConflict, "already exists"}
+func NotImplemented(err error) Response {
+ message := "not implemented"
+ if err != nil {
+ message = err.Error()
+ }
+ return &errorResponse{http.StatusNotImplemented, message}
+}
+
+func NotFound(err error) Response {
+ message := "not found"
+ if err != nil {
+ message = err.Error()
+ }
+ return &errorResponse{http.StatusNotFound, message}
+}
+
+func Forbidden(err error) Response {
+ message := "not authorized"
+ if err != nil {
+ message = err.Error()
+ }
+ return &errorResponse{http.StatusForbidden, message}
+}
+
+func Conflict(err error) Response {
+ message := "already exists"
+ if err != nil {
+ message = err.Error()
+ }
+ return &errorResponse{http.StatusConflict, message}
+}
func Unavailable(err error) Response {
message := "unavailable"
@@ -526,17 +552,17 @@ func SmartError(err error) Response {
case nil:
return EmptySyncResponse
case os.ErrNotExist:
- return NotFound
+ return NotFound(err)
case sql.ErrNoRows:
- return NotFound
+ return NotFound(err)
case db.ErrNoSuchObject:
- return NotFound
+ return NotFound(err)
case os.ErrPermission:
- return Forbidden
+ return Forbidden(err)
case db.ErrAlreadyDefined:
- return Conflict
+ return Conflict(err)
case sqlite3.ErrConstraintUnique:
- return Conflict
+ return Conflict(err)
default:
return InternalError(err)
}
diff --git a/lxd/storage_pools.go b/lxd/storage_pools.go
index 17bff4bee..3c0568774 100644
--- a/lxd/storage_pools.go
+++ b/lxd/storage_pools.go
@@ -499,7 +499,7 @@ func storagePoolDelete(d *Daemon, r *http.Request) Response {
poolID, err := d.cluster.StoragePoolGetID(poolName)
if err != nil {
- return NotFound
+ return NotFound(err)
}
// If this is not an internal cluster request, check if the storage
diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go
index 12a46a149..ede82ff1f 100644
--- a/lxd/storage_volumes.go
+++ b/lxd/storage_volumes.go
@@ -223,7 +223,7 @@ func doVolumeCreateOrCopy(d *Daemon, poolName string, req *api.StorageVolumesPos
func doVolumeMigration(d *Daemon, poolName string, req *api.StorageVolumesPost) Response {
// Validate migration mode
if req.Source.Mode != "pull" && req.Source.Mode != "push" {
- return NotImplemented
+ return NotImplemented(fmt.Errorf("mode '%s' not implemented", req.Source.Mode))
}
storage, err := storagePoolVolumeDBCreateInternal(d.State(), poolName, req)
@@ -418,8 +418,10 @@ func storagePoolVolumeTypePost(d *Daemon, r *http.Request) Response {
// Check that the name isn't already in use.
_, err = d.cluster.StoragePoolNodeVolumeGetTypeID(req.Name,
storagePoolVolumeTypeCustom, poolID)
- if err == nil || err != nil && err != db.ErrNoSuchObject {
- return Conflict
+ if err == nil {
+ return Conflict(fmt.Errorf("name '%s' already in use", req.Name))
+ } else if err != nil && err != db.ErrNoSuchObject {
+ return Conflict(err)
}
doWork := func() error {
@@ -758,7 +760,7 @@ func storagePoolVolumeTypeDelete(d *Daemon, r *http.Request) Response {
s, err := storagePoolVolumeInit(d.State(), poolName, volumeName, volumeType)
if err != nil {
- return NotFound
+ return NotFound(err)
}
switch volumeType {
More information about the lxc-devel
mailing list