Replace submit queue with Tide in docs #2701

nikhita · 2018-09-23T10:39:08Z

Ref: https://groups.google.com/d/topic/kubernetes-dev/sp0dOVt4WsA/discussion

The submit queue is also mentioned in many places in sig-scalability/ but I don't want to correct them in this PR since it touches their charters, etc.

/cc @spiffxp @cblecker @BenTheElder @guineveresaenger

nikhita · 2018-09-23T10:39:42Z

contributors/devel/automation.md

 ## Submit Queue

+Submit Queue is deprecated. Kubernetes uses


Should this be removed completely or be marked as deprecated?

the submit queue doesn't even exist anymore (rm'ed) but people might have still heard of it.. I think we should probably keep this for a bit.

I would change the heading to Tide, and this text can say something like "This project formerly used a Submit Queue, it has since been replaced by Tide"

nikhita · 2018-09-23T10:40:03Z

contributors/devel/e2e-tests.md

-queue](http://submit-queue.k8s.io/) on a subset of those tests if they fail (the
-subset is defined in the [munger config](https://git.k8s.io/test-infra/mungegithub/mungers/submit-queue.go)
-via the `jenkins-jobs` flag; note we also block on	`kubernetes-build` and
-`kubernetes-test-go` jobs for build and unit and integration tests).


Where is this defined now?

I don't think it looked like this for quite some time. currently it depends on if a job is considered optional by prow, or for other repos if they've configured additional contexts (EG for additional CI statuses not from prow).. this block could use a thoroughy rewrite. IT also only addresses e2e tests, without mentioning build, unit, integration, ...

nikhita · 2018-09-23T10:40:22Z

contributors/devel/e2e-tests.md

-quota,) they should remain with the `[Feature:.+]` label, and the suites that
-run them should be incorporated into the
-[munger config](https://git.k8s.io/test-infra/mungegithub/mungers/submit-queue.go)
-via the `jenkins-jobs` flag.


What is the equivalent alternative for this now?

ideally don't run this sort of thing in presubmit to begin with, we can have optionally requested tests with always_run: false set on them in prow config, and we can have periodic / postsubmit to cover features that aren't necessarily PR blocking.

there's also an entirely seperate concept of release blocking jobs not covered here I think. these docs are really stale

BenTheElder · 2018-09-24T08:18:31Z

/assign @cjwagner

BenTheElder · 2018-09-24T08:19:26Z

contributors/devel/automation.md

@@ -48,6 +51,10 @@ If these tests pass a second time, the PR will be merged when this PR finishes r

 ## Github Munger


this probably doesn't need to be kept

BenTheElder · 2018-09-24T08:19:46Z

contributors/devel/e2e-tests.md

@@ -576,7 +576,7 @@ suite, it receives a `[Feature:.+]` label, e.g. `[Feature:Performance]` or
 `[Feature:Ingress]`. `[Feature:.+]` tests are not run in our core suites,
 instead running in custom suites. If a feature is experimental or alpha and is
 not enabled by default due to being incomplete or potentially subject to
-breaking changes, it does *not* block the merge-queue, and thus should run in
+breaking changes, it does *not* block the merge pool, and thus should run in


this might be confusing without more background on pools

It does not block PR merges

spiffxp

Thank you for doing this, a few suggestions

spiffxp · 2018-09-24T22:27:00Z

contributors/devel/automation.md

 ## Submit Queue

+Submit Queue is deprecated. Kubernetes uses


I would change the heading to Tide, and this text can say something like "This project formerly used a Submit Queue, it has since been replaced by Tide"

spiffxp · 2018-09-24T22:27:51Z

contributors/devel/e2e-tests.md

@@ -576,7 +576,7 @@ suite, it receives a `[Feature:.+]` label, e.g. `[Feature:Performance]` or
 `[Feature:Ingress]`. `[Feature:.+]` tests are not run in our core suites,
 instead running in custom suites. If a feature is experimental or alpha and is
 not enabled by default due to being incomplete or potentially subject to
-breaking changes, it does *not* block the merge-queue, and thus should run in
+breaking changes, it does *not* block the merge pool, and thus should run in


It does not block PR merges

spiffxp · 2018-09-24T22:29:34Z

contributors/devel/e2e-tests.md

@@ -684,14 +681,11 @@ contend for resources; see above about [kinds of tests](#kinds_of_tests).

 Generally, a feature starts as `experimental`, and will be run in some suite
 owned by the team developing the feature. If a feature is in beta or GA, it
-*should* block the merge-queue. In moving from experimental to beta or GA, tests
+*should* block the merge pool. In moving from experimental to beta or GA, tests


block PR merges and releases

spiffxp · 2018-09-24T22:31:30Z

contributors/devel/e2e-tests.md

-via the `jenkins-jobs` flag; note we also block on	`kubernetes-build` and
-`kubernetes-test-go` jobs for build and unit and integration tests).
+continuous basis, and block merges via [Tide](https://git.k8s.io/test-infra/prow/cmd/tide)
+on a subset of those tests if they fail.


This whole section reads like it's left over from the old days of post-submits blocking the submit queue. In that spirit, I would rewrite this to talk about release-blocking jobs, and reference https://testgrid.k8s.io/sig-release-master-blocking

spiffxp · 2018-09-24T22:32:08Z

contributors/devel/writing-good-e2e-tests.md

@@ -10,7 +10,7 @@ designing, writing and debugging your end-to-end tests.  In
 particular, "flaky" tests, which pass most of the time but fail
 intermittently for difficult-to-diagnose reasons are extremely costly
 in terms of blurring our regression signals and slowing down our
-automated merge queue.  Up-front time and effort designing your test
+automated merge pool.  Up-front time and effort designing your test


s/automated merge pool/merge velocity

spiffxp · 2018-09-24T22:38:13Z

contributors/guide/pull-requests.md

@@ -72,7 +72,7 @@ Here's the process the pull request goes through on its way from submission to m

 1. If you're **not** a member of the Kubernetes organization, a Reviewer/Kubernetes Member checks that the pull request is safe to test. If so, they comment `/ok-to-test`. Pull requests by Kubernetes organization [members](/community-membership.md) do not need this step. Now the pull request is considered to be trusted, and the pre-submit tests will run:

-    1. Automatic tests run. See the current list of tests on the [MERGE REQUIREMENTS tab, at this link](https://submit-queue.k8s.io/#/info)
+    1. Automatic tests run. See the current list of tests on the [MERGE REQUIREMENTS tab, at this link](https://prow.k8s.io/tide#/info)


This link doesn't actually show tests that are run for every PR. We're sort of lacking a clear list:

The presubmits themselves are spread amongst a bunch of yaml files in https://github.com/kubernetes/test-infra/tree/master/config/jobs/kubernetes

We could point to https://k8s-testgrid.appspot.com/presubmits-kubernetes-blocking

We could point to https://prow.k8s.io/?repo=kubernetes%2Fkubernetes&type=presubmit

I guess the last link is my vote. But none of these clearly answer "how can I find out what these jobs do"

spiffxp · 2018-09-24T22:38:48Z

contributors/guide/pull-requests.md

-    1. Author has signed the CLA (`cncf-cla: yes` label added to pull request)
-    1. No changes made since last `lgtm` label applied
+1. The pull request enters the [merge pool](https://prow.k8s.io/tide)
+if the merge criteria is met. The [PR dashboard](https://prow.k8s.io/pr) shows


s/criteria is/criteria are/

spiffxp · 2018-09-24T22:39:09Z

contributors/guide/pull-requests.md


 That's the last step. Your pull request is now merged.

 ## Marking Unfinished Pull Requests

-If you want to solicit reviews before the implementation of your pull request is complete, you should hold your pull request to ensure that the merge queue does not pick it up and attempt to merge it. There are two methods to achieve this:
+If you want to solicit reviews before the implementation of your pull request is complete, you should hold your pull request to ensure that the merge pool does not pick it up and attempt to merge it. There are two methods to achieve this:


s/the merge pool/tide/

nikhita · 2018-09-25T07:31:42Z

@BenTheElder @spiffxp Addressed comments. Ptal

Is it ok to do the re-write of https://github.com/kubernetes/community/blob/master/contributors/devel/e2e-tests.md as a follow up?

cjwagner · 2018-09-25T20:53:03Z

contributors/devel/automation.md

+This project formerly used a Submit Queue, it has since been replaced by
+[Tide](https://git.k8s.io/test-infra/prow/cmd/tide).
+
+### Submit Queue


We don't need this section anymore since the SQ is gone now and the links are dead.

cjwagner · 2018-09-25T20:58:15Z

contributors/guide/owners.md

+batch as it can (“tide comes in”), and merges them (“tide goes out”).
+
+- [Blunderbuss plugin](https://git.k8s.io/test-infra/prow/plugins/blunderbuss):
+  - responsible for determining **reviewers** and assigning to them


Blunderbuss requests reviews. It does not assign.

cjwagner · 2018-09-25T20:59:10Z

contributors/guide/owners.md

+- [Blunderbuss plugin](https://git.k8s.io/test-infra/prow/plugins/blunderbuss):
+  - responsible for determining **reviewers** and assigning to them
+- [Tide](https://git.k8s.io/test-infra/prow/cmd/tide):
+  - responsible for merging PRs
  - responsible for updating a GitHub status check explaining why a PR can't be merged (eg: a


The other important thing that Tide does is trigger batch tests and retrigger stale PR tests.

cjwagner · 2018-09-25T21:00:40Z

contributors/guide/pull-requests.md

@@ -72,7 +72,7 @@ Here's the process the pull request goes through on its way from submission to m

 1. If you're **not** a member of the Kubernetes organization, a Reviewer/Kubernetes Member checks that the pull request is safe to test. If so, they comment `/ok-to-test`. Pull requests by Kubernetes organization [members](/community-membership.md) do not need this step. Now the pull request is considered to be trusted, and the pre-submit tests will run:

-    1. Automatic tests run. See the current list of tests on the [MERGE REQUIREMENTS tab, at this link](https://submit-queue.k8s.io/#/info)
+    1. Automatic tests run. See the current list of tests on the [MERGE REQUIREMENTS tab, at this link](https://prow.k8s.io/?repo=kubernetes%2Fkubernetes&type=presubmit)


There is no MERGE REQUIREMENTS tab at this link.

cjwagner · 2018-09-25T21:01:24Z

contributors/guide/pull-requests.md

@@ -82,24 +82,26 @@ Here's the process the pull request goes through on its way from submission to m
 1. (Optional) Some reviewers prefer that you squash commits at this step
 1. Follow the bot suggestions to assign an OWNER who will add the `/approve` label to the pull request

-Once the tests pass, all failures are commented as flakes, or the reviewer adds the labels `/lgtm` and `/approved`, the pull request enters the final merge queue. The merge queue is needed to make sure no incompatible changes have been introduced by other pull requests since the tests were last run on your pull request.
+Once the tests pass, all failures are commented as flakes, or the reviewer adds the labels `/lgtm` and `/approved`, the pull request enters the final merge pool. The merge pool is needed to make sure no incompatible changes have been introduced by other pull requests since the tests were last run on your pull request.


all failures are commented as flakes

What does this mean?

What does this mean?

Not sure, it was part of the old docs. I've updated it. Ptal.

cjwagner

Thanks for making the changes. I missed a couple things on my last review though.

cjwagner · 2018-09-27T18:01:16Z

contributors/devel/automation.md

-The status of the submit-queue is [online.](http://submit-queue.k8s.io/)
-
-### Ready to merge status
+#### Ready to merge status

 A PR is considered "ready for merging" by the submit queue if it matches the set
 of conditions listed in the [merge requirements tab](http://submit-queue.k8s.io/#/info)


Update this link to point to the tide dashboard. https://prow.k8s.io/tide

cjwagner · 2018-09-27T18:03:31Z

contributors/guide/contributor-cheatsheet.md

@@ -15,7 +15,7 @@ A list of common resources when contributing to Kubernetes.
 ## Workflow

 - [Gubernator Dashboard - k8s.reviews](https://k8s-gubernator.appspot.com/pr)
- [Submit Queue](https://submit-queue.k8s.io)
+- [Tide](https://prow.k8s.io/tide)


Please also remove reference to the cherrypick queue below.

nikhita · 2018-10-06T17:46:25Z

I've created an issue #2759 for a follow-up update to the e2e tests doc.

spiffxp · 2018-10-08T17:16:32Z

/approve
agree with the followup, will leave /lgtm for @cjwagner

k8s-ci-robot · 2018-10-08T17:16:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: spiffxp

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~contributors/devel/OWNERS~~ [spiffxp]
~~contributors/guide/OWNERS~~ [spiffxp]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

cjwagner

/lgtm

k8s-ci-robot requested review from BenTheElder, cblecker, guineveresaenger and spiffxp September 23, 2018 10:39

nikhita commented Sep 23, 2018

View reviewed changes

k8s-ci-robot assigned cjwagner Sep 24, 2018

BenTheElder reviewed Sep 24, 2018

View reviewed changes

spiffxp reviewed Sep 24, 2018

View reviewed changes

nikhita force-pushed the replace-submit-queue-with-tide branch from 6508dcc to d00356a Compare September 25, 2018 07:29

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 25, 2018

cjwagner reviewed Sep 25, 2018

View reviewed changes

nikhita force-pushed the replace-submit-queue-with-tide branch from d00356a to 30a07da Compare September 27, 2018 14:54

cjwagner reviewed Sep 27, 2018

View reviewed changes

Replace submit queue with Tide in docs

c2bd5cb

nikhita force-pushed the replace-submit-queue-with-tide branch from 30a07da to c2bd5cb Compare October 6, 2018 17:38

nikhita mentioned this pull request Oct 6, 2018

Update e2e testing guide #2759

Open

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 8, 2018

cjwagner approved these changes Oct 8, 2018

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 8, 2018

k8s-ci-robot merged commit f1e2ffa into kubernetes:master Oct 8, 2018

nikhita deleted the replace-submit-queue-with-tide branch October 9, 2018 05:31

		@@ -48,6 +51,10 @@ If these tests pass a second time, the PR will be merged when this PR finishes r

		## Github Munger

Replace submit queue with Tide in docs #2701

Replace submit queue with Tide in docs #2701

Conversation

nikhita commented Sep 23, 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

BenTheElder commented Sep 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spiffxp left a 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

nikhita commented Sep 25, 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

cjwagner left a 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

nikhita commented Oct 6, 2018

spiffxp commented Oct 8, 2018

k8s-ci-robot commented Oct 8, 2018

cjwagner left a comment

Choose a reason for hiding this comment