Initial go-provider integration #890

blampe · 2023-12-12T20:32:19Z

NB: This the first PR on top of the buildkit feature branch, which I've just cut from master.

This extends the hybrid provider to also include a native buildx provider implemented with pulumi-go-provider.

The image resource doesn't do anything yet, but tests are included for 100% coverage. I'd like to keep that number as high as possible.

Fixes #884.

iwahbe

LGTM

guineveresaenger

My main concerns are around the practicality of handling Yet Another Provider here. Is there a plan for reconciliation once we deem this resource production ready?

of course it is unlikely that users will use all three providers simultaneously, except perhaps as apart of an migration/upgrade.

guineveresaenger · 2023-12-14T19:48:16Z

provider/hybrid.go

@@ -27,19 +27,31 @@ type dockerHybridProvider struct {
 	version         string
 	bridgedProvider rpc.ResourceProviderServer
 	nativeProvider  rpc.ResourceProviderServer
+	buildxProvider  rpc.ResourceProviderServer


Curious why we need a whole new provider here - is this specifically so we can run a client config in the provider?

It's strictly for dogfooding go-pulumi-provider, which I'm using to help speed up development. Working at a higher level than resource.PropertyMap is a huge time saver, and the integration test helpers are also great.

guineveresaenger · 2023-12-14T19:58:15Z

provider/resources.go

 						Description: "The builder based on moby/buildkit project",
 					},
 				},
 			},
 		},
 		ExtraResources: map[string]schema.ResourceSpec{
+			dockerResource("buildx", "Image").String(): {


I like this name spacing but am a little concerned that putting this into a new module may make it more difficult to reconcile once the resource is no longer experimental.

Do we have the ability to alias things if this needs to move around? Is there an alternative name you'd suggest?

We do have the ability to alias via the bridge; it's what we did for Imagev3 => v4 and I do believe it supports moving resources between modules as well. I was talking more from a rollout strategy point.

guineveresaenger · 2023-12-14T19:58:58Z

provider/internal/buildx_test.go

+
+func newServer() integration.Server {
+	provider := NewBuildxProvider()
+	return integration.NewServer("docker", semver.Version{Major: 4}, provider)


this is very cool!

blampe · 2023-12-15T19:54:53Z

E2E buildx tests #996
Support inline dockerfiles, named and remote contexts #1000
API types for caches, exports, and platforms #993
Multi-platform caching for buildx #981
Handle unknown inputs and buildOnPreview #980
Share build context hashing #962
buildx auth #948
buildx.Image MVP #899
Upgrade Docker SDK #898
Initial go-provider integration #890 👈
feature/buildkit

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @blampe and the rest of your teammates on Graphite

blampe

My main concerns are around the practicality of handling Yet Another Provider here. Is there a plan for reconciliation once we deem this resource production ready?

I think "provider" is a bit overloaded here. From the user's perspective, a provider is just a gRPC server.

From our perspective as maintainers, the "providers" here are effectively just gRPC handlers, and we happen to have 3 ways of handling requests. There's nothing wrong with that, and I'd want to understand the value we'd get from reconciling before considering it.

of course it is unlikely that users will use all three providers simultaneously, except perhaps as apart of an migration/upgrade.

There shouldn't be any issue mixing/matching the provider implementation, and we should expect users to at least have Image resources coexisting with buildx.Image. I'll add a test around this as part of the auth work.

blampe · 2023-12-15T21:06:38Z

provider/hybrid.go

@@ -27,19 +27,31 @@ type dockerHybridProvider struct {
 	version         string
 	bridgedProvider rpc.ResourceProviderServer
 	nativeProvider  rpc.ResourceProviderServer
+	buildxProvider  rpc.ResourceProviderServer


It's strictly for dogfooding go-pulumi-provider, which I'm using to help speed up development. Working at a higher level than resource.PropertyMap is a huge time saver, and the integration test helpers are also great.

blampe · 2023-12-15T21:08:21Z

provider/resources.go

 						Description: "The builder based on moby/buildkit project",
 					},
 				},
 			},
 		},
 		ExtraResources: map[string]schema.ResourceSpec{
+			dockerResource("buildx", "Image").String(): {


Do we have the ability to alias things if this needs to move around? Is there an alternative name you'd suggest?

github-actions · 2024-01-18T17:28:48Z

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

buildx/image.Image

Maintainer note: consult the runbook for dealing with any breaking changes.

This essentially exposes `newProvider` so we can attach an existing `HostClient` to it. I only see this used in one test, so I opted for a breaking change instead of adding another function. Refs pulumi/pulumi-docker#890

blampe requested review from guineveresaenger and iwahbe December 12, 2023 20:32

iwahbe approved these changes Dec 13, 2023

View reviewed changes

guineveresaenger reviewed Dec 14, 2023

View reviewed changes

This was referenced Dec 15, 2023

Upgrade Docker SDK #898

Closed

buildx.Image MVP #899

Closed

blampe commented Dec 15, 2023

View reviewed changes

blampe force-pushed the 884-pulumi-go-provider branch from d3f1104 to 85d05da Compare December 21, 2023 21:54

blampe mentioned this pull request Jan 18, 2024

buildx auth #948

Closed

blampe force-pushed the 884-pulumi-go-provider branch from 85d05da to cc147c0 Compare January 18, 2024 17:25

blampe force-pushed the 884-pulumi-go-provider branch from 7965d25 to 34a13a0 Compare January 18, 2024 20:01

This was referenced Jan 26, 2024

Share build context hashing #962

Closed

Enable attaching RawServer to existing HostClient pulumi/pulumi-go-provider#188

Merged

This was referenced Jan 30, 2024

Native buildx resource prototype #885

Closed

Handle unknown inputs and buildOnPreview #980

Closed

Multi-platform caching for buildx #981

Closed

blampe force-pushed the 884-pulumi-go-provider branch 2 times, most recently from 76ad325 to 9fb4662 Compare February 9, 2024 22:22

blampe mentioned this pull request Feb 13, 2024

API types for caches, exports, and platforms #993

Closed

blampe force-pushed the 884-pulumi-go-provider branch from 9fb4662 to 64676b2 Compare February 15, 2024 19:13

blampe mentioned this pull request Feb 15, 2024

E2E buildx tests #996

Closed

blampe force-pushed the 884-pulumi-go-provider branch from 64676b2 to ee294d5 Compare February 15, 2024 20:24

blampe mentioned this pull request Feb 16, 2024

Support inline dockerfiles, named and remote contexts #1000

Closed

blampe force-pushed the 884-pulumi-go-provider branch from ee294d5 to d3a3b24 Compare February 27, 2024 19:14

blampe added 3 commits March 5, 2024 11:15

go-provider scaffolding

382a4c9

generate sdk

a0fa698

Generate schema from inferred resource

d418b47

blampe added 7 commits March 5, 2024 11:15

Regenerate SDK

3309e7c

lint

66df4ee

more robust bridging

c4c1f5a

lint

1960989

regenerate sdk

4ab7634

fix RawServer host

8c0f36c

bump go-provider

c124573

blampe force-pushed the 884-pulumi-go-provider branch from d3a3b24 to c124573 Compare March 5, 2024 19:17

blampe closed this Apr 19, 2024

mjeffryes assigned blampe Apr 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial go-provider integration #890

Initial go-provider integration #890

blampe commented Dec 12, 2023

iwahbe left a comment

guineveresaenger left a comment

guineveresaenger Dec 14, 2023

blampe Dec 15, 2023

guineveresaenger Dec 14, 2023

blampe Dec 15, 2023

guineveresaenger Dec 27, 2023

guineveresaenger Dec 14, 2023

blampe commented Dec 15, 2023 •

edited

Loading

blampe left a comment

blampe Dec 15, 2023

blampe Dec 15, 2023

github-actions bot commented Jan 18, 2024 •

edited

Loading

Initial go-provider integration #890

Initial go-provider integration #890

Conversation

blampe commented Dec 12, 2023

iwahbe left a comment

Choose a reason for hiding this comment

guineveresaenger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blampe commented Dec 15, 2023 • edited Loading

blampe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 18, 2024 • edited Loading

Does the PR have any schema changes?

Does the PR have any schema changes?

New resources:

blampe commented Dec 15, 2023 •

edited

Loading

github-actions bot commented Jan 18, 2024 •

edited

Loading