From 0ed08f5fd8a9d994b99da2bf132b81c2cebd9352 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Wed, 28 Jun 2023 14:50:59 +0000 Subject: [PATCH] Various fixes * enable foreign key constraints on sqlite * on delete cascade for addresses and status messages * add debug server config option * fix rr allocation Signed-off-by: Gabriel Adrian Samfira --- apiserver/routers/routers.go | 11 +++++++++++ cmd/garm/main.go | 5 +++++ config/config.go | 3 ++- database/sql/jobs.go | 7 ++++--- database/sql/models.go | 14 +++++++------- params/params.go | 6 +++--- runner/pool/pool.go | 8 ++++---- runner/pool/util.go | 4 ---- 8 files changed, 36 insertions(+), 22 deletions(-) diff --git a/apiserver/routers/routers.go b/apiserver/routers/routers.go index 00875576..67f1ec75 100644 --- a/apiserver/routers/routers.go +++ b/apiserver/routers/routers.go @@ -15,8 +15,10 @@ package routers import ( + _ "expvar" // Register the expvar handlers "io" "net/http" + _ "net/http/pprof" // Register the pprof handlers "github.com/gorilla/mux" "github.com/prometheus/client_golang/prometheus/promhttp" @@ -40,6 +42,15 @@ func WithMetricsRouter(parentRouter *mux.Router, disableAuth bool, metricsMiddle return parentRouter } +func WithDebugServer(parentRouter *mux.Router) *mux.Router { + if parentRouter == nil { + return nil + } + + parentRouter.PathPrefix("/debug/pprof/").Handler(http.DefaultServeMux) + return parentRouter +} + func NewAPIRouter(han *controllers.APIController, logWriter io.Writer, authMiddleware, initMiddleware, instanceMiddleware auth.Middleware) *mux.Router { router := mux.NewRouter() logMiddleware := util.NewLoggingMiddleware(logWriter) diff --git a/cmd/garm/main.go b/cmd/garm/main.go index 9449e341..2ad404dd 100644 --- a/cmd/garm/main.go +++ b/cmd/garm/main.go @@ -154,6 +154,11 @@ func main() { router = routers.WithMetricsRouter(router, cfg.Metrics.DisableAuth, metricsMiddleware) } + if cfg.Default.DebugServer { + log.Printf("setting up debug routes") + router = routers.WithDebugServer(router) + } + corsMw := mux.CORSMethodMiddleware(router) router.Use(corsMw) diff --git a/config/config.go b/config/config.go index 909e26f4..a0eca1e3 100644 --- a/config/config.go +++ b/config/config.go @@ -120,6 +120,7 @@ type Default struct { // LogFile is the location of the log file. LogFile string `toml:"log_file,omitempty" json:"log-file"` EnableLogStreamer bool `toml:"enable_log_streamer"` + DebugServer bool `toml:"debug_server" json:"debug-server"` } func (d *Default) Validate() error { @@ -337,7 +338,7 @@ func (s *SQLite) Validate() error { } func (s *SQLite) ConnectionString() (string, error) { - return fmt.Sprintf("%s?_journal_mode=WAL", s.DBFile), nil + return fmt.Sprintf("%s?_journal_mode=WAL&_foreign_keys=ON", s.DBFile), nil } // MySQL is the config entry for the mysql section diff --git a/database/sql/jobs.go b/database/sql/jobs.go index 96ab75a4..ffe14378 100644 --- a/database/sql/jobs.go +++ b/database/sql/jobs.go @@ -22,6 +22,7 @@ func sqlWorkflowJobToParamsJob(job WorkflowJob) (params.Job, error) { return params.Job{}, errors.Wrap(err, "unmarshaling labels") } } + return params.Job{ ID: job.ID, RunID: job.RunID, @@ -200,15 +201,15 @@ func (s *sqlDatabase) CreateOrUpdateJob(ctx context.Context, job params.Job) (pa workflowJob.RunnerName = job.RunnerName } - if job.RepoID != uuid.Nil { + if job.RepoID != nil { workflowJob.RepoID = job.RepoID } - if job.OrgID != uuid.Nil { + if job.OrgID != nil { workflowJob.OrgID = job.OrgID } - if job.EnterpriseID != uuid.Nil { + if job.EnterpriseID != nil { workflowJob.EnterpriseID = job.EnterpriseID } if err := s.conn.Save(&workflowJob).Error; err != nil { diff --git a/database/sql/models.go b/database/sql/models.go index 7a15fd41..ef368768 100644 --- a/database/sql/models.go +++ b/database/sql/models.go @@ -130,8 +130,8 @@ type InstanceStatusUpdate struct { EventLevel params.EventLevel Message string `gorm:"type:text"` - InstanceID uuid.UUID - Instance Instance `gorm:"foreignKey:InstanceID"` + InstanceID uuid.UUID `gorm:"index:instance_id"` + Instance Instance `gorm:"foreignKey:InstanceID"` } type Instance struct { @@ -144,7 +144,7 @@ type Instance struct { OSArch params.OSArch OSName string OSVersion string - Addresses []Address `gorm:"foreignKey:InstanceID"` + Addresses []Address `gorm:"foreignKey:InstanceID;constraint:OnDelete:CASCADE,OnUpdate:CASCADE;"` Status common.InstanceStatus RunnerStatus common.RunnerStatus CallbackURL string @@ -158,7 +158,7 @@ type Instance struct { PoolID uuid.UUID Pool Pool `gorm:"foreignKey:PoolID"` - StatusMessages []InstanceStatusUpdate `gorm:"foreignKey:InstanceID"` + StatusMessages []InstanceStatusUpdate `gorm:"foreignKey:InstanceID;constraint:OnDelete:CASCADE,OnUpdate:CASCADE;"` } type User struct { @@ -218,13 +218,13 @@ type WorkflowJob struct { // entity type, in response to one workflow event. Thus, we will get 3 webhooks // with the same run_id and job id. Record all involved entities in the same job // if we have them configured in garm. - RepoID uuid.UUID `gorm:"index"` + RepoID *uuid.UUID `gorm:"index"` Repository Repository `gorm:"foreignKey:RepoID"` - OrgID uuid.UUID `gorm:"index"` + OrgID *uuid.UUID `gorm:"index"` Organization Organization `gorm:"foreignKey:OrgID"` - EnterpriseID uuid.UUID `gorm:"index"` + EnterpriseID *uuid.UUID `gorm:"index"` Enterprise Enterprise `gorm:"foreignKey:EnterpriseID"` LockedBy uuid.UUID diff --git a/params/params.go b/params/params.go index 73c1c85e..b3b61e47 100644 --- a/params/params.go +++ b/params/params.go @@ -463,9 +463,9 @@ type Job struct { // entity type, in response to one workflow event. Thus, we will get 3 webhooks // with the same run_id and job id. Record all involved entities in the same job // if we have them configured in garm. - RepoID uuid.UUID `json:"repo_id,omitempty"` - OrgID uuid.UUID `json:"org_id,omitempty"` - EnterpriseID uuid.UUID `json:"enterprise_id,omitempty"` + RepoID *uuid.UUID `json:"repo_id,omitempty"` + OrgID *uuid.UUID `json:"org_id,omitempty"` + EnterpriseID *uuid.UUID `json:"enterprise_id,omitempty"` LockedBy uuid.UUID diff --git a/runner/pool/pool.go b/runner/pool/pool.go index 5adae80d..2efb65fa 100644 --- a/runner/pool/pool.go +++ b/runner/pool/pool.go @@ -904,11 +904,11 @@ func (r *basePoolManager) paramsWorkflowJobToParamsJob(job params.WorkflowJob) ( switch r.helper.PoolType() { case params.EnterprisePool: - jobParams.EnterpriseID = asUUID + jobParams.EnterpriseID = &asUUID case params.RepositoryPool: - jobParams.RepoID = asUUID + jobParams.RepoID = &asUUID case params.OrganizationPool: - jobParams.OrgID = asUUID + jobParams.OrgID = &asUUID default: return jobParams, errors.Errorf("unknown pool type: %s", r.helper.PoolType()) } @@ -1563,7 +1563,7 @@ func (r *basePoolManager) consumeQueuedJobs() error { jobLabels := []string{ fmt.Sprintf("%s%d", jobLabelPrefix, job.ID), } - for { + for i := 0; i < poolRR.Len(); i++ { pool, err := poolRR.Next() if err != nil { log.Printf("[PoolRR %s] could not find a pool to create a runner for job %d: %s", r.helper.String(), job.ID, err) diff --git a/runner/pool/util.go b/runner/pool/util.go index aa09c9f7..3b6860eb 100644 --- a/runner/pool/util.go +++ b/runner/pool/util.go @@ -19,10 +19,6 @@ func (p *poolRoundRobin) Next() (params.Pool, error) { if len(p.pools) == 0 { return params.Pool{}, runnerErrors.ErrNoPoolsAvailable } - if p.next >= uint32(len(p.pools)) { - p.Reset() - return params.Pool{}, runnerErrors.ErrNoPoolsAvailable - } n := atomic.AddUint32(&p.next, 1) return p.pools[(int(n)-1)%len(p.pools)], nil