[squid-dev] Strategy about build farm nodes

[Alex Rousskov]

On 4/28/21 1:45 AM, Francesco Chemolli wrote:

>   I'm moving here the discussion from PR #806 about what strategy to
> have for CI tests, looking for an agreement.

> We have 3 classes of tests ni our CI farm
> (https://build.squid-cache.org/)

> - PR staging tests, triggered by commit hooks on GitHub (possibly with
> human approval)

>    the job is 5-pr-auto (misnamed, it tests trunk with the PR applied).

FYI: The "auto" branch is "master branch + PR". When PR is merged,
master becomes identical to the auto branch. The name "auto" is fairly
common in CI AFAICT. In Anubis (and relatively widespread/common)
terminology[1], the words "staged" and "staging" mean what you describe
as "trunk with the PR applied" below.

[1]
https://github.com/measurement-factory/anubis/blob/master/README.md#pull-request-merging-algorithm

AFAICT, this class should _follow_ the 5-pr-test class (described below)
because a PR is not staged until it is approved and passes 5-pr-test checks.


>    To be successful, it needs to successfully compile and unit-test all
> combinations of clang and gcc on the latest stable version of the most
> popular linux distros, at this time centos-8, debian-testing,
> fedora-33, opensuse-leap, ubuntu-focal


> - PR build tests run , after a PR is approved, also triggered by GitHub
>   the job is 5-pr-test

AFAICT, this job tests the "GitHub-generated merge commit for the PR
branch". That commit may become stale if master advances while there
were no PR branch commits. Unlike the GitHub-generated merge commit, the
"auto" branch (mentioned above) is kept in sync with master by Anubis.

We do not merge the GitHub-generated merge commit into master.

If 5-pr-test requires a PR approval, then something probably went wrong
somewhere. Testing PR branch or the GitHub-generated merge commit should
not require a PR approval. Perhaps you meant the "OK to test" _testing_
approval. That testing approval is indeed required and expected for
testing GitHub-generated merge commits on vulnerable Jenkins.


>   To be successful, it needs to compile and unit-test all combinations
> of clang and gcc on LTS and most recent versions of most popular linux
> distros, at this time: centos-7,
> debian-stable, debian-unstable, fedora-32, fedora-34, ubuntu-bionic, ubuntu-hirsute

> - full-scale build tests (jobs: trunk-matrix, trunk-arm32-matrix,
> trunk-arm64-matrix, trunk-freebsd13-clang-matrix, trunk-openbsd-matrix)
>   these test everything we can test, including bleeding edge distros
> such as getntoo, rawhide, tumbleweed, and the latest compilers. 

Please clarify what branches/commits these "full-scale build tests" test.

Ideally, all this should be documented somewhere on Squid wiki!


> Failing
> these will not block a PR from mergeing, but there is an expectation
> that build regressions will be fixed

If we expect a fix, then the failure has to block something important,
like the responsible PR (ideally), the next numbered release, or all
PRs. What does this failure block today, if anything?


> The change in policy since last week

There was no change in any Project policies AFAIK. You have changed
Jenkins setup, but that is not a change in policy. It is likely that
there is no policy at all right now -- just ad hoc decisions by
sysadmins. It would be a stretch to call that current approach a
"policy", but even if we do call it that, then that "policy" has not
"changed" :-).

Alternatively, Jenkins configuration changes have violated a Project
policy (one cannot _change_ a Project policy unilaterally; one can only
violate it). I prefer the "no policy" version above though :-).


> is that the PR-blocking builds used
> to depend on unstabledistros (fedora-rawhide and opensuse-tumbleweed),
> I've removed that today as part of the discussion on PR #806
> This policy would allow keeping stable distros uptodate and matching
> expected user experience, while not introducing instability that would
> come in from the unstable distros
> One distro that's notably missing is centos-stream, this is due to
> technical issues with getting a good docker image for it, when available
> I'll add it

> Would this work as a general policy?

I am not sure what exact general policy you are proposing -- please
extract it from the lengthy prose above (removing the ever-changing
low-level specifics to the extent possible, to arrive at something truly
"general"). FWIW, if the fleshed out proposal violates the existing "do
not break master" principle below, then I do not think it will work
well. I suspect that a focus on vague optimization targets like
"expected user experience" will not allow us to formulate a usable
policy we can all agree on, but I would be happy to be proven wrong.


I can propose the following. Perhaps knowing my state of mind will help
you finalize your proposal.

We already have a "do not break master" principle:

* Master always passes all current development-related tests.


I propose to add the following principle:

* A test failure blocks the change responsible for that failure.


Direct consequences of that principle are:

* If the breaking change is a CI change (e.g., an OS upgrade on a
Jenkins test node), then the change must be reverted (until it becomes
non-blocking).

* If the breaking change is a code change, then the corresponding pull
request must be fixed before it is merged. We already apply this
principle (or are very close to faithfully applying it).


The above applies to what I called, for the lack of a better term,
"development-related" tests. These are the tests applied to staged
commits[1] today. Until Squid releases are automated and "always ready",
we should have "release-related" tests as well. It is not clear to me
whether such automated release-related tests exist today in our CI, but,
if they do exist, then the same principles can be applied to releases:

* A new release passes all contemporary release-related tests.
* A test failure blocks the change responsible for that failure.

Just like for development-related tests, a release-related test failure
would imply that either the tests have to be fixed or the release
candidate has to be rejected.

N.B. I am talking about releases separately from PRs primarily because I
suspect that some of the Jenkins tests may not be applicable to PRs but
are currently considered applicable to releases. For example, we are OK
with master build failures on OS Foo, but we still want all official
releases to build on that OS. If that is not the case today, then we do
not have to complicate things by separating development concerns from
release concerns!


HTH,

Alex.