roachtest: de-flake decommission/mixed-versions

[tbg]

The test was fully decommissioning and then recommissioning nodes, which
does no longer work now that the master branch is 21.1 and thus the
predecessor version in this test is 20.2, which does know about fully
decommissioning (decommission attempts run via 20.1 nodes never set
the status to fully decommissioned, so we got away with it so far).

Pare down the test so that it carries out one random partial and full
decommission cycle alongside a partial version upgrade, which is
reasonably all we can do unless we want to get into the business
of wiping and re-adding nodes to the cluster in this test, which
I don't think we do as it doesn't work well with the step-driven
format this test is using.

Fixes #55798.

Release note: None

[cockroach-teamcity]

This change is 

[tbg]

Still issues here, btw. When we're decommissioning a node through itself, it's possible that the cluster will kick the node out before it manages to convince itself it's good to go. We need to terminate this through the RPC layer, which in turn motivated me to work on #54939.

[andreimatei]

🐶

When we're decommissioning a node through itself, it's possible that the cluster will kick the node out before it manages to convince itself it's good to go.

What does this mean? Why does it matter what node we're decommissioning "through"? That's just about what machine we ssh to run the decommission command on, right?

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @andreimatei, @irfansharif, and @tbg)

---

pkg/cmd/roachtest/mixed_version_decommission.go, line 62 at r1 (raw file):

		// Partially decommission a random node from another random node. We
		// use the predecessor CLI to do so.
		partialDecommissionStep(h.getRandNode(), h.getRandNode(), predecessorVersion),

I think this "from another node" part is confusing. Wouldn't it be much clearer if we always ran the cockroach node decommission commands locally (on the test runner machine)? Even if that means downloading the predecessor version locally.

[tbg]

Yes, but what matters more is which node the decommission process talks to.

Here, the decommission process is on n1:
client ---gRPC---> n1.Decommission(n1)

you could also decommission n1 through n2, in which case n2 would be orchestrating the process (i.e. scanning the metas, marking liveness as decommissioning, etc):

client ---gRPC--> n2.Decommission(n1)

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @andreimatei and @irfansharif)

---

pkg/cmd/roachtest/mixed_version_decommission.go, line 62 at r1 (raw file):

Previously, andreimatei (Andrei Matei) wrote…

I think this "from another node" part is confusing. Wouldn't it be much clearer if we always ran the cockroach node decommission commands locally (on the test runner machine)? Even if that means downloading the predecessor version locally.

While we could do this for the predecessor version (since it is always downloaded) in general we try to stay away from this pattern since the local arch can and often is different from the other, plus there would be a need to manually construct a more complicated command line (host, port , etc). When run on node X, the cli goes through node X by default, which is convenient.

[tbg]

bors r=andreimatei

I'm avoiding the self-decommission in this test. Instead, I filed an issue and added a skipped acceptance test that only decommissions self. Will look into that separately.

[craig]

Build failed:

• GitHub CI (Cockroach)

[tbg]

bors r=andreimatei

forgot bazel update

[craig]

Build succeeded:

• GitHub CI (Cockroach)