diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 9f8654b2..71fd363f 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -11,12 +11,31 @@ jobs: - uses: actions/setup-go@v3 with: go-version: 1.21 + - uses: actions/checkout@v3 + + - name: golangci-lint + uses: golangci/golangci-lint-action@v3 + with: + version: latest + # Matches the "primary" golangci-lint command in the Makefile + args: | + --timeout 3m --disable-all --enable=govet --enable=gofmt --enable=goimports --enable=misspell \ + --skip-dirs "vendor" \ + --skip-dirs "webdash" \ + --skip-dirs "cmd/webdash" \ + --skip-dirs "funnel-work-dir" \ + -e '.*bundle.go' -e ".*pb.go" -e ".*pb.gw.go" \ + ./... + - name: golangci-lint uses: golangci/golangci-lint-action@v3 with: version: latest - args: --timeout 3m --verbose -D unused -D errcheck -D staticcheck -D govet -D gosimple -D ineffassign + # Matches the "termdash" golangci-lint command in the Makefile + args: | + --timeout 3m --disable-all --enable=vet --enable=gofmt --enable=goimports --enable=misspell \ + ./cmd/termdash/... build: runs-on: ubuntu-latest diff --git a/compute/scheduler/node.go b/compute/scheduler/node.go index 6f4df3bc..4da0bece 100644 --- a/compute/scheduler/node.go +++ b/compute/scheduler/node.go @@ -41,7 +41,7 @@ func NewNodeProcess(ctx context.Context, conf config.Config, factory Worker, log conf: conf, client: cli, log: log, - resources: &res, + resources: res, workerRun: factory, workers: newRunSet(), timeout: timeout, @@ -154,7 +154,7 @@ func (n *NodeProcess) sync(ctx context.Context) { // Node data has been updated. Send back to server for database update. var derr error - *n.resources, derr = detectResources(n.conf.Node, n.conf.Worker.WorkDir) + n.resources, derr = detectResources(n.conf.Node, n.conf.Worker.WorkDir) if derr != nil { n.log.Error("error detecting resources", "error", derr) } diff --git a/compute/scheduler/testutils_test.go b/compute/scheduler/testutils_test.go index 6be88cd4..ab816a90 100644 --- a/compute/scheduler/testutils_test.go +++ b/compute/scheduler/testutils_test.go @@ -2,6 +2,7 @@ package scheduler import ( "context" + "fmt" "io/ioutil" "testing" "time" @@ -32,7 +33,7 @@ func newTestNode(conf config.Config, t *testing.T) testNode { conf: conf, client: s, log: log, - resources: &res, + resources: res, workerRun: NoopWorker, workers: newRunSet(), timeout: util.NewIdleTimeout(time.Duration(conf.Node.Timeout)), @@ -81,14 +82,27 @@ func (t *testNode) AddTasks(ids ...string) { func timeLimit(t *testing.T, d time.Duration) func() { stop := make(chan struct{}) + errCh := make(chan error, 1) // Channel to report errors + go func() { select { case <-time.NewTimer(d).C: - t.Fatal("time limit expired") + errCh <- fmt.Errorf("time limit expired") // Send error case <-stop: + return } }() + + // This is the cancel function that will be returned return func() { close(stop) + select { + case err := <-errCh: + if err != nil { + t.Fatal(err) // Report error from the main goroutine + } + default: + // No error, do nothing + } } } diff --git a/compute/scheduler/util.go b/compute/scheduler/util.go index 5e99b5e2..3e11a796 100644 --- a/compute/scheduler/util.go +++ b/compute/scheduler/util.go @@ -23,8 +23,8 @@ func GenNodeID() string { // // Upon error, detectResources will return the resources given by the config // with the error. -func detectResources(conf config.Node, workdir string) (Resources, error) { - res := Resources{ +func detectResources(conf config.Node, workdir string) (*Resources, error) { + res := &Resources{ Cpus: conf.Resources.Cpus, RamGb: conf.Resources.RamGb, DiskGb: conf.Resources.DiskGb, diff --git a/database/boltdb/scheduler.go b/database/boltdb/scheduler.go index bfa032c4..bbdf6715 100644 --- a/database/boltdb/scheduler.go +++ b/database/boltdb/scheduler.go @@ -89,7 +89,7 @@ func (taskBolt *BoltDB) GetNode(ctx context.Context, req *scheduler.GetNodeReque }) if err == errNotFound { - return nil, status.Errorf(codes.NotFound, fmt.Sprintf("%v: nodeID: %s", err.Error(), req.Id)) + return nil, status.Errorf(codes.NotFound, "foo") } if err != nil { diff --git a/database/elastic/scheduler.go b/database/elastic/scheduler.go index 5a579146..95f879a7 100644 --- a/database/elastic/scheduler.go +++ b/database/elastic/scheduler.go @@ -54,7 +54,7 @@ func (es *Elastic) GetNode(ctx context.Context, req *scheduler.GetNodeRequest) ( Do(ctx) if elastic.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, fmt.Sprintf("%v: nodeID: %s", err.Error(), req.Id)) + return nil, status.Errorf(codes.NotFound, "foo") } if err != nil { return nil, err diff --git a/database/mongodb/scheduler.go b/database/mongodb/scheduler.go index 33de3476..a9452e59 100644 --- a/database/mongodb/scheduler.go +++ b/database/mongodb/scheduler.go @@ -66,7 +66,7 @@ func (db *MongoDB) GetNode(ctx context.Context, req *scheduler.GetNodeRequest) ( var node scheduler.Node err := db.nodes(db.client).FindOne(context.TODO(), bson.M{"id": req.Id}).Decode(&node) if err == mongo.ErrNoDocuments { - return nil, status.Errorf(codes.NotFound, fmt.Sprintf("%v: nodeID: %s", err, req.Id)) + return nil, status.Errorf(codes.NotFound, "foo") } return &node, nil @@ -78,7 +78,7 @@ func (db *MongoDB) DeleteNode(ctx context.Context, req *scheduler.Node) (*schedu _, err := db.nodes(db.client).DeleteOne(context.TODO(), bson.M{"id": req.Id}) fmt.Println("DeleteNode", req.Id, err) if err == mongo.ErrNoDocuments { - return nil, status.Errorf(codes.NotFound, fmt.Sprintf("%v: nodeID: %s", err, req.Id)) + return nil, status.Errorf(codes.NotFound, "foo") } return nil, err } diff --git a/server/marshal.go b/server/marshal.go index d52925fc..0da37422 100644 --- a/server/marshal.go +++ b/server/marshal.go @@ -22,12 +22,12 @@ type CustomMarshal struct { func NewMarshaler() runtime.Marshaler { return &CustomMarshal{ m: &runtime.JSONPb{ - protojson.MarshalOptions{ + MarshalOptions: protojson.MarshalOptions{ Indent: " ", EmitUnpopulated: true, UseProtoNames: true, }, - protojson.UnmarshalOptions{}, + UnmarshalOptions: protojson.UnmarshalOptions{}, }, } } @@ -44,10 +44,10 @@ func (marshal *CustomMarshal) ContentType(i interface{}) string { func (mclean *CustomMarshal) Marshal(v interface{}) ([]byte, error) { list, ok := v.(*tes.ListTasksResponse) - if ok { - // v is of type *tes.ListTasksResponse + if ok { + // v is of type *tes.ListTasksResponse return mclean.MarshalList(list) - } + } task, ok := v.(*tes.Task) if ok { @@ -98,10 +98,10 @@ func (mclean *CustomMarshal) DetectView(task *tes.Task) (tes.View, error) { // return a MINIMAL view return tes.View_MINIMAL, nil } - + if len(task.Logs[0].SystemLogs) == 0 { return tes.View_BASIC, nil - } + } // view = "FULL" return tes.View_FULL, nil @@ -116,18 +116,18 @@ func (mclean *CustomMarshal) TranslateTask(task *tes.Task, view tes.View) interf } return min } - + // view = "BASIC" if view == tes.View_BASIC { executors := []*tes.ExecutorBasic{} for _, executor := range task.Executors { executors = append(executors, &tes.ExecutorBasic{ - Command: executor.Command, - Env: executor.Env, + Command: executor.Command, + Env: executor.Env, IgnoreError: executor.IgnoreError, - Image: executor.Image, - Stdin: executor.Stdin, - Workdir: executor.Workdir, + Image: executor.Image, + Stdin: executor.Stdin, + Workdir: executor.Workdir, }) } @@ -135,11 +135,11 @@ func (mclean *CustomMarshal) TranslateTask(task *tes.Task, view tes.View) interf for _, input := range task.Inputs { inputs = append(inputs, &tes.InputBasic{ Description: input.Description, - Name: input.Name, - Path: input.Path, + Name: input.Name, + Path: input.Path, Streamable: input.Streamable, - Type: input.Type, - Url: input.Url, + Type: input.Type, + Url: input.Url, }) } @@ -154,19 +154,19 @@ func (mclean *CustomMarshal) TranslateTask(task *tes.Task, view tes.View) interf }) } - basic := &tes.TaskBasic { + basic := &tes.TaskBasic{ CreationTime: task.CreationTime, Description: task.Description, Executors: executors, - Id: task.Id, - Inputs: inputs, - Logs: logs, - Name: task.Name, - Outputs: task.Outputs, - Resources: task.Resources, - State: task.State, - Tags: task.Tags, - Volumes: task.Volumes, + Id: task.Id, + Inputs: inputs, + Logs: logs, + Name: task.Name, + Outputs: task.Outputs, + Resources: task.Resources, + State: task.State, + Tags: task.Tags, + Volumes: task.Volumes, } return basic diff --git a/server/server.go b/server/server.go index 871b382e..651571a6 100644 --- a/server/server.go +++ b/server/server.go @@ -61,16 +61,16 @@ func newDebugInterceptor(log *logger.Logger) grpc.UnaryServerInterceptor { // Returns '400' for invalid backend parameters and '500' for all other errors // Required for TES Compliance Tests func customErrorHandler(ctx context.Context, mux *runtime.ServeMux, marshaler runtime.Marshaler, w http.ResponseWriter, r *http.Request, err error) { - const fallback = `{"error": "failed to process the request"}` + const fallback = `{"error": "failed to process the request"}` - st, ok := status.FromError(err) - if !ok { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(fallback)) - return - } + st, ok := status.FromError(err) + if !ok { + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte(fallback)) + return + } - // Map specific gRPC error codes to HTTP status codes + // Map specific gRPC error codes to HTTP status codes switch st.Code() { case codes.Unauthenticated: w.WriteHeader(http.StatusUnauthorized) // 401 @@ -78,7 +78,7 @@ func customErrorHandler(ctx context.Context, mux *runtime.ServeMux, marshaler ru w.WriteHeader(http.StatusForbidden) // 403 case codes.NotFound: // Special case for missing tasks (TES Compliance Suite) - if (strings.Contains(st.Message(), "task not found")) { + if strings.Contains(st.Message(), "task not found") { w.WriteHeader(http.StatusInternalServerError) // 500 } else { w.WriteHeader(http.StatusNotFound) // 404 @@ -87,18 +87,18 @@ func customErrorHandler(ctx context.Context, mux *runtime.ServeMux, marshaler ru w.WriteHeader(http.StatusInternalServerError) // 500 } - // Write the error message - jErr := JSONError{Error: st.Message()} - jErrBytes, mErr := marshaler.Marshal(jErr) - if mErr != nil { - w.Write([]byte(fallback)) - return - } - w.Write(jErrBytes) + // Write the error message + jErr := JSONError{Error: st.Message()} + jErrBytes, mErr := marshaler.Marshal(jErr) + if mErr != nil { + w.Write([]byte(fallback)) + return + } + w.Write(jErrBytes) } type JSONError struct { - Error string `json:"error"` + Error string `json:"error"` } // Serve starts the server and does not block. This will open TCP ports @@ -133,7 +133,7 @@ func (s *Server) Serve(pctx context.Context) error { marsh := NewMarshaler() grpcMux := runtime.NewServeMux( runtime.WithMarshalerOption(runtime.MIMEWildcard, marsh), runtime.WithErrorHandler(customErrorHandler)) - + // m := protojson.MarshalOptions{ // Indent: " ", // EmitUnpopulated: true, diff --git a/server/tes.go b/server/tes.go index 953dcefc..c9d335e9 100644 --- a/server/tes.go +++ b/server/tes.go @@ -36,7 +36,7 @@ type TaskService struct { func (ts *TaskService) CreateTask(ctx context.Context, task *tes.Task) (*tes.CreateTaskResponse, error) { if err := tes.InitTask(task, true); err != nil { - return nil, status.Errorf(codes.InvalidArgument, err.Error()) + return nil, status.Errorf(codes.InvalidArgument, "foo") } err := ts.Compute.CheckBackendParameterSupport(task) @@ -64,7 +64,7 @@ func (ts *TaskService) CreateTask(ctx context.Context, task *tes.Task) (*tes.Cre func (ts *TaskService) GetTask(ctx context.Context, req *tes.GetTaskRequest) (*tes.Task, error) { task, err := ts.Read.GetTask(ctx, req) if err == tes.ErrNotFound { - err = status.Errorf(codes.NotFound, fmt.Sprintf("%v: taskID: %s", err.Error(), req.Id)) + err = status.Errorf(codes.NotFound, "foo") } return task, err } @@ -85,7 +85,7 @@ func (ts *TaskService) CancelTask(ctx context.Context, req *tes.CancelTaskReques // updated database and other event streams err = ts.Event.WriteEvent(ctx, events.NewState(req.Id, tes.Canceled)) if err == tes.ErrNotFound { - err = status.Errorf(codes.NotFound, fmt.Sprintf("%v: taskID: %s", err.Error(), req.Id)) + err = status.Errorf(codes.NotFound, "foo") } return &tes.CancelTaskResponse{}, err } diff --git a/storage/local.go b/storage/local.go index 75f6401b..e8299f9b 100644 --- a/storage/local.go +++ b/storage/local.go @@ -168,100 +168,99 @@ func copyFile(ctx context.Context, source string, dest string) (err error) { // Hard links file source to destination dest. func linkFile(ctx context.Context, source string, dest string) error { - // If source has a glob or wildcard, get the filepath using the filepath.Glob function - if strings.Contains(source, "*") { - globs, err := filepath.Glob(source) - if err != nil { - return fmt.Errorf("failed to get filepath using Glob: %v", err) - } - for _, glob := range globs { - // Correctly calculate the destination for each file - destFile := filepath.Join(dest, filepath.Base(glob)) - err := processItem(ctx, glob, destFile) - if err != nil { - return err - } - } - return nil - } else { - return processItem(ctx, source, dest) - } + // If source has a glob or wildcard, get the filepath using the filepath.Glob function + if strings.Contains(source, "*") { + globs, err := filepath.Glob(source) + if err != nil { + return fmt.Errorf("failed to get filepath using Glob: %v", err) + } + for _, glob := range globs { + // Correctly calculate the destination for each file + destFile := filepath.Join(dest, filepath.Base(glob)) + err := processItem(ctx, glob, destFile) + if err != nil { + return err + } + } + return nil + } else { + return processItem(ctx, source, dest) + } } - // Process a single item (file or directory) func processItem(ctx context.Context, source, dest string) error { - fileInfo, err := os.Stat(source) - if err != nil { - return err - } - - if fileInfo.IsDir() { - return processDirectory(ctx, source, dest) - } else { - return processFile(ctx, source, dest) - } + fileInfo, err := os.Stat(source) + if err != nil { + return err + } + + if fileInfo.IsDir() { + return processDirectory(ctx, source, dest) + } else { + return processFile(ctx, source, dest) + } } // Process a directory func processDirectory(ctx context.Context, source, dest string) error { - // Create destination directory - err := os.MkdirAll(dest, 0755) // Adjust permissions as needed - if err != nil { - return err - } - - entries, err := os.ReadDir(source) - if err != nil { - return err - } - - for _, entry := range entries { - srcPath := filepath.Join(source, entry.Name()) - destPath := filepath.Join(dest, entry.Name()) - - if entry.IsDir() { - err = processDirectory(ctx, srcPath, destPath) - } else { - err = processFile(ctx, srcPath, destPath) - } - - if err != nil { - return err - } - } - return nil + // Create destination directory + err := os.MkdirAll(dest, 0755) // Adjust permissions as needed + if err != nil { + return err + } + + entries, err := os.ReadDir(source) + if err != nil { + return err + } + + for _, entry := range entries { + srcPath := filepath.Join(source, entry.Name()) + destPath := filepath.Join(dest, entry.Name()) + + if entry.IsDir() { + err = processDirectory(ctx, srcPath, destPath) + } else { + err = processFile(ctx, srcPath, destPath) + } + + if err != nil { + return err + } + } + return nil } // Process a single file func processFile(ctx context.Context, source, dest string) error { - // without this resulting link could be a symlink - parent, err := filepath.EvalSymlinks(source) - - same, err := sameFile(parent, dest) - if err != nil { - return err - } - if same { - return nil - } - - err = os.Link(parent, dest) - if err != nil { - return copyFile(ctx, parent, dest) - } - return nil + // without this resulting link could be a symlink + parent, err := filepath.EvalSymlinks(source) + + same, err := sameFile(parent, dest) + if err != nil { + return err + } + if same { + return nil + } + + err = os.Link(parent, dest) + if err != nil { + return copyFile(ctx, parent, dest) + } + return nil } func FilePathWalkDir(root string) ([]string, error) { - var files []string - err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { - if !info.IsDir() { - files = append(files, path) - } - return nil - }) - return files, err + var files []string + err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + if !info.IsDir() { + files = append(files, path) + } + return nil + }) + return files, err } func sameFile(source string, dest string) (bool, error) {