feat(database): add support for pipelines

[jbrockopp]

Dependent on go-vela/types#223

Part of the effort for go-vela/community#460

NOTE: I had to modify hooks to account for go-vela/types#239

This adds support for a new pipelines resource to the github.com/go-vela/server/database package.

As a part of this, I used a new strategy to add this logic which attempts to solve a few things:

• reduce code complexity
• reduce code duplication
• improve ability to add support for other database drivers
• provider a cleaner abstraction for database resources

If this new approach is liked and folks feel the pros outweigh the cons, I'll replicate this for the other resources.

If not, then I'll modify this PR accordingly to follow our existing pattern.

Structure

One distinguishable change with this approach is all code for pipelines is under a database/pipeline package:

$ tree -d database
database
├── pipeline
├── postgres
│   ├── ddl
│   └── dml
└── sqlite
    ├── ddl
    └── dml

7 directories

You'll notice that no dml package is nested under the pipeline package.

This new approach would no longer require raw SQL queries for integrating with the pipelines table.

Instead, we leverage our DB library's (https://gorm.io/) agnostic abstraction for integrating with that table.

This is great because we reduce our total LOC and complexity when we need to add/update functionality.

To be specific, when we want to add new code for pipelines, ideally I should only have to update the one package.

We no longer have to update code specific to postgres, sqlite or any future drivers we may support for Vela.

However, it comes with a cost of becoming reliant on the GORM library which can be a downside.

Resource Specific Interface

Inside the package exists a PipelineService interface that declares all functions necessary for this code:

server/database/pipeline/service.go

Lines 11 to 49 in 093f2e1

	// PipelineService represents the Vela interface for pipeline
	// functions with the supported Database backends.
	//
	// nolint: revive // ignore name stutter
	type PipelineService interface {
	// Pipeline Data Definition Language Functions
	//
	// https://en.wikipedia.org/wiki/Data_definition_language
	// CreateIndexes creates the indexes for the pipelines table.
	CreateIndexes() error
	// CreateTable defines a function that creates the pipelines table.
	CreateTable(string) error
	// Pipeline Data Manipulation Language Functions
	//
	// https://en.wikipedia.org/wiki/Data_manipulation_language
	// CountPipelines defines a function that gets the count of all pipelines.
	CountPipelines() (int64, error)
	// CountPipelinesForRepo defines a function that gets the count of pipelines by repo ID.
	CountPipelinesForRepo(*library.Repo) (int64, error)
	// CreatePipeline defines a function that creates a new pipeline.
	CreatePipeline(*library.Pipeline) error
	// DeletePipeline defines a function that deletes an existing pipeline.
	DeletePipeline(*library.Pipeline) error
	// GetPipeline defines a function that gets a pipeline by ID.
	GetPipeline(int64) (*library.Pipeline, error)
	// GetPipelineForRepo defines a function that gets a pipeline by number and repo ID.
	GetPipelineForRepo(int, *library.Repo) (*library.Pipeline, error)
	// LastPipelineForRepo defines a function that gets the last pipeline by repo ID.
	LastPipelineForRepo(*library.Repo) (*library.Pipeline, error)
	// ListPipelines defines a function that gets a list of all pipelines.
	ListPipelines() ([]*library.Pipeline, error)
	// ListPipelinesForRepo defines a function that gets a list of pipelines by repo ID.
	ListPipelinesForRepo(*library.Repo, int, int) ([]*library.Pipeline, int64, error)
	// UpdatePipeline defines a function that updates an existing pipeline.
	UpdatePipeline(*library.Pipeline) error
	}

Most of the naming for these functions was adopted from the existing code barring the Get prefix.

And then we create an engine which implements that service interface:

server/database/pipeline/pipeline.go

Lines 25 to 39 in 093f2e1

	// engine represents the pipeline functionality that implements the PipelineService interface.
	engine struct {
	// engine configuration settings used in pipeline functions
	config *config
	// gorm.io/gorm database client used in pipeline functions
	//
	// https://pkg.go.dev/gorm.io/gorm#DB
	client *gorm.DB
	// sirupsen/logrus logger used in pipeline functions
	//
	// https://pkg.go.dev/github.com/sirupsen/logrus#Entry
	logger *logrus.Entry
	}

[codecov]

Codecov Report

Merging #574 (3741949) into master (e48007c) will increase coverage by 0.53%.
The diff coverage is 76.86%.

@@            Coverage Diff             @@
##           master     #574      +/-   ##
==========================================
+ Coverage   54.56%   55.09%   +0.53%     
==========================================
  Files         181      195      +14     
  Lines       15347    15723     +376     
==========================================
+ Hits         8374     8663     +289     
- Misses       6639     6699      +60     
- Partials      334      361      +27     

Impacted Files	Coverage Δ
database/pipeline/pipeline.go	51.72% <51.72%> (ø)
database/postgres/postgres.go	59.14% <64.28%> (+1.10%)	⬆️
database/sqlite/sqlite.go	59.91% <65.38%> (+0.29%)	⬆️
database/pipeline/list.go	66.66% <66.66%> (ø)
database/pipeline/get.go	71.42% <71.42%> (ø)
database/pipeline/list_repo.go	72.72% <72.72%> (ø)
database/pipeline/create.go	76.00% <76.00%> (ø)
database/pipeline/last_repo.go	76.00% <76.00%> (ø)
database/pipeline/update.go	76.00% <76.00%> (ø)
database/pipeline/get_repo.go	76.92% <76.92%> (ø)
... and 8 more

[kneal]

Overall it looks fantastic, one question what is the goal of SkipCreation for the pipeline endpoints?

[kneal]

Is this the build number?

[jbrockopp]

No, but it follows the same train of logic 👍

Like builds today, each pipeline in the table will have a number field that increments based off the repo_id.

That field doesn't directly map to the build number because pipelines have a one-to-many relation with builds 😃

You can have multiple builds all stemming from the same pipeline i.e. a build that gets restarted multiple times

[jbrockopp]

Overall it looks fantastic, one question what is the goal of SkipCreation for the pipeline endpoints?

To answer your question directly, the SkipCreation logic won't be used for the API pipeline endpoints 👍

In case others have the same or similar questions, I'll provide some context on SkipCreation in general 😃

Then I'll explain a bit more on why we're referencing it in the pipeline package.

This functionality was first added here: #455

By default, Vela will always attempt to create all tables and indexes in the database 👍

However, we enable Vela installation admins to disable this behavior by setting this flag to true 😄

The reason why we're using SkipCreation in the pipeline package is because of how the new code is structured.

When we create the pipeline engine, which implements the PipelineService, we create the table and indexes too:

server/database/pipeline/pipeline.go

Lines 62 to 79 in 093f2e1

	// check if we should skip creating pipeline database objects
	if e.config.SkipCreation {
	e.logger.Warning("skipping creation of pipelines table and indexes in the database")
	return e, nil
	}
	// create the pipelines table
	err := e.CreateTable(e.client.Config.Dialector.Name())
	if err != nil {
	return nil, fmt.Errorf("unable to create %s table: %w", constants.TablePipeline, err)
	}
	// create the indexes for the pipelines table
	err = e.CreateIndexes()
	if err != nil {
	return nil, fmt.Errorf("unable to create indexes for %s table: %w", constants.TablePipeline, err)
	}

So, when we create the database client for postgres or sqlite, we need to pass this flag to that engine:

server/database/postgres/postgres.go

Lines 94 to 98 in 093f2e1

	// create the services for the database
	err = createServices(c)
	if err != nil {
	return nil, err
	}

->

server/database/postgres/postgres.go

Lines 358 to 369 in 093f2e1

	// create the database agnostic pipeline service
	//
	// https://pkg.go.dev/github.com/go-vela/server/database/pipeline#New
	c.PipelineService, err = pipeline.New(
	pipeline.WithClient(c.Postgres),
	pipeline.WithCompressionLevel(c.config.CompressionLevel),
	pipeline.WithLogger(c.Logger),
	pipeline.WithSkipCreation(c.config.SkipCreation),
	)
	if err != nil {
	return err
	}

[ecrupper]

I really like the design change. + 1 for getting it applied to the rest of the resources.

Everything else LGTM.

[JordanSussman]

minor feedback around pagination

[JordanSussman]

Seems like we should have a maximum and/or default value for the number of pieplines to be returned to avoid returning massive amounts of data. What do you think @jbrockopp?

[jbrockopp]

We could certainly explore that in the future 👍

But to keep in line with existing functionality, I'm not sure we should make that change at this time.

For context, the List functions for resources were designed to not contain pagination.

As an example from builds:

server/database/service.go

Lines 37 to 39 in e48007c

	// GetBuildList defines a function that gets
	// a list of all builds.
	GetBuildList() ([]*library.Build, error)

->

server/database/postgres/build_list.go

Lines 15 to 16 in e48007c

	// GetBuildList gets a list of all builds from the database.
	func (c *client) GetBuildList() ([]*library.Build, error) {

->

server/database/postgres/dml/build.go

Lines 8 to 13 in e48007c

	// ListBuilds represents a query to
	// list all builds in the database.
	ListBuilds = `
	SELECT *
	FROM builds;
	`

The only time these List functions are actually called are for the admin endpoints (restricted to platform admins):

server/api/admin/build.go

Line 50 in e48007c

b, err := database.FromContext(c).GetBuildList()

The idea is there are times where Vela installation admins may need to query all objects in a table.

The most prominent use-case we've seen for this is the migration utilities:

https://github.com/go-vela/community/tree/master/migrations

[kneal]

LGTM 🐬