Prepare `main` to become new default branch by thanethomson · Pull Request #9095 · tendermint/tendermint

thanethomson · 2022-07-26T12:11:34Z

This PR aims to sync the entire .github folder in main with that in master, updating the configuration for the new repo strategy.

One minor update to the Makefile was necessary in order to keep the tests split into groups, and some updates to some YAML files were necessary to get the linter to pass.

While I was at it, I've now updated the various issue/PR templates to better reflect our current workflow. Please let me know whether those updates seem reasonable. Some of the main changes there:

Deleted .github/auto-comment.yml - doesn't seem like it was being used
.github/ISSUE_TEMPLATE/{proposal,feature-request}.md - removed the #### For Admin Use section. Doesn't look like we really pay attention to it?
.github/ISSUE_TEMPLATE/config.yml - hopefully I've configured this correctly (as per the GitHub docs).
.github/ISSUE_TEMPLATE/*.md - updated the instructions in the comments to redirect users to the discussions section of the repo if they want to ask questions instead of logging issues.

.github/workflows/release.yml

.github/workflows/e2e-manual.yml

.github/workflows/fuzz-nightly.yml

tychoish · 2022-07-26T12:26:23Z

.github/workflows/fuzz-nightly.yml

          name: suppressions
          path: test/fuzz/**/suppressions
-          retention-days: 1
+          retention-days: 3


I don't know that there's any benefit in retaining these results, unless we have a plan to use them

What was the reasoning in having this in the first place?

I think it makes sense to keep the crashers for 3 days -- if we are notified on Friday afternoon, we have an opportunity to review on Monday.

But off the top of my head, I don't know what the suppressions are, so I can't speak to why they should or should not be archived.

I'm not sure. As far as I know the fuzz tests are basically useless, (e.g. they've never caught a bug in the last 18 months, as far as I know, aside from sometimes unexpectedly breaking compile, in silly but legitimate ways.)

.github/workflows/jepsen.yml

tychoish · 2022-07-26T12:31:48Z

.github/workflows/tests.yml

+            Makefile
+      - name: Run Go Tests
+        run: |
+          make test-group-${{ matrix.part }} NUM_SPLIT=6


we should make sure that the number of splits here (6) are the right number of splits given the runtime of the tests and overhead.

How do we do that?

Look at the runtime of the tests in the patch, and make sure that they're more or less balanced, I think. It's reasonably impressionistic. The way this all works is that it takes a sorted list of all of the packages that have tests and splits them, but number of tests and test runtimes in CI don't split along the same lines, so you could have one group that takes 10 minutes and 5 that take 2, and in that case, you don't actually get any responsiveness by having more splits, because the long poll is in one test.

We always used to have 4 splits, and we upped it to 6 when the tests were more flaky because it meant when you restarted the tests (to be able to merge something) you'd be able to see the failure more easily and potentially reduce the chance that more tests would flake on the restart.

for i in {0..5}; do /usr/bin/time -f "Test 0$i: %E" make test-group-0$i NUM_SPLIT=6; done Test 00: 0:23.59 Test 01: 0:08.11 Test 02: 0:04.93 Test 03: 2:00.23 Test 04: 0:18.79 Test 05: 0:41.22

I think this seems reasonable?

.github/CODEOWNERS

creachadair · 2022-07-27T19:27:05Z

rpc/openapi/openapi.yaml

@@ -537,8 +537,7 @@ paths:
            type: array


Maybe this is fine, but are these changes actually in the v0.34 line? This didn't seem to be related to the CI parts of the change offhand.

These fixes are just to get the linter to pass. I also went and checked the response schemas against the structs on the main branch and it seems there were a few minor inconsistencies there, so I fixed the schema where it was incorrect.

creachadair · 2022-07-27T19:29:03Z

.github/workflows/tests.yml

-          key: ${{ runner.os }}-${{ github.sha }}-tm-binary
-        if: env.GIT_DIFF
-
-  test_abci_apps:


Are these really no longer relevant?

It seems as though these were removed in #6684. I'll revert this for now until we re-apply that patch.

Actually, these tests were moved to the build workflow.

mark-rushakoff

Overall LGTM, but I'll leave approval to those more familiar with the previous CI implementations. Left a few comments on things that stood out to me.

.github/PULL_REQUEST_TEMPLATE.md

.github/linters/markdownlint.yml

.github/workflows/build.yml

mark-rushakoff · 2022-07-27T18:20:28Z

.github/workflows/fuzz-nightly.yml

      - name: Install go-fuzz
        working-directory: test/fuzz
-        run: go get -u github.com/dvyukov/go-fuzz/go-fuzz github.com/dvyukov/go-fuzz/go-fuzz-build
+        run: go install github.com/dvyukov/go-fuzz/go-fuzz@latest github.com/dvyukov/go-fuzz/go-fuzz-build@latest


Fuzzing is built in to go1.18, so we should prefer the tooling built into the toolchain. I think that is fine to wait for a later PR, though.

I think this was implemented later on toward v0.35/v0.36. We can look at porting that work at some point.

mark-rushakoff · 2022-07-27T18:21:52Z

.github/workflows/fuzz-nightly.yml

          name: suppressions
          path: test/fuzz/**/suppressions
-          retention-days: 1
+          retention-days: 3


I think it makes sense to keep the crashers for 3 days -- if we are notified on Friday afternoon, we have an opportunity to review on Monday.

But off the top of my head, I don't know what the suppressions are, so I can't speak to why they should or should not be archived.

mark-rushakoff · 2022-07-27T18:23:55Z

.github/workflows/lint.yml

+name: Golang Linter
+# Lint runs golangci-lint over the entire Tendermint repository.
+#
+# This workflow is run on every pull request and push to main.


It would be nice to mention how to run this locally. (I think it's just make lint?)

Yeah, it's just make lint. I've now added this to the end of the comment.

tychoish

I think the deletion of the minnet-kubernetes directory is probably fine, but I'm not sure it's in scope for the rest of this PR

tychoish · 2022-07-29T14:34:28Z

.github/CODEOWNERS


-/spec @cason @sergio-mena @jmalicevic @milosevic
+# Spec related changes can be approved by the protocol design team
+/spec   @josef-widder @milosevic @cason @sergio-mena @jmalicevic


it's potentially a bit confusing to have duplication between this and the full team.

tychoish · 2022-07-29T14:48:36Z

.github/workflows/e2e-nightly-main.yml

        working-directory: test/e2e
        run: ./run-multiple.sh networks/nightly/*-group${{ matrix.group }}-*.toml

  e2e-nightly-fail-2:


should the -2 here also drop?

Not sure. This is how it is on master:

tendermint/.github/workflows/e2e-nightly-master.yml

Line 43 in d433ebe

e2e-nightly-fail-2:

Removed it. It seems redundant.

tychoish · 2022-07-29T14:50:58Z

.golangci.yml

+    # - lll
    # - maligned
-    # - misspell
+    - misspell


I kind of think that we shouldn't change the linter config on the branch.

I'm not changing it, I'm just sync'ing with master. See

tendermint/.golangci.yml

Line 29 in d433ebe

- misspell

tychoish · 2022-07-29T14:51:35Z

rpc/openapi/openapi.yaml

          type: string
          example: "Dialing seeds in progress. See /net_info for details"

+    BlockSearchResponse:


I don't think we should add this without the corresponding implementation.

It's already on main (i.e. in v0.34):

tendermint/rpc/core/types/responses.go

Line 209 in 4309f54

type ResultBlockSearch struct {

While fixing the linting issues in the OpenAPI config, I discovered that there were a bunch of things that were missing/incorrect. So I've fixed them here to match the code on main.

See

tendermint/rpc/openapi/openapi.yaml

Line 1067 in 4309f54

$ref: "#/components/schemas/BlockSearchResponse"

on main for example, where there's no definition of BlockSearchResponse in the OpenAPI config.

thanethomson · 2022-07-29T14:57:19Z

I think the deletion of the minnet-kubernetes directory is probably fine, but I'm not sure it's in scope for the rest of this PR

To get the YAML linting to pass I either had to fix all of the k8s scripts, or just delete them, so I deleted them (since this is what happened to them on master).

.github/ISSUE_TEMPLATE/bug-report.md

.github/PULL_REQUEST_TEMPLATE.md

.github/codecov.yml

.github/workflows/build.yml

.github/workflows/fuzz-nightly.yml

.github/workflows/lint.yml

.github/workflows/release.yml

.github/workflows/tests.yml

.golangci.yml

Remove mintnet as discussed on team call. closes #1941

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Update Makefile with changes from #7372 Signed-off-by: Thane Thomson <connect@thanethomson.com> * Sync main GitHub config with master and update Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove unnecesary dot folders Signed-off-by: Thane Thomson <connect@thanethomson.com> * Sync dotfiles Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove unused Jepsen tests for now Signed-off-by: Thane Thomson <connect@thanethomson.com> * tools: remove k8s (#6625) Remove mintnet as discussed on team call. closes #1941 * Restore nightly fuzz testing of P2P addrbook and pex Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix YAML lints Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix YAML formatting nits Signed-off-by: Thane Thomson <connect@thanethomson.com> * More YAML nits Signed-off-by: Thane Thomson <connect@thanethomson.com> * github: fix linter configuration errors and occluded errors (#6400) * Minor fixes to OpenAPI spec to sync with structs on main Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove .github/auto-comment.yml - does not appear to be used Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add issue config with link to discussions Signed-off-by: Thane Thomson <connect@thanethomson.com> * Adjust issue/PR templates to suit current process Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove unused RC branch config from release workflow Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix wildcard matching in build jobs config Signed-off-by: Thane Thomson <connect@thanethomson.com> * Document markdownlint config Signed-off-by: Thane Thomson <connect@thanethomson.com> * Restore manual E2E test group config Signed-off-by: Thane Thomson <connect@thanethomson.com> * Document linter workflow with local execution instructions Signed-off-by: Thane Thomson <connect@thanethomson.com> * Document and fix minor nit in Super-Linter markdownlint config Signed-off-by: Thane Thomson <connect@thanethomson.com> * Update .github/ISSUE_TEMPLATE/bug-report.md Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> * Update pull request template to add language around discussions/issues Signed-off-by: Thane Thomson <connect@thanethomson.com> * .golangci.yml: Deleted commented-out lines Signed-off-by: Thane Thomson <connect@thanethomson.com> * ci: Drop "-2" from e2e-nightly-fail workflow Signed-off-by: Thane Thomson <connect@thanethomson.com> * Address triviality concern in PR template Signed-off-by: Thane Thomson <connect@thanethomson.com> Co-authored-by: Marko <marbar3778@yahoo.com> Co-authored-by: Sam Kleinman <garen@tychoish.com> Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

…#9157) * Update CODEOWNERS to use teams (#9129) * Update CODEOWNERS to use teams Update the `CODEOWNERS` file to use the @tendermint/tendermint-engineering and @tendermint/tendermint-research teams as opposed to adding people one by one. This makes repository administration somewhat easier to manage, especially when onboarding/offboarding people. Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add Ethan as superuser Signed-off-by: Thane Thomson <connect@thanethomson.com> * Prepare `main` to become new default branch (#9095) * Update Makefile with changes from #7372 Signed-off-by: Thane Thomson <connect@thanethomson.com> * Sync main GitHub config with master and update Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove unnecesary dot folders Signed-off-by: Thane Thomson <connect@thanethomson.com> * Sync dotfiles Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove unused Jepsen tests for now Signed-off-by: Thane Thomson <connect@thanethomson.com> * tools: remove k8s (#6625) Remove mintnet as discussed on team call. closes #1941 * Restore nightly fuzz testing of P2P addrbook and pex Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix YAML lints Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix YAML formatting nits Signed-off-by: Thane Thomson <connect@thanethomson.com> * More YAML nits Signed-off-by: Thane Thomson <connect@thanethomson.com> * github: fix linter configuration errors and occluded errors (#6400) * Minor fixes to OpenAPI spec to sync with structs on main Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove .github/auto-comment.yml - does not appear to be used Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add issue config with link to discussions Signed-off-by: Thane Thomson <connect@thanethomson.com> * Adjust issue/PR templates to suit current process Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove unused RC branch config from release workflow Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix wildcard matching in build jobs config Signed-off-by: Thane Thomson <connect@thanethomson.com> * Document markdownlint config Signed-off-by: Thane Thomson <connect@thanethomson.com> * Restore manual E2E test group config Signed-off-by: Thane Thomson <connect@thanethomson.com> * Document linter workflow with local execution instructions Signed-off-by: Thane Thomson <connect@thanethomson.com> * Document and fix minor nit in Super-Linter markdownlint config Signed-off-by: Thane Thomson <connect@thanethomson.com> * Update .github/ISSUE_TEMPLATE/bug-report.md Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> * Update pull request template to add language around discussions/issues Signed-off-by: Thane Thomson <connect@thanethomson.com> * .golangci.yml: Deleted commented-out lines Signed-off-by: Thane Thomson <connect@thanethomson.com> * ci: Drop "-2" from e2e-nightly-fail workflow Signed-off-by: Thane Thomson <connect@thanethomson.com> * Address triviality concern in PR template Signed-off-by: Thane Thomson <connect@thanethomson.com> Co-authored-by: Marko <marbar3778@yahoo.com> Co-authored-by: Sam Kleinman <garen@tychoish.com> Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> Co-authored-by: Thane Thomson <connect@thanethomson.com> Co-authored-by: Marko <marbar3778@yahoo.com> Co-authored-by: Sam Kleinman <garen@tychoish.com> Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

thanethomson commented Jul 26, 2022

View reviewed changes

.github/workflows/release.yml Outdated Show resolved Hide resolved

thanethomson mentioned this pull request Jul 26, 2022

Migrate master to main #9096

Closed

12 tasks

tychoish reviewed Jul 26, 2022

View reviewed changes

mark-rushakoff mentioned this pull request Jul 26, 2022

abci/tests: stop abci server started in addr test #9093

Merged

thanethomson force-pushed the thane/main-ci branch 3 times, most recently from 463bcf9 to ca63b83 Compare July 27, 2022 14:26

thanethomson marked this pull request as ready for review July 27, 2022 14:31

thanethomson requested review from cason, cmwaters, creachadair, ebuchman, jmalicevic, milosevic, sergio-mena and williambanfield as code owners July 27, 2022 14:31

creachadair reviewed Jul 27, 2022

View reviewed changes

.github/CODEOWNERS Outdated Show resolved Hide resolved

creachadair reviewed Jul 27, 2022

View reviewed changes

mark-rushakoff reviewed Jul 27, 2022

View reviewed changes

tychoish assigned thanethomson Jul 28, 2022

thanethomson force-pushed the thane/main-ci branch from 0194d9a to 8b6d482 Compare July 28, 2022 17:01

thanethomson marked this pull request as draft July 28, 2022 19:11

thanethomson marked this pull request as ready for review July 28, 2022 20:48

thanethomson changed the title ~~ci: Update main CI config to resemble master's~~ Prepare main to become new default branch Jul 29, 2022

tychoish reviewed Jul 29, 2022

View reviewed changes

williambanfield suggested changes Jul 29, 2022

View reviewed changes

thanethomson mentioned this pull request Jul 29, 2022

ci: Should we remove the code coverage workflow? #9128

Closed

williambanfield approved these changes Jul 29, 2022

View reviewed changes

tychoish approved these changes Jul 29, 2022

View reviewed changes

tac0turtle and others added 22 commits July 29, 2022 15:13

tools: remove k8s (#6625)

7a10909

Remove mintnet as discussed on team call. closes #1941

Restore nightly fuzz testing of P2P addrbook and pex

78e5592

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Fix YAML lints

c393e3c

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Fix YAML formatting nits

c068bfc

Signed-off-by: Thane Thomson <connect@thanethomson.com>

More YAML nits

d3d0d11

Signed-off-by: Thane Thomson <connect@thanethomson.com>

github: fix linter configuration errors and occluded errors (#6400)

639c293

Minor fixes to OpenAPI spec to sync with structs on main

c2571be

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Remove .github/auto-comment.yml - does not appear to be used

fbb2127

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Add issue config with link to discussions

ac40597

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Adjust issue/PR templates to suit current process

bb266e9

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Remove unused RC branch config from release workflow

04f98ec

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Fix wildcard matching in build jobs config

01676e3

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Document markdownlint config

99d1495

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Restore manual E2E test group config

daa3a67

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Document linter workflow with local execution instructions

a7708be

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Document and fix minor nit in Super-Linter markdownlint config

6c2d88e

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Update .github/ISSUE_TEMPLATE/bug-report.md

99d71d7

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

Apply suggestions from code review

5aab6c0

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

Update pull request template to add language around discussions/issues

db90b00

Signed-off-by: Thane Thomson <connect@thanethomson.com>

.golangci.yml: Deleted commented-out lines

815c9c5

Signed-off-by: Thane Thomson <connect@thanethomson.com>

ci: Drop "-2" from e2e-nightly-fail workflow

7c6e69c

Signed-off-by: Thane Thomson <connect@thanethomson.com>

Address triviality concern in PR template

e51a78a

Signed-off-by: Thane Thomson <connect@thanethomson.com>

thanethomson force-pushed the thane/main-ci branch from fd719d1 to e51a78a Compare July 29, 2022 19:15

thanethomson requested review from a team July 29, 2022 19:15

thanethomson merged commit 85636f6 into main Jul 29, 2022

thanethomson deleted the thane/main-ci branch July 29, 2022 19:20

sergio-mena mentioned this pull request Aug 2, 2022

Update feature/abci++ppp branch with latest infra changes in main #9157

Merged

3 tasks

Conversation

thanethomson commented Jul 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

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!

mark-rushakoff left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

tychoish 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!

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!

thanethomson commented Jul 29, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thanethomson commented Jul 26, 2022 •

edited

Loading