diff --git a/pkg/client/sync_client.go b/pkg/client/sync_client.go index dadb8bd4..a0a75081 100644 --- a/pkg/client/sync_client.go +++ b/pkg/client/sync_client.go @@ -394,6 +394,9 @@ func (client *SyncClient) DownloadToDst(srcFilePath, dstFilePath string) error { } } dst, err := os.Create(dstFilePath) + if err != nil { + return errors.Wrapf(err, "failed to create the dst file before download") + } defer dst.Close() httpClient := &http.Client{Timeout: 0} diff --git a/pkg/datasource/service.go b/pkg/datasource/service.go index 5f1a5e50..b3e29ac5 100644 --- a/pkg/datasource/service.go +++ b/pkg/datasource/service.go @@ -158,36 +158,33 @@ func (s *Service) waitForBeginning() { ticker := time.NewTicker(RetryInterval) defer ticker.Stop() for { - select { - case <-ticker.C: - if count >= RetryCount { - s.lock.Lock() - s.timeoutBegin = true - s.dsInfo.State = string(types.StateFailed) - s.dsInfo.Message = TimeoutBeginErrorMessage - s.lock.Unlock() - if err := s.syncClient.Delete(s.filePath); err != nil { - s.log.Errorf("DataSource Service: failed to do cleanup after timeout waiting for the datasource processing begin: %v", err) - } - return + <-ticker.C + if count >= RetryCount { + s.lock.Lock() + s.timeoutBegin = true + s.dsInfo.State = string(types.StateFailed) + s.dsInfo.Message = TimeoutBeginErrorMessage + s.lock.Unlock() + if err := s.syncClient.Delete(s.filePath); err != nil { + s.log.WithError(err).Error("DataSource Service: failed to do cleanup after timeout waiting for the datasource processing begin") } + return + } - count++ + count++ - dsInfo, err := s.syncDataSourceFileInfo() - if err != nil { - s.log.Debugf("DataSource Service: failed to get the datasource file info, the processing may be not begin yet: %v", err) - continue - } + dsInfo, err := s.syncDataSourceFileInfo() + if err != nil { + s.log.Debugf("DataSource Service: failed to get the datasource file info, the processing may be not begin yet: %v", err) + continue + } - notBeginYet := dsInfo.State == "" || dsInfo.State == string(types.StatePending) || dsInfo.State == string(types.StateStarting) - if !notBeginYet { - return - } - s.log.Debugf("DataSource Service: datasource file is state %v, the processing is not begin yet", dsInfo.State) + notBeginYet := dsInfo.State == "" || dsInfo.State == string(types.StatePending) || dsInfo.State == string(types.StateStarting) + if !notBeginYet { + return } + s.log.Debugf("DataSource Service: datasource file is state %v, the processing is not begin yet", dsInfo.State) } - } func (s *Service) restoreFromBackupURL() (err error) { @@ -315,7 +312,9 @@ func (s *Service) Get(writer http.ResponseWriter, request *http.Request) { return } writer.Header().Set("Content-Type", "application/json") - writer.Write(outgoingJSON) + if _, err := writer.Write(outgoingJSON); err != nil { + logrus.WithError(err).Warn("Failed to write response") + } } func (s *Service) Transfer(writer http.ResponseWriter, request *http.Request) { diff --git a/pkg/manager/server_test.go b/pkg/manager/server_test.go index 140b67a3..4ac2468f 100644 --- a/pkg/manager/server_test.go +++ b/pkg/manager/server_test.go @@ -314,6 +314,7 @@ func (s *TestSuite) TestSingleBackingImageFetch(c *C) { c.Assert(dsInfo.FilePath, Equals, dsFilePath) bi, err := getAndWaitFileState(cli1, biName, biUUID, string(types.StateReady), 30) + c.Assert(err, IsNil) c.Assert(bi.Status.CurrentChecksum, Equals, checksum) _, err = os.Stat(dsFilePath) diff --git a/pkg/manager/service.go b/pkg/manager/service.go index d096ddc3..4c45e4c1 100644 --- a/pkg/manager/service.go +++ b/pkg/manager/service.go @@ -147,7 +147,9 @@ func (m *Manager) monitoring() { done = true break case <-ticker.C: - m.listAndUpdate() + if _, err := m.listAndUpdate(); err != nil { + m.log.WithError(err).Warn("failed to list and update backing image files") + } } if done { break @@ -372,7 +374,7 @@ func (m *Manager) Sync(ctx context.Context, req *rpc.SyncRequest) (resp *rpc.Bac // sender.Send is a non-blocking call sender := client.NewBackingImageManagerClient(req.FromAddress) if err = sender.Send(req.Spec.Name, req.Spec.Uuid, toAddress); err != nil { - err = errors.Wrapf(err, "sender failed to request backing image sending to %v", toAddress) + err = errors.Wrapf(err, "sender failed to request backing image sending to %v", toAddress) // nolint:ineffassign,staticcheck return } diff --git a/pkg/sync/server_test.go b/pkg/sync/server_test.go index 6cbfed12..dbf610af 100644 --- a/pkg/sync/server_test.go +++ b/pkg/sync/server_test.go @@ -643,7 +643,7 @@ func (s *SyncTestSuite) TestReadyFileValidation(c *C) { fInfo, err = getAndWaitFileState(cli, curPath, string(types.StateUnknown), 1) c.Assert(err, IsNil) c.Check(fInfo.ModificationTime == oldModificationTime, Equals, false) - fInfo, err = getAndWaitFileState(cli, curPath, string(types.StateFailed), 10) + _, err = getAndWaitFileState(cli, curPath, string(types.StateFailed), 10) c.Assert(err, IsNil) err = cli.Delete(curPath) diff --git a/pkg/sync/service.go b/pkg/sync/service.go index 5a2685d8..6e507570 100644 --- a/pkg/sync/service.go +++ b/pkg/sync/service.go @@ -131,7 +131,9 @@ func (s *Service) List(writer http.ResponseWriter, request *http.Request) { return } writer.Header().Set("Content-Type", "application/json") - writer.Write(outgoingJSON) + if _, err := writer.Write(outgoingJSON); err != nil { + logrus.WithError(err).Warn("Failed to write response") + } } func (s *Service) Get(writer http.ResponseWriter, request *http.Request) { @@ -157,7 +159,9 @@ func (s *Service) Get(writer http.ResponseWriter, request *http.Request) { return } writer.Header().Set("Content-Type", "application/json") - writer.Write(outgoingJSON) + if _, err := writer.Write(outgoingJSON); err != nil { + logrus.WithError(err).Warn("Failed to write response") + } } func (s *Service) Delete(writer http.ResponseWriter, request *http.Request) { @@ -203,7 +207,6 @@ func (s *Service) cleanup(filePath string, deleteFile bool) { if sf != nil && deleteFile { sf.Delete() } - return } func (s *Service) DownloadToDst(writer http.ResponseWriter, request *http.Request) { @@ -249,7 +252,6 @@ func (s *Service) DownloadToDst(writer http.ResponseWriter, request *http.Reques err = ioErr return } - return } func (s *Service) checkAndInitSyncFile(filePath, uuid, diskUUID, expectedChecksum string, size int64) (*SyncingFile, error) { diff --git a/pkg/sync/sync_file.go b/pkg/sync/sync_file.go index 3751b5d9..63b93677 100644 --- a/pkg/sync/sync_file.go +++ b/pkg/sync/sync_file.go @@ -267,22 +267,20 @@ func (sf *SyncingFile) waitForProcessingBeginWithTimeout() { ticker := time.NewTicker(RetryInterval) defer ticker.Stop() for { - select { - case <-ticker.C: - count++ - sf.lock.Lock() - notBeginYet := sf.state == "" || sf.state == types.StatePending || sf.state == types.StateStarting - if !notBeginYet { - sf.lock.Unlock() - return - } - if count >= RetryCount { - sf.handleFailureNoLock(fmt.Errorf("failed to wait for processing begin in %v seconds, current state %v", RetryCount, sf.state)) - sf.lock.Unlock() - return - } + <-ticker.C + count++ + sf.lock.Lock() + notBeginYet := sf.state == "" || sf.state == types.StatePending || sf.state == types.StateStarting + if !notBeginYet { sf.lock.Unlock() + return + } + if count >= RetryCount { + sf.handleFailureNoLock(fmt.Errorf("failed to wait for processing begin in %v seconds, current state %v", RetryCount, sf.state)) + sf.lock.Unlock() + return } + sf.lock.Unlock() } } @@ -291,18 +289,16 @@ func (sf *SyncingFile) WaitForStateNonPending() error { ticker := time.NewTicker(RetryInterval) defer ticker.Stop() for { - select { - case <-ticker.C: - count++ - sf.lock.RLock() - state := sf.state - sf.lock.RUnlock() - if state != types.StatePending { - return nil - } - if count >= LargeRetryCount { - return fmt.Errorf("sync file is still in empty state after %v second", LargeRetryCount) - } + <-ticker.C + count++ + sf.lock.RLock() + state := sf.state + sf.lock.RUnlock() + if state != types.StatePending { + return nil + } + if count >= LargeRetryCount { + return fmt.Errorf("sync file is still in empty state after %v second", LargeRetryCount) } } } @@ -387,7 +383,6 @@ func (sf *SyncingFile) Delete() { if err := os.RemoveAll(configFilePath); err != nil { sf.log.Warnf("SyncingFile: failed to delete sync file config file %v: %v", configFilePath, err) } - return } func (sf *SyncingFile) GetFileReader() (io.ReadCloser, error) { diff --git a/scripts/validate b/scripts/validate index b02ef99c..5c83da06 100755 --- a/scripts/validate +++ b/scripts/validate @@ -10,20 +10,8 @@ PACKAGES="$(find -name '*.go' | xargs -I{} dirname {} | cut -f2 -d/ | sort -u | echo Running: go vet go vet ${PACKAGES} -if [ ! -z "${DRONE_REPO}" ] && [ ! -z "${DRONE_PULL_REQUEST}" ]; then - wget https://github.com/$DRONE_REPO/pull/$DRONE_PULL_REQUEST.patch - echo "Running: golangci-lint run --new-from-patch=${DRONE_PULL_REQUEST}.patch" - golangci-lint run --new-from-patch="${DRONE_PULL_REQUEST}.patch" - rm "${DRONE_PULL_REQUEST}.patch" -elif [ ! -z "${DRONE_COMMIT_REF}" ]; then - echo "Running: golangci-lint run --new-from-rev=${DRONE_COMMIT_REF}" - golangci-lint run --new-from-rev=${DRONE_COMMIT_REF} -else - git symbolic-ref -q HEAD && REV="HEAD" || REV="HEAD^" - headSHA=$(git rev-parse --short=12 ${REV}) - echo "Running: golangci-lint run --new-from-rev=${headSHA}" - golangci-lint run --new-from-rev=${headSHA} -fi +echo "Running: golangci-lint" +golangci-lint run --timeout=5m echo Running: go fmt test -z "$(go fmt ${PACKAGES} | tee /dev/stderr)"