Skip to content

feat: persist prebuild definitions on template import #16951

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 88 commits into from
Apr 7, 2025

Conversation

SasSwart
Copy link
Contributor

@SasSwart SasSwart commented Mar 17, 2025

This PR allows provisioners to recognise and report prebuild definitions to the coder control plane. It also allows the coder control plane to then persist these to its store.

closes coder/internal#507

SasSwart and others added 10 commits March 13, 2025 19:24
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
appeasing linter

Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Copy link
Contributor Author

@SasSwart SasSwart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self Review notes

@SasSwart SasSwart force-pushed the prebuilds-db branch 3 times, most recently from 70ecdcc to 7e062e2 Compare March 17, 2025 08:14
SasSwart and others added 11 commits March 17, 2025 12:11
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
appeasing linter

Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Copy link
Member

@mtojek mtojek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SasSwart Are you sure about the scope of the PR? I see a lot of changes pulled from mainline.

InvalidateAfterSecs: sql.NullInt32{
Int32: 0,
Valid: false,
}, // TODO: implement cache invalidation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re TODO: perhaps create an issue for that and link it here

Copy link
Member

@mtojek mtojek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Base automatically changed from prebuilds-db to main April 3, 2025 08:58
@SasSwart SasSwart merged commit 0b2b643 into main Apr 7, 2025
37 checks passed
@SasSwart SasSwart deleted the jjs/insert-prebuilds branch April 7, 2025 08:35
@github-actions github-actions bot locked and limited conversation to collaborators Apr 7, 2025
Copy link
Contributor

@dannykopping dannykopping left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Post-merge review

@@ -182,7 +182,6 @@ func TestDBAuthzRecursive(t *testing.T) {
method.Name == "PGLocks" {
continue
}
// Log the name of the last method, so if there is a panic, it is
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional deletion?

@@ -883,10 +884,24 @@ func ConvertState(ctx context.Context, modules []*tfjson.StateModule, rawGraph s
)
}

if len(preset.Prebuilds) != 1 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this not warn incorrectly when there are no prebuilds defined at all (which is perfectly valid)?

Shouldn't this be len(preset.Prebuilds) > 1?

@@ -2,7 +2,7 @@ terraform {
required_providers {
coder = {
source = "coder/coder"
version = "2.1.3"
version = "2.3.0-pre2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that coder/terraform-provider-coder#373 is merged we should cut a release and reference it here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recognise and persist prebuild definitions on template import
4 participants