Opened 3 months ago

Last modified 3 days ago

#29280 needs_revision task

Use Chutney in Tor's CI

Reported by: cohosh Owned by:
Priority: High Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: CI, PTs 029-backport 034-backport 035-backport 040-backport, network-team-roadmap-2019-Q1Q2
Cc: cohosh Actual Points: 1
Parent ID: #29267 Points: 1
Reviewer: teor Sponsor: Sponsor19

Description

Get Chutney working for continuous integration. This will also help with Tor in general and so has broader benefits than just CI for pluggable transports.

Child Tickets

TicketStatusOwnerSummaryComponent
#28013closedcatalystrun test-network-all in Travis CICore Tor/Tor
#29490newChutney fails (sometimes?) when tor is built with --enable-coverageCore Tor/Tor
#29491newChutney fails when Tor is built with --enable-nssCore Tor/Tor
#29703closedteorBackport test-network.sh fixes to 0.2.9Core Tor/Tor
#29748closednickmLet test-network.sh tolerate some number of failuresCore Tor/Chutney
#30049newWork out how to test mixed-version chutney networks in Tor's CICore Tor/Tor

Change History (17)

comment:1 Changed 2 months ago by nickm

Component: ObfuscationCore Tor/Tor
Milestone: Tor: 0.4.1.x-final

comment:2 Changed 7 weeks ago by nickm

Actual Points: 1
Keywords: 029-backport 034-backport 035-backport 040-backport added
Status: newneeds_review

Please see the branches below. They add a new make target, "make test-network-forgiving" that tries to run test-network.sh a few times, and succeeds if it succeeds once. They add a builder to do this on travis.

All of them passed on my first attempt when I pushed them to github myself, so let's see whether CI likes them a second time.

comment:3 Changed 7 weeks ago by nickm

Reviewer: teor

comment:4 Changed 6 weeks ago by gaba

Keywords: network-team-roadmap-2019-Q1Q2 added

comment:5 Changed 6 weeks ago by teor

Status: needs_reviewneeds_revision

I (accidentally) did a review on the 0.3.5 pull request:
https://github.com/torproject/tor/pull/736

In general, the code looks good. I have one design question, and one code question.

comment:6 Changed 6 weeks ago by teor

I am not sure if we need to fix #29689 as part of this ticket.

It seems like it might be annoying if someone uses tor/src/test/test-network.sh from an unusual path, or with an unusual directory structure.

comment:7 Changed 6 weeks ago by nickm

I think we can do #29689 separately, as it's logically independent of this?

comment:8 Changed 6 weeks ago by nickm

Status: needs_revisionneeds_review

Hi! Answered your questions on the tickets; please let me know which way you think I should proceed.

comment:9 in reply to:  7 Changed 6 weeks ago by teor

Replying to nickm:

I think we can do #29689 separately, as it's logically independent of this?

I think you're right, let's do it some other time.

I'll put it on my fast-fix backlog.

comment:10 in reply to:  8 ; Changed 6 weeks ago by teor

Status: needs_reviewneeds_revision

Replying to nickm:

Hi! Answered your questions on the tickets; please let me know which way you think I should proceed.

I think we should add new chutney-related code to the chutney repository: that's been our policy since 0.2.9. And in this particular case, we will need test-network-forgiving.sh to have useful CI for the chutney repository.

There's a longer explanation on the pull request.

I opened #29703 to backport some test-network.sh fixes from 0.3.0 to 0.2.9 so that we can reliably call chutney's test-network.sh from tor's test-network.sh.

It's also worth being aware of #29060: I think one of its commits may break tor's chutney CI on master.

comment:11 in reply to:  10 Changed 6 weeks ago by teor

Replying to teor:

I opened #29703 to backport some test-network.sh fixes from 0.3.0 to 0.2.9 so that we can reliably call chutney's test-network.sh from tor's test-network.sh.

Hmm, there's another option if you don't want to backport:

  1. put test-network-forgiving.sh in 0.2.9, and call it from the 0.2.9 Makefile
  2. delete test-network-forgiving.sh in 0.3.4 and later
  3. implement a test-network-forgiving mode in chutney's test-network.sh, and call it from 0.3.4 and later Makefiles

This seems like the worst of both worlds: we'll be making changes in 0.2.9 and chutney if there are any bugs. Historically, we're not good at keeping tor and chutney in sync.

It's also worth being aware of #29060: I think one of its commits may break tor's chutney CI on master.

I've asked for a revert on that ticket.

comment:12 Changed 4 weeks ago by teor

nickm, #29703 is ready for you to rebase onto.

comment:13 Changed 4 weeks ago by nickm

Status: needs_revisionneeds_review

I've made revised branches:

comment:14 Changed 2 weeks ago by teor

Summary: Use Chutney for CIUse Chutney in Tor's CI

comment:15 Changed 6 days ago by teor

I've just finished #29729, and discovered a lot of interesting things about chutney, tor, and Travis in the process.

I'll have some feedback on these pull requests tomorrow (~18 hours).

comment:16 Changed 3 days ago by teor

Points: 1
Status: needs_reviewneeds_revision

From a commit message:

Previously we had "make check" launched whenever DISTCHECK was
false. Now we'd like to turn it off in a few other circumstances,
like running chutney. Maybe stem too?

Yes, let's turn off "make check" for stem.

The default network in chutney is bridges+hs-v2, but I discovered in #29729 that it is really unstable in chutney's CI. (Sometimes all the jobs work, and sometimes multiple jobs fail. I think it might be due to network or CPU load issues on machines.)

So let's change to a subset of the CI tests that work in chutney?

So for 0.2.9, let's use:

  • bridges-min
  • hs-v2-min

And for 0.3.4 and later, let's add:

  • hs-v3-min

Each test should only take 30-60 seconds, so I think it's worth running these 3 tests. (That's about 2 extra minutes per branch, or an 18 extra minutes for a full merge-forward from 0.2.9.)

Here's the full list of all the tests in chutney from #29729:
https://github.com/torproject/chutney/pull/25/files#diff-354f30a63fb0907d4ad57269548329e3R29

We're missing:

  • basic-min
  • single-onion-v23
  • and all the IPv6 tests
    • We can only run IPv6 tests in macOS Travis, because Linux Travis doesn't support IPv6.

I opened #30212 to add them later.

Then let's let CI run for a while to evaluate the chutney tests.

If we think chutney is too unstable in Tor's Chutney job, or Chutney's CI, we could:

  • run chutney in offline mode (#30183), or
  • add a sleep after failure in test-network --forgiving (#30210).

comment:17 Changed 3 days ago by teor

Oops, I forgot to review 0.3.4 and later:

I am not so sure about this part of the merge commit to 0.3.4:

++    - env: DISTCHECK="yes" RUST_OPTIONS="--enable-rust --enable-cargo-online-mode" HARDENING_OPTIONS="" SKIP_MAKE_CHECK="yes"

I don't think we should skip make check on the only online rust build. We could move online to the dirauth build, and skip make check on distcheck?

Also, in future, let's try to do semantic changes like this in a separate commit?
https://trac.torproject.org/projects/tor/wiki/org/teams/NetworkTeam/MergePolicy?action=diff&version=11

In 0.3.5, you dropped the SKIP_MAKE_CHECK you added in 0.3.4:

 -    ## But without hardening (see above)
 -    - env: DISTCHECK="yes" RUST_OPTIONS="--enable-rust --enable-cargo-online-mode" HARDENING_OPTIONS="" SKIP_MAKE_CHECK="yes"
 +    - env: DISTCHECK="yes" RUST_OPTIONS="--enable-rust --enable-cargo-online-mode"

We should also re-run all these branches with the latest chutney.

Note: See TracTickets for help on using tickets.