From 94a3e3a5639ff6e01968e342293d2a9f21e00866 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 3 May 2024 10:14:26 -0500 Subject: [PATCH] chore: allow terraform & echo built-in provisioners (#13121) * chore: allow terraform & echo built-in provisioners Built-in provisioners serve all specified types. This allows running terraform, echo, or both in built in. The cli flag to control the types is hidden by default, to be used primarily for testing purposes. --- cli/server.go | 143 ++++++++++++++----------- cli/server_test.go | 15 ++- cli/testdata/server-config.yaml.golden | 9 +- coderd/apidoc/docs.go | 12 ++- coderd/apidoc/swagger.json | 12 ++- coderd/coderd.go | 19 ++-- coderd/coderdtest/coderdtest.go | 2 +- codersdk/deployment.go | 46 +++++--- codersdk/organizations.go | 11 ++ docs/api/general.md | 2 +- docs/api/schemas.md | 22 ++-- site/e2e/playwright.config.ts | 3 +- site/src/api/typesGenerated.ts | 2 +- 13 files changed, 180 insertions(+), 118 deletions(-) diff --git a/cli/server.go b/cli/server.go index 5b804f9278..e96c8cacdc 100644 --- a/cli/server.go +++ b/cli/server.go @@ -944,6 +944,13 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. var provisionerdWaitGroup sync.WaitGroup defer provisionerdWaitGroup.Wait() provisionerdMetrics := provisionerd.NewMetrics(options.PrometheusRegistry) + + // Built in provisioner daemons will support the same types. + // By default, this is the slice {"terraform"} + provisionerTypes := make([]codersdk.ProvisionerType, 0) + for _, pt := range vals.Provisioner.DaemonTypes { + provisionerTypes = append(provisionerTypes, codersdk.ProvisionerType(pt)) + } for i := int64(0); i < vals.Provisioner.Daemons.Value(); i++ { suffix := fmt.Sprintf("%d", i) // The suffix is added to the hostname, so we may need to trim to fit into @@ -952,7 +959,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. name := fmt.Sprintf("%s-%s", hostname, suffix) daemonCacheDir := filepath.Join(cacheDir, fmt.Sprintf("provisioner-%d", i)) daemon, err := newProvisionerDaemon( - ctx, coderAPI, provisionerdMetrics, logger, vals, daemonCacheDir, errCh, &provisionerdWaitGroup, name, + ctx, coderAPI, provisionerdMetrics, logger, vals, daemonCacheDir, errCh, &provisionerdWaitGroup, name, provisionerTypes, ) if err != nil { return xerrors.Errorf("create provisioner daemon: %w", err) @@ -1340,6 +1347,7 @@ func newProvisionerDaemon( errCh chan error, wg *sync.WaitGroup, name string, + provisionerTypes []codersdk.ProvisionerType, ) (srv *provisionerd.Server, err error) { ctx, cancel := context.WithCancel(ctx) defer func() { @@ -1359,79 +1367,88 @@ func newProvisionerDaemon( return nil, xerrors.Errorf("mkdir work dir: %w", err) } + // Omit any duplicates + provisionerTypes = slice.Unique(provisionerTypes) + + // Populate the connector with the supported types. connector := provisionerd.LocalProvisioners{} - if cfg.Provisioner.DaemonsEcho { - echoClient, echoServer := drpc.MemTransportPipe() - wg.Add(1) - go func() { - defer wg.Done() - <-ctx.Done() - _ = echoClient.Close() - _ = echoServer.Close() - }() - wg.Add(1) - go func() { - defer wg.Done() - defer cancel() + for _, provisionerType := range provisionerTypes { + switch provisionerType { + case codersdk.ProvisionerTypeEcho: + echoClient, echoServer := drpc.MemTransportPipe() + wg.Add(1) + go func() { + defer wg.Done() + <-ctx.Done() + _ = echoClient.Close() + _ = echoServer.Close() + }() + wg.Add(1) + go func() { + defer wg.Done() + defer cancel() - err := echo.Serve(ctx, &provisionersdk.ServeOptions{ - Listener: echoServer, - WorkDirectory: workDir, - Logger: logger.Named("echo"), - }) - if err != nil { - select { - case errCh <- err: - default: - } - } - }() - connector[string(database.ProvisionerTypeEcho)] = sdkproto.NewDRPCProvisionerClient(echoClient) - } else { - tfDir := filepath.Join(cacheDir, "tf") - err = os.MkdirAll(tfDir, 0o700) - if err != nil { - return nil, xerrors.Errorf("mkdir terraform dir: %w", err) - } - - tracer := coderAPI.TracerProvider.Tracer(tracing.TracerName) - terraformClient, terraformServer := drpc.MemTransportPipe() - wg.Add(1) - go func() { - defer wg.Done() - <-ctx.Done() - _ = terraformClient.Close() - _ = terraformServer.Close() - }() - wg.Add(1) - go func() { - defer wg.Done() - defer cancel() - - err := terraform.Serve(ctx, &terraform.ServeOptions{ - ServeOptions: &provisionersdk.ServeOptions{ - Listener: terraformServer, - Logger: logger.Named("terraform"), + err := echo.Serve(ctx, &provisionersdk.ServeOptions{ + Listener: echoServer, WorkDirectory: workDir, - }, - CachePath: tfDir, - Tracer: tracer, - }) - if err != nil && !xerrors.Is(err, context.Canceled) { - select { - case errCh <- err: - default: + Logger: logger.Named("echo"), + }) + if err != nil { + select { + case errCh <- err: + default: + } } + }() + connector[string(database.ProvisionerTypeEcho)] = sdkproto.NewDRPCProvisionerClient(echoClient) + case codersdk.ProvisionerTypeTerraform: + tfDir := filepath.Join(cacheDir, "tf") + err = os.MkdirAll(tfDir, 0o700) + if err != nil { + return nil, xerrors.Errorf("mkdir terraform dir: %w", err) } - }() - connector[string(database.ProvisionerTypeTerraform)] = sdkproto.NewDRPCProvisionerClient(terraformClient) + tracer := coderAPI.TracerProvider.Tracer(tracing.TracerName) + terraformClient, terraformServer := drpc.MemTransportPipe() + wg.Add(1) + go func() { + defer wg.Done() + <-ctx.Done() + _ = terraformClient.Close() + _ = terraformServer.Close() + }() + wg.Add(1) + go func() { + defer wg.Done() + defer cancel() + + err := terraform.Serve(ctx, &terraform.ServeOptions{ + ServeOptions: &provisionersdk.ServeOptions{ + Listener: terraformServer, + Logger: logger.Named("terraform"), + WorkDirectory: workDir, + }, + CachePath: tfDir, + Tracer: tracer, + }) + if err != nil && !xerrors.Is(err, context.Canceled) { + select { + case errCh <- err: + default: + } + } + }() + + connector[string(database.ProvisionerTypeTerraform)] = sdkproto.NewDRPCProvisionerClient(terraformClient) + default: + return nil, fmt.Errorf("unknown provisioner type %q", provisionerType) + } } return provisionerd.New(func(dialCtx context.Context) (proto.DRPCProvisionerDaemonClient, error) { // This debounces calls to listen every second. Read the comment // in provisionerdserver.go to learn more! - return coderAPI.CreateInMemoryProvisionerDaemon(dialCtx, name) + return coderAPI.CreateInMemoryProvisionerDaemon(dialCtx, name, provisionerTypes) }, &provisionerd.Options{ Logger: logger.Named(fmt.Sprintf("provisionerd-%s", name)), UpdateInterval: time.Second, diff --git a/cli/server_test.go b/cli/server_test.go index 065131fd97..3ca57cf0ce 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -1367,7 +1367,8 @@ func TestServer(t *testing.T) { "--in-memory", "--http-address", ":0", "--access-url", "http://example.com", - "--provisioner-daemons-echo", + "--provisioner-daemons=3", + "--provisioner-types=echo", "--log-human", fiName, ) clitest.Start(t, root) @@ -1385,7 +1386,8 @@ func TestServer(t *testing.T) { "--in-memory", "--http-address", ":0", "--access-url", "http://example.com", - "--provisioner-daemons-echo", + "--provisioner-daemons=3", + "--provisioner-types=echo", "--log-human", fi, ) clitest.Start(t, root) @@ -1403,7 +1405,8 @@ func TestServer(t *testing.T) { "--in-memory", "--http-address", ":0", "--access-url", "http://example.com", - "--provisioner-daemons-echo", + "--provisioner-daemons=3", + "--provisioner-types=echo", "--log-json", fi, ) clitest.Start(t, root) @@ -1424,7 +1427,8 @@ func TestServer(t *testing.T) { "--in-memory", "--http-address", ":0", "--access-url", "http://example.com", - "--provisioner-daemons-echo", + "--provisioner-daemons=3", + "--provisioner-types=echo", "--log-stackdriver", fi, ) // Attach pty so we get debug output from the command if this test @@ -1459,7 +1463,8 @@ func TestServer(t *testing.T) { "--in-memory", "--http-address", ":0", "--access-url", "http://example.com", - "--provisioner-daemons-echo", + "--provisioner-daemons=3", + "--provisioner-types=echo", "--log-human", fi1, "--log-json", fi2, "--log-stackdriver", fi3, diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index 4366dbc4d9..bf49239bc4 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -379,10 +379,11 @@ provisioning: # state for a long time, consider increasing this. # (default: 3, type: int) daemons: 3 - # Whether to use echo provisioner daemons instead of Terraform. This is for E2E - # tests. - # (default: false, type: bool) - daemonsEcho: false + # The supported job types for the built-in provisioners. By default, this is only + # the terraform type. Supported types: terraform,echo. + # (default: terraform, type: string-array) + daemonTypes: + - terraform # Deprecated and ignored. # (default: 1s, type: duration) daemonPollInterval: 1s diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index b3607a1c7a..9e746d2df6 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -10484,11 +10484,15 @@ const docTemplate = `{ "daemon_psk": { "type": "string" }, - "daemons": { - "type": "integer" + "daemon_types": { + "type": "array", + "items": { + "type": "string" + } }, - "daemons_echo": { - "type": "boolean" + "daemons": { + "description": "Daemons is the number of built-in terraform provisioners.", + "type": "integer" }, "force_cancel_interval": { "type": "integer" diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index e73b54f3c9..d0e60d65aa 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -9418,11 +9418,15 @@ "daemon_psk": { "type": "string" }, - "daemons": { - "type": "integer" + "daemon_types": { + "type": "array", + "items": { + "type": "string" + } }, - "daemons_echo": { - "type": "boolean" + "daemons": { + "description": "Daemons is the number of built-in terraform provisioners.", + "type": "integer" }, "force_cancel_interval": { "type": "integer" diff --git a/coderd/coderd.go b/coderd/coderd.go index 0bd6a5a4ae..9dcda1a715 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1348,7 +1348,7 @@ func compressHandler(h http.Handler) http.Handler { // CreateInMemoryProvisionerDaemon is an in-memory connection to a provisionerd. // Useful when starting coderd and provisionerd in the same process. -func (api *API) CreateInMemoryProvisionerDaemon(dialCtx context.Context, name string) (client proto.DRPCProvisionerDaemonClient, err error) { +func (api *API) CreateInMemoryProvisionerDaemon(dialCtx context.Context, name string, provisionerTypes []codersdk.ProvisionerType) (client proto.DRPCProvisionerDaemonClient, err error) { tracer := api.TracerProvider.Tracer(tracing.TracerName) clientSession, serverSession := drpc.MemTransportPipe() defer func() { @@ -1365,18 +1365,21 @@ func (api *API) CreateInMemoryProvisionerDaemon(dialCtx context.Context, name st return nil, xerrors.Errorf("unable to fetch default org for in memory provisioner: %w", err) } + dbTypes := make([]database.ProvisionerType, 0, len(provisionerTypes)) + for _, tp := range provisionerTypes { + dbTypes = append(dbTypes, database.ProvisionerType(tp)) + } + //nolint:gocritic // in-memory provisioners are owned by system daemon, err := api.Database.UpsertProvisionerDaemon(dbauthz.AsSystemRestricted(dialCtx), database.UpsertProvisionerDaemonParams{ Name: name, OrganizationID: defaultOrg.ID, CreatedAt: dbtime.Now(), - Provisioners: []database.ProvisionerType{ - database.ProvisionerTypeEcho, database.ProvisionerTypeTerraform, - }, - Tags: provisionersdk.MutateTags(uuid.Nil, nil), - LastSeenAt: sql.NullTime{Time: dbtime.Now(), Valid: true}, - Version: buildinfo.Version(), - APIVersion: proto.CurrentVersion.String(), + Provisioners: dbTypes, + Tags: provisionersdk.MutateTags(uuid.Nil, nil), + LastSeenAt: sql.NullTime{Time: dbtime.Now(), Valid: true}, + Version: buildinfo.Version(), + APIVersion: proto.CurrentVersion.String(), }) if err != nil { return nil, xerrors.Errorf("failed to create in-memory provisioner daemon: %w", err) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index b3288c84b8..14a2fb9231 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -578,7 +578,7 @@ func NewProvisionerDaemon(t testing.TB, coderAPI *coderd.API) io.Closer { }() daemon := provisionerd.New(func(dialCtx context.Context) (provisionerdproto.DRPCProvisionerDaemonClient, error) { - return coderAPI.CreateInMemoryProvisionerDaemon(dialCtx, "test") + return coderAPI.CreateInMemoryProvisionerDaemon(dialCtx, "test", []codersdk.ProvisionerType{codersdk.ProvisionerTypeEcho}) }, &provisionerd.Options{ Logger: coderAPI.Logger.Named("provisionerd").Leveled(slog.LevelDebug), UpdateInterval: 250 * time.Millisecond, diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 8b11e2c827..2aa675727b 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -406,12 +406,13 @@ type ExternalAuthConfig struct { } type ProvisionerConfig struct { - Daemons serpent.Int64 `json:"daemons" typescript:",notnull"` - DaemonsEcho serpent.Bool `json:"daemons_echo" typescript:",notnull"` - DaemonPollInterval serpent.Duration `json:"daemon_poll_interval" typescript:",notnull"` - DaemonPollJitter serpent.Duration `json:"daemon_poll_jitter" typescript:",notnull"` - ForceCancelInterval serpent.Duration `json:"force_cancel_interval" typescript:",notnull"` - DaemonPSK serpent.String `json:"daemon_psk" typescript:",notnull"` + // Daemons is the number of built-in terraform provisioners. + Daemons serpent.Int64 `json:"daemons" typescript:",notnull"` + DaemonTypes serpent.StringArray `json:"daemon_types" typescript:",notnull"` + DaemonPollInterval serpent.Duration `json:"daemon_poll_interval" typescript:",notnull"` + DaemonPollJitter serpent.Duration `json:"daemon_poll_jitter" typescript:",notnull"` + ForceCancelInterval serpent.Duration `json:"force_cancel_interval" typescript:",notnull"` + DaemonPSK serpent.String `json:"daemon_psk" typescript:",notnull"` } type RateLimitConfig struct { @@ -1413,15 +1414,30 @@ when required by your organization's security policy.`, YAML: "daemons", }, { - Name: "Echo Provisioner", - Description: "Whether to use echo provisioner daemons instead of Terraform. This is for E2E tests.", - Flag: "provisioner-daemons-echo", - Env: "CODER_PROVISIONER_DAEMONS_ECHO", - Hidden: true, - Default: "false", - Value: &c.Provisioner.DaemonsEcho, - Group: &deploymentGroupProvisioning, - YAML: "daemonsEcho", + Name: "Provisioner Daemon Types", + Description: fmt.Sprintf("The supported job types for the built-in provisioners. By default, this is only the terraform type. Supported types: %s.", + strings.Join([]string{ + string(ProvisionerTypeTerraform), string(ProvisionerTypeEcho), + }, ",")), + Flag: "provisioner-types", + Env: "CODER_PROVISIONER_TYPES", + Hidden: true, + Default: string(ProvisionerTypeTerraform), + Value: serpent.Validate(&c.Provisioner.DaemonTypes, func(values *serpent.StringArray) error { + if values == nil { + return nil + } + + for _, value := range *values { + if err := ProvisionerTypeValid(value); err != nil { + return err + } + } + + return nil + }), + Group: &deploymentGroupProvisioning, + YAML: "daemonTypes", }, { Name: "Poll Interval", diff --git a/codersdk/organizations.go b/codersdk/organizations.go index cb353dff27..f887d5ea4d 100644 --- a/codersdk/organizations.go +++ b/codersdk/organizations.go @@ -27,6 +27,17 @@ const ( ProvisionerTypeTerraform ProvisionerType = "terraform" ) +// ProvisionerTypeValid accepts string or ProvisionerType for easier usage. +// Will validate the enum is in the set. +func ProvisionerTypeValid[T ProvisionerType | string](pt T) error { + switch string(pt) { + case string(ProvisionerTypeEcho), string(ProvisionerTypeTerraform): + return nil + default: + return fmt.Errorf("provisioner type '%s' is not supported", pt) + } +} + // Organization is the JSON representation of a Coder organization. type Organization struct { ID uuid.UUID `table:"id" json:"id" validate:"required" format:"uuid"` diff --git a/docs/api/general.md b/docs/api/general.md index 02399a6e3e..52313409cb 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -325,8 +325,8 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "daemon_poll_interval": 0, "daemon_poll_jitter": 0, "daemon_psk": "string", + "daemon_types": ["string"], "daemons": 0, - "daemons_echo": true, "force_cancel_interval": 0 }, "proxy_health_status_interval": 0, diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 040e6adcdd..26b38a7c1e 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -2053,8 +2053,8 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "daemon_poll_interval": 0, "daemon_poll_jitter": 0, "daemon_psk": "string", + "daemon_types": ["string"], "daemons": 0, - "daemons_echo": true, "force_cancel_interval": 0 }, "proxy_health_status_interval": 0, @@ -2426,8 +2426,8 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "daemon_poll_interval": 0, "daemon_poll_jitter": 0, "daemon_psk": "string", + "daemon_types": ["string"], "daemons": 0, - "daemons_echo": true, "force_cancel_interval": 0 }, "proxy_health_status_interval": 0, @@ -3691,22 +3691,22 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "daemon_poll_interval": 0, "daemon_poll_jitter": 0, "daemon_psk": "string", + "daemon_types": ["string"], "daemons": 0, - "daemons_echo": true, "force_cancel_interval": 0 } ``` ### Properties -| Name | Type | Required | Restrictions | Description | -| ----------------------- | ------- | -------- | ------------ | ----------- | -| `daemon_poll_interval` | integer | false | | | -| `daemon_poll_jitter` | integer | false | | | -| `daemon_psk` | string | false | | | -| `daemons` | integer | false | | | -| `daemons_echo` | boolean | false | | | -| `force_cancel_interval` | integer | false | | | +| Name | Type | Required | Restrictions | Description | +| ----------------------- | --------------- | -------- | ------------ | --------------------------------------------------------- | +| `daemon_poll_interval` | integer | false | | | +| `daemon_poll_jitter` | integer | false | | | +| `daemon_psk` | string | false | | | +| `daemon_types` | array of string | false | | | +| `daemons` | integer | false | | Daemons is the number of built-in terraform provisioners. | +| `force_cancel_interval` | integer | false | | | ## codersdk.ProvisionerDaemon diff --git a/site/e2e/playwright.config.ts b/site/e2e/playwright.config.ts index 7bc8ead8e4..0e10c1ff34 100644 --- a/site/e2e/playwright.config.ts +++ b/site/e2e/playwright.config.ts @@ -59,7 +59,8 @@ export default defineConfig({ "--telemetry=false", "--dangerous-disable-rate-limits", "--provisioner-daemons 10", - "--provisioner-daemons-echo", + // TODO: Enable some terraform provisioners + "--provisioner-types=echo", "--web-terminal-renderer=dom", "--pprof-enable", ] diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 5488d5c161..d91a6b430d 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -838,7 +838,7 @@ export interface PrometheusConfig { // From codersdk/deployment.go export interface ProvisionerConfig { readonly daemons: number; - readonly daemons_echo: boolean; + readonly daemon_types: string[]; readonly daemon_poll_interval: number; readonly daemon_poll_jitter: number; readonly force_cancel_interval: number;