Skip to content

Commit

Permalink
only load body if required on syncthing api calls (#887)
Browse files Browse the repository at this point in the history 
* only load body if required

* user friendly errors

Signed-off-by: Ramiro Berrelleza <rberrelleza@gmail.com>
  • Loading branch information
@rberrelleza
rberrelleza committed Jun 1, 2020
1 parent 971ddde commit 1944ebf
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 45 deletions.
24 changes: 14 additions & 10 deletions cmd/up.go
View file
Expand Up @@ -129,9 +129,9 @@ func Up() *cobra.Command {

log.Yellow("couldn't upgrade syncthing, will try again later")
fmt.Println()
} else {
log.Success("Dependencies successfully installed")
}

log.Success("Dependencies successfully installed")
}

utils.CheckLocalWatchesConfiguration()
Expand All @@ -140,6 +140,7 @@ func Up() *cobra.Command {
if err != nil {
return err
}

if err := dev.UpdateNamespace(namespace); err != nil {
return err
}
Expand Down Expand Up @@ -172,7 +173,7 @@ func RunUp(dev *model.Dev, autoDeploy, build, forcePull, resetSyncthing bool) er
}

if up.Dev.ExecuteOverSSHEnabled() {
log.Success("Experimental SSH mode enabled")
log.Info("execute over SSH mode enabled")
}

defer up.shutdown()
Expand Down Expand Up @@ -307,20 +308,23 @@ func (up *UpContext) Activate(autoDeploy, build, resetSyncthing bool) {

log.Success("Development environment activated")

err = up.sync(resetSyncthing && !up.retry)
if err != nil {
if err := up.sync(resetSyncthing && !up.retry); err != nil {
if !pods.Exists(up.Pod, up.Dev.Namespace, up.Client) {
log.Yellow("\nConnection lost to your development environment, reconnecting...\n")
up.shutdown()
continue
}

up.Exit <- err
return
}

up.success = true

if up.retry {
analytics.TrackReconnect(true, up.getClusterType(), up.isSwap)
}

up.retry = true

log.Success("Files synchronized")
Expand All @@ -331,7 +335,8 @@ func (up *UpContext) Activate(autoDeploy, build, resetSyncthing bool) {
up.Running <- up.runCommand()
}()

prevError := up.WaitUntilExitOrInterrupt()
prevError := up.waitUntilExitOrInterrupt()

if isTerm {
log.Debug("Restoring terminal")
if err := term.RestoreTerminal(inFd, state); err != nil {
Expand Down Expand Up @@ -400,8 +405,8 @@ func (up *UpContext) getCurrentDeployment(autoDeploy bool) (*appsv1.Deployment,
return up.Dev.GevSandbox(), true, nil
}

// WaitUntilExitOrInterrupt blocks execution until a stop signal is sent or a disconnect event or an error
func (up *UpContext) WaitUntilExitOrInterrupt() error {
// waitUntilExitOrInterrupt blocks execution until a stop signal is sent or a disconnect event or an error
func (up *UpContext) waitUntilExitOrInterrupt() error {
for {
select {
case err := <-up.Running:
Expand Down Expand Up @@ -749,8 +754,7 @@ func (up *UpContext) synchronizeFiles() error {
}
}()

err := up.Sy.WaitForCompletion(up.Context, up.Dev, reporter)
if err != nil {
if err := up.Sy.WaitForCompletion(up.Context, up.Dev, reporter); err != nil {
if err == errors.ErrUnknownSyncError {
analytics.TrackSyncError()
return errors.UserError{
Expand Down
8 changes: 4 additions & 4 deletions cmd/up_test.go
View file
Expand Up @@ -26,17 +26,17 @@ import (
"github.com/okteto/okteto/pkg/model"
)

func TestWaitUntilExitOrInterrupt(t *testing.T) {
func Test_waitUntilExitOrInterrupt(t *testing.T) {
up := UpContext{}
up.Running = make(chan error, 1)
up.Running <- nil
err := up.WaitUntilExitOrInterrupt()
err := up.waitUntilExitOrInterrupt()
if err != nil {
t.Errorf("exited with error instead of nil: %s", err)
}

up.Running <- fmt.Errorf("custom-error")
err = up.WaitUntilExitOrInterrupt()
err = up.waitUntilExitOrInterrupt()
if err == nil {
t.Errorf("didn't report proper error")
}
Expand All @@ -47,7 +47,7 @@ func TestWaitUntilExitOrInterrupt(t *testing.T) {

up.Disconnect = make(chan error, 1)
up.Disconnect <- errors.ErrLostSyncthing
err = up.WaitUntilExitOrInterrupt()
err = up.waitUntilExitOrInterrupt()
if err != errors.ErrLostSyncthing {
t.Errorf("exited with error %s instead of %s", err, errors.ErrLostSyncthing)
}
Expand Down
27 changes: 19 additions & 8 deletions pkg/syncthing/api.go
View file
Expand Up @@ -21,6 +21,8 @@ import (
"net/http"
"path"
"time"

"github.com/okteto/okteto/pkg/log"
)

type addAPIKeyTransport struct {
Expand All @@ -41,16 +43,17 @@ func NewAPIClient() *http.Client {
}

// APICall calls the syncthing API and returns the parsed json or an error
func (s *Syncthing) APICall(ctx context.Context, url, method string, code int, params map[string]string, local bool, body []byte) ([]byte, error) {
func (s *Syncthing) APICall(ctx context.Context, url, method string, code int, params map[string]string, local bool, body []byte, readBody bool) ([]byte, error) {
var urlPath string
if local {
urlPath = path.Join(s.GUIAddress, url)
} else {
urlPath = path.Join(s.RemoteGUIAddress, url)
}

req, err := http.NewRequest(method, fmt.Sprintf("http://%s", urlPath), bytes.NewBuffer(body))
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to initialize syncthing API request: %w", err)
}

req = req.WithContext(ctx)
Expand All @@ -66,17 +69,25 @@ func (s *Syncthing) APICall(ctx context.Context, url, method string, code int, p

resp, err := s.Client.Do(req)
if err != nil {
return nil, err
log.Infof("fail to call syncthing API at %s: %s", url, err)
return nil, fmt.Errorf("failed to call syncthing API: %w", err)
}

defer resp.Body.Close()
body, err = ioutil.ReadAll(resp.Body)
if err != nil {
return nil, err
}

if resp.StatusCode != code {
return nil, fmt.Errorf("bad response from syncthing api %s %d: %s", req.URL.String(), resp.StatusCode, string(body))
log.Infof("unexpected response from syncthing API %s %d: %s", req.URL.String(), resp.StatusCode, string(body))
return nil, fmt.Errorf("unexpected response from syncthing API %s %d: %s", req.URL.String(), resp.StatusCode, string(body))
}

if !readBody {
return nil, nil
}

body, err = ioutil.ReadAll(resp.Body)
if err != nil {
log.Infof("failed to read response from syncthing API at %s: %s", url, err)
return nil, fmt.Errorf("failed to read response from syncthing API")
}

return body, nil
Expand Down
44 changes: 21 additions & 23 deletions pkg/syncthing/syncthing.go
View file
Expand Up @@ -31,7 +31,7 @@ import (
"time"

"github.com/okteto/okteto/pkg/config"
"github.com/okteto/okteto/pkg/errors"
okerr "github.com/okteto/okteto/pkg/errors"
"github.com/okteto/okteto/pkg/log"
"github.com/okteto/okteto/pkg/model"
"golang.org/x/crypto/bcrypt"
Expand Down Expand Up @@ -224,11 +224,11 @@ func (s *Syncthing) initConfig() error {
}

if err := ioutil.WriteFile(filepath.Join(s.Home, certFile), cert, 0700); err != nil {
return err
return fmt.Errorf("failed to write syncthing certificate: %w", err)
}

if err := ioutil.WriteFile(filepath.Join(s.Home, keyFile), key, 0700); err != nil {
return err
return fmt.Errorf("failed to write syncthing key: %w", err)
}

return nil
Expand All @@ -238,12 +238,13 @@ func (s *Syncthing) initConfig() error {
func (s *Syncthing) UpdateConfig() error {
buf := new(bytes.Buffer)
if err := configTemplate.Execute(buf, s); err != nil {
return err
return fmt.Errorf("failed to write syncthing configuration template: %w", err)
}

if err := ioutil.WriteFile(filepath.Join(s.Home, configFile), buf.Bytes(), 0700); err != nil {
return err
return fmt.Errorf("failed to write syncthing configuration file: %w", err)
}

return nil
}

Expand All @@ -267,18 +268,15 @@ func (s *Syncthing) Run(ctx context.Context) error {
s.cmd.Env = append(os.Environ(), "STNOUPGRADE=1")

if err := s.cmd.Start(); err != nil {
return err
return fmt.Errorf("failed to start syncthing: %w", err)
}

if s.cmd.Process == nil {
return nil
}

if err := ioutil.WriteFile(
pidPath,
[]byte(strconv.Itoa(s.cmd.Process.Pid)),
0600); err != nil {
return err
if err := ioutil.WriteFile(pidPath, []byte(strconv.Itoa(s.cmd.Process.Pid)), 0600); err != nil {
return fmt.Errorf("failed to write syncthing pid file: %w", err)
}

s.pid = s.cmd.Process.Pid
Expand All @@ -292,7 +290,7 @@ func (s *Syncthing) WaitForPing(ctx context.Context, local bool) error {
ticker := time.NewTicker(100 * time.Millisecond)
log.Infof("waiting for syncthing local=%t to be ready...", local)
for i := 0; i < 150; i++ {
_, err := s.APICall(ctx, "rest/system/ping", "GET", 200, nil, local, nil)
_, err := s.APICall(ctx, "rest/system/ping", "GET", 200, nil, local, nil, false)
if err == nil {
return nil
}
Expand All @@ -313,7 +311,7 @@ func (s *Syncthing) SendStignoreFile(ctx context.Context, dev *model.Dev) {
log.Infof("sending '.stignore' file to the remote syncthing...")
params := getFolderParameter(dev)
ignores := &Ignores{}
body, err := s.APICall(ctx, "rest/db/ignores", "GET", 200, params, true, nil)
body, err := s.APICall(ctx, "rest/db/ignores", "GET", 200, params, true, nil, true)
if err != nil {
log.Infof("error getting 'rest/db/ignores' syncthing API: %s", err)
return
Expand All @@ -337,7 +335,7 @@ func (s *Syncthing) SendStignoreFile(ctx context.Context, dev *model.Dev) {
if err != nil {
log.Infof("error marshaling 'rest/db/ignores': %s", err)
}
_, err = s.APICall(ctx, "rest/db/ignores", "POST", 200, params, false, body)
_, err = s.APICall(ctx, "rest/db/ignores", "POST", 200, params, false, body, false)
if err != nil {
log.Infof("error posting 'rest/db/ignores' syncthing API: %s", err)
return
Expand All @@ -348,7 +346,7 @@ func (s *Syncthing) SendStignoreFile(ctx context.Context, dev *model.Dev) {
func (s *Syncthing) ResetDatabase(ctx context.Context, dev *model.Dev, local bool) error {
log.Infof("reseting syncthing database local=%t...", local)
params := getFolderParameter(dev)
_, err := s.APICall(ctx, "rest/system/reset", "POST", 200, params, local, nil)
_, err := s.APICall(ctx, "rest/system/reset", "POST", 200, params, local, nil, false)
if err != nil {
log.Infof("error posting 'rest/system/reset' local=%t syncthing API: %s", local, err)
return err
Expand All @@ -360,7 +358,7 @@ func (s *Syncthing) ResetDatabase(ctx context.Context, dev *model.Dev, local boo
func (s *Syncthing) Overwrite(ctx context.Context, dev *model.Dev) error {
log.Infof("overriding local changes to the remote syncthing...")
params := getFolderParameter(dev)
_, err := s.APICall(ctx, "rest/db/override", "POST", 200, params, true, nil)
_, err := s.APICall(ctx, "rest/db/override", "POST", 200, params, true, nil, false)
if err != nil {
log.Infof("error posting 'rest/db/override' syncthing API: %s", err)
return err
Expand All @@ -382,7 +380,7 @@ func (s *Syncthing) WaitForScanning(ctx context.Context, dev *model.Dev, local b
return ctx.Err()
}

body, err := s.APICall(ctx, "rest/db/status", "GET", 200, params, local, nil)
body, err := s.APICall(ctx, "rest/db/status", "GET", 200, params, local, nil, true)
if err != nil {
log.Debugf("error calling 'rest/db/status' local=%t syncthing API: %s", local, err)
continue
Expand Down Expand Up @@ -454,7 +452,7 @@ func (s *Syncthing) WaitForCompletion(ctx context.Context, dev *model.Dev, repor
}
retries++
if retries >= 60 {
return errors.ErrUnknownSyncError
return okerr.ErrUnknownSyncError
}
continue
}
Expand All @@ -470,7 +468,7 @@ func (s *Syncthing) WaitForCompletion(ctx context.Context, dev *model.Dev, repor
func (s *Syncthing) GetStatus(ctx context.Context, dev *model.Dev, local bool) (*Status, error) {
params := getFolderParameter(dev)
status := &Status{}
body, err := s.APICall(ctx, "rest/db/status", "GET", 200, params, local, nil)
body, err := s.APICall(ctx, "rest/db/status", "GET", 200, params, local, nil, true)
if err != nil {
return nil, err
}
Expand All @@ -491,7 +489,7 @@ func (s *Syncthing) GetCompletion(ctx context.Context, dev *model.Dev, local boo
params["device"] = localDeviceID
}
completion := &Completion{}
body, err := s.APICall(ctx, "rest/db/completion", "GET", 200, params, local, nil)
body, err := s.APICall(ctx, "rest/db/completion", "GET", 200, params, local, nil, true)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -524,7 +522,7 @@ func (s *Syncthing) GetFolderErrors(ctx context.Context, dev *model.Dev, local b
params["timeout"] = "15"
params["events"] = "FolderErrors"
folderErrorsList := []FolderErrors{}
body, err := s.APICall(ctx, "rest/events", "GET", 200, params, local, nil)
body, err := s.APICall(ctx, "rest/events", "GET", 200, params, local, nil, true)
if err != nil {
return err
}
Expand Down Expand Up @@ -555,7 +553,7 @@ func (s *Syncthing) GetFolderErrors(ctx context.Context, dev *model.Dev, local b
// Restart restarts the syncthing process
func (s *Syncthing) Restart(ctx context.Context) error {
log.Infof("restarting syncthing...")
_, err := s.APICall(ctx, "rest/system/restart", "POST", 200, nil, true, nil)
_, err := s.APICall(ctx, "rest/system/restart", "POST", 200, nil, true, nil, false)
return err
}

Expand Down Expand Up @@ -598,7 +596,7 @@ func (s *Syncthing) Save(dev *model.Dev) error {

syncthingInfoFile := config.GetSyncthingInfoFile(dev.Namespace, dev.Name)
if err := ioutil.WriteFile(syncthingInfoFile, marshalled, 0600); err != nil {
return err
return fmt.Errorf("failed to write syncthing info file: %w", err)
}

return nil
Expand Down

0 comments on commit 1944ebf

Please sign in to comment.