Skip to content

interop: add builder and path options in compositions' groups#1367

Merged
laurentsenta merged 11 commits intomasterfrom
feat/builder-in-composition-simple
Jul 7, 2022
Merged

interop: add builder and path options in compositions' groups#1367
laurentsenta merged 11 commits intomasterfrom
feat/builder-in-composition-simple

Conversation

@laurentsenta
Copy link
Copy Markdown
Contributor

@laurentsenta laurentsenta commented Jun 27, 2022

  • Adds a builder option in groups, that lets a user override the builder used by a test,
  • Adds a build_config.path option for docker:go and docker:generic that lets a user define a subfolder for their test.

We provide ARG PLAN_PATH to the docker:generic container as a helper (a user might also set the value themselves through build config options).

With these two features, we can define a test that builds go and rust code, or go and node, or node, and rust, etc.

Fixes #1357 and #1305

Example of composition.yml

[global]
  plan = "integrations_mixed_builders"
  case = "issue-1357-mix-builder-configuration"
  total_instances = 2
  runner = "local:docker"

[[groups]]
  id = "go"
  instances = { count = 1 }
  builder = "docker:go" # override builder here

  [groups.build_config]
    path = "go/" # define base path here

  [groups.run.test_params]
    expected_implementation = "go"

[[groups]]
  id = "generic"
  instances = { count = 1 }
  builder = "docker:generic" # override builder here

  [groups.build_config]
    path = "generic/" # define base path here
    build_args = { image = "golang:1.16-buster" }

  [groups.run.test_params]
    expected_implementation = "generic"

Tasks

@laurentsenta laurentsenta force-pushed the feat/builder-in-composition-simple branch from 1e99db6 to 419193a Compare June 27, 2022 16:51
@laurentsenta laurentsenta changed the title interop: add builder and path option in grousp interop: add builder and path option in groups Jun 27, 2022
@laurentsenta laurentsenta changed the title interop: add builder and path option in groups interop: add builder and path options in compositions' groups Jun 27, 2022
@laurentsenta laurentsenta force-pushed the feat/builder-in-composition-simple branch 5 times, most recently from 0719a02 to 2c2e7ec Compare June 28, 2022 09:20
COPY . .

ARG PLAN_PATH
RUN cd plan/${PLAN_PATH} && go build -a -o /testplan
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is how you use PLAN_PATH in docker:generic builder.

@laurentsenta
Copy link
Copy Markdown
Contributor Author

@mxinden asking for your review, if you have time to take a look at the API (the manifest in the PR description) and how we use it in the Dockerfile.

Copy link
Copy Markdown
Contributor

@galargh galargh left a comment

Choose a reason for hiding this comment

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

🥇

Copy link
Copy Markdown
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

🚀 Interface looks good to me. That should be all we need to build both Go and Rust.

@@ -0,0 +1,21 @@
name = "integrations"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this manifest.toml still needed even though we have the composition file?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yea, right now they are required, we might eventually drop them later.

@@ -0,0 +1,14 @@
ARG image
FROM ${image} AS builder
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am guessing that this is how I would be passing feature flags from the composition file to the cargo build command? Works for me. Great.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes! And we'd use the PLAN_PATH below to dispatch between go and rust code.


// Trickle builder configuration
for _, grp := range c.Groups {
if grp.Builder == "" {
Copy link
Copy Markdown
Contributor

@brdji brdji Jul 5, 2022

Choose a reason for hiding this comment

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

The only thing I would improve here is avoid code duplication (lines 30-31, when validating the composition file). The check for group and global builder is the same (builder != "")
Maybe extract it to a separate validation function, e.g. validateGroupBuilder(gs Groups) ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the thorough review, you're right,
I dropped the validation in prepareForBuild since that's a different operation.

@laurentsenta laurentsenta force-pushed the feat/builder-in-composition-simple branch from 1454919 to a751b25 Compare July 7, 2022 14:13
@laurentsenta laurentsenta merged commit 42b4355 into master Jul 7, 2022
@laurentsenta laurentsenta deleted the feat/builder-in-composition-simple branch July 7, 2022 15:01
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.

Add builders parameters to groups in composition files

4 participants