Skip to content
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

Implement Hybrid Functionality and add custom Image resource #413

Merged
merged 67 commits into from
Nov 18, 2022

Conversation

guineveresaenger
Copy link
Contributor

@guineveresaenger guineveresaenger commented Nov 2, 2022

This pull request restructures this provider to support both TF based and pulumi-native(custom) Resources.
Specifically, this adds Image as an "extra" custom resource to the resources declaration file (resources.go),
and creates a hybrid provider that handles logic to switch between a custom resource and a TF based resource.

Of note

  • To implement hybrid functionality, the call to tfbridge.Main is circumvented and all RPC methods are implemented directly in hybrid.go.
  • The logic to distinguish between a custom resource and a TF resource is currently hardcoded via the dockerImageTok variable, as that is the only custom resource currently needed.
  • The schema contains the foundation for the behavior of Image as it is currently implemented, with the following deviations:
    • digest is discontinued as per deprecation message
    • id is discontinued, as per deprecation message
    • localImageName is discontinued, as users can achieve the same behavior by setting skipPush to true and image tags are lightweight and repeatable.
  • A dockerBuild method uses the Docker client to start implementing much of the expected (current) behavior of Image:
    • Handles user input from the Pulumi program with logic around OneOf (Note that OneOf is not fully supported in all languages so it may make sense to simplify the schema for cacheFrom and build)
    • Build image locally
    • Push to Registry
    • Build context enabled
    • SkipPush enabled
    • Buildtime args enabled
    • Logs build and push output to Pulumi Info
    • Errors properly on failure to push and outputs the docker client error
    • No separate logic for docker tags - this is a bonus, as the client handles this implicitly via looking for a :mytag at the end of the image name (defaults to :latest)
    • TODO: handle registry login for ImagePush to use cacheFrom when set
    • TODO: handle setting of env input to translate to setting the BuilderVersion constant in the docker client
    • TODO: deprecate/gather usage information on extraOptions as we will need to parse them individually as client options rather can command line flags
    • TODO: add behavior for target
    • TODO: add behavior for crossplat build

Edited to add Nov.17:

  • All Docker Image integration tests have been updated and unskipped and are passing (because we have schematized inputs now, several languages will need to migrate from a DockerBuild type to a DockerBuildArgs type).
  • We have full implementation with Checkpoint objects for Create, Update, and Diff
  • Read is currently returning only an ID, as a full Read would be a new feature
  • The unique identifier for the Image resource is imageName - not sure if this is optimal; we may want to extract a build hash here instead.

Fixes #403

@t0yv0
Copy link
Member

t0yv0 commented Nov 3, 2022

It looks like the overlays used to be written by hand for every language and with this instead we get automatic extensibility in the languages dimension, that's a huge win!

Comment on lines +172 to +275
if v := req.GetVersion(); v != 0 {
return nil, fmt.Errorf("unsupported schema version %d", v)
}
return &rpc.GetSchemaResponse{Schema: string(p.schemaBytes)}, nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I don't think this layer of the GetSchema will ever be called since the schema will be vended by the top-level provider interface. This can probably fail with an assertion here instead.

provider/hybrid.go Show resolved Hide resolved
tok := urn.Type().String()
fmt.Println(tok)
// TODO: implement this for actual!!!
if tok == dockerImageToken {
Copy link
Contributor

Choose a reason for hiding this comment

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

Another interesting pattern when it comes to multiplexing custom resource handlers is what is done in the pulumiservice provider: https://github.com/pulumi/pulumi-pulumiservice/blob/main/provider/pkg/provider/provider.go. Essentially you could add a resource interface which could be used to invoke the handlers for each of the supported resources as done in that provider.

@github-actions
Copy link

github-actions bot commented Nov 3, 2022

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

1 similar comment
@github-actions
Copy link

github-actions bot commented Nov 3, 2022

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@guineveresaenger
Copy link
Contributor Author

The integration test failures are expected here as the tests need to be updated.

@github-actions
Copy link

github-actions bot commented Nov 9, 2022

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

8 similar comments
@github-actions
Copy link

github-actions bot commented Nov 9, 2022

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@github-actions
Copy link

github-actions bot commented Nov 9, 2022

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@github-actions
Copy link

github-actions bot commented Nov 9, 2022

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@github-actions
Copy link

github-actions bot commented Nov 9, 2022

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@guineveresaenger guineveresaenger changed the title Implement Hybrid Functionality Implement Hybrid Functionality and add custom Image resource Nov 14, 2022
@guineveresaenger guineveresaenger marked this pull request as ready for review November 14, 2022 20:12
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

6 similar comments
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

1 similar comment
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.

New resources:

  • index/image.Image

@guineveresaenger guineveresaenger merged commit d58ed38 into master Nov 18, 2022
@pulumi-bot pulumi-bot deleted the implement-hybrid branch November 18, 2022 23:04
maravedi added a commit to maravedi/examples that referenced this pull request Aug 30, 2023
docker.DockerBuild was changed to DockerBuildArgs back in Novermber of 2022: pulumi/pulumi-docker#413 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement ExtraResources feature for Docker Image
3 participants