this test is cool! hadn't looked at a version upgrade test before -- i like how you wrote this

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @RichardJCai)

pkg/cmd/roachtest/privilege_version_upgrade.go, line 30 at r1 (raw file):

		Name:       "versionupgrade/privileges",
		Owner:      OwnerSQLExperience,
		MinVersion: "v20.1.0",

oh hm, i thought MinVersion got removed recently? i might be mistaken

pkg/cmd/roachtest/privilege_version_upgrade.go, line 66 at r1 (raw file):

			expectedGrants = append(expectedGrants, []string{"test", "testuser", privilege})
		}
		r.CheckQueryResults(t, `SHOW GRANTS ON DATABASE test`, expectedGrants)

is the order of the results stable? might be worth adding an extra ORDER BY 1, 2, 3 to be sure. (same for the other SHOW GRANTS below

pkg/cmd/roachtest/privilege_version_upgrade.go, line 206 at r1 (raw file):

	// An empty string will lead to the cockroach binary specified by flag
	// `cockroach` to be used.
	const mainVersion = ""

nit: would this be better named currentVersion or something? also, why don't we want to use buildVersion here?

pkg/cmd/roachtest/privilege_version_upgrade.go, line 221 at r1 (raw file):

		steps = append(
			steps,
			createUserStep(1),

nit: since loadNode is always 1, could it be defined as a const rather than passed as a parameter?

pkg/cmd/roachtest/privilege_version_upgrade.go, line 273 at r1 (raw file):

			continue
		}
		if i%2 == 0 {

i thought for a second if this would be a good place to randomize, but i'm not sure. of course the downside is that it could get annoying to reproduce, but on the pro side, the test will always log the expected privileges it it fails and we might get more coverage... idk if it's worth it though

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)

pkg/cmd/roachtest/privilege_version_upgrade.go, line 30 at r1 (raw file):

nVer

Yeah you're right looks like its gone

pkg/cmd/roachtest/privilege_version_upgrade.go, line 66 at r1 (raw file):

The underlying SHOW GRANTS query actually has an ORDER BY 1,2,3

pkg/cmd/roachtest/privilege_version_upgrade.go, line 206 at r1 (raw file):

Yeah I think currentVersion makes sense. buildVersion probably also makes sense

pkg/cmd/roachtest/privilege_version_upgrade.go, line 221 at r1 (raw file):

Done

pkg/cmd/roachtest/privilege_version_upgrade.go, line 273 at r1 (raw file):

Yeah imo I don't think it's worth it to randomize.
I don't think randomizing will do much since we're just trying to make sure each privilege stays consistent after upgrading. Testing different combinations won't really expand coverage.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)

pkg/cmd/roachtest/privilege_version_upgrade.go, line 206 at r1 (raw file):

Nvm using buildVersion errors out, seems like it doesn't find the right binary name or the url doesn't match the buildVersion exactly.

		Wraps: (3) /Users/rcai/go/src/github.com/cockroachdb/cockroach/bin/roachprod stage richardc-1625856233-01-n3cpu4:3 release vv21.2.0-alpha.00000000 --dir='vv21.2.0-alpha.00000000' returned
		  | stderr:
		  | curl: (22) The requested URL returned error: 404 Not Found
		  |
		  | gzip: stdin: unexpected end of file
		  | tar: Child returned status 1
		  | tar: Error is not recoverable: exiting now
		  | Error: COMMAND_PROBLEM: exit status 2
		  | (1) COMMAND_PROBLEM
		  | Wraps: (2) Node 3. Command with error:
		  |   | ```
		  |   |
		  |   | tmpdir="$(mktemp -d /tmp/cockroach-release.XXX)" && \
		  |   | dir='vv21.2.0-alpha.00000000' && \
		  |   | curl -f -s -S -o- https://s3.amazonaws.com/binaries.cockroachdb.com/cockroach-vv21.2.0-alpha.00000000.linux-amd64.tgz | tar xfz - -C "${tmpdir}" --strip-components 1 && \
		  |   | mv ${tmpdir}/cockroach ${dir}/cockroach && \
		  |   | mkdir -p ${dir}/lib && \
		  |   | if [ -d ${tmpdir}/lib ]; then mv ${tmpdir}/lib/* ${dir}/lib; fi && \
		  |   | chmod 755 ${dir}/cockroach
		  |   |
		  |   | ```
		  | Wraps: (3) exit status 2
		  | Error types: (1) errors.Cmd (2) *hintdetail.withDetail (3) *exec.ExitError
		  |
		  | stdout:
		  | Resolved release url for cockroach version vv21.2.0-alpha.00000000: https://s3.amazonaws.com/binaries.cockroachdb.com/cockroach-vv21.2.0-alpha.00000000.linux-amd64.tgz

Oops yeah I think I forgot to add the file or something

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)

looks good!

Reviewed 2 of 3 files at r1, 2 of 2 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)