8000 Merge branch 'dk/is-prebuild-claim' of github.com:/coder/coder into d… · coder/coder@ce63b24 · GitHub
[go: up one dir, main page]
Skip to content

Commit ce63b24

Browse files
dannykoppingdannykopping
committed
Merge branch 'dk/is-prebuild-claim' of github.com:/coder/coder into dk/logreplacements
2 parents 11a2c5a + d0f00ce commit ce63b24

File tree

18 files changed

+104
-57
lines changed
  • wsbuilder
  • enterprise/coderd/prebuilds
  • go.mod
  • go.sum
  • provisionerd/proto
  • provisionersdk/proto
  • provisioner/terraform
  • site/e2e

    • 18 files changed

    +104
    -57
    lines changed

    ‎cli/testdata/coder_provisioner_list_--output_json.golden

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -7,7 +7,7 @@
    77
    "last_seen_at": "====[timestamp]=====",
    88
    "name": "test",
    99
    "version": "v0.0.0-devel",
    10-
    "api_version": "1.4",
    10+
    "api_version": "1.5",
    1111
    "provisioners": [
    1212
    "echo"
    1313
    ],

    ‎coderd/prebuilds/api.go

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -32,7 +32,7 @@ type ReconciliationOrchestrator interface {
    3232
    // TrackResourceReplacement handles a pathological situation whereby a terraform resource is replaced due to drift,
    3333
    // which can obviate the whole point of pre-provisioning a prebuilt workspace.
    3434
    // See more detail at https://coder.com/docs/admin/templates/extending-templates/prebuilt-workspaces.md#preventing-resource-replacement.
    35-
    TrackResourceReplacement(ctx context.Context, workspaceID, buildID, claimantID uuid.UUID, replacements []*sdkproto.ResourceReplacement)
    35+
    TrackResourceReplacement(ctx context.Context, workspaceID, buildID uuid.UUID, replacements []*sdkproto.ResourceReplacement)
    3636
    }
    3737

    3838
    type Reconciler interface {

    ‎coderd/prebuilds/noop.go

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -13,7 +13,7 @@ type NoopReconciler struct{}
    1313

    1414
    func (NoopReconciler) Run(context.Context) {}
    1515
    func (NoopReconciler) Stop(context.Context, error) {}
    16-
    func (NoopReconciler) TrackResourceReplacement(context.Context, uuid.UUID, uuid.UUID, uuid.UUID, []*sdkproto.ResourceReplacement) {
    16+
    func (NoopReconciler) TrackResourceReplacement(context.Context, uuid.UUID, uuid.UUID, []*sdkproto.ResourceReplacement) {
    1717
    }
    1818
    func (NoopReconciler) ReconcileAll(context.Context) error { return nil }
    1919
    func (NoopReconciler) SnapshotState(context.Context, database.Store) (*GlobalSnapshot, error) {

    ‎coderd/provisionerdserver/provisionerdserver.go

    Lines changed: 7 additions & 12 deletions
    Original file line numberDiff line numberDiff line change
    @@ -620,11 +620,6 @@ func (s *server) acquireProtoJob(ctx context.Context, job database.ProvisionerJo
    620620
    }
    621621
    }
    622622

    623-
    var prebuildClaimedForUserID string
    624-
    if input.PrebuildClaimedByUser != uuid.Nil {
    625-
    prebuildClaimedForUserID = input.PrebuildClaimedByUser.String()
    626-
    }
    627-
    628623
    protoJob.Type = &proto.AcquiredJob_WorkspaceBuild_{
    629624
    WorkspaceBuild: &proto.AcquiredJob_WorkspaceBuild{
    630625
    WorkspaceBuildId: workspaceBuild.ID.String(),
    @@ -654,7 +649,7 @@ func (s *server) acquireProtoJob(ctx context.Context, job database.ProvisionerJo
    654649
    WorkspaceOwnerLoginType: string(owner.LoginType),
    655650
    WorkspaceOwnerRbacRoles: ownerRbacRoles,
    656651
    IsPrebuild: input.IsPrebuild,
    657-
    PrebuildClaimForUserId: prebuildClaimedForUserID,
    652+
    IsPrebuildClaim: input.IsPrebuildClaim,
    658653
    },
    659654
    LogLevel: input.LogLevel,
    660655
    },
    @@ -1736,7 +1731,7 @@ func (s *server) CompleteJob(ctx context.Context, completed *proto.CompletedJob)
    17361731
    orchestrator := s.PrebuildsOrchestrator.Load()
    17371732
    if resourceReplacements := completed.GetWorkspaceBuild().GetResourceReplacements(); orchestrator != nil && len(resourceReplacements) > 0 {
    17381733
    // Fire and forget.
    1739-
    go (*orchestrator).TrackResourceReplacement(context.Background(), workspace.ID, workspaceBuild.ID, input.PrebuildClaimedByUser, resourceReplacements)
    1734+
    go (*orchestrator).TrackResourceReplacement(context.Background(), workspace.ID, workspaceBuild.ID, resourceReplacements)
    17401735
    }
    17411736
    }
    17421737

    @@ -2489,11 +2484,11 @@ type TemplateVersionImportJob struct {
    24892484

    24902485
    // WorkspaceProvisionJob is the payload for the "workspace_provision" job type.
    24912486
    type WorkspaceProvisionJob struct {
    2492-
    WorkspaceBuildID uuid.UUID `json:"workspace_build_id"`
    2493-
    DryRun bool `json:"dry_run"`
    2494-
    IsPrebuild bool `json:"is_prebuild,omitempty"`
    2495-
    PrebuildClaimedByUser uuid.UUID `json:"prebuild_claimed_by,omitempty"`
    2496-
    LogLevel string `json:"log_level,omitempty"`
    2487+
    WorkspaceBuildID uuid.UUID `json:"workspace_build_id"`
    2488+
    DryRun bool `json:"dry_run"`
    2489+
    IsPrebuild bool `json:"is_prebuild,omitempty"`
    2490+
    IsPrebuildClaim bool `json:"is_prebuild_claim,omitempty"`
    2491+
    LogLevel string `json:"log_level,omitempty"`
    24972492
    }
    24982493

    24992494
    // TemplateVersionDryRunJob is the payload for the "template_version_dry_run" job type.

    ‎coderd/provisionerdserver/provisionerdserver_test.go

    Lines changed: 3 additions & 3 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1798,9 +1798,9 @@ func TestCompleteJob(t *testing.T) {
    17981798
    Input: must(json.Marshal(provisionerdserver.WorkspaceProvisionJob{
    17991799
    WorkspaceBuildID: build.ID,
    18001800

    1801+
    IsPrebuild: false,
    18011802
    // Mark the job as a prebuilt workspace claim.
    1802-
    PrebuildClaimedByUser: uuid.New(),
    1803-
    IsPrebuild: false,
    1803+
    IsPrebuildClaim: true,
    18041804
    })),
    18051805
    OrganizationID: pd.OrganizationID,
    18061806
    })
    @@ -1847,7 +1847,7 @@ type mockPrebuildsOrchestrator struct {
    18471847
    done chan struct{}
    18481848
    }
    18491849

    1850-
    func (m *mockPrebuildsOrchestrator) TrackResourceReplacement(_ context.Context, _, _, _ uuid.UUID, replacements []*sdkproto.ResourceReplacement) {
    1850+
    func (m *mockPrebuildsOrchestrator) TrackResourceReplacement(_ context.Context, _, _ uuid.UUID, replacements []*sdkproto.ResourceReplacement) {
    18511851
    m.replacements = replacements
    18521852
    m.done <- struct{}{}
    18531853
    }

    ‎coderd/workspaces.go

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -713,7 +713,7 @@ func createWorkspace(
    713713
    builder = builder.TemplateVersionPresetID(req.TemplateVersionPresetID)
    714714
    }
    715715
    if claimedWorkspace != nil {
    716-
    builder = builder.MarkPrebuildClaimedBy(owner.ID)
    716+
    builder = builder.MarkPrebuildClaim()
    717717
    }
    718718

    719719
    if req.EnableDynamicParameters && api.Experiments.Enabled(codersdk.ExperimentDynamicParameters) {

    ‎coderd/wsbuilder/wsbuilder.go

    Lines changed: 9 additions & 8 deletions
    Original file line numberDiff line numberDiff line change
    @@ -76,8 +76,7 @@ type Builder struct {
    7676
    parameterValues *[]string
    7777
    templateVersionPresetParameterValues []database.TemplateVersionPresetParameter
    7878

    79-
    prebuild bool
    80-
    prebuildClaimedBy uuid.UUID
    79+
    prebuild, prebuildClaim bool
    8180

    8281
    verifyNoLegacyParametersOnce bool
    8382
    }
    @@ -174,15 +173,17 @@ func (b Builder) RichParameterValues(p []codersdk.WorkspaceBuildParameter) Build
    174173
    return b
    175174
    }
    176175

    176+
    // MarkPrebuild indicates that a prebuilt workspace is being built.
    177177
    func (b Builder) MarkPrebuild() Builder {
    178178
    // nolint: revive
    179179
    b.prebuild = true
    180180
    return b
    181181
    }
    182182

    183-
    func (b Builder) MarkPrebuildClaimedBy(userID uuid.UUID) Builder {
    183+
    // MarkPrebuildClaim indicates that a prebuilt workspace is being claimed.
    184+
    func (b Builder) MarkPrebuildClaim() Builder {
    184185
    // nolint: revive
    185-
    b.prebuildClaimedBy = userID
    186+
    b.prebuildClaim = true
    186187
    return b
    187188
    }
    188189

    @@ -322,10 +323,10 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object
    322323

    323324
    workspaceBuildID := uuid.New()
    324325
    input, err := json.Marshal(provisionerdserver.WorkspaceProvisionJob{
    325-
    WorkspaceBuildID: workspaceBuildID,
    326-
    LogLevel: b.logLevel,
    327-
    IsPrebuild: b.prebuild,
    328-
    PrebuildClaimedByUser: b.prebuildClaimedBy,
    326+
    WorkspaceBuildID: workspaceBuildID,
    327+
    LogLevel: b.logLevel,
    328+
    IsPrebuild: b.prebuild,
    329+
    IsPrebuildClaim: b.prebuildClaim,
    329330
    })
    330331
    if err != nil {
    331332
    return nil, nil, nil, BuildError{

    ‎enterprise/coderd/prebuilds/reconcile.go

    Lines changed: 5 additions & 5 deletions
    Original file line numberDiff line numberDiff line change
    @@ -630,21 +630,21 @@ func (c *StoreReconciler) provision(
    630630
    return nil
    631631
    }
    632632

    633-
    func (c *StoreReconciler) TrackResourceReplacement(ctx context.Context, workspaceID, buildID, claimantID uuid.UUID, replacements []*sdkproto.ResourceReplacement) {
    633+
    func (c *StoreReconciler) TrackResourceReplacement(ctx context.Context, workspaceID, buildID uuid.UUID, replacements []*sdkproto.ResourceReplacement) {
    634634
    // Set authorization context since this may be called in the background (i.e. with a bare context).
    635635
    // nolint:gocritic // Necessary to query all the required data.
    636636
    ctx = dbauthz.AsSystemRestricted(ctx)
    637637
    // Since this may be called in a fire-and-forget fashion, we need to give up at some point.
    638638
    trackCtx, trackCancel := context.WithTimeout(ctx, time.Minute)
    639639
    defer trackCancel()
    640640

    641-
    if err := c.trackResourceReplacement(trackCtx, workspaceID, buildID, claimantID, replacements); err != nil {
    641+
    if err := c.trackResourceReplacement(trackCtx, workspaceID, buildID, replacements); err != nil {
    642642
    c.logger.Error(ctx, "failed to track resource replacement", slog.Error(err))
    643643
    }
    644644
    }
    645645

    646646
    // nolint:revive // Shut up it's fine.
    647-
    func (c *StoreReconciler) trackResourceReplacement(ctx context.Context, workspaceID, buildID, claimantID uuid.UUID, replacements []*sdkproto.ResourceReplacement) error {
    647+
    func (c *StoreReconciler) trackResourceReplacement(ctx context.Context, workspaceID, buildID uuid.UUID, replacements []*sdkproto.ResourceReplacement) error {
    648648
    if err := ctx.Err(); err != nil {
    649649
    return err
    650650
    }
    @@ -677,9 +677,9 @@ func (c *StoreReconciler) trackResourceReplacement(ctx context.Context, workspac
    677677
    return xerrors.Errorf("fetch template preset for template version ID %q: %w", prebuild.TemplateVersionID.String(), err)
    678678
    }
    679679

    680-
    claimant, err := c.store.GetUserByID(ctx, claimantID)
    680+
    claimant, err := c.store.GetUserByID(ctx, workspace.OwnerID) // At this point, the workspace is owned by the new owner.
    681681
    if err != nil {
    682-
    return xerrors.Errorf("fetch claimant %q: %w", claimantID.String(), err)
    682+
    return xerrors.Errorf("fetch claimant %q: %w", workspace.OwnerID.String(), err)
    683683
    }
    684684

    685685
    // Use the claiming build here (not prebuild) because both should be equivalent, and we might as well spot inconsistencies now.

    ‎enterprise/coderd/prebuilds/reconcile_test.go

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -979,7 +979,7 @@ func TestTrackResourceReplacement(t *testing.T) {
    979979
    }))
    980980

    981981
    // When: a claim occurred and resource replacements are detected (_how_ is out of scope of this test).
    982-
    reconciler.TrackResourceReplacement(ctx, prebuiltWorkspace.ID, prebuild.ID, userID, []*sdkproto.ResourceReplacement{
    982+
    reconciler.TrackResourceReplacement(ctx, prebuiltWorkspace.ID, prebuild.ID, []*sdkproto.ResourceReplacement{
    983983
    {
    984984
    Resource: "docker_container[0]",
    985985
    Paths: []string{"env", "image"},

    ‎go.mod

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -101,7 +101,7 @@ require (
    101101
    github.com/coder/quartz v0.1.3
    102102
    github.com/coder/retry v1.5.1
    103103
    github.com/coder/serpent v0.10.0
    104-
    github.com/coder/terraform-provider-coder/v2 v2.4.0
    104+
    github.com/coder/terraform-provider-coder/v2 v2.4.1
    105105
    github.com/coder/websocket v1.8.13
    106106
    github.com/coder/wgtunnel v0.1.13-0.20240522110300-ade90dfb2da0
    107107
    github.com/coreos/go-oidc/v3 v3.14.1

    ‎go.sum

    Lines changed: 2 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -921,8 +921,8 @@ github.com/coder/tailscale v1.1.1-0.20250422090654-5090e715905e h1:nope/SZfoLB9M
    921921
    github.com/coder/tailscale v1.1.1-0.20250422090654-5090e715905e/go.mod h1:1ggFFdHTRjPRu9Yc1yA7nVHBYB50w9Ce7VIXNqcW6Ko=
    922922
    github.com/coder/terraform-config-inspect v0.0.0-20250107175719-6d06d90c630e h1:JNLPDi2P73laR1oAclY6jWzAbucf70ASAvf5mh2cME0=
    923923
    github.com/coder/terraform-config-inspect v0.0.0-20250107175719-6d06d90c630e/go.mod h1:Gz/z9Hbn+4KSp8A2FBtNszfLSdT2Tn/uAKGuVqqWmDI=
    924-
    github.com/coder/terraform-provider-coder/v2 v2.4.0 h1:uuFmF03IyahAZLXEukOdmvV9hGfUMJSESD8+G5wkTcM=
    925-
    github.com/coder/terraform-provider-coder/v2 v2.4.0/go.mod h1:2kaBpn5k9ZWtgKq5k4JbkVZG9DzEqR4mJSmpdshcO+s=
    924+
    github.com/coder/terraform-provider-coder/v2 v2.4.1 h1:+HxLJVENJ+kvGhibQ0jbr8Evi6M857d9691ytxNbv90=
    925+
    github.com/coder/terraform-provider-coder/v2 v2.4.1/go.mod h1:2kaBpn5k9ZWtgKq5k4JbkVZG9DzEqR4mJSmpdshcO+s=
    926926
    github.com/coder/trivy v0.0.0-20250409153844-e6b004bc465a h1:yryP7e+IQUAArlycH4hQrjXQ64eRNbxsV5/wuVXHgME=
    927927
    github.com/coder/trivy v0.0.0-20250409153844-e6b004bc465a/go.mod h1:dDvq9axp3kZsT63gY2Znd1iwzfqDq3kXbQnccIrjRYY=
    928928
    github.com/coder/websocket v1.8.13 h1:f3QZdXy7uGVz+4uCJy2nTZyM0yTBj8yANEHhqlXZ9FE=

    ‎provisioner/terraform/executor.go

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -315,7 +315,7 @@ func (e *executor) plan(ctx, killCtx context.Context, env, vars []string, logr l
    315315
    // When a prebuild claim attempt is made, log a warning if a resource is due to be replaced, since this will obviate
    316316
    // the point of prebuilding if the expensive resource is replaced once claimed!
    317317
    var (
    318-
    isPrebuildClaimAttempt = !destroy && metadata.GetPrebuildClaimForUserId() != ""
    318+
    isPrebuildClaimAttempt = !destroy && metadata.GetIsPrebuildClaim()
    319319
    resReps []*proto.ResourceReplacement
    320320
    )
    321321
    if repsFromPlan := findResourceReplacements(plan); len(repsFromPlan) > 0 {

    ‎provisioner/terraform/provision.go

    Lines changed: 3 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -270,6 +270,9 @@ func provisionEnv(
    270270
    if metadata.GetIsPrebuild() {
    271271
    env = append(env, provider.IsPrebuildEnvironmentVariable()+"=true")
    272272
    }
    273+
    if metadata.GetIsPrebuildClaim() {
    274+
    env = append(env, provider.IsPrebuildClaimEnvironmentVariable()+"=true")
    275+
    }
    273276

    274277
    for key, value := range provisionersdk.AgentScriptEnv() {
    275278
    env = append(env, key+"="+value)

    ‎provisioner/terraform/provision_test.go

    Lines changed: 43 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -25,6 +25,7 @@ import (
    2525

    2626
    "cdr.dev/slog"
    2727
    "cdr.dev/slog/sloggers/slogtest"
    28+
    2829
    "github.com/coder/coder/v2/codersdk/drpc"
    2930
    "github.com/coder/coder/v2/provisioner/terraform"
    3031
    "github.com/coder/coder/v2/provisionersdk"
    @@ -977,7 +978,7 @@ func TestProvision(t *testing.T) {
    977978
    required_providers {
    978979
    coder = {
    979980
    source = "coder/coder"
    980-
    version = "2.3.0-pre2"
    981+
    version = ">= 2.4.1"
    981982
    }
    982983
    }
    983984
    }
    @@ -994,7 +995,8 @@ func TestProvision(t *testing.T) {
    994995
    },
    995996
    Request: &proto.PlanRequest{
    996997
    Metadata: &proto.Metadata{
    997-
    IsPrebuild: true,
    998+
    IsPrebuild: true,
    999+
    IsPrebuildClaim: false,
    9981000
    },
    9991001
    },
    10001002
    Response: &proto.PlanComplete{
    @@ -1008,6 +1010,45 @@ func TestProvision(t *testing.T) {
    10081010
    }},
    10091011
    },
    10101012
    },
    1013+
    {
    1014+
    Name: "is-prebuild-claim",
    1015+
    Files: map[string]string{
    1016+
    "main.tf": `terraform {
    1017+
    required_providers {
    1018+
    coder = {
    1019+
    source = "coder/coder"
    1020+
    version = ">= 2.4.1"
    1021+
    }
    1022+
    }
    1023+
    }
    1024+
    data "coder_workspace" "me" {}
    1025+
    resource "null_resource" "example" {}
    1026+
    resource "coder_metadata" "example" {
    1027+
    resource_id = null_resource.example.id
    1028+
    item {
    1029+
    key = "is_prebuild_claim"
    1030+
    value = data.coder_workspace.me.is_prebuild_claim
    1031+
    }
    1032+
    }
    1033+
    `,
    1034+
    },
    1035+
    Request: &proto.PlanRequest{
    1036+
    Metadata: &proto.Metadata{
    1037+
    IsPrebuild: false,
    1038+
    IsPrebuildClaim: true,
    1039+
    },
    1040+
    },
    1041+
    Response: &proto.PlanComplete{
    1042+
    Resources: []*proto.Resource{{
    1043+
    Name: "example",
    1044+
    Type: "null_resource",
    1045+
    Metadata: []*proto.Resource_Metadata{{
    1046+
    Key: "is_prebuild_claim",
    1047+
    Value: "true",
    1048+
    }},
    1049+
    }},
    1050+
    },
    1051+
    },
    10111052
    }
    10121053

    10131054
    // Remove unused cache dirs before running tests.

    ‎provisionerd/proto/version.go

    Lines changed: 6 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -12,12 +12,17 @@ import "github.com/coder/coder/v2/apiversion"
    1212
    //
    1313
    // API v1.4:
    1414
    // - Add new field named `devcontainers` in the Agent.
    15+
    //
    16+
    // API v1.5:
    17+
    // - Add new field named `is_prebuild_claim` in the Metadata message.
    1518
    const (
    1619
    CurrentMajor = 1
    17-
    CurrentMinor = 4
    20+
    CurrentMinor = 5
    1821
    )
    1922

    2023
    // CurrentVersion is the current provisionerd API version.
    2124
    // Breaking changes to the provisionerd API **MUST** increment
    2225
    // CurrentMajor above.
    26+
    // Non-breaking changes to the provisionerd API **MUST** increment
    27+
    // CurrentMinor above.
    2328
    var CurrentVersion = apiversion.New(CurrentMajor, CurrentMinor)

    ‎provisionersdk/proto/provisioner.pb.go

    Lines changed: 10 additions & 11 deletions
    Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

    ‎provisionersdk/proto/provisioner.proto

    Lines changed: 3 additions & 3 deletions
    Original file line numberDiff line numberDiff line change
    @@ -298,9 +298,9 @@ message Metadata {
    298298
    string workspace_build_id = 17;
    299299
    string workspace_owner_login_type = 18;
    300300
    repeated Role workspace_owner_rbac_roles = 19;
    301-
    bool is_prebuild = 20;
    302-
    string running_workspace_agent_token = 21;
    303-
    string prebuild_claim_for_user_id = 22;
    301+
    bool is_prebuild = 20; // Indicates that a prebuilt workspace is being built.
    302+
    string running_workspace_agent_token = 21; // Preserves the running agent token of a prebuilt workspace so it can reinitialize.
    303+
    bool is_prebuild_claim = 22; // Indicates that a prebuilt workspace is being claimed.
    304304
    }
    305305

    306306
    // Config represents execution configuration shared by all subsequent requests in the Session

    0 commit comments

    Comments
     (0)
    0