Skip to content

Commit

Permalink
fix: golangci-lint error
Browse files Browse the repository at this point in the history
Signed-off-by: PoAn Yang <poan.yang@suse.com>
  • Loading branch information
FrankYang0529 authored and derekbit committed Apr 26, 2024
1 parent de77482 commit 5ae373f
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 50 deletions.
15 changes: 10 additions & 5 deletions app/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,21 @@ func StartCmd() cli.Command {
func cleanup(pm *process.Manager) {
logrus.Infof("Trying to gracefully shut down %v", types.ProcessManagerGrpcService)

pmResp, err := pm.ProcessList(nil, &rpc.ProcessListRequest{})
pmResp, err := pm.ProcessList(context.TODO(), &rpc.ProcessListRequest{})
if err != nil {
logrus.WithError(err).Errorf("Failed to list processes before shutting down %v", types.ProcessManagerGrpcService)
return
}
for _, p := range pmResp.Processes {
pm.ProcessDelete(nil, &rpc.ProcessDeleteRequest{
if _, err := pm.ProcessDelete(context.TODO(), &rpc.ProcessDeleteRequest{
Name: p.Spec.Name,
})
}); err != nil {
logrus.WithError(err).Errorf("Failed to delete process %s", p.Spec.Name)
}
}

for i := 0; i < types.WaitCount; i++ {
pmResp, err := pm.ProcessList(nil, &rpc.ProcessListRequest{})
pmResp, err := pm.ProcessList(context.TODO(), &rpc.ProcessListRequest{})
if err != nil {
logrus.WithError(err).Errorf("Failed to list processes when shutting down %v", types.ProcessManagerGrpcService)
break
Expand Down Expand Up @@ -275,7 +277,7 @@ func start(c *cli.Context) (err error) {
listeners[types.SpdkGrpcService] = spdkGRPCListener
}

g, ctx := errgroup.WithContext(ctx)
g, _ := errgroup.WithContext(ctx)

// Register signal handler
sigs := make(chan os.Signal, 1)
Expand Down Expand Up @@ -375,6 +377,9 @@ func setupDiskGRPCServer(ctx context.Context, listen, spdkServiceAddress string,

func setupSPDKGRPCServer(ctx context.Context, portRange, listen string) (*grpc.Server, net.Listener, error) {
portStart, portEnd, err := util.ParsePortRange(portRange)
if err != nil {
return nil, nil, err
}

srv, err := spdk.NewServer(ctx, portStart, portEnd)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func RPCToInstance(obj *rpc.InstanceResponse) *Instance {
Name: obj.Spec.Name,
Type: obj.Spec.Type,
//lint:ignore SA1019 replaced with DataEngine
BackendStoreDriver: obj.Spec.BackendStoreDriver.String(),
BackendStoreDriver: obj.Spec.BackendStoreDriver.String(), // nolint: staticcheck
DataEngine: dataEngines[obj.Spec.DataEngine.String()],
PortCount: obj.Spec.PortCount,
PortArgs: obj.Spec.PortArgs,
Expand Down
1 change: 1 addition & 0 deletions pkg/disk/disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ func (s *Server) startMonitoring() {
case <-s.ctx.Done():
logrus.Infof("%s: stopped monitoring replicas due to the context done", types.DiskGrpcService)
done = true
default:
}
if done {
break
Expand Down
1 change: 1 addition & 0 deletions pkg/instance/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ func (s *Server) startMonitoring() {
case <-s.ctx.Done():
logrus.Infof("%s: stopped monitoring replicas due to the context done", types.InstanceGrpcService)
done = true
default:
}
if done {
break
Expand Down
14 changes: 11 additions & 3 deletions pkg/process/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"path/filepath"
"sync"
"syscall"

"github.com/sirupsen/logrus"
)

type Executor interface {
Expand Down Expand Up @@ -72,23 +74,29 @@ func (bc *BinaryCommand) StopWithSignal(signal syscall.Signal) {
bc.RLock()
defer bc.RUnlock()
if bc.Process != nil {
bc.Process.Signal(signal)
if err := bc.Process.Signal(signal); err != nil {
logrus.WithError(err).Error("failed to send signal to process")
}
}
}

func (bc *BinaryCommand) Stop() {
bc.RLock()
defer bc.RUnlock()
if bc.Process != nil {
bc.Process.Signal(syscall.SIGINT)
if err := bc.Process.Signal(syscall.SIGINT); err != nil {
logrus.WithError(err).Error("failed to send signal to process")
}
}
}

func (bc *BinaryCommand) Kill() {
bc.RLock()
defer bc.RUnlock()
if bc.Process != nil {
bc.Process.Signal(syscall.SIGKILL)
if err := bc.Process.Signal(syscall.SIGKILL); err != nil {
logrus.WithError(err).Error("failed to send signal to process")
}
}
}

Expand Down
56 changes: 33 additions & 23 deletions pkg/process/process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"testing"
"time"

"github.com/google/uuid"
rpc "github.com/longhorn/types/pkg/generated/imrpc"
"github.com/sirupsen/logrus"
"google.golang.org/grpc"
Expand Down Expand Up @@ -38,10 +37,6 @@ type TestSuite struct {

var _ = Suite(&TestSuite{})

func generateUUID() string {
return uuid.New().String()
}

type ProcessWatcher struct {
grpc.ServerStream
}
Expand Down Expand Up @@ -85,18 +80,21 @@ func (s *TestSuite) TestCRUD(c *C) {
go func(i int) {
defer wg.Done()
name := "test_crud_process-" + strconv.Itoa(i)
go s.pm.ProcessWatch(nil, pw)
go func() {
err := s.pm.ProcessWatch(nil, pw)
c.Assert(err, IsNil)
}()

createReq := &rpc.ProcessCreateRequest{
Spec: createProcessSpec(name, TestBinary),
}
createResp, err := s.pm.ProcessCreate(nil, createReq)
createResp, err := s.pm.ProcessCreate(context.TODO(), createReq)
c.Assert(err, IsNil)
c.Assert(createResp.Status.State, Not(Equals), types.ProcessStateStopping)
c.Assert(createResp.Status.State, Not(Equals), types.ProcessStateStopped)
c.Assert(createResp.Status.State, Not(Equals), types.ProcessStateError)

getResp, err := s.pm.ProcessGet(nil, &rpc.ProcessGetRequest{
getResp, err := s.pm.ProcessGet(context.TODO(), &rpc.ProcessGetRequest{
Name: name,
})
c.Assert(err, IsNil)
Expand All @@ -105,7 +103,7 @@ func (s *TestSuite) TestCRUD(c *C) {
c.Assert(getResp.Status.State, Not(Equals), types.ProcessStateStopped)
c.Assert(getResp.Status.State, Not(Equals), types.ProcessStateError)

listResp, err := s.pm.ProcessList(nil, &rpc.ProcessListRequest{})
listResp, err := s.pm.ProcessList(context.TODO(), &rpc.ProcessListRequest{})
c.Assert(err, IsNil)
c.Assert(listResp.Processes[name], NotNil)
c.Assert(listResp.Processes[name].Spec.Name, Equals, name)
Expand All @@ -115,7 +113,7 @@ func (s *TestSuite) TestCRUD(c *C) {

running := false
for j := 0; j < RetryCount; j++ {
getResp, err := s.pm.ProcessGet(nil, &rpc.ProcessGetRequest{
getResp, err := s.pm.ProcessGet(context.TODO(), &rpc.ProcessGetRequest{
Name: name,
})
c.Assert(err, IsNil)
Expand All @@ -130,7 +128,7 @@ func (s *TestSuite) TestCRUD(c *C) {
deleteReq := &rpc.ProcessDeleteRequest{
Name: name,
}
deleteResp, err := s.pm.ProcessDelete(nil, deleteReq)
deleteResp, err := s.pm.ProcessDelete(context.TODO(), deleteReq)
c.Assert(err, IsNil)
c.Assert(deleteResp.Deleted, Equals, true)
c.Assert(deleteResp.Status.State, Not(Equals), types.ProcessStateStarting)
Expand All @@ -150,7 +148,10 @@ func (s *TestSuite) TestProcessDeletion(c *C) {
wg.Add(1)
go func(i int) {
defer wg.Done()
go s.pm.ProcessWatch(nil, pw)
go func() {
err := s.pm.ProcessWatch(nil, pw)
c.Assert(err, IsNil)
}()
name := "test_process_deletion-" + strconv.Itoa(i)

assertProcessCreation(c, s.pm, name, TestBinary)
Expand Down Expand Up @@ -190,7 +191,10 @@ func (s *TestSuite) TestProcessReplace(c *C) {
wg.Add(1)
go func(i int) {
defer wg.Done()
go s.pm.ProcessWatch(nil, pw)
go func() {
err := s.pm.ProcessWatch(nil, pw)
c.Assert(err, IsNil)
}()
name := "test_process_replace-" + strconv.Itoa(i)
assertProcessCreation(c, s.pm, name, TestBinary)
assertProcessReplace(c, s.pm, name, TestBinaryReplace)
Expand Down Expand Up @@ -218,12 +222,15 @@ func (s *TestSuite) TestProcessReplaceMissingBinary(c *C) {
wg.Add(1)
go func(i int) {
defer wg.Done()
go s.pm.ProcessWatch(nil, pw)
go func() {
err := s.pm.ProcessWatch(nil, pw)
c.Assert(err, NotNil)
}()
name := "test_process_missing_binary_replace-" + strconv.Itoa(i)
assertProcessCreation(c, s.pm, name, TestBinary)

// replacement for a missing binary should error
_, err := s.pm.ProcessReplace(nil, &rpc.ProcessReplaceRequest{
_, err := s.pm.ProcessReplace(context.TODO(), &rpc.ProcessReplaceRequest{
Spec: createProcessSpec(name, TestBinaryMissing),
TerminateSignal: "SIGHUP",
})
Expand All @@ -244,15 +251,18 @@ func (s *TestSuite) TestProcessReplaceDuringDeletion(c *C) {
wg.Add(1)
go func(i int) {
defer wg.Done()
go s.pm.ProcessWatch(nil, pw)
go func() {
err := s.pm.ProcessWatch(nil, pw)
c.Assert(err, NotNil)
}()
name := "test_process_deletion_replace-" + strconv.Itoa(i)
assertProcessCreation(c, s.pm, name, TestBinary)
assertProcessDeletion(c, s.pm, name)

// TODO: we should change the deletion handling in a future version
// since deletion happens async this might error or not
// depending on if the process has already been deleted from the map
replaceResp, err := s.pm.ProcessReplace(nil, &rpc.ProcessReplaceRequest{
replaceResp, err := s.pm.ProcessReplace(context.TODO(), &rpc.ProcessReplaceRequest{
Spec: createProcessSpec(name, TestBinaryReplace),
TerminateSignal: "SIGHUP",
})
Expand Down Expand Up @@ -290,7 +300,7 @@ func assertProcessReplace(c *C, pm *Manager, name, binary string) {
Spec: createProcessSpec(name, binary),
TerminateSignal: "SIGHUP",
}
rsp, err := pm.ProcessReplace(nil, replaceReq)
rsp, err := pm.ProcessReplace(context.TODO(), replaceReq)

c.Assert(err, IsNil)
c.Assert(rsp, NotNil)
Expand All @@ -301,13 +311,13 @@ func assertProcessCreation(c *C, pm *Manager, name, binary string) {
Spec: createProcessSpec(name, binary),
}

createResp, err := pm.ProcessCreate(nil, createReq)
createResp, err := pm.ProcessCreate(context.TODO(), createReq)
c.Assert(err, IsNil)
c.Assert(createResp.Status.State, Not(Equals), types.ProcessStateStopping)
c.Assert(createResp.Status.State, Not(Equals), types.ProcessStateStopped)
c.Assert(createResp.Status.State, Not(Equals), types.ProcessStateError)

createResp, err = pm.ProcessCreate(nil, createReq)
createResp, err = pm.ProcessCreate(context.TODO(), createReq)
c.Assert(createResp, IsNil)
c.Assert(err, NotNil)
c.Assert(status.Code(err), Equals, codes.AlreadyExists)
Expand All @@ -325,7 +335,7 @@ func assertProcessDeletion(c *C, pm *Manager, name string) {
deleteReq := &rpc.ProcessDeleteRequest{
Name: name,
}
deleteResp, err := pm.ProcessDelete(nil, deleteReq)
deleteResp, err := pm.ProcessDelete(context.TODO(), deleteReq)
if err == nil {
c.Assert(deleteResp.Deleted, Equals, true)
} else {
Expand All @@ -346,7 +356,7 @@ func createProcessSpec(name, binary string) *rpc.ProcessSpec {

func waitForProcessState(pm *Manager, name string, predicate func(process *rpc.ProcessResponse) bool) (bool, error) {
for j := 0; j < RetryCount; j++ {
getResp, err := pm.ProcessGet(nil, &rpc.ProcessGetRequest{
getResp, err := pm.ProcessGet(context.TODO(), &rpc.ProcessGetRequest{
Name: name,
})

Expand All @@ -367,7 +377,7 @@ func waitForProcessListState(pm *Manager, predicate func(processes map[string]*r
// or that it has some marker that it's in the process of deletion unfortunately the deletion marker is only on the rpc response
// and not the process struct so there is no way for the process list to signal deletion
for j := 0; j < RetryCount; j++ {
listResp, err := pm.ProcessList(nil, &rpc.ProcessListRequest{})
listResp, err := pm.ProcessList(context.TODO(), &rpc.ProcessListRequest{})
if err != nil {
return false, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ type Proxy struct {
rpc.UnimplementedProxyEngineServiceServer
ctx context.Context
logsDir string
shutdownCh chan error
HealthChecker HealthChecker
ops map[rpc.DataEngine]ProxyOps
}
Expand Down Expand Up @@ -86,6 +85,7 @@ func (p *Proxy) startMonitoring() {
case <-p.ctx.Done():
logrus.Infof("%s: stopped monitoring replicas due to the context done", types.ProxyGRPCService)
done = true
default:
}
if done {
break
Expand Down
8 changes: 7 additions & 1 deletion pkg/util/grpcutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"time"

"google.golang.org/grpc"
"google.golang.org/grpc/backoff"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/keepalive"
Expand All @@ -29,7 +30,12 @@ func Connect(endpoint string, tlsConfig *tls.Config, dialOptions ...grpc.DialOpt
return nil, err
}

dialOptions = append(dialOptions, grpc.WithBackoffMaxDelay(time.Second))
dialOptions = append(dialOptions, grpc.WithConnectParams(grpc.ConnectParams{
Backoff: backoff.Config{
MaxDelay: time.Second,
},
}))

if tlsConfig != nil {
dialOptions = append(dialOptions, grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)))
} else {
Expand Down
2 changes: 0 additions & 2 deletions pkg/util/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ type LonghornFormatter struct {
*logrus.TextFormatter

LogsDir string

logFiles []*os.File
}

type LonghornWriter struct {
Expand Down
16 changes: 2 additions & 14 deletions scripts/validate
Original file line number Diff line number Diff line change
Expand Up @@ -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="origin/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)"

0 comments on commit 5ae373f

Please sign in to comment.