From 9e1c4e60e52bc363f1c942a76f9b0f9547e88719 Mon Sep 17 00:00:00 2001 From: Alex Suraci Date: Tue, 25 Feb 2014 17:10:04 -0800 Subject: [PATCH 1/2] remove docker/queue singletons; inject in main --- cmd/drone/drone.go | 4 +++- cmd/droned/drone.go | 11 ++++++++++- pkg/build/docker/client.go | 2 -- pkg/handler/hooks.go | 22 +++++++++++++++------- pkg/queue/queue.go | 7 ------- pkg/queue/runner.go | 2 +- 6 files changed, 29 insertions(+), 19 deletions(-) diff --git a/cmd/drone/drone.go b/cmd/drone/drone.go index 812db1f88..d183e5e79 100644 --- a/cmd/drone/drone.go +++ b/cmd/drone/drone.go @@ -123,6 +123,8 @@ func vet(path string) { } func run(path string) { + dockerClient := docker.New() + // parse the Drone yml file s, err := script.ParseBuildFile(path) if err != nil { @@ -175,7 +177,7 @@ func run(path string) { // loop through and create builders for _, b := range builds { //script.Builds { - builder := build.New(docker.DefaultClient) + builder := build.New(dockerClient) builder.Build = b builder.Repo = &code builder.Key = key diff --git a/cmd/droned/drone.go b/cmd/droned/drone.go index 9d796851a..8bfd350ab 100644 --- a/cmd/droned/drone.go +++ b/cmd/droned/drone.go @@ -5,7 +5,9 @@ import ( "flag" "log" "net/http" + "runtime" "strings" + "time" "code.google.com/p/go.net/websocket" "github.com/GeertJohan/go.rice" @@ -13,10 +15,12 @@ import ( _ "github.com/mattn/go-sqlite3" "github.com/russross/meddler" + "github.com/drone/drone/pkg/build/docker" "github.com/drone/drone/pkg/channel" "github.com/drone/drone/pkg/database" "github.com/drone/drone/pkg/database/migrate" "github.com/drone/drone/pkg/handler" + "github.com/drone/drone/pkg/queue" ) var ( @@ -118,6 +122,11 @@ func setupStatic() { // setup routes for serving dynamic content. func setupHandlers() { + queueRunner := queue.NewRunner(docker.New(), 300*time.Second) + queue := queue.Start(runtime.NumCPU(), queueRunner) + + hookHandler := handler.NewHookHandler(queue) + m := pat.New() m.Get("/login", handler.ErrorHandler(handler.Login)) m.Post("/login", handler.ErrorHandler(handler.Authorize)) @@ -177,7 +186,7 @@ func setupHandlers() { m.Get("/account/admin/users", handler.AdminHandler(handler.AdminUserList)) // handlers for GitHub post-commit hooks - m.Post("/hook/github.com", handler.ErrorHandler(handler.Hook)) + m.Post("/hook/github.com", handler.ErrorHandler(hookHandler.Hook)) // handlers for first-time installation m.Get("/install", handler.ErrorHandler(handler.Install)) diff --git a/pkg/build/docker/client.go b/pkg/build/docker/client.go index 123e52468..bb33a28fb 100644 --- a/pkg/build/docker/client.go +++ b/pkg/build/docker/client.go @@ -29,8 +29,6 @@ const ( // Enables verbose logging to the Terminal window var Logging = true -var DefaultClient = New() // TEMPORARY; PLEASE CONSTRUCT/INJECT YOURSELF - // New creates an instance of the Docker Client func New() *Client { c := &Client{} diff --git a/pkg/handler/hooks.go b/pkg/handler/hooks.go index 75f775993..f7aaed24b 100644 --- a/pkg/handler/hooks.go +++ b/pkg/handler/hooks.go @@ -13,10 +13,19 @@ import ( "github.com/drone/go-github/github" ) +type HookHandler struct { + queue *queue.Queue +} + +func NewHookHandler(queue *queue.Queue) *HookHandler { + return &HookHandler{ + queue: queue, + } +} + // Processes a generic POST-RECEIVE hook and // attempts to trigger a build. -func Hook(w http.ResponseWriter, r *http.Request) error { - +func (h *HookHandler) Hook(w http.ResponseWriter, r *http.Request) error { // handle github ping if r.Header.Get("X-Github-Event") == "ping" { return RenderText(w, http.StatusText(http.StatusOK), http.StatusOK) @@ -25,7 +34,7 @@ func Hook(w http.ResponseWriter, r *http.Request) error { // if this is a pull request route // to a different handler if r.Header.Get("X-Github-Event") == "pull_request" { - PullRequestHook(w, r) + h.PullRequestHook(w, r) return nil } @@ -160,14 +169,13 @@ func Hook(w http.ResponseWriter, r *http.Request) error { //realtime.CommitPending(repo.UserID, repo.TeamID, repo.ID, commit.ID, repo.Private) //realtime.BuildPending(repo.UserID, repo.TeamID, repo.ID, commit.ID, build.ID, repo.Private) - queue.Add(&queue.BuildTask{Repo: repo, Commit: commit, Build: build, Script: buildscript}) //Push(repo, commit, build, buildscript) + h.queue.Add(&queue.BuildTask{Repo: repo, Commit: commit, Build: build, Script: buildscript}) //Push(repo, commit, build, buildscript) // OK! return RenderText(w, http.StatusText(http.StatusOK), http.StatusOK) } -func PullRequestHook(w http.ResponseWriter, r *http.Request) { - +func (h *HookHandler) PullRequestHook(w http.ResponseWriter, r *http.Request) { // get the payload of the message // this should contain a json representation of the // repository and commit details @@ -276,7 +284,7 @@ func PullRequestHook(w http.ResponseWriter, r *http.Request) { // notify websocket that a new build is pending // TODO we should, for consistency, just put this inside Queue.Add() - queue.Add(&queue.BuildTask{Repo: repo, Commit: commit, Build: build, Script: buildscript}) + h.queue.Add(&queue.BuildTask{Repo: repo, Commit: commit, Build: build, Script: buildscript}) // OK! RenderText(w, http.StatusText(http.StatusOK), http.StatusOK) diff --git a/pkg/queue/queue.go b/pkg/queue/queue.go index 057a2c5aa..ccac7a161 100644 --- a/pkg/queue/queue.go +++ b/pkg/queue/queue.go @@ -1,11 +1,8 @@ package queue import ( - "github.com/drone/drone/pkg/build/docker" "github.com/drone/drone/pkg/build/script" . "github.com/drone/drone/pkg/model" - "runtime" - "time" ) // A Queue dispatches tasks to workers. @@ -25,10 +22,6 @@ type BuildTask struct { Script *script.Build } -var defaultQueue = Start(runtime.NumCPU(), newRunner(docker.DefaultClient, 300*time.Second)) // TEMPORARY; INJECT PLEASE - -var Add = defaultQueue.Add // TEMPORARY; INJECT PLEASE - func Start(workers int, runner Runner) *Queue { // get the number of CPUs. Since builds // tend to be CPU-intensive we should only diff --git a/pkg/queue/runner.go b/pkg/queue/runner.go index 8bb0a7f0b..41a435316 100644 --- a/pkg/queue/runner.go +++ b/pkg/queue/runner.go @@ -19,7 +19,7 @@ type runner struct { timeout time.Duration } -func newRunner(dockerClient *docker.Client, timeout time.Duration) *runner { +func NewRunner(dockerClient *docker.Client, timeout time.Duration) Runner { return &runner{ dockerClient: dockerClient, timeout: timeout, From a198657a7ec8dfe425f0a2c856e72d93b6e40d69 Mon Sep 17 00:00:00 2001 From: Alex Suraci Date: Tue, 25 Feb 2014 17:14:14 -0800 Subject: [PATCH 2/2] Runner -> BuildRunner to be more descriptive also kill some stale comments --- pkg/queue/{runner.go => build_runner.go} | 14 +++++++------- pkg/queue/queue.go | 12 ++---------- pkg/queue/worker.go | 2 +- 3 files changed, 10 insertions(+), 18 deletions(-) rename pkg/queue/{runner.go => build_runner.go} (63%) diff --git a/pkg/queue/runner.go b/pkg/queue/build_runner.go similarity index 63% rename from pkg/queue/runner.go rename to pkg/queue/build_runner.go index 41a435316..0c04ad102 100644 --- a/pkg/queue/runner.go +++ b/pkg/queue/build_runner.go @@ -10,29 +10,29 @@ import ( "github.com/drone/drone/pkg/build/script" ) -type Runner interface { +type BuildRunner interface { Run(buildScript *script.Build, repo *repo.Repo, key []byte, buildOutput io.Writer) (success bool, err error) } -type runner struct { +type buildRunner struct { dockerClient *docker.Client timeout time.Duration } -func NewRunner(dockerClient *docker.Client, timeout time.Duration) Runner { - return &runner{ +func NewBuildRunner(dockerClient *docker.Client, timeout time.Duration) BuildRunner { + return &buildRunner{ dockerClient: dockerClient, timeout: timeout, } } -func (r *runner) Run(buildScript *script.Build, repo *repo.Repo, key []byte, buildOutput io.Writer) (bool, error) { - builder := build.New(r.dockerClient) +func (runner *buildRunner) Run(buildScript *script.Build, repo *repo.Repo, key []byte, buildOutput io.Writer) (bool, error) { + builder := build.New(runner.dockerClient) builder.Build = buildScript builder.Repo = repo builder.Key = key builder.Stdout = buildOutput - builder.Timeout = r.timeout + builder.Timeout = runner.timeout err := builder.Run() diff --git a/pkg/queue/queue.go b/pkg/queue/queue.go index ccac7a161..cb7cfba96 100644 --- a/pkg/queue/queue.go +++ b/pkg/queue/queue.go @@ -22,20 +22,12 @@ type BuildTask struct { Script *script.Build } -func Start(workers int, runner Runner) *Queue { - // get the number of CPUs. Since builds - // tend to be CPU-intensive we should only - // execute 1 build per CPU. - // must be at least 1 - // if ncpu < 1 { - // ncpu = 1 - // } - +// Start N workers with the given build runner. +func Start(workers int, runner BuildRunner) *Queue { tasks := make(chan *BuildTask) queue := &Queue{tasks: tasks} - // spawn a worker for each CPU for i := 0; i < workers; i++ { worker := worker{ runner: runner, diff --git a/pkg/queue/worker.go b/pkg/queue/worker.go index a43071091..58a3b669d 100644 --- a/pkg/queue/worker.go +++ b/pkg/queue/worker.go @@ -17,7 +17,7 @@ import ( ) type worker struct { - runner Runner + runner BuildRunner } // work is a function that will infinitely