Merge pull request #447 from parauliya/Fix_381

Fixed #381: 'tink workflow state' command returns an error if workflow does not exist
tinkerbell · Mar 10, 2021 · c4e5d39 · c4e5d39
2 parents 5e1f0fd + e7fdb8d
commit c4e5d39
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 5 deletions.
diff --git a/db/workflow.go b/db/workflow.go
@@ -350,13 +350,13 @@ func (d TinkDB) GetWorkflow(ctx context.Context, id string) (Workflow, error) {
 	if err == nil {
 		return Workflow{ID: id, Template: tmp, Hardware: tar}, nil
 	}
-
 	if err != sql.ErrNoRows {
 		err = errors.Wrap(err, "SELECT")
 		d.logger.Error(err)
+		return Workflow{}, err
 	}
 
-	return Workflow{}, nil
+	return Workflow{}, errors.New("Workflow with id " + id + " does not exist")
 }
 
 // DeleteWorkflow deletes a workflow
@@ -549,8 +549,9 @@ func (d TinkDB) GetWorkflowContexts(ctx context.Context, wfID string) (*pb.Workf
 	if err != sql.ErrNoRows {
 		err = errors.Wrap(err, "SELECT from worflow_state")
 		d.logger.Error(err)
+		return &pb.WorkflowContext{}, err
 	}
-	return &pb.WorkflowContext{}, nil
+	return &pb.WorkflowContext{}, errors.New("Workflow with id " + wfID + " does not exist")
 }
 
 // GetWorkflowActions : gives you the action list of workflow

diff --git a/grpc-server/workflow.go b/grpc-server/workflow.go
@@ -102,8 +102,8 @@ func (s *server) GetWorkflow(ctx context.Context, in *workflow.GetRequest) (*wor
 			l = l.With("detail", pqErr.Detail, "where", pqErr.Where)
 		}
 		l.Error(err)
+		return &workflow.Workflow{}, err
 	}
-
 	fields := map[string]string{
 		"id": w.Template,
 	}
@@ -200,7 +200,7 @@ func (s *server) GetWorkflowContext(ctx context.Context, in *workflow.GetRequest
 	metrics.CacheInFlight.With(labels).Inc()
 	defer metrics.CacheInFlight.With(labels).Dec()
 
-	const msg = "getting a workflow"
+	const msg = "getting a workflow context"
 	labels["op"] = "get"
 
 	metrics.CacheTotals.With(labels).Inc()
@@ -216,6 +216,7 @@ func (s *server) GetWorkflowContext(ctx context.Context, in *workflow.GetRequest
 			l = l.With("detail", pqErr.Detail, "where", pqErr.Where)
 		}
 		l.Error(err)
+		return &workflow.WorkflowContext{}, err
 	}
 	wf := &workflow.WorkflowContext{
 		WorkflowId:           w.WorkflowId,

diff --git a/grpc-server/workflow_test.go b/grpc-server/workflow_test.go
@@ -147,6 +147,18 @@ func TestGetWorkflow(t *testing.T) {
 				expectedError: false,
 			},
 		},
+		"WorkflowDoesNotExist": {
+			args: args{
+				db: mock.DB{
+					GetWorkflowFunc: func(ctx context.Context, workflowID string) (db.Workflow, error) {
+						return db.Workflow{}, errors.New("Workflow with id " + workflowID + " does not exist")
+					},
+				},
+			},
+			want: want{
+				expectedError: true,
+			},
+		},
 	}
 
 	ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
@@ -169,3 +181,52 @@ func TestGetWorkflow(t *testing.T) {
 		})
 	}
 }
+
+func TestGetWorkflowContext(t *testing.T) {
+	type (
+		args struct {
+			db mock.DB
+		}
+		want struct {
+			expectedError bool
+		}
+	)
+	testCases := map[string]struct {
+		args args
+		want want
+	}{
+		"WorkflowDoesNotExist": {
+			args: args{
+				db: mock.DB{
+					GetWorkflowContextsFunc: func(ctx context.Context, workflowID string) (*workflow.WorkflowContext, error) {
+						w := workflow.WorkflowContext{}
+						return &w, errors.New("Workflow with id " + workflowID + " does not exist")
+					},
+				},
+			},
+			want: want{
+				expectedError: true,
+			},
+		},
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
+	defer cancel()
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			s := testServer(t, tc.args.db)
+			res, err := s.GetWorkflowContext(ctx, &workflow.GetRequest{
+				Id: workflowID,
+			})
+			if err != nil {
+				assert.Error(t, err)
+				assert.Empty(t, res)
+				assert.True(t, tc.want.expectedError)
+				return
+			}
+			assert.NoError(t, err)
+			assert.NotEmpty(t, res)
+			assert.False(t, tc.want.expectedError)
+		})
+	}
+}