fix: stop inserting provisioner daemons into the database (#9108)

Signed-off-by: Spike Curtis <spike@coder.com>
2023-09-08 14:37:36 +04:00 · 2023-09-08 14:37:36 +04:00 · 8d7eb1728c
parent fb3616c37e
commit 8d7eb1728c
4 changed files with 24 additions and 75 deletions
--- a/coderd/coderd.go
+++ b/coderd/coderd.go
@ -46,7 +46,6 @@ import (
 	"github.com/coder/coder/v2/coderd/batchstats"
 	"github.com/coder/coder/v2/coderd/database"
 	"github.com/coder/coder/v2/coderd/database/dbauthz"
-	"github.com/coder/coder/v2/coderd/database/dbtime"
 	"github.com/coder/coder/v2/coderd/database/pubsub"
 	"github.com/coder/coder/v2/coderd/gitauth"
 	"github.com/coder/coder/v2/coderd/gitsshkey"
@ -1078,32 +1077,24 @@ func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context, debounce ti
 		}
 	}()

-	name := namesgenerator.GetRandomName(1)
-	// nolint:gocritic // Inserting a provisioner daemon is a system function.
-	daemon, err := api.Database.InsertProvisionerDaemon(dbauthz.AsSystemRestricted(ctx), database.InsertProvisionerDaemonParams{
-		ID:           uuid.New(),
-		CreatedAt:    dbtime.Now(),
-		Name:         name,
-		Provisioners: []database.ProvisionerType{database.ProvisionerTypeEcho, database.ProvisionerTypeTerraform},
-		Tags: database.StringMap{
-			provisionerdserver.TagScope: provisionerdserver.ScopeOrganization,
-		},
+	tags, err := json.Marshal(database.StringMap{
+		provisionerdserver.TagScope: provisionerdserver.ScopeOrganization,
 	})
-	if err != nil {
-		return nil, xerrors.Errorf("insert provisioner daemon %q: %w", name, err)
-	}
-
-	tags, err := json.Marshal(daemon.Tags)
 	if err != nil {
 		return nil, xerrors.Errorf("marshal tags: %w", err)
 	}

 	mux := drpcmux.New()
+	name := namesgenerator.GetRandomName(1)
+	logger := api.Logger.Named(fmt.Sprintf("inmem-provisionerd-%s", name))
+	logger.Info(ctx, "starting in-memory provisioner daemon")
 	srv, err := provisionerdserver.NewServer(
 		api.AccessURL,
-		daemon.ID,
-		api.Logger.Named(fmt.Sprintf("provisionerd-%s", daemon.Name)),
-		daemon.Provisioners,
+		uuid.New(),
+		logger,
+		[]database.ProvisionerType{
+			database.ProvisionerTypeEcho, database.ProvisionerTypeTerraform,
+		},
 		tags,
 		api.Database,
 		api.Pubsub,
@ -1133,16 +1124,14 @@ func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context, debounce ti
 				if xerrors.Is(err, io.EOF) {
 					return
 				}
-				api.Logger.Debug(ctx, "drpc server error", slog.Error(err))
+				logger.Debug(ctx, "drpc server error", slog.Error(err))
 			},
 		},
 	)
 	go func() {
 		err := server.Serve(ctx, serverSession)
-		if err != nil && !xerrors.Is(err, io.EOF) {
-			api.Logger.Debug(ctx, "provisioner daemon disconnected", slog.Error(err))
-		}
-		// close the sessions so we don't leak goroutines serving them.
+		logger.Info(ctx, "provisioner daemon disconnected", slog.Error(err))
+		// close the sessions, so we don't leak goroutines serving them.
 		_ = clientSession.Close()
 		_ = serverSession.Close()
 	}()
--- a/codersdk/organizations.go
+++ b/codersdk/organizations.go
@ -154,6 +154,9 @@ func (c *Client) Organization(ctx context.Context, id uuid.UUID) (Organization,
 }

 // ProvisionerDaemons returns provisioner daemons available.
+//
+// Deprecated: We no longer track provisioner daemons as they connect.  This function may return historical data
+// but new provisioner daemons will not appear.
 func (c *Client) ProvisionerDaemons(ctx context.Context) ([]ProvisionerDaemon, error) {
 	res, err := c.Request(ctx, http.MethodGet,
 		// TODO: the organization path parameter is currently ignored.
--- a/enterprise/coderd/provisionerdaemons.go
+++ b/enterprise/coderd/provisionerdaemons.go
@ -25,7 +25,6 @@ import (
 	"cdr.dev/slog"
 	"github.com/coder/coder/v2/coderd"
 	"github.com/coder/coder/v2/coderd/database"
-	"github.com/coder/coder/v2/coderd/database/dbtime"
 	"github.com/coder/coder/v2/coderd/httpapi"
 	"github.com/coder/coder/v2/coderd/httpmw"
 	"github.com/coder/coder/v2/coderd/provisionerdserver"
@ -198,25 +197,7 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
 		slog.F("provisioners", provisioners),
 		slog.F("tags", tags),
 	)
-	daemon, err := api.Database.InsertProvisionerDaemon(ctx, database.InsertProvisionerDaemonParams{
-		ID:           uuid.New(),
-		CreatedAt:    dbtime.Now(),
-		Name:         name,
-		Provisioners: provisioners,
-		Tags:         tags,
-	})
-	if err != nil {
-		if !xerrors.Is(err, context.Canceled) {
-			log.Error(ctx, "write provisioner daemon", slog.Error(err))
-		}
-		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
-			Message: "Internal error writing provisioner daemon.",
-			Detail:  err.Error(),
-		})
-		return
-	}
-
-	rawTags, err := json.Marshal(daemon.Tags)
+	rawTags, err := json.Marshal(tags)
 	if err != nil {
 		if !xerrors.Is(err, context.Canceled) {
 			log.Error(ctx, "marshal provisioner tags", slog.Error(err))
@ -263,11 +244,13 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
 		return
 	}
 	mux := drpcmux.New()
+	logger := api.Logger.Named(fmt.Sprintf("ext-provisionerd-%s", name))
+	logger.Info(ctx, "starting external provisioner daemon")
 	srv, err := provisionerdserver.NewServer(
 		api.AccessURL,
-		daemon.ID,
-		api.Logger.Named(fmt.Sprintf("provisionerd-%s", daemon.Name)),
-		daemon.Provisioners,
+		uuid.New(),
+		logger,
+		provisioners,
 		rawTags,
 		api.Database,
 		api.Pubsub,
@ -302,12 +285,12 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
 			if xerrors.Is(err, io.EOF) {
 				return
 			}
-			api.Logger.Debug(ctx, "drpc server error", slog.Error(err))
+			logger.Debug(ctx, "drpc server error", slog.Error(err))
 		},
 	})
 	err = server.Serve(ctx, session)
+	logger.Info(ctx, "provisioner daemon disconnected", slog.Error(err))
 	if err != nil && !xerrors.Is(err, io.EOF) {
-		api.Logger.Debug(ctx, "provisioner daemon disconnected", slog.Error(err))
 		_ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("serve: %s", err))
 		return
 	}
--- a/enterprise/coderd/provisionerdaemons_test.go
+++ b/enterprise/coderd/provisionerdaemons_test.go
@ -47,9 +47,6 @@ func TestProvisionerDaemonServe(t *testing.T) {
 		})
 		require.NoError(t, err)
 		srv.DRPCConn().Close()
-		daemons, err := client.ProvisionerDaemons(ctx)
-		require.NoError(t, err)
-		require.Len(t, daemons, 1)
 	})

 	t.Run("NoLicense", func(t *testing.T) {
@ -68,11 +65,6 @@ func TestProvisionerDaemonServe(t *testing.T) {
 		var apiError *codersdk.Error
 		require.ErrorAs(t, err, &apiError)
 		require.Equal(t, http.StatusForbidden, apiError.StatusCode())
-
-		// querying provisioner daemons is forbidden without license
-		_, err = client.ProvisionerDaemons(ctx)
-		require.ErrorAs(t, err, &apiError)
-		require.Equal(t, http.StatusForbidden, apiError.StatusCode())
 	})

 	t.Run("Organization", func(t *testing.T) {
@ -98,9 +90,6 @@ func TestProvisionerDaemonServe(t *testing.T) {
 		var apiError *codersdk.Error
 		require.ErrorAs(t, err, &apiError)
 		require.Equal(t, http.StatusForbidden, apiError.StatusCode())
-		daemons, err := client.ProvisionerDaemons(ctx)
-		require.NoError(t, err)
-		require.Len(t, daemons, 0)
 	})

 	t.Run("OrganizationNoPerms", func(t *testing.T) {
@ -126,9 +115,6 @@ func TestProvisionerDaemonServe(t *testing.T) {
 		var apiError *codersdk.Error
 		require.ErrorAs(t, err, &apiError)
 		require.Equal(t, http.StatusForbidden, apiError.StatusCode())
-		daemons, err := client.ProvisionerDaemons(ctx)
-		require.NoError(t, err)
-		require.Len(t, daemons, 0)
 	})

 	t.Run("UserLocal", func(t *testing.T) {
@ -214,9 +200,6 @@ func TestProvisionerDaemonServe(t *testing.T) {
 		require.NoError(t, err)
 		err = srv.DRPCConn().Close()
 		require.NoError(t, err)
-		daemons, err := client.ProvisionerDaemons(ctx)
-		require.NoError(t, err)
-		require.Len(t, daemons, 1)
 	})

 	t.Run("PSK_daily_cost", func(t *testing.T) {
@ -347,9 +330,6 @@ func TestProvisionerDaemonServe(t *testing.T) {
 		var apiError *codersdk.Error
 		require.ErrorAs(t, err, &apiError)
 		require.Equal(t, http.StatusForbidden, apiError.StatusCode())
-		daemons, err := client.ProvisionerDaemons(ctx)
-		require.NoError(t, err)
-		require.Len(t, daemons, 0)
 	})

 	t.Run("NoAuth", func(t *testing.T) {
@ -378,9 +358,6 @@ func TestProvisionerDaemonServe(t *testing.T) {
 		var apiError *codersdk.Error
 		require.ErrorAs(t, err, &apiError)
 		require.Equal(t, http.StatusForbidden, apiError.StatusCode())
-		daemons, err := client.ProvisionerDaemons(ctx)
-		require.NoError(t, err)
-		require.Len(t, daemons, 0)
 	})

 	t.Run("NoPSK", func(t *testing.T) {
@ -409,8 +386,5 @@ func TestProvisionerDaemonServe(t *testing.T) {
 		var apiError *codersdk.Error
 		require.ErrorAs(t, err, &apiError)
 		require.Equal(t, http.StatusForbidden, apiError.StatusCode())
-		daemons, err := client.ProvisionerDaemons(ctx)
-		require.NoError(t, err)
-		require.Len(t, daemons, 0)
 	})
 }