Adding Boskos / Mason configuration and tools by sebastienvas · Pull Request #690 · istio/test-infra

sebastienvas · 2018-02-14T23:56:28Z

In order to review, please look at the commits as the vendor update hides all the changes.

ldemailly · 2018-02-15T07:50:43Z

👍

too bad the repo history will carry those forever ;-)

sebastienvas · 2018-02-15T17:44:22Z

/retest

chxchx

Now I see why checking in vendor might be a bad idea

chxchx · 2018-02-22T01:37:47Z

boskos/cmd/mason_client/main.go

+	c1, acquireCancel := context.WithTimeout(context.Background(), timeout)
+	c2, updateCancel := context.WithCancel(context.Background())
+	defer acquireCancel()
+	defer updateCancel()


I think stacked defers are executed in LIFO order so I think it makes more sense to move defer updateCancel() after error checking on acquire().

chxchx · 2018-02-22T02:12:20Z

boskos/gcp/gce.go

+				},
+			},
+		},
+		ServiceAccounts: []*compute.ServiceAccount{


I thought you need something like

ServiceAccounts: []*compute.ServiceAccount{ &compute.ServiceAccount{ Email: "default", Scopes: config.Scopes, }, },

so we know the element we put in the slice is a pointer?

gometalinter will complain if you do that.

chxchx · 2018-02-22T02:17:40Z

boskos/gcp/gcloud.go

+	"github.com/sirupsen/logrus"
+)
+
+func runCommand(name string, args ...string) error {


Why not use the existing Shell() utility we have in test-infra?
If you insist, perhaps make this a util as well?

chxchx · 2018-02-22T02:21:56Z

boskos/gcp/gke.go

+				return err
+			}
+			switch newOp.Status {
+			case "DONE":


Are these string standardized? Maybe define they as enum instead of hardcoding?

are there no other values? default?

added logging

chxchx · 2018-02-22T02:23:44Z

boskos/gcp/test-configs.yaml

+          - http-server
+          - https-server
+          scopes:
+          - https://www.googleapis.com/auth/cloud-platform


pro tip: config your editor to always add new line at the end

chxchx · 2018-02-22T02:28:15Z

scripts/set-boskos-project.sh

+# Print commands
+set -x
+
+USERS=()


just curious, how is this different from

USERS=( 'serviceAccount:boskos@istio-testing.iam.gserviceaccount.com' 'serviceAccount:istio-prow-test-job@istio-testing.iam.gserviceaccount.com' 'group:mdb.istio-testing@google.com' )

chxchx · 2018-02-22T02:29:53Z

scripts/update-e2e-cluster.sh

  CLUSTER_NAME="${REPO}-e2e-rbac-rotation-${i}"
-  result=$(gcloud container clusters create ${CLUSTER_NAME} --zone ${ZONE} --project ${PROJECT_NAME} --cluster-version ${CLUSTER_VERSION} \
-  --machine-type ${MACHINE_TYPE} --num-nodes ${NUM_NODES} --no-enable-legacy-authorization --enable-kubernetes-alpha --quiet \
+  // Passing KUBECONFIG as an env will override default ~/.kube/config


Is this meant to be a comment? Start with # instead?

Using scripts/update_deps.sh

rkpagadala · 2018-02-26T23:44:31Z

boskos/cmd/mason/main.go

@@ -0,0 +1,62 @@
+// Copyright 2017 Istio Authors


change 2017 to 2018 here and elsewhere for new files.

rkpagadala · 2018-02-26T23:45:46Z

boskos/cmd/mason_client/main.go

@@ -0,0 +1,191 @@
+// Copyright 2017 Istio Authors


rkpagadala · 2018-02-26T23:56:52Z

boskos/gcp/gke.go

+				return err
+			}
+			switch newOp.Status {
+			case "DONE":


are there no other values? default?

rkpagadala · 2018-02-27T00:13:29Z

boskos/README.md

+
+Boskos runs on a kubernetes cluster. Initially only Prow jobs will be using
+Boskos. Since Boskos service is only available intra cluster, it is important
+that Boskos runs in the same cluster were test jobs are running.


where test jobs are running.

rkpagadala · 2018-02-27T00:15:15Z

boskos/README.md

+## Mason
+
+Mason is used for resources that have resources dependency. Mason resource are
+virtual, they only exist inside other resources. An example is a mason resource


An example of a mason resource

rkpagadala · 2018-02-27T00:21:50Z

boskos/gcp/gke.go

+			case operationDone:
 				return nil
-			case "ABORTING":
+			case operationAborting:


any other operations status that we need to care about? do we need default?

sebastienvas

PTAL

rkpagadala · 2018-02-27T21:36:47Z

/lgtm
assuming you will fix the failure in the tests

istio-merge-robot · 2018-02-27T21:37:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rkpagadala

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these OWNERS Files:

~~OWNERS~~ [rkpagadala]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

istio-merge-robot · 2018-02-27T21:43:15Z

/lgtm cancel //PR changed after LGTM, removing LGTM. @rkpagadala @sebastienvas

googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Feb 14, 2018

sebastienvas force-pushed the boskos branch from 9939289 to 0f0ccea Compare February 15, 2018 00:05

sebastienvas requested review from chxchx and rkpagadala February 16, 2018 19:32

sebastienvas force-pushed the boskos branch from 10e5f6e to 1e2faa1 Compare February 16, 2018 19:41

chxchx reviewed Feb 22, 2018

View reviewed changes

sebastienvas added 17 commits February 26, 2018 15:03

Adds config files and client for boskos

eba5385

Update bazel version and update scripts

86b6810

Update Vendors

cb1690b

Using scripts/update_deps.sh

Updates script to not change ~/.kube/config

8d63c67

Removing load balancer for boskos service

fb59484

Bumping up k8s version to 1.9

02b689a

Update mason to update config every minute

12357cc

Update mason deployment

f85042f

Adds documentation

655fa30

Fixed linter issues

9bfad2b

Only saving content

732beeb

Use latest version of debian strech image

18b9979

Update reaper deployment with mason cleanup

d9b9535

Use busy state as this is what reaper expect

9b9aa9b

Resolve code review comments

0de3dbb

Fix bazel build

46a5601

Add more projects to boskos

41058fb

sebastienvas force-pushed the boskos branch from 104746c to 41058fb Compare February 26, 2018 23:03

rkpagadala reviewed Feb 26, 2018

View reviewed changes

rkpagadala reviewed Feb 27, 2018

View reviewed changes

sebastienvas mentioned this pull request Feb 27, 2018

Enable cluster provisioning on all e2e istio/istio#3790

Merged

Fix review comments

22324d0

sebastienvas commented Feb 27, 2018

View reviewed changes

Fix typos in README

c61eab4

istio-testing assigned rkpagadala Feb 27, 2018

istio-testing added the lgtm label Feb 27, 2018

istio-merge-robot added the approved label Feb 27, 2018

Merge branch 'master' of https://github.com/istio/test-infra into boskos

b83ddda

istio-merge-robot removed the lgtm label Feb 27, 2018

sebastienvas merged commit 100ca90 into istio:master Feb 27, 2018

sebastienvas deleted the boskos branch February 27, 2018 21:47

Conversation

sebastienvas commented Feb 14, 2018

Uh oh!

ldemailly commented Feb 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sebastienvas commented Feb 15, 2018

Uh oh!

chxchx left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sebastienvas Feb 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sebastienvas left a comment

Choose a reason for hiding this comment

Uh oh!

rkpagadala commented Feb 27, 2018

Uh oh!

istio-merge-robot commented Feb 27, 2018

Uh oh!

istio-merge-robot commented Feb 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ldemailly commented Feb 15, 2018 •

edited

Loading

sebastienvas Feb 23, 2018 •

edited

Loading