From e1678a0342d6801640cfd778cbe2732d11ad6567 Mon Sep 17 00:00:00 2001 From: PoAn Yang Date: Tue, 30 Apr 2024 10:51:54 +0800 Subject: [PATCH] fix: golangci-lint error Signed-off-by: PoAn Yang (cherry picked from commit 2709238902957fd8002caa89614c3dbf1e235b91) --- pkg/client/sync_client.go | 3 +++ pkg/datasource/service.go | 49 +++++++++++++++++++------------------- pkg/manager/server_test.go | 1 + pkg/manager/service.go | 6 +++-- pkg/sync/server_test.go | 2 +- pkg/sync/service.go | 10 ++++---- pkg/sync/sync_file.go | 49 +++++++++++++++++--------------------- scripts/validate | 16 ++----------- 8 files changed, 63 insertions(+), 73 deletions(-) diff --git a/pkg/client/sync_client.go b/pkg/client/sync_client.go index 68bdb8b0..c9991b72 100644 --- a/pkg/client/sync_client.go +++ b/pkg/client/sync_client.go @@ -352,6 +352,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 7d9ea803..fb9e916c 100644 --- a/pkg/datasource/service.go +++ b/pkg/datasource/service.go @@ -154,36 +154,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) downloadFromURL(parameters map[string]string) (err error) { @@ -298,7 +295,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 8b420d76..8f183385 100644 --- a/pkg/manager/server_test.go +++ b/pkg/manager/server_test.go @@ -313,6 +313,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 12a5d27d..2d44ddc1 100644 --- a/pkg/manager/service.go +++ b/pkg/manager/service.go @@ -139,7 +139,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 @@ -364,7 +366,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 696edf94..d89b578b 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 22eee077..ba8deb41 100644 --- a/pkg/sync/service.go +++ b/pkg/sync/service.go @@ -130,7 +130,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) { @@ -156,7 +158,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) { @@ -202,7 +206,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) { @@ -248,7 +251,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 4186c627..57e3fce8 100644 --- a/pkg/sync/sync_file.go +++ b/pkg/sync/sync_file.go @@ -243,22 +243,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() } } @@ -267,18 +265,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) } } } @@ -363,7 +359,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)"