From ce52ecefec3f6d673017802db9be1468c8d44ec6 Mon Sep 17 00:00:00 2001 From: Kelly Deng Date: Fri, 13 Nov 2020 12:32:38 -0500 Subject: [PATCH 1/4] converge if statements in TestGetTemplate Signed-off-by: Kelly Deng --- grpc-server/template_test.go | 91 ++++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 36 deletions(-) diff --git a/grpc-server/template_test.go b/grpc-server/template_test.go index 09c19dc1a..b2f059594 100644 --- a/grpc-server/template_test.go +++ b/grpc-server/template_test.go @@ -11,6 +11,19 @@ import ( ) const ( + templateNotFoundID = "abstract-beef-beyond-meat-abominations" + templateNotFoundName = "my-awesome-mock-name" + templateNotFoundTemplate = `version: "0.1" +name: not_found_workflow +global_timeout: 600 +tasks: + - name: "not found" + worker: "{{.device_1}}" + actions: + - name: "not_found" + image: not-found + timeout: 60` + templateID1 = "7cd79119-1959-44eb-8b82-bc15bad4888e" templateName1 = "template_1" template1 = `version: "0.1" @@ -124,6 +137,9 @@ func TestGetTemplate(t *testing.T) { ) testCases := map[string]struct { args args + id string + name string + data string err bool }{ "SuccessfulTemplateGet_Name": { @@ -135,18 +151,18 @@ func TestGetTemplate(t *testing.T) { GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) { t.Log("in get template func") - if fields["id"] == templateID1 { - return templateID1, templateName1, template1, nil - } - if fields["name"] == templateName1 { + if fields["id"] == templateID1 || fields["name"] == templateName1 { return templateID1, templateName1, template1, nil } - return "", "", "", errors.New("failed to get template") + return templateNotFoundID, templateNotFoundName, templateNotFoundTemplate, errors.New("failed to get template") }, }, getRequest: &pb.GetRequest{GetBy: &pb.GetRequest_Name{Name: templateName1}}, }, - err: false, + id: templateID1, + name: templateName1, + data: template1, + err: false, }, "FailedTemplateGet_Name": { @@ -158,18 +174,18 @@ func TestGetTemplate(t *testing.T) { GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) { t.Log("in get template func") - if fields["id"] == templateID1 { - return templateID1, templateName1, template1, nil - } - if fields["name"] == templateName1 { + if fields["id"] == templateID1 || fields["name"] == templateName1 { return templateID1, templateName1, template1, nil } - return "", "", "", errors.New("failed to get template") + return templateNotFoundID, templateNotFoundName, templateNotFoundTemplate, errors.New("failed to get template") }, }, getRequest: &pb.GetRequest{GetBy: &pb.GetRequest_Name{Name: templateName2}}, }, - err: true, + id: templateNotFoundID, + name: templateNotFoundName, + data: templateNotFoundTemplate, + err: true, }, "SuccessfulTemplateGet_ID": { @@ -181,18 +197,18 @@ func TestGetTemplate(t *testing.T) { GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) { t.Log("in get template func") - if fields["id"] == templateID1 { - return templateID1, templateName1, template1, nil - } - if fields["name"] == templateName1 { + if fields["id"] == templateID1 || fields["name"] == templateName1 { return templateID1, templateName1, template1, nil } - return "", "", "", errors.New("failed to get template") + return templateNotFoundID, templateNotFoundName, templateNotFoundTemplate, errors.New("failed to get template") }, }, getRequest: &pb.GetRequest{GetBy: &pb.GetRequest_Id{Id: templateID1}}, }, - err: false, + id: templateID1, + name: templateName1, + data: template1, + err: false, }, "FailedTemplateGet_ID": { @@ -204,18 +220,18 @@ func TestGetTemplate(t *testing.T) { GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) { t.Log("in get template func") - if fields["id"] == templateID1 { - return templateID1, templateName1, template1, nil - } - if fields["name"] == templateName1 { + if fields["id"] == templateID1 || fields["name"] == templateName1 { return templateID1, templateName1, template1, nil } - return "", "", "", errors.New("failed to get template") + return templateNotFoundID, templateNotFoundName, templateNotFoundTemplate, errors.New("failed to get template") }, }, getRequest: &pb.GetRequest{GetBy: &pb.GetRequest_Id{Id: templateID2}}, }, - err: true, + id: templateNotFoundID, + name: templateNotFoundName, + data: templateNotFoundTemplate, + err: true, }, "FailedTemplateGet_EmptyRequest": { @@ -227,18 +243,18 @@ func TestGetTemplate(t *testing.T) { GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) { t.Log("in get template func") - if fields["id"] == templateID1 { + if fields["id"] == templateID1 || fields["name"] == templateName1 { return templateID1, templateName1, template1, nil } - if fields["name"] == templateName1 { - return templateID1, templateName1, template1, nil - } - return "", "", "", errors.New("failed to get template") + return templateNotFoundID, templateNotFoundName, templateNotFoundTemplate, errors.New("failed to get template") }, }, getRequest: &pb.GetRequest{}, }, - err: true, + id: templateNotFoundID, + name: templateNotFoundName, + data: templateNotFoundTemplate, + err: true, }, "FailedTemplateGet_NilRequest": { @@ -250,17 +266,17 @@ func TestGetTemplate(t *testing.T) { GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) { t.Log("in get template func") - if fields["id"] == templateID1 { - return templateID1, templateName1, template1, nil - } - if fields["name"] == templateName1 { + if fields["id"] == templateID1 || fields["name"] == templateName1 { return templateID1, templateName1, template1, nil } - return "", "", "", errors.New("failed to get template") + return templateNotFoundID, templateNotFoundName, templateNotFoundTemplate, errors.New("failed to get template") }, }, }, - err: true, + id: templateNotFoundID, + name: templateNotFoundName, + data: templateNotFoundTemplate, + err: true, }, } @@ -278,6 +294,9 @@ func TestGetTemplate(t *testing.T) { assert.Nil(t, err) assert.NotEmpty(t, res) } + assert.Equal(t, res.Id, tc.id) + assert.Equal(t, res.Name, tc.name) + assert.Equal(t, res.Data, tc.data) }) } } From 7473ee98f69e650ea463f282902b8897b5049f06 Mon Sep 17 00:00:00 2001 From: Kelly Deng Date: Thu, 12 Nov 2020 13:15:20 -0500 Subject: [PATCH 2/4] fix: get returns sql.ErrNoRows when hardware not found to avoid 'Unknown desc = unexpected end of JSON input' error Signed-off-by: Kelly Deng --- db/db.go | 13 +++---------- db/workflow.go | 7 ++++--- grpc-server/workflow.go | 5 +++-- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/db/db.go b/db/db.go index 26268f0a7..9eba92e0c 100644 --- a/db/db.go +++ b/db/db.go @@ -102,18 +102,11 @@ func get(ctx context.Context, db *sql.DB, query string, args ...interface{}) (st buf := []byte{} err := row.Scan(&buf) - if err == nil { - return string(buf), nil - } - - if err != sql.ErrNoRows { - err = errors.Wrap(err, "SELECT") + if err != nil { logger.Error(err) - } else { - err = nil + return "", errors.Wrap(err, "SELECT") } - - return "", err + return string(buf), nil } // buildGetCondition builds a where condition string in the format "column_name = 'field_value' AND" diff --git a/db/workflow.go b/db/workflow.go index 64d68baae..fa344d66e 100644 --- a/db/workflow.go +++ b/db/workflow.go @@ -40,7 +40,7 @@ func (d TinkDB) CreateWorkflow(ctx context.Context, wf Workflow, data string, id err = insertActionList(ctx, d.instance, data, id, tx) if err != nil { - return errors.Wrap(err, "Failed to insert in workflow_state") + return errors.Wrap(err, "failed to insert in workflow_state") } err = insertInWorkflow(ctx, d.instance, wf, tx) @@ -109,9 +109,10 @@ func insertActionList(ctx context.Context, db *sql.DB, yamlData string, id uuid. workerID, err := getWorkerID(ctx, db, task.WorkerAddr) if err != nil { + if errors.Cause(err) == sql.ErrNoRows { + return errors.Wrapf(err, "hardware %s not found", task.WorkerAddr) + } return err - } else if workerID == "" { - return fmt.Errorf("hardware mentioned with reference %s not found", task.WorkerAddr) } workerUID, err := uuid.Parse(workerID) if err != nil { diff --git a/grpc-server/workflow.go b/grpc-server/workflow.go index d0a466d8c..9f6cf6eb9 100644 --- a/grpc-server/workflow.go +++ b/grpc-server/workflow.go @@ -69,8 +69,9 @@ func (s *server) CreateWorkflow(ctx context.Context, in *workflow.CreateRequest) if pqErr := db.Error(err); pqErr != nil { l = l.With("detail", pqErr.Detail, "where", pqErr.Where) } - l.Error(err) - return &workflow.CreateResponse{}, err + e := errors.Wrap(err, "failed to create workflow") + l.Error(e) + return &workflow.CreateResponse{}, e } l := logger.With("workflowID", id.String()) From 8f0e53c3767df603947cb122c5736ffaf6eea08a Mon Sep 17 00:00:00 2001 From: Kelly Deng Date: Wed, 18 Nov 2020 16:33:52 -0500 Subject: [PATCH 3/4] fix error message format Signed-off-by: Kelly Deng --- db/workflow.go | 27 +++++++++++++++++---------- grpc-server/workflow.go | 5 ++--- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/db/workflow.go b/db/workflow.go index fa344d66e..7190e0fea 100644 --- a/db/workflow.go +++ b/db/workflow.go @@ -40,12 +40,12 @@ func (d TinkDB) CreateWorkflow(ctx context.Context, wf Workflow, data string, id err = insertActionList(ctx, d.instance, data, id, tx) if err != nil { - return errors.Wrap(err, "failed to insert in workflow_state") + return errors.Wrap(err, "failed to create workflow") } err = insertInWorkflow(ctx, d.instance, wf, tx) if err != nil { - return errors.Wrap(err, "Failed to workflow") + return errors.Wrap(err, "failed to create workflow") } err = tx.Commit() @@ -109,10 +109,7 @@ func insertActionList(ctx context.Context, db *sql.DB, yamlData string, id uuid. workerID, err := getWorkerID(ctx, db, task.WorkerAddr) if err != nil { - if errors.Cause(err) == sql.ErrNoRows { - return errors.Wrapf(err, "hardware %s not found", task.WorkerAddr) - } - return err + return errors.WithMessage(err, "unable to insert into action list") } workerUID, err := uuid.Parse(workerID) if err != nil { @@ -690,7 +687,11 @@ func getWorkerIDbyMac(ctx context.Context, db *sql.DB, mac string) (string, erro data @> $1 ` - return get(ctx, db, query, arg) + id, err := get(ctx, db, query, arg) + if errors.Cause(err) == sql.ErrNoRows { + err = errors.WithMessage(errors.New(mac), "mac") + } + return id, err } func getWorkerIDbyIP(ctx context.Context, db *sql.DB, ip string) (string, error) { @@ -734,7 +735,11 @@ func getWorkerIDbyIP(ctx context.Context, db *sql.DB, ip string) (string, error) ) ` - return get(ctx, db, query, instance, hardwareOrManagement) + id, err := get(ctx, db, query, instance, hardwareOrManagement) + if errors.Cause(err) == sql.ErrNoRows { + err = errors.WithMessage(errors.New(ip), "ip") + } + return id, err } func getWorkerID(ctx context.Context, db *sql.DB, addr string) (string, error) { @@ -744,10 +749,12 @@ func getWorkerID(ctx context.Context, db *sql.DB, addr string) (string, error) { if ip == nil || ip.To4() == nil { return "", fmt.Errorf("invalid worker address: %s", addr) } - return getWorkerIDbyIP(ctx, db, addr) + id, err := getWorkerIDbyIP(ctx, db, addr) + return id, errors.WithMessage(err, "no worker found") } - return getWorkerIDbyMac(ctx, db, addr) + id, err := getWorkerIDbyMac(ctx, db, addr) + return id, errors.WithMessage(err, "no worker found") } func init() { diff --git a/grpc-server/workflow.go b/grpc-server/workflow.go index 9f6cf6eb9..d0a466d8c 100644 --- a/grpc-server/workflow.go +++ b/grpc-server/workflow.go @@ -69,9 +69,8 @@ func (s *server) CreateWorkflow(ctx context.Context, in *workflow.CreateRequest) if pqErr := db.Error(err); pqErr != nil { l = l.With("detail", pqErr.Detail, "where", pqErr.Where) } - e := errors.Wrap(err, "failed to create workflow") - l.Error(e) - return &workflow.CreateResponse{}, e + l.Error(err) + return &workflow.CreateResponse{}, err } l := logger.With("workflowID", id.String()) From 99800d55b9075a3d2260777ab2317d1eeab647fa Mon Sep 17 00:00:00 2001 From: Kelly Deng Date: Sat, 14 Nov 2020 13:03:54 -0500 Subject: [PATCH 4/4] clean up some error messages Signed-off-by: Kelly Deng --- db/template.go | 2 +- grpc-server/workflow.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/db/template.go b/db/template.go index dc656e0e5..47e0d1cc8 100644 --- a/db/template.go +++ b/db/template.go @@ -49,7 +49,7 @@ func (d TinkDB) CreateTemplate(ctx context.Context, name string, data string, id func (d TinkDB) GetTemplate(ctx context.Context, fields map[string]string) (string, string, string, error) { getCondition, err := buildGetCondition(fields) if err != nil { - return "", "", "", errors.Wrap(err, "failed to build get condition") + return "", "", "", errors.Wrap(err, "failed to get template") } query := ` diff --git a/grpc-server/workflow.go b/grpc-server/workflow.go index d0a466d8c..d2523be6f 100644 --- a/grpc-server/workflow.go +++ b/grpc-server/workflow.go @@ -25,8 +25,8 @@ var state = map[int32]workflow.State{ } const ( - errFailedToGetTemplate = "failed to get template with ID: %s" - errTemplateParsing = "failed to parse template with ID: %s" + errFailedToGetTemplate = "failed to get template with ID %s" + errTemplateParsing = "failed to parse template with ID %s" ) // CreateWorkflow implements workflow.CreateWorkflow