From a0118e1ddb689e447048f8a53ad1667c1de8b4d0 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 14 Oct 2022 09:44:48 -0700 Subject: [PATCH 1/4] Add comments to pipeline run proto message --- pkg/server/gen/server.pb.go | 11 +++++++---- pkg/server/gen/server.swagger.json | 9 ++++++--- pkg/server/proto/server.proto | 3 +++ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/pkg/server/gen/server.pb.go b/pkg/server/gen/server.pb.go index 4a4015d4dfc..019b0eb49a3 100644 --- a/pkg/server/gen/server.pb.go +++ b/pkg/server/gen/server.pb.go @@ -14844,10 +14844,13 @@ type PipelineRun struct { Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` // The sequence number for this pipeline run. - Sequence uint64 `protobuf:"varint,2,opt,name=sequence,proto3" json:"sequence,omitempty"` - Pipeline *Ref_Pipeline `protobuf:"bytes,3,opt,name=pipeline,proto3" json:"pipeline,omitempty"` - Jobs []*Ref_Job `protobuf:"bytes,4,rep,name=jobs,proto3" json:"jobs,omitempty"` - State PipelineRun_State `protobuf:"varint,5,opt,name=state,proto3,enum=hashicorp.waypoint.PipelineRun_State" json:"state,omitempty"` + Sequence uint64 `protobuf:"varint,2,opt,name=sequence,proto3" json:"sequence,omitempty"` + // The pipeline this run is apart of. + Pipeline *Ref_Pipeline `protobuf:"bytes,3,opt,name=pipeline,proto3" json:"pipeline,omitempty"` + // The full list of jobs that are associated with this run. + Jobs []*Ref_Job `protobuf:"bytes,4,rep,name=jobs,proto3" json:"jobs,omitempty"` + // The current state of this pipeline run, + State PipelineRun_State `protobuf:"varint,5,opt,name=state,proto3,enum=hashicorp.waypoint.PipelineRun_State" json:"state,omitempty"` } func (x *PipelineRun) Reset() { diff --git a/pkg/server/gen/server.swagger.json b/pkg/server/gen/server.swagger.json index 19a9a2be4d4..25359b2429e 100644 --- a/pkg/server/gen/server.swagger.json +++ b/pkg/server/gen/server.swagger.json @@ -9477,16 +9477,19 @@ "description": "The sequence number for this pipeline run." }, "pipeline": { - "$ref": "#/definitions/hashicorp.waypoint.Ref.Pipeline" + "$ref": "#/definitions/hashicorp.waypoint.Ref.Pipeline", + "description": "The pipeline this run is apart of." }, "jobs": { "type": "array", "items": { "$ref": "#/definitions/hashicorp.waypoint.Ref.Job" - } + }, + "description": "The full list of jobs that are associated with this run." }, "state": { - "$ref": "#/definitions/hashicorp.waypoint.PipelineRun.State" + "$ref": "#/definitions/hashicorp.waypoint.PipelineRun.State", + "title": "The current state of this pipeline run," } } }, diff --git a/pkg/server/proto/server.proto b/pkg/server/proto/server.proto index 9cab66b11db..bc3ec7d5418 100644 --- a/pkg/server/proto/server.proto +++ b/pkg/server/proto/server.proto @@ -4870,10 +4870,13 @@ message PipelineRun { // The sequence number for this pipeline run. uint64 sequence = 2; + // The pipeline this run is apart of. Ref.Pipeline pipeline = 3; + // The full list of jobs that are associated with this run. repeated Ref.Job jobs = 4; + // The current state of this pipeline run, State state = 5; enum State { From e900731397749f0d5af86a9911d0550ab2f5a66a Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 14 Oct 2022 09:45:01 -0700 Subject: [PATCH 2/4] pipeline: Properly determine when pipeline run is complete Prior to this commit, when we'd go to complete a pipeline run, we would only look at the "last" job in the jobs slice for a pipeline run message. This check doesn't seem to work anymore, and relying on the last job in a slice can be fragile. This commit fixes this behavior by checking all job ids in a pipeline run and seeing if they have finished as well before attempting to mark the pipeline run as a success. Fixes #4032 --- internal/server/boltdbstate/job.go | 24 ++++++++++++++++---- pkg/server/singleprocess/service_pipeline.go | 1 + 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/internal/server/boltdbstate/job.go b/internal/server/boltdbstate/job.go index 09d64557360..72b26d6c9fa 100644 --- a/internal/server/boltdbstate/job.go +++ b/internal/server/boltdbstate/job.go @@ -1772,11 +1772,25 @@ func (s *State) pipelineComplete(jobId string) error { if job.State == pb.Job_ERROR { run.State = pb.PipelineRun_ERROR } else if job.State == pb.Job_SUCCESS { - // If job Id matches last job queued by pipeline. - // We will have to change this in the future when pipeline steps run in parallel, - // and the last job queued may not be the last job to complete in the pipeline - // TODO:XX figure out how ^ - if job.Id == run.Jobs[len(run.Jobs)-1].Id { + // Look at all job ids in a run and check if any are not SUCCESS + runComplete := true + for _, j := range run.Jobs { + if j.Id == job.Id { + continue + } + + rj, err := s.JobById(j.Id, nil) + if err != nil { + return err + } + + if rj.State != pb.Job_SUCCESS { + runComplete = false + break + } + } + + if runComplete { run.State = pb.PipelineRun_SUCCESS s.log.Trace("pipeline run is complete", "job", job.Id, "pipeline", job.Pipeline.PipelineId, "run", run.Sequence) } diff --git a/pkg/server/singleprocess/service_pipeline.go b/pkg/server/singleprocess/service_pipeline.go index b3b15e41e10..67fb80ce7a3 100644 --- a/pkg/server/singleprocess/service_pipeline.go +++ b/pkg/server/singleprocess/service_pipeline.go @@ -512,6 +512,7 @@ func (s *Service) buildStepJobs( } } + // Include a list of all associated jobs for this specific run pipelineRun.Jobs = append(pipelineRun.Jobs, &pb.Ref_Job{Id: job.Id}) stepJobs = append(stepJobs, &pb.QueueJobRequest{ Job: job, From f10ccb3d0caccd4306290c5d0eaac7234455c76c Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 14 Oct 2022 09:49:46 -0700 Subject: [PATCH 3/4] Add changelog --- .changelog/4053.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/4053.txt diff --git a/.changelog/4053.txt b/.changelog/4053.txt new file mode 100644 index 00000000000..0d8c4ef4c70 --- /dev/null +++ b/.changelog/4053.txt @@ -0,0 +1,3 @@ +```release-note:bug +pipelines: Properly mark a pipeline run as complete +``` From f50812b242f90a61f2f161cbc3f5cff8ef172776 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 17 Oct 2022 10:20:46 -0700 Subject: [PATCH 4/4] Docs: change docstring for pipeline ref in run --- pkg/server/gen/server.pb.go | 2 +- pkg/server/gen/server.swagger.json | 2 +- pkg/server/proto/server.proto | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/server/gen/server.pb.go b/pkg/server/gen/server.pb.go index 019b0eb49a3..6d3a0fca38f 100644 --- a/pkg/server/gen/server.pb.go +++ b/pkg/server/gen/server.pb.go @@ -14845,7 +14845,7 @@ type PipelineRun struct { Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` // The sequence number for this pipeline run. Sequence uint64 `protobuf:"varint,2,opt,name=sequence,proto3" json:"sequence,omitempty"` - // The pipeline this run is apart of. + // The pipeline associated with this run. Pipeline *Ref_Pipeline `protobuf:"bytes,3,opt,name=pipeline,proto3" json:"pipeline,omitempty"` // The full list of jobs that are associated with this run. Jobs []*Ref_Job `protobuf:"bytes,4,rep,name=jobs,proto3" json:"jobs,omitempty"` diff --git a/pkg/server/gen/server.swagger.json b/pkg/server/gen/server.swagger.json index 25359b2429e..3ca47ad5f2b 100644 --- a/pkg/server/gen/server.swagger.json +++ b/pkg/server/gen/server.swagger.json @@ -9478,7 +9478,7 @@ }, "pipeline": { "$ref": "#/definitions/hashicorp.waypoint.Ref.Pipeline", - "description": "The pipeline this run is apart of." + "description": "The pipeline associated with this run." }, "jobs": { "type": "array", diff --git a/pkg/server/proto/server.proto b/pkg/server/proto/server.proto index bc3ec7d5418..af940597127 100644 --- a/pkg/server/proto/server.proto +++ b/pkg/server/proto/server.proto @@ -4870,7 +4870,7 @@ message PipelineRun { // The sequence number for this pipeline run. uint64 sequence = 2; - // The pipeline this run is apart of. + // The pipeline associated with this run. Ref.Pipeline pipeline = 3; // The full list of jobs that are associated with this run.