feat(engine): hatchery as a uservice - auth #3326

yesnault · 2018-09-14T13:04:22Z

close #3315 (with 5f461f6 too)

Signed-off-by: Yvonnick Esnault [email protected]

Signed-off-by: Yvonnick Esnault <[email protected]>

yesnault · 2018-09-14T21:42:50Z

WIP do not merge yet

golangcibot · 2018-09-14T21:43:12Z

engine/api/hatchery.go

@@ -34,37 +32,35 @@ func (api *API) registerHatcheryHandler() service.Handler {
 			return sdk.WrapError(err, "registerHatcheryHandler> Cannot load hatchery %s", hatch.Name)
 		}

+		tx, errBegin := api.mustDB().Begin()
+		defer tx.Rollback()


Error return value of tx.Rollback is not checked

Signed-off-by: Yvonnick Esnault <[email protected]>

ovh-cds · 2018-09-16T20:49:00Z

CDS Report ut-engine#5975.1 ✘

Stage 1
- Engine Test with DB ✘

sguiheux · 2018-09-17T07:23:16Z

engine/api/workflow/dao_node_run_job.go

+		if hatchery.RatioService == 100 {
+			where = " AND contains_service = true "
+		} else if hatchery.RatioService == 0 {
+			where = " AND (contains_service is NULL or contains_service = false) "


contains_service is NULL . is mandatory ???

sguiheux · 2018-09-17T07:24:11Z

engine/api/workflow/gorp_model.go

@@ -89,6 +89,7 @@ type JobRun struct {
 	ExecGroups             sql.NullString `db:"exec_groups"`
 	PlatformPluginBinaries sql.NullString `db:"platform_plugin_binaries"`
 	BookedBy               sdk.Hatchery   `db:"-"`
+	ContainsService        sql.NullBool   `db:"contains_service"`


sql.Bool ? ( is false is applied on old lines)

sguiheux · 2018-09-17T07:31:48Z

engine/sql/128_hatchery_type.sql

+
+ALTER TABLE hatchery 
+    ADD COLUMN worker_model_id BIGINT,
+    ADD COLUMN model_type VARCHAR(50) DEFAULT '',


sguiheux · 2018-09-17T07:36:27Z

engine/sql/128_hatchery_type.sql

+-- +migrate Up
+
+ALTER TABLE hatchery 
+    ADD COLUMN worker_model_id BIGINT,


Remove worker_model_id, localhatchery must check requirement from its path

fsamin · 2018-09-17T07:22:24Z

engine/api/hatchery/hatchery.go

 	var errg error
-	h.UID, errg = generateID()
+	hatchery.UID, errg = generateID()


why not using sdk.UUID() ?

fsamin · 2018-09-17T07:22:51Z

engine/api/hatchery/gorp_model.go

+)
+
+// Hatchery is a gorp wrapper around sdk.Hatchery
+type Hatchery sdk.Hatchery


this type should be private

fsamin · 2018-09-17T07:27:43Z

engine/api/hatchery/hatchery.go

+			Cmd:   "worker --api={{.API}} --token={{.Token}} --basedir={{.BaseDir}} --model={{.Model}} --name={{.Name}} --hatchery={{.Hatchery}} --hatchery-name={{.HatcheryName}} --insecure={{.HTTPInsecure}} --booked-workflow-job-id={{.WorkflowJobID}} --booked-pb-job-id={{.PipelineBuildJobID}} --single-use --force-exit",
+		}
+		if err := worker.InsertWorkerModel(db, &hatchery.Model); err != nil && strings.Contains(err.Error(), "idx_worker_model_name") {
+			return sdk.ErrModelNameExist


may be we should update the model and not raise an error

fsamin · 2018-09-17T07:30:06Z

engine/api/hatchery/hatchery.go

 		}
-		return nil, err
+		return nil, sdk.ErrNotFound


fsamin · 2018-09-17T07:30:34Z

engine/api/hatchery/hatchery.go

-
-	var h sdk.Hatchery
-	var wmID sql.NullInt64
+	var hatchery Hatchery
 	hasher := sha512.New()
 	hashed := base64.StdEncoding.EncodeToString(hasher.Sum([]byte(token)))


fsamin · 2018-09-17T07:30:58Z

engine/api/hatchery/hatchery.go

-
-	return nil
-}
-
 func generateID() (string, error) {


remove this, sdk.UUID exists

fsamin · 2018-09-17T07:34:51Z

engine/api/services.go

+				}
+				if svcs[i].Type == "hatchery" {
+					log.Info("killDeadServices> Delete dead hatchery: %v", &svcs[i].Name)
+					if err := hatchery.DeleteHatcheryByName(db, svcs[i].Name); err != nil {


we shouldn't have to delete manually the hatchery when we remove the service. There should be a foreign key and a delete cascade

fsamin · 2018-09-17T07:36:08Z

engine/api/services.go

+			}
+		case <-ctx.Done():
+			if err := ctx.Err(); err != nil {
+				log.Error("Exiting killDeadServices: %v", err)


Signed-off-by: Yvonnick Esnault <[email protected]>

fsamin · 2018-09-19T09:36:38Z

engine/api/api_routes.go

@@ -248,7 +246,6 @@ func (api *API) InitRouter() {
 	r.Handle("/project/{permProjectKey}/push/workflows", r.POST(api.postWorkflowPushHandler))

 	// Workflows run
-	r.Handle("/project/{permProjectKey}/runs", r.GET(api.getWorkflowAllRunsHandler, EnableTracing()))


do not delete this

fsamin · 2018-09-19T09:38:14Z

engine/api/hatchery/hatchery.go

 		WHERE (
-			hatchery.group_id = ANY(
+			services.type = 'hatchery'


use the sdk constant

fsamin · 2018-09-19T09:38:24Z

engine/api/hatchery/hatchery.go

 		WHERE (
-			hatchery.group_id = ANY(
+			services.type = 'hatchery'


fsamin · 2018-09-19T09:40:14Z

engine/api/mon_db.go

-	}
-
-	if _, err := workflow.LoadNodeJobRunQueue(ctx, api.mustDB(), api.Cache, permissions, groupsID, usr, nil, nil, nil); err != nil {
+	if _, err := workflow.LoadNodeJobRunQueue(ctx, api.mustDB(), api.Cache, "", nil, permissions, groupsID, usr, nil, nil, nil); err != nil {


too mush optional parameters
we need more specialized function of an option struct

fsamin · 2018-09-19T09:43:10Z

engine/api/workflow_run.go

@@ -136,18 +136,6 @@ func (api *API) searchWorkflowRun(ctx context.Context, w http.ResponseWriter, r
 	return service.WriteJSON(w, runs, code)
 }

-func (api *API) getWorkflowAllRunsHandler() service.Handler {


fsamin · 2018-09-19T09:44:48Z

engine/api/workflow/dao_node_run_job.go

@@ -60,14 +60,29 @@ func LoadNodeJobRunQueue(ctx context.Context, db gorp.SqlExecutor, store cache.S
 		statuses = []string{sdk.StatusWaiting.String()}
 	}

+	var where string


the sql query is unreadable :(
how can we improve this code ?

fsamin · 2018-09-19T09:47:56Z

engine/api/workflow_queue.go

@@ -753,7 +754,15 @@ func getSinceUntilLimitHeader(ctx context.Context, w http.ResponseWriter, r *htt
 		limit, _ = strconv.Atoi(limitHeader)
 	}

-	return since, until, limit
+	ratioServiceHeader := r.Header.Get("X-CDS-Ratio-Service")


why header and not queryParam ?

fsamin · 2018-09-19T09:48:04Z

engine/api/workflow_queue.go

@@ -702,7 +702,7 @@ func (api *API) countWorkflowJobQueueHandler() service.Handler {

 func (api *API) getWorkflowJobQueueHandler() service.Handler {
 	return func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {
-		since, until, limit := getSinceUntilLimitHeader(ctx, w, r)
+		since, until, limit, modelType, ratioService := getQueueHeaders(ctx, w, r)


Is there any tests on this new options

tests added

bnjjj · 2018-09-19T09:50:31Z

engine/hatchery/local/local.go

+		}
+	}
+
+	for _, r := range tmp {


declare capa here with a good length or capacity :)

Signed-off-by: Yvonnick Esnault <[email protected]>

ovh-cds · 2018-09-20T13:45:07Z

CDS Report build-engine-cli-tools#6067.0 ✘

Build minimal
- cds cli legacy ✔
- cdsctl linux ✘
- Engine Build linux amd64 ✔
- tools/debpacker ✔
- tools/os-ansible-inventory ✔
- Worker linux amd64 / 386 ✔

Signed-off-by: Yvonnick Esnault <[email protected]>

fsamin · 2018-09-20T14:01:59Z

engine/api/build_test.go

 	"github.com/stretchr/testify/assert"

+	"github.com/ovh/cds/engine/api/services"
+	"github.com/ovh/cds/engine/api/sessionstore"
+	"github.com/ovh/cds/sdk/namesgenerator"


Signed-off-by: Yvonnick Esnault <[email protected]>

feat(engine): add type, model type and ratio in hatchery table

79dc71f

Signed-off-by: Yvonnick Esnault <[email protected]>

yesnault requested review from bnjjj, fsamin, richardlt and sguiheux as code owners September 14, 2018 13:04

richardlt approved these changes Sep 14, 2018

View reviewed changes

wip

98706ea

Signed-off-by: Yvonnick Esnault <[email protected]>

yesnault changed the title ~~feat(engine): add type, model type and ratio in hatchery table~~ WIP feat(engine): add type, model type and ratio in hatchery table Sep 14, 2018

golangcibot reviewed Sep 14, 2018

View reviewed changes

yesnault added 2 commits September 15, 2018 16:40

hatchery as a service (heartbeat)

4a3f15c

Signed-off-by: Yvonnick Esnault <[email protected]>

wip

ee54c99

Signed-off-by: Yvonnick Esnault <[email protected]>

ovh deleted a comment from ovh-cds Sep 15, 2018

yesnault added 7 commits September 15, 2018 17:34

quicker spawn worker from hatchery:local

c27db38

Signed-off-by: Yvonnick Esnault <[email protected]>

wip

1eb30cb

Signed-off-by: Yvonnick Esnault <[email protected]>

wip

03fc134

Signed-off-by: Yvonnick Esnault <[email protected]>

wip

a7f6b74

Signed-off-by: Yvonnick Esnault <[email protected]>

wip

e41281a

Signed-off-by: Yvonnick Esnault <[email protected]>

wip

3aa3108

Signed-off-by: Yvonnick Esnault <[email protected]>

cr

d72de9e

Signed-off-by: Yvonnick Esnault <[email protected]>

yesnault changed the title ~~WIP feat(engine): add type, model type and ratio in hatchery table~~ feat(engine): add type, model type and ratio in hatchery table Sep 16, 2018

sguiheux requested changes Sep 17, 2018

View reviewed changes

fsamin requested changes Sep 17, 2018

View reviewed changes

cr

9cd2073

Signed-off-by: Yvonnick Esnault <[email protected]>

ovh deleted a comment from ovh-cds Sep 17, 2018

wip

3da40a5

Signed-off-by: Yvonnick Esnault <[email protected]>

yesnault changed the title ~~feat(engine): add type, model type and ratio in hatchery table~~ WIP feat(engine): add type, model type and ratio in hatchery table Sep 17, 2018

ovh deleted a comment from ovh-cds Sep 18, 2018

cr

dc3c60e

Signed-off-by: Yvonnick Esnault <[email protected]>

fsamin requested changes Sep 19, 2018

View reviewed changes

bnjjj suggested changes Sep 19, 2018

View reviewed changes

engine/hatchery/local/local.go Outdated

}

}

for _, r := range tmp {

Copy link

Contributor

bnjjj Sep 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

declare capa here with a good length or capacity :)

sguiheux approved these changes Sep 19, 2018

View reviewed changes

yesnault added 5 commits September 19, 2018 14:12

cr

4207f90

Signed-off-by: Yvonnick Esnault <[email protected]>

cr

2551a3b

Signed-off-by: Yvonnick Esnault <[email protected]>

cr

3037f3d

Signed-off-by: Yvonnick Esnault <[email protected]>

cr

87f8b19

Signed-off-by: Yvonnick Esnault <[email protected]>

cr

9aef64b

Signed-off-by: Yvonnick Esnault <[email protected]>

This comment has been minimized.

Sign in to view

cr

de61545

Signed-off-by: Yvonnick Esnault <[email protected]>

This comment has been minimized.

Sign in to view

yesnault added 2 commits September 19, 2018 18:15

last test on modelType

e1959d7

Signed-off-by: Yvonnick Esnault <[email protected]>

cr

9b8904e

Signed-off-by: Yvonnick Esnault <[email protected]>

bnjjj approved these changes Sep 20, 2018

View reviewed changes

yesnault added 2 commits September 20, 2018 15:29

cr

e8aad4e

Signed-off-by: Yvonnick Esnault <[email protected]>

cf

bc23c47

Signed-off-by: Yvonnick Esnault <[email protected]>

yesnault added 2 commits September 20, 2018 16:03

wip

db77eab

Signed-off-by: Yvonnick Esnault <[email protected]>

cr

a466bd7

Signed-off-by: Yvonnick Esnault <[email protected]>

fsamin requested changes Sep 20, 2018

View reviewed changes

fsamin approved these changes Sep 20, 2018

View reviewed changes

cr

9750a8a

Signed-off-by: Yvonnick Esnault <[email protected]>

yesnault merged commit 5470fc5 into master Sep 20, 2018

yesnault deleted the ye-store-hatchery-type branch October 16, 2018 19:06

This was referenced Feb 20, 2022

[Snyk] Security upgrade prismjs from 1.21.0 to 1.27.0 mbs-technologie/cds#13

Open

[Snyk] Security upgrade prismjs from 1.23.0 to 1.27.0 Visva92/cds#8

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(engine): hatchery as a uservice - auth #3326

feat(engine): hatchery as a uservice - auth #3326

yesnault commented Sep 14, 2018 •

edited

Loading

yesnault commented Sep 14, 2018

golangcibot Sep 14, 2018

ovh-cds commented Sep 16, 2018

sguiheux Sep 17, 2018

sguiheux Sep 17, 2018

sguiheux Sep 17, 2018

sguiheux Sep 17, 2018

fsamin Sep 17, 2018

fsamin Sep 17, 2018

fsamin Sep 17, 2018

fsamin Sep 17, 2018

fsamin Sep 17, 2018

fsamin Sep 17, 2018

fsamin Sep 17, 2018

fsamin Sep 17, 2018

fsamin Sep 19, 2018

fsamin Sep 19, 2018

fsamin Sep 19, 2018

fsamin Sep 19, 2018

fsamin Sep 19, 2018

fsamin Sep 19, 2018

fsamin Sep 19, 2018

fsamin Sep 19, 2018

yesnault Sep 19, 2018

bnjjj Sep 19, 2018

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ovh-cds commented Sep 20, 2018

fsamin Sep 20, 2018

feat(engine): hatchery as a uservice - auth #3326

feat(engine): hatchery as a uservice - auth #3326

Conversation

yesnault commented Sep 14, 2018 • edited Loading

yesnault commented Sep 14, 2018

Choose a reason for hiding this comment

ovh-cds commented Sep 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ovh-cds commented Sep 20, 2018

Choose a reason for hiding this comment

yesnault commented Sep 14, 2018 •

edited

Loading