refactor: always reuse users in notifications scaletest (#28793)

Signed-off-by: Callum Styan <callumstyan@gmail.com>

cli/exp_scaletest_internal_test.go

@@ -10,9 +10,9 @@ import (
 )
 
 // TestFilterScaletestUsersByPrefix covers the pure user-selection logic behind
-// notifications --reuse-users: an infix pool must not pick up users from the
-// default pool (isolation), non-scaletest users are ignored, and the username
-// guard rejects users that only match the prefix in another field.
+// the notifications scaletest user reuse: an infix pool must not pick up users
+// from the default pool (isolation), non-scaletest users are ignored, and the
+// username guard rejects users that only match the prefix in another field.
 func TestFilterScaletestUsersByPrefix(t *testing.T) {
 	t.Parallel()
 

cli/exp_scaletest_notifications.go

@@ -21,7 +21,6 @@ import (
 	"cdr.dev/slog/v3"
 	notificationsLib "github.com/coder/coder/v2/coderd/notifications"
 	"github.com/coder/coder/v2/codersdk"
-	"github.com/coder/coder/v2/scaletest/createusers"
 	"github.com/coder/coder/v2/scaletest/harness"
 	"github.com/coder/coder/v2/scaletest/loadtestutil"
 	"github.com/coder/coder/v2/scaletest/notifications"
@@ -36,7 +35,6 @@ func (r *RootCmd) scaletestNotifications() *serpent.Command {
 		smtpRequestTimeout      time.Duration
 		dialTimeout             time.Duration
 		noCleanup               bool
-		reuseUsers              bool
 		usernameInfix           string
 		smtpAPIURL              string
 
@@ -120,17 +118,13 @@ func (r *RootCmd) scaletestNotifications() *serpent.Command {
 			}()
 
 			var adminReuse, regularReuse []notificationReuseUser
-			if reuseUsers {
-				_, _ = fmt.Fprintln(inv.Stderr, "Reusing existing scaletest users...")
-				// Bound token lifetime to just beyond the run so tokens orphaned by a
-				// hard kill expire quickly rather than at the deployment default.
-				tokenLifetime := notificationTimeout + dialTimeout + time.Hour
-				adminReuse, regularReuse, err = selectNotificationReuseUsers(ctx, client, usernameInfix, int(templateAdminCount), int(regularUserCount), tokenLifetime)
-				if err != nil {
-					return err
-				}
-			} else {
-				_, _ = fmt.Fprintln(inv.Stderr, "Creating users...")
+			_, _ = fmt.Fprintln(inv.Stderr, "Reusing existing scaletest users...")
+			// Bound token lifetime to just beyond the run so tokens orphaned by a
+			// hard kill expire quickly rather than at the deployment default.
+			tokenLifetime := notificationTimeout + dialTimeout + time.Hour
+			adminReuse, regularReuse, err = selectNotificationReuseUsers(ctx, client, usernameInfix, int(templateAdminCount), int(regularUserCount), tokenLifetime)
+			if err != nil {
+				return err
 			}
 
 			dialBarrier := &sync.WaitGroup{}
@@ -168,13 +162,8 @@ func (r *RootCmd) scaletestNotifications() *serpent.Command {
 					SMTPApiURL:               smtpAPIURL,
 					SMTPRequestTimeout:       smtpRequestTimeout,
 					SMTPHttpClient:           smtpHTTPClient,
-				}
-				if reuseUsers {
-					config.SessionToken = adminReuse[i].sessionToken
-					config.PreCreatedUser = adminReuse[i].user
-				} else {
-					config.User = createusers.Config{OrganizationID: me.OrganizationIDs[0]}
-					config.Roles = []string{codersdk.RoleTemplateAdmin}
+					SessionToken:             adminReuse[i].sessionToken,
+					PreCreatedUser:           adminReuse[i].user,
 				}
 				if err := config.Validate(); err != nil {
 					return xerrors.Errorf("validate config: %w", err)
@@ -188,13 +177,8 @@ func (r *RootCmd) scaletestNotifications() *serpent.Command {
 					DialBarrier:           dialBarrier,
 					ReceivingWatchBarrier: templateAdminWatchBarrier,
 					Metrics:               metrics,
-				}
-				if reuseUsers {
-					config.SessionToken = regularReuse[i].sessionToken
-					config.PreCreatedUser = regularReuse[i].user
-				} else {
-					config.User = createusers.Config{OrganizationID: me.OrganizationIDs[0]}
-					config.Roles = []string{}
+					SessionToken:          regularReuse[i].sessionToken,
+					PreCreatedUser:        regularReuse[i].user,
 				}
 				if err := config.Validate(); err != nil {
 					return xerrors.Errorf("validate config: %w", err)
@@ -322,16 +306,10 @@ func (r *RootCmd) scaletestNotifications() *serpent.Command {
 			Description: "Do not clean up resources after the test completes.",
 			Value:       serpent.BoolOf(&noCleanup),
 		},
-		{
-			Flag:        "reuse-users",
-			Env:         "CODER_SCALETEST_NOTIFICATION_REUSE_USERS",
-			Description: "Reuse existing scaletest users instead of creating new ones. Enough users, including template admins, must already exist (see \"coder exp scaletest create-users\") or the command errors. Run only one user-selecting scaletest command at a time to avoid overlapping selections.",
-			Value:       serpent.BoolOf(&reuseUsers),
-		},
 		{
 			Flag:        "username-infix",
 			Env:         "CODER_SCALETEST_NOTIFICATION_USERNAME_INFIX",
-			Description: "Username infix identifying the user pool to reuse when --reuse-users is set. It must match the --username-infix used by the create-users run that provisioned the pool: for example asdf selects users named scaletest-asdf-<random>-<id> so notifications reuses only its own pool and does not compete with other load generators. Leave empty to select any scaletest- user. Ignored without --reuse-users.",
+			Description: "Username infix identifying the user pool to reuse. It must match the --username-infix used by the create-users run that provisioned the pool: for example asdf selects users named scaletest-asdf-<random>-<id> so notifications reuses only its own pool and does not compete with other load generators. Leave empty to select any scaletest- user.",
 			Value:       serpent.StringOf(&usernameInfix),
 		},
 		{

cli/exp_scaletest_test.go

@@ -116,9 +116,10 @@ func TestScaleTestCreateUsers(t *testing.T) {
 	require.Equal(t, 1, templateAdmins)
 }
 
-// TestScaleTestNotifications_ReuseUsersInsufficient verifies that --reuse-users
-// checks the pool up front and exits with an actionable error when not enough
-// scaletest users (or template admins) exist, rather than creating any.
+// TestScaleTestNotifications_ReuseUsersInsufficient verifies that the
+// notifications scaletest checks the user pool up front and exits with an
+// actionable error when not enough scaletest users (or template admins) exist,
+// rather than creating any.
 func TestScaleTestNotifications_ReuseUsersInsufficient(t *testing.T) {
 	t.Parallel()
 
@@ -138,7 +139,6 @@ func TestScaleTestNotifications_ReuseUsersInsufficient(t *testing.T) {
 	inv, root := clitest.New(t, "exp", "scaletest", "notifications",
 		"--user-count", "2",
 		"--template-admin-percentage", "50",
-		"--reuse-users",
 		"--dial-timeout", "5s",
 		"--notification-timeout", "5s",
 		"--scaletest-prometheus-address", "127.0.0.1:0",

scaletest/notifications/config.go

@@ -9,23 +9,15 @@ import (
 	"golang.org/x/xerrors"
 
 	"github.com/coder/coder/v2/codersdk"
-	"github.com/coder/coder/v2/scaletest/createusers"
 )
 
 type Config struct {
-	// User is the configuration for the user to create.
-	User createusers.Config `json:"user"`
-
-	// Roles are the roles to assign to the user.
-	Roles []string `json:"roles"`
-
-	// SessionToken, when set, makes the runner reuse PreCreatedUser instead of
-	// creating one. User and Roles are then ignored.
+	// SessionToken authenticates the runner as PreCreatedUser.
 	SessionToken string `json:"-"`
 
-	// PreCreatedUser is the user to run as when SessionToken is set. It must
-	// already hold any role needed to receive the notifications under test. Only
-	// ID and Email are used.
+	// PreCreatedUser is the existing user to run as. It must already hold any
+	// role needed to receive the notifications under test. Only ID, Username and
+	// Email are used.
 	PreCreatedUser codersdk.User `json:"-"`
 
 	// NotificationTimeout is how long to wait for notifications after triggering.
@@ -56,20 +48,12 @@ type Config struct {
 }
 
 func (c Config) Validate() error {
-	if c.SessionToken != "" {
-		// Reuse mode does not create a user, so only the existing user's ID is required.
-		if c.PreCreatedUser.ID == uuid.Nil {
-			return xerrors.New("pre_created_user must be set when session_token is set")
-		}
-	} else {
-		// The runner always needs an org; ensure we propagate it into the user config.
-		if c.User.OrganizationID == uuid.Nil {
-			return xerrors.New("user organization_id must be set")
-		}
-
-		if err := c.User.Validate(); err != nil {
-			return xerrors.Errorf("user config: %w", err)
-		}
+	// The runner always reuses an existing user; it never creates one.
+	if c.SessionToken == "" {
+		return xerrors.New("session_token must be set")
+	}
+	if c.PreCreatedUser.ID == uuid.Nil {
+		return xerrors.New("pre_created_user must be set")
 	}
 
 	if c.DialBarrier == nil {

scaletest/notifications/run.go

@@ -19,7 +19,6 @@ import (
 	"cdr.dev/slog/v3/sloggers/sloghuman"
 	"github.com/coder/coder/v2/coderd/tracing"
 	"github.com/coder/coder/v2/codersdk"
-	"github.com/coder/coder/v2/scaletest/createusers"
 	"github.com/coder/coder/v2/scaletest/harness"
 	"github.com/coder/coder/v2/scaletest/loadtestutil"
 	"github.com/coder/coder/v2/scaletest/smtpmock"
@@ -31,8 +30,6 @@ type Runner struct {
 	client *codersdk.Client
 	cfg    Config
 
-	createUserRunner *createusers.Runner
-
 	// websocketReceiptTimes stores the receipt time for websocket notifications
 	websocketReceiptTimes   map[uuid.UUID]time.Time
 	websocketReceiptTimesMu sync.RWMutex
@@ -65,7 +62,7 @@ var (
 	_ harness.Collectable = &Runner{}
 )
 
-func (r *Runner) Run(ctx context.Context, id string, logs io.Writer) error {
+func (r *Runner) Run(ctx context.Context, _ string, logs io.Writer) error {
 	ctx, span := tracing.StartSpan(ctx)
 	defer span.End()
 
@@ -88,47 +85,15 @@ func (r *Runner) Run(ctx context.Context, id string, logs io.Writer) error {
 	r.client.SetLogger(logger)
 	r.client.SetLogBodies(true)
 
-	var (
-		newUser       codersdk.User
-		newUserClient *codersdk.Client
-	)
-	if r.cfg.SessionToken != "" {
-		// Reuse mode assigns no roles; the user must already hold any role needed
-		// to receive the notifications under test.
-		newUser = r.cfg.PreCreatedUser
-		newUserClient = codersdk.New(r.client.URL,
-			codersdk.WithSessionToken(r.cfg.SessionToken),
-			codersdk.WithLogger(logger),
-			codersdk.WithLogBodies())
+	// The runner always reuses an existing user; it never creates one. The user
+	// must already hold any role needed to receive the notifications under test.
+	newUser := r.cfg.PreCreatedUser
+	newUserClient := codersdk.New(r.client.URL,
+		codersdk.WithSessionToken(r.cfg.SessionToken),
+		codersdk.WithLogger(logger),
+		codersdk.WithLogBodies())
 
-		logger.Info(ctx, "reusing existing user", slog.F("username", newUser.Username), slog.F("user_id", newUser.ID.String()))
-	} else {
-		r.createUserRunner = createusers.NewRunner(r.client, r.cfg.User)
-		newUserAndToken, err := r.createUserRunner.RunReturningUser(ctx, id, logs)
-		if err != nil {
-			r.cfg.Metrics.AddError("create_user")
-			return xerrors.Errorf("create user: %w", err)
-		}
-		newUser = newUserAndToken.User
-		newUserClient = codersdk.New(r.client.URL,
-			codersdk.WithSessionToken(newUserAndToken.SessionToken),
-			codersdk.WithLogger(logger),
-			codersdk.WithLogBodies())
-
-		logger.Info(ctx, "runner user created", slog.F("username", newUser.Username), slog.F("user_id", newUser.ID.String()))
-
-		if len(r.cfg.Roles) > 0 {
-			logger.Info(ctx, "assigning roles to user", slog.F("roles", r.cfg.Roles))
-
-			_, err := r.client.UpdateUserRoles(ctx, newUser.ID.String(), codersdk.UpdateRoles{
-				Roles: r.cfg.Roles,
-			})
-			if err != nil {
-				r.cfg.Metrics.AddError("assign_roles")
-				return xerrors.Errorf("assign roles: %w", err)
-			}
-		}
-	}
+	logger.Info(ctx, "reusing existing user", slog.F("username", newUser.Username), slog.F("user_id", newUser.ID.String()))
 
 	logger.Info(ctx, "notification runner is ready")
 
@@ -194,14 +159,9 @@ func (r *Runner) Run(ctx context.Context, id string, logs io.Writer) error {
 	return nil
 }
 
-func (r *Runner) Cleanup(ctx context.Context, id string, logs io.Writer) error {
-	if r.createUserRunner != nil {
-		_, _ = fmt.Fprintln(logs, "Cleaning up user...")
-		if err := r.createUserRunner.Cleanup(ctx, id, logs); err != nil {
-			return xerrors.Errorf("cleanup user: %w", err)
-		}
-	}
-
+func (*Runner) Cleanup(_ context.Context, _ string, _ io.Writer) error {
+	// The runner reuses an existing user and never creates one, so there is
+	// nothing to clean up.
 	return nil
 }
 

scaletest/notifications/run_test.go

@@ -22,8 +22,8 @@ import (
 	notificationsLib "github.com/coder/coder/v2/coderd/notifications"
 	"github.com/coder/coder/v2/coderd/notifications/dispatch"
 	"github.com/coder/coder/v2/coderd/notifications/types"
+	"github.com/coder/coder/v2/coderd/rbac"
 	"github.com/coder/coder/v2/codersdk"
-	"github.com/coder/coder/v2/scaletest/createusers"
 	"github.com/coder/coder/v2/scaletest/notifications"
 	"github.com/coder/coder/v2/scaletest/smtpmock"
 	"github.com/coder/coder/v2/testutil"
@@ -62,13 +62,13 @@ func TestRun(t *testing.T) {
 
 	// Start receiving runners who will receive notifications
 	receivingRunners := make([]*notifications.Runner, 0, numReceivingUsers)
+	receivingUsernames := make([]string, 0, numReceivingUsers)
 	for i := range numReceivingUsers {
+		userClient, user := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID, rbac.RoleOwner())
+		receivingUsernames = append(receivingUsernames, user.Username)
 		runnerCfg := notifications.Config{
-			User: createusers.Config{
-				OrganizationID: firstUser.OrganizationID,
-				Username:       "receiving-user-" + strconv.Itoa(i),
-			},
-			Roles:                    []string{codersdk.RoleOwner},
+			SessionToken:             userClient.SessionToken(),
+			PreCreatedUser:           user,
 			NotificationTimeout:      testutil.WaitLong,
 			DialTimeout:              testutil.WaitLong,
 			Metrics:                  metrics,
@@ -89,11 +89,10 @@ func TestRun(t *testing.T) {
 	// Start regular user runners who will maintain websocket connections
 	regularRunners := make([]*notifications.Runner, 0, numRegularUsers)
 	for i := range numRegularUsers {
+		userClient, user := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID)
 		runnerCfg := notifications.Config{
-			User: createusers.Config{
-				OrganizationID: firstUser.OrganizationID,
-			},
-			Roles:                 []string{},
+			SessionToken:          userClient.SessionToken(),
+			PreCreatedUser:        user,
 			NotificationTimeout:   testutil.WaitLong,
 			DialTimeout:           testutil.WaitLong,
 			Metrics:               metrics,
@@ -116,9 +115,9 @@ func TestRun(t *testing.T) {
 		dialBarrier.Wait()
 
 		for i := 0; i < numReceivingUsers; i++ {
-			err := sendInboxNotification(runCtx, t, db, inboxHandler, "receiving-user-"+strconv.Itoa(i), notificationsLib.TemplateUserAccountCreated)
+			err := sendInboxNotification(runCtx, t, db, inboxHandler, receivingUsernames[i], notificationsLib.TemplateUserAccountCreated)
 			require.NoError(t, err)
-			err = sendInboxNotification(runCtx, t, db, inboxHandler, "receiving-user-"+strconv.Itoa(i), notificationsLib.TemplateUserAccountDeleted)
+			err = sendInboxNotification(runCtx, t, db, inboxHandler, receivingUsernames[i], notificationsLib.TemplateUserAccountDeleted)
 			require.NoError(t, err)
 		}
 
@@ -142,10 +141,11 @@ func TestRun(t *testing.T) {
 	err = cleanupEg.Wait()
 	require.NoError(t, err)
 
+	// The runner reuses existing users and never deletes them, so every
+	// pre-created user remains alongside the first user.
 	users, err := client.Users(ctx, codersdk.UsersRequest{})
 	require.NoError(t, err)
-	require.Len(t, users.Users, 1)
-	require.Equal(t, firstUser.UserID, users.Users[0].ID)
+	require.Len(t, users.Users, 1+numReceivingUsers+numRegularUsers)
 
 	for _, runner := range receivingRunners {
 		metrics := runner.GetMetrics()
@@ -216,13 +216,13 @@ func TestRunWithSMTP(t *testing.T) {
 
 	// Start receiving runners who will receive notifications
 	receivingRunners := make([]*notifications.Runner, 0, numReceivingUsers)
+	receivingUsernames := make([]string, 0, numReceivingUsers)
 	for i := range numReceivingUsers {
+		userClient, user := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID, rbac.RoleOwner())
+		receivingUsernames = append(receivingUsernames, user.Username)
 		runnerCfg := notifications.Config{
-			User: createusers.Config{
-				OrganizationID: firstUser.OrganizationID,
-				Username:       "receiving-user-" + strconv.Itoa(i),
-			},
-			Roles:                    []string{codersdk.RoleOwner},
+			SessionToken:             userClient.SessionToken(),
+			PreCreatedUser:           user,
 			NotificationTimeout:      testutil.WaitLong,
 			DialTimeout:              testutil.WaitLong,
 			Metrics:                  metrics,
@@ -246,11 +246,10 @@ func TestRunWithSMTP(t *testing.T) {
 	// Start regular user runners who will maintain websocket connections
 	regularRunners := make([]*notifications.Runner, 0, numRegularUsers)
 	for i := range numRegularUsers {
+		userClient, user := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID)
 		runnerCfg := notifications.Config{
-			User: createusers.Config{
-				OrganizationID: firstUser.OrganizationID,
-			},
-			Roles:                 []string{},
+			SessionToken:          userClient.SessionToken(),
+			PreCreatedUser:        user,
 			NotificationTimeout:   testutil.WaitLong,
 			DialTimeout:           testutil.WaitLong,
 			Metrics:               metrics,
@@ -277,9 +276,9 @@ func TestRunWithSMTP(t *testing.T) {
 		}
 
 		for i := 0; i < numReceivingUsers; i++ {
-			err := sendInboxNotification(runCtx, t, db, inboxHandler, "receiving-user-"+strconv.Itoa(i), notificationsLib.TemplateUserAccountCreated)
+			err := sendInboxNotification(runCtx, t, db, inboxHandler, receivingUsernames[i], notificationsLib.TemplateUserAccountCreated)
 			require.NoError(t, err)
-			err = sendInboxNotification(runCtx, t, db, inboxHandler, "receiving-user-"+strconv.Itoa(i), notificationsLib.TemplateUserAccountDeleted)
+			err = sendInboxNotification(runCtx, t, db, inboxHandler, receivingUsernames[i], notificationsLib.TemplateUserAccountDeleted)
 			require.NoError(t, err)
 		}
 
@@ -306,10 +305,11 @@ func TestRunWithSMTP(t *testing.T) {
 	err = cleanupEg.Wait()
 	require.NoError(t, err)
 
+	// The runner reuses existing users and never deletes them, so every
+	// pre-created user remains alongside the first user.
 	users, err := client.Users(ctx, codersdk.UsersRequest{})
 	require.NoError(t, err)
-	require.Len(t, users.Users, 1)
-	require.Equal(t, firstUser.UserID, users.Users[0].ID)
+	require.Len(t, users.Users, 1+numReceivingUsers+numRegularUsers)
 
 	// Verify that notifications were received via both websocket and SMTP
 	for _, runner := range receivingRunners {