💯

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ericharmeling)

A couple nits related to CI testing and local testing.

Reviewed all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dhartunian)

pkg/server/tenant_admin.go line 162 at r1 (raw file):

	}

	j, err := jobsHelper(

See below for context on this suggestion.

Suggestion:

	}

	t.sqlServer.cfg.TestingKnobs = base.TestingKnobs{}

	j, err := jobsHelper(

pkg/server/tenant_admin.go line 167 at r1 (raw file):

		userName,
		t.sqlServer,
		t.sqlServer.cfg,

When I test the jobs endpoint locally (using ./cockroach demo --multitenant=true), the cluster crashes with the following error:

* interface conversion: base.ModuleTestingKnobs is nil, not *jobs.TestingKnobs
* (1) attached stack trace
*   -- stack trace:
*   | runtime.gopanic
*   | 	GOROOT/src/runtime/panic.go:1038
*   | runtime.panicdottypeE
*   | 	GOROOT/src/runtime/iface.go:261
*   | runtime.panicdottypeI
*   | 	GOROOT/src/runtime/iface.go:271
*   | github.com/cockroachdb/cockroach/pkg/server.jobsHelper.func2
*   | 	github.com/cockroachdb/cockroach/pkg/server/admin.go:2124
*   | github.com/cockroachdb/cockroach/pkg/server.jobsHelper
*   | 	github.com/cockroachdb/cockroach/pkg/server/admin.go:2129
*   | github.com/cockroachdb/cockroach/pkg/server.(*tenantAdminServer).Jobs
*   | 	github.com/cockroachdb/cockroach/pkg/server/tenant_admin.go:162
* ...

Locally, I just set the problematic field to the zero value for base.TestingKnobs, and it resolved this issue. IDK if this is the best approach, but it worked for me. 😁

(Aside: I assume this was a problem because jobsHelper normally expects a pointer to server.cfg.BaseConfig, and not sqlServer.cfg and that field wasn't getting initialized properly?)

pkg/server/tenant_admin.go line 182 at r1 (raw file):
nit: linter is complaining

pkg/server/tenant_admin.go:176:1: receiver name s should be consistent with previous receiver name t for tenantAdminServer

Suggestion:

func (t *tenantAdminServer) Job(
	ctx context.Context, request *serverpb.JobRequest,
) (_ *serverpb.JobResponse, retErr error) {
	ctx = t.AnnotateCtx(ctx)

	userName, err := userFromContext(ctx)
	if err != nil {
		return nil, serverError(ctx, err)
	}
	r, err := jobHelper(ctx, request, userName, t.sqlServer)
	if err != nil {
		return nil, serverError(ctx, err)
	}
	return r, nil
}

@ericharmeling PTAL, I added a separate commit that does some light refactoring of tests so they can be shared between packages.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dhartunian, @ericharmeling, and @knz)

pkg/server/tenant_admin.go line 167 at r1 (raw file):

what I just pushed up works without further tweaking of the config variables.

pkg/server/tenant_admin.go line 182 at r1 (raw file):

Done.

Nice! Apologies for the delay. There are still some linting errors, but otherwise LGTM. Thank you!

Reviewed 9 of 9 files at r2, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dhartunian)