From c61ec9fe80157339ac5cdf126010c02877bb81f6 Mon Sep 17 00:00:00 2001 From: Dinos Kousidis Date: Sun, 19 Jan 2020 17:17:52 +0100 Subject: [PATCH] Add detailed error message in fluxctl sync * Extends v6.GitConfig to include `Error` field. * In case of `fluxctl sync` error, prints the `Error` field in the output. * Bumps api version. Signed-off-by: Dinos Kousidis --- cmd/fluxctl/sync_cmd.go | 4 ++-- pkg/api/api.go | 4 ++-- pkg/api/v12/api.go | 22 ++++++++++++++++++++++ pkg/api/v6/api.go | 1 + pkg/daemon/daemon.go | 14 ++++++++++++++ pkg/http/client/client.go | 7 +++++++ pkg/http/daemon/server.go | 13 +++++++++++++ pkg/http/routes.go | 2 ++ pkg/http/transport.go | 1 + pkg/remote/logging.go | 10 ++++++++++ pkg/remote/metrics.go | 11 +++++++++++ pkg/remote/mock.go | 8 ++++++++ pkg/remote/rpc/baseclient.go | 5 +++++ pkg/remote/rpc/server.go | 20 ++++++++++++++++++++ 14 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 pkg/api/v12/api.go diff --git a/cmd/fluxctl/sync_cmd.go b/cmd/fluxctl/sync_cmd.go index c3d3880f65..67db05a890 100644 --- a/cmd/fluxctl/sync_cmd.go +++ b/cmd/fluxctl/sync_cmd.go @@ -35,7 +35,7 @@ func (opts *syncOpts) RunE(cmd *cobra.Command, args []string) error { ctx := context.Background() - gitConfig, err := opts.API.GitRepoConfig(ctx, false) + gitConfig, err := opts.API.GitRepoConfigWithError(ctx, false) if err != nil { return err } @@ -46,7 +46,7 @@ func (opts *syncOpts) RunE(cmd *cobra.Command, args []string) error { case git.RepoReady: break default: - return fmt.Errorf("git repository %s is not ready to sync (status: %s)", gitConfig.Remote.URL, string(gitConfig.Status)) + return fmt.Errorf("git repository %s is not ready to sync\n(Full error: %v)", gitConfig.Remote.URL, gitConfig.Error) } fmt.Fprintf(cmd.OutOrStderr(), "Synchronizing with %s\n", gitConfig.Remote.URL) diff --git a/pkg/api/api.go b/pkg/api/api.go index 41429368e4..975d2bb7cb 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -1,10 +1,10 @@ package api -import "github.com/fluxcd/flux/pkg/api/v11" +import v12 "github.com/fluxcd/flux/pkg/api/v12" // Server defines the minimal interface a Flux must satisfy to adequately serve a // connecting fluxctl. This interface specifically does not facilitate connecting // to Weave Cloud. type Server interface { - v11.Server + v12.Server } diff --git a/pkg/api/v12/api.go b/pkg/api/v12/api.go new file mode 100644 index 0000000000..49c756fddd --- /dev/null +++ b/pkg/api/v12/api.go @@ -0,0 +1,22 @@ +// This package defines the types for Flux API version 12. +package v12 + +import ( + "context" + + v11 "github.com/fluxcd/flux/pkg/api/v11" + v6 "github.com/fluxcd/flux/pkg/api/v6" +) + +// GitConfig extends the v6.GitConfig with Error +type GitConfig struct { + v6.GitConfig + Error string `json:"errors"` +} + +// Server in version 12 +type Server interface { + v11.Server + + GitRepoConfigWithError(ctx context.Context, regenerate bool) (GitConfig, error) +} diff --git a/pkg/api/v6/api.go b/pkg/api/v6/api.go index 7646f7621c..734aa8951d 100644 --- a/pkg/api/v6/api.go +++ b/pkg/api/v6/api.go @@ -58,6 +58,7 @@ type GitConfig struct { Remote GitRemoteConfig `json:"remote"` PublicSSHKey ssh.PublicKey `json:"publicSSHKey"` Status git.GitRepoStatus `json:"status"` + Error string `json:"errors"` } type Deprecated interface { diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 777b2997fe..8d9ec9b63c 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -14,6 +14,7 @@ import ( "github.com/fluxcd/flux/pkg/api" "github.com/fluxcd/flux/pkg/api/v10" "github.com/fluxcd/flux/pkg/api/v11" + "github.com/fluxcd/flux/pkg/api/v12" "github.com/fluxcd/flux/pkg/api/v6" "github.com/fluxcd/flux/pkg/api/v9" "github.com/fluxcd/flux/pkg/cluster" @@ -640,6 +641,19 @@ func (d *Daemon) GitRepoConfig(ctx context.Context, regenerate bool) (v6.GitConf }, nil } +func (d *Daemon) GitRepoConfigWithError(ctx context.Context, regenerate bool) (v12.GitConfig, error) { + gitConfig, err := d.GitRepoConfig(ctx, regenerate) + if err != nil { + return v12.GitConfig{}, err + } + _, errorField := d.Repo.Status() + + return v12.GitConfig{ + gitConfig, + errorField.Error(), + }, nil +} + // Non-api.Server methods // WithWorkingClone applies the given func to a fresh, writable clone diff --git a/pkg/http/client/client.go b/pkg/http/client/client.go index 32be770d02..f20a8b8315 100644 --- a/pkg/http/client/client.go +++ b/pkg/http/client/client.go @@ -15,6 +15,7 @@ import ( "github.com/fluxcd/flux/pkg/api" "github.com/fluxcd/flux/pkg/api/v10" "github.com/fluxcd/flux/pkg/api/v11" + "github.com/fluxcd/flux/pkg/api/v12" "github.com/fluxcd/flux/pkg/api/v6" "github.com/fluxcd/flux/pkg/api/v9" fluxerr "github.com/fluxcd/flux/pkg/errors" @@ -130,6 +131,12 @@ func (c *Client) GitRepoConfig(ctx context.Context, regenerate bool) (v6.GitConf return res, err } +func (c *Client) GitRepoConfigWithError(ctx context.Context, regenerate bool) (v12.GitConfig, error) { + var res v12.GitConfig + err := c.methodWithResp(ctx, "POST", &res, transport.GitRepoConfigWithError, regenerate) + return res, err +} + // --- Request helpers // Post is a simple query-param only post request diff --git a/pkg/http/daemon/server.go b/pkg/http/daemon/server.go index 451e9814d0..b3db366467 100644 --- a/pkg/http/daemon/server.go +++ b/pkg/http/daemon/server.go @@ -64,6 +64,7 @@ func NewHandler(s api.Server, r *mux.Router) http.Handler { r.Get(transport.SyncStatus).HandlerFunc(handle.SyncStatus) r.Get(transport.Export).HandlerFunc(handle.Export) r.Get(transport.GitRepoConfig).HandlerFunc(handle.GitRepoConfig) + r.Get(transport.GitRepoConfigWithError).HandlerFunc(handle.GitRepoConfigWithError) // These handlers persist to support requests from older fluxctls. In general we // should avoid adding references to them so that they can eventually be removed. @@ -231,6 +232,18 @@ func (s HTTPServer) GitRepoConfig(w http.ResponseWriter, r *http.Request) { transport.JSONResponse(w, r, res) } +func (s HTTPServer) GitRepoConfigWithError(w http.ResponseWriter, r *http.Request) { + var regenerate bool + if err := json.NewDecoder(r.Body).Decode(®enerate); err != nil { + transport.WriteError(w, r, http.StatusBadRequest, err) + } + res, err := s.server.GitRepoConfigWithError(r.Context(), regenerate) + if err != nil { + transport.ErrorResponse(w, r, err) + } + transport.JSONResponse(w, r, res) +} + // --- handlers supporting deprecated requests func (s HTTPServer) UpdateImages(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/http/routes.go b/pkg/http/routes.go index 3d062e7eba..5860f75c70 100644 --- a/pkg/http/routes.go +++ b/pkg/http/routes.go @@ -15,6 +15,7 @@ const ( SyncStatus = "SyncStatus" Export = "Export" GitRepoConfig = "GitRepoConfig" + GitRepoConfigWithError = "GitRepoConfigWithError" UpdateImages = "UpdateImages" UpdatePolicies = "UpdatePolicies" @@ -38,4 +39,5 @@ const ( RegisterDaemonV9 = "RegisterDaemonV9" RegisterDaemonV10 = "RegisterDaemonV10" RegisterDaemonV11 = "RegisterDaemonV11" + RegisterDaemonV12 = "RegisterDaemonV12" ) diff --git a/pkg/http/transport.go b/pkg/http/transport.go index 58338f7a34..3108d71417 100644 --- a/pkg/http/transport.go +++ b/pkg/http/transport.go @@ -43,6 +43,7 @@ func NewAPIRouter() *mux.Router { r.NewRoute().Name(SyncStatus).Methods("GET").Path("/v6/sync").Queries("ref", "{ref}") r.NewRoute().Name(Export).Methods("HEAD", "GET").Path("/v6/export") r.NewRoute().Name(GitRepoConfig).Methods("POST").Path("/v9/git-repo-config") + r.NewRoute().Name(GitRepoConfigWithError).Methods("POST").Path("/v12/git-repo-config-with-error") // These routes persist to support requests from older fluxctls. In general we // should avoid adding references to them so that they can eventually be removed. diff --git a/pkg/remote/logging.go b/pkg/remote/logging.go index 0c6578376a..05f03308be 100644 --- a/pkg/remote/logging.go +++ b/pkg/remote/logging.go @@ -8,6 +8,7 @@ import ( "github.com/fluxcd/flux/pkg/api" "github.com/fluxcd/flux/pkg/api/v10" "github.com/fluxcd/flux/pkg/api/v11" + "github.com/fluxcd/flux/pkg/api/v12" "github.com/fluxcd/flux/pkg/api/v6" "github.com/fluxcd/flux/pkg/api/v9" "github.com/fluxcd/flux/pkg/job" @@ -107,6 +108,15 @@ func (p *ErrorLoggingServer) GitRepoConfig(ctx context.Context, regenerate bool) return p.server.GitRepoConfig(ctx, regenerate) } +func (p *ErrorLoggingServer) GitRepoConfigWithError(ctx context.Context, regenerate bool) (_ v12.GitConfig, err error) { + defer func() { + if err != nil { + p.logger.Log("method", "GitRepoConfigWithError", "error", err) + } + }() + return p.server.GitRepoConfigWithError(ctx, regenerate) +} + func (p *ErrorLoggingServer) Ping(ctx context.Context) (err error) { defer func() { if err != nil { diff --git a/pkg/remote/metrics.go b/pkg/remote/metrics.go index 5fe0cb2a77..eadb17633e 100644 --- a/pkg/remote/metrics.go +++ b/pkg/remote/metrics.go @@ -11,6 +11,7 @@ import ( "github.com/fluxcd/flux/pkg/api" "github.com/fluxcd/flux/pkg/api/v10" "github.com/fluxcd/flux/pkg/api/v11" + "github.com/fluxcd/flux/pkg/api/v12" "github.com/fluxcd/flux/pkg/api/v6" "github.com/fluxcd/flux/pkg/api/v9" "github.com/fluxcd/flux/pkg/job" @@ -118,6 +119,16 @@ func (i *instrumentedServer) SyncStatus(ctx context.Context, cursor string) (_ [ return i.s.SyncStatus(ctx, cursor) } +func (i *instrumentedServer) GitRepoConfigWithError(ctx context.Context, regenerate bool) (_ v12.GitConfig, err error) { + defer func(begin time.Time) { + requestDuration.With( + fluxmetrics.LabelMethod, "GitRepoConfigWithError", + fluxmetrics.LabelSuccess, fmt.Sprint(err == nil), + ).Observe(time.Since(begin).Seconds()) + }(time.Now()) + return i.s.GitRepoConfigWithError(ctx, regenerate) +} + func (i *instrumentedServer) GitRepoConfig(ctx context.Context, regenerate bool) (_ v6.GitConfig, err error) { defer func(begin time.Time) { requestDuration.With( diff --git a/pkg/remote/mock.go b/pkg/remote/mock.go index 61ebd396b1..28efe5937c 100644 --- a/pkg/remote/mock.go +++ b/pkg/remote/mock.go @@ -11,6 +11,7 @@ import ( "github.com/fluxcd/flux/pkg/api" "github.com/fluxcd/flux/pkg/api/v10" "github.com/fluxcd/flux/pkg/api/v11" + "github.com/fluxcd/flux/pkg/api/v12" "github.com/fluxcd/flux/pkg/api/v6" "github.com/fluxcd/flux/pkg/api/v9" "github.com/fluxcd/flux/pkg/guid" @@ -49,6 +50,9 @@ type MockServer struct { GitRepoConfigAnswer v6.GitConfig GitRepoConfigError error + + GitRepoConfigWithErrorAnswer v12.GitConfig + GitRepoConfigWithErrorError error } func (p *MockServer) Ping(ctx context.Context) error { @@ -104,6 +108,10 @@ func (p *MockServer) GitRepoConfig(ctx context.Context, regenerate bool) (v6.Git return p.GitRepoConfigAnswer, p.GitRepoConfigError } +func (p *MockServer) GitRepoConfigWithError(ctx context.Context, regenerate bool) (v12.GitConfig, error) { + return p.GitRepoConfigWithErrorAnswer, p.GitRepoConfigWithErrorError +} + var _ api.Server = &MockServer{} // -- Battery of tests for an api.Server implementation. Since these diff --git a/pkg/remote/rpc/baseclient.go b/pkg/remote/rpc/baseclient.go index 02dc3ae23c..9b00c23e8d 100644 --- a/pkg/remote/rpc/baseclient.go +++ b/pkg/remote/rpc/baseclient.go @@ -8,6 +8,7 @@ import ( "github.com/fluxcd/flux/pkg/api" "github.com/fluxcd/flux/pkg/api/v10" "github.com/fluxcd/flux/pkg/api/v11" + "github.com/fluxcd/flux/pkg/api/v12" "github.com/fluxcd/flux/pkg/api/v6" "github.com/fluxcd/flux/pkg/api/v9" "github.com/fluxcd/flux/pkg/job" @@ -67,3 +68,7 @@ func (bc baseClient) SyncStatus(context.Context, string) ([]string, error) { func (bc baseClient) GitRepoConfig(context.Context, bool) (v6.GitConfig, error) { return v6.GitConfig{}, remote.UpgradeNeededError(errors.New("GitRepoConfig method not implemented")) } + +func (bc baseClient) GitRepoConfigWithError(context.Context, bool) (v12.GitConfig, error) { + return v12.GitConfig{}, remote.UpgradeNeededError(errors.New("GitRepoConfigWithError method not implemented")) +} diff --git a/pkg/remote/rpc/server.go b/pkg/remote/rpc/server.go index 7f38250f84..c38014bc15 100644 --- a/pkg/remote/rpc/server.go +++ b/pkg/remote/rpc/server.go @@ -8,6 +8,7 @@ import ( "time" "github.com/fluxcd/flux/pkg/api/v10" + "github.com/fluxcd/flux/pkg/api/v12" "github.com/pkg/errors" @@ -207,6 +208,11 @@ type GitRepoConfigResponse struct { ApplicationError *fluxerr.Error } +type GitRepoConfigWithErrorResponse struct { + Result v12.GitConfig + ApplicationError *fluxerr.Error +} + func (p *RPCServer) GitRepoConfig(regenerate bool, resp *GitRepoConfigResponse) error { ctx, cancel := context.WithTimeout(context.Background(), p.timeout) defer cancel() @@ -220,3 +226,17 @@ func (p *RPCServer) GitRepoConfig(regenerate bool, resp *GitRepoConfigResponse) } return err } + +func (p *RPCServer) GitRepoConfigWithError(regenerate bool, resp *GitRepoConfigWithErrorResponse) error { + ctx, cancel := context.WithTimeout(context.Background(), p.timeout) + defer cancel() + v, err := p.s.GitRepoConfigWithError(ctx, regenerate) + resp.Result = v + if err != nil { + if err, ok := errors.Cause(err).(*fluxerr.Error); ok { + resp.ApplicationError = err + return nil + } + } + return err +}