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

building from workspace is not possible with inherited or path depencies #40

Open
divagant-martian opened this issue Oct 7, 2022 · 10 comments

Comments

@divagant-martian
Copy link

More of a tracking issue that falls more on testground itself: Using workspaces is not really possible/useful right now.

In a scenario like this:

related-testground-plans
├─ Cargo.toml # a workspace
├─ planA
│   ├── Cargo.toml # specify utility as a path dependency
│   ├── Dockerfile
│   ├── manifest.toml
├─ planA
│   ├── Cargo.toml # specify utility as a path dependency
│   ├── Dockerfile
│   ├── manifest.toml
└─ utility
       └──  Cargo.toml

the utility directory currently is not accessible from the docker build context. An option would be able to define the testplans in the root of the directory but I don't see a way. Maybe related testground/testground#1466
This makes rust workspaces particularly disadvantageous

@divagant-martian
Copy link
Author

@laurentsenta @mxinden would you have a workaround for this? So far I've searched in the documentation ways of setting the dockerfile path from the manifest and have the dockerfiles in the root

related-testground-plans
├─ Cargo.toml # a workspace
├─ planA.Dockerfile
├─ planB.Dockerfile
├─ planA
│   ├── Cargo.toml # specify utility as a path dependency
│   ├── manifest.toml
├─ planA
│   ├── Cargo.toml # specify utility as a path dependency
│   ├── manifest.toml
└─ utility
       └──  Cargo.toml

If there is a way to do this I have not found it

Also moving the dockerfiles one directory up and passing as parameter to the testground cli. This also doesn't seem to work
Any ideas are welcome that keek the workspace

@ackintosh
Copy link
Contributor

ackintosh commented Oct 24, 2022

The followings are how I'm running planA for now. That works but not cool...


# checkout https://github.com/testground/testground/pull/1503 and run `make install`
cd path/to/ackintosh/testground
make install
  • Add a dockerfile param to planA/manifest.toml:
  name = "planA"

  [defaults]
  builder = "docker:generic"
  runner = "local:docker"

  [builders."docker:generic"]
  enabled = true
+ dockerfile = "planA.Dockerfile"

  [[testcases]]
  name = "planA"
  • Copy planA/manifest.toml to the root:
related-testground-plans
├─ Cargo.toml # a workspace
├─ manifest.toml # *** Copied from `planA` ***
├─ planA.Dockerfile
├─ planB.Dockerfile
├─ planA
│   ├── Cargo.toml # specify utility as a path dependency
│   ├── manifest.toml
├─ planB
│   ├── Cargo.toml # specify utility as a path dependency
│   ├── manifest.toml
└─ utility
       └──  Cargo.toml
  • Make sure related-testground-plans is linked to the root:
$ ls -l ~/testground/plans/
related-testground-plans -> /path/to/related-testground-plans
  • Run planA
# NOTE: Pass `related-testground-plans`, not `planA` to the `--plan` param.
testground run single --plan=related-testground-plans --testcase=planA --builder=docker:generic --runner=local:docker --instances=16

You can also run planB by copying planB/manifest.toml to the root.

@laurentsenta
Copy link
Contributor

laurentsenta commented Oct 30, 2022

Thanks for reaching out @divagant-martian and for sharing a solution @ackintosh !

I'm not familiar with rust workspaces and I'd like to take some time to learn about your use case and improve support
for rust workspaces (captured in testground/testground#1498). We're all busy with labweek & IPFS Camp at the moment, but we'll review the Dockerfile PR as soon as we have some bandwidth available.

  • Could you share a minimal repo that we could use as a testsuite?
    I guess having a workspace with one library, and a single plan that records "success" would be enough.

This is how I would do this for now:
(it looks very similar to how we build the ping plan in the https://github.com/libp2p/test-plans/ repository)

Define your test at the root of your workspace (single manifest at the root),
and use a composition file with a path build args which tells testground "use the dockerfile from subfolder xxxx".

The structure would look this:

related-testground-plans
├─ Cargo.toml # a workspace
├─ manifest.toml # *** merged parameter from planA and planB ***
├─ planA
│   ├── Cargo.toml # specify utility as a path dependency
│   ├── Dockerfile
├─ planB
│   ├── Cargo.toml # specify utility as a path dependency
│   ├── Dockerfile
└─ utility
       └──  Cargo.toml

Then in a composition file, use the path parameter to tell testground "please build code from this subpath":

[[groups]]
  id = "plan-a"
  instances = { count = 2 }
  builder = "docker:generic"

  [groups.build_config]
    path = "planA/"

Then in the Dockerfile, copy the whole source folder (COPY . .) but use the PLAN_PATH` arg passed by testground to build the plan's binary:

ARG image
FROM ${image} AS builder

WORKDIR /build
ENV CGO_ENABLED 0
COPY . .

ARG PLAN_PATH

# you can access every subfolders here,
# for example:
RUN cat plan/planA/Cargo.toml
RUN cat plan/planB/Cargo.toml

# you can use `PLAN_PATH` to build the "right" plan:
RUN cd plan/${PLAN_PATH} && cargo build

FROM ${image}
COPY --from=builder /testplan /testplan
EXPOSE 6060
ENTRYPOINT [ "/testplan"]

You can then switch between plan by using a different plan parameter in your composition file.

@divagant-martian
Copy link
Author

@laurentsenta I checked the plans you linked but I'm still not sure how to apply them, haven't tried hard enough tbh so will do that.
Meanwhile here is a repo showing the issue [email protected]:divagant-martian/shared-deps-testground-example.git

Caveats:

  • Right now the issue is compilation of the plan itself so the plan contents are not relevant. Plan A just prints something to stdout
  • The plan to test is plan A, plan B has an empty Dockerfile and empty manifest since they are also not relevant
  • If you check the Cargo.toml of Plan A there are two notable parts there:
    • [dependencies]
      serde_json = { workspace = true }
      utility = { path = "../utility" }
      serde_json is defined to be the same as the workspace one. This means I need access to the root Cargo.toml in compilation time.
      utility is defined as a path dependency, which means having access to the sibling folder utility as well.
  • Each one of those alone produces a different error (which is simply not having access to the right files) but for completeness these are those:
    • Having just the dependency that is referenced as a path
      diff --git a/plan_a/Cargo.toml b/plan_a/Cargo.toml
      index eaf9515..db4b7d2 100644
      --- a/plan_a/Cargo.toml
      +++ b/plan_a/Cargo.toml
      @@ -7,4 +7,4 @@ edition = "2021"
      
       [dependencies]
       serde_json = { workspace = true }
      -utility = { path = "../utility" }
      
      Results in
      Step 7/13 : RUN cd ./plan/ && cargo build --release
       ---> Running in b5a79398ff4a
      error: failed to parse manifest at `/usr/src/test-plan/plan/Cargo.toml`
      Caused by:
      failed to find a workspace root
      
    • Having just the dependency that is inherited from the workspace
      diff --git a/plan_a/Cargo.toml b/plan_a/Cargo.toml
      index eaf9515..88a58a1 100644
      --- a/plan_a/Cargo.toml
      +++ b/plan_a/Cargo.toml
      @@ -6,5 +6,4 @@ edition = "2021"
       # See more keys and their definitions at https://doc.rust-lang.org          /cargo/reference/manifest.html
      
       [dependencies]
      -serde_json = { workspace = true }
       utility = { path = "../utility" }
      
      Results in
      Step 7/13 : RUN cd ./plan/ && cargo build --release
       ---> Running in 226a094f3263
      error: failed to get `utility` as a dependency of package `plan_a v0.1.0 (/usr/src/test-plan/plan)`
      
      Caused by:
        failed to load source for dependency `utility`
      
      Caused by:
        Unable to update /usr/src/test-plan/utility
      
      Caused by:
        failed to read `/usr/src/test-plan/utility/Cargo.toml`
      
      Caused by:
        No such file or directory (os error 2)
      
  • Running plan_a after import as
    testground run single --plan=plan_a --testcase=plan_a --builder=docker:generic --runner=local:docker --instances=3
    

@laurentsenta
Copy link
Contributor

@divagant-martian

Thanks for sharing your example repo, I took some time to fix it, this might become a useful example.

When you call testground run single --plan=plan_a --builder=docker:generic, testground uploads the test folder: shared-deps-testground-example/plan_a.

By then your docker builder, and cargo build doesn't have access to the utility library or the root Cargo.toml.
This is expected; we don't want the docker:generic builder to upload folders from your machines that are not in your plan.

The solution is to make sure your test plan is defined at the root folder of your source, then use a composition file with a path config to notify the builder which subfolder to use:

  [groups.build_config]
    path = "planA/"

I updated your repo to support workspaces and proposed 2 compositions (hardcoded plus a generic one).
Check the commit history for the steps I followed to fix it, and the README on how to use it:
https://github.com/laurentsenta/shared-deps-testground-example/commits/master

@divagant-martian
Copy link
Author

it took me a bit as well to apply it to a project, but it proved useful. I think it would be nice to document this somewhere. What would be the best place for this?

@divagant-martian
Copy link
Author

Also, After some effort and more than a couple mistakes I managed to cache dependencies in the workspace using https://crates.io/crates/cargo-chef

@laurentsenta
Copy link
Contributor

Nice, could you share your setup? It's probably something we'll want to reuse and share, and eventually turn into a native Testground builder.

At the moment, because this would be mostly rust-specific (workspace, cargo-chef, etc) I'd recommend we create a doc folder in this repo. @mxinden wdyt?

@divagant-martian
Copy link
Author

Here is the setup applied to our example repo divagant-martian/shared-deps-testground-example@941464f

@mxinden
Copy link
Member

mxinden commented Nov 12, 2022

Do I understand that the approach taken with cargo chef is superior to our current caching approach?

sdk-rust/Dockerfile

Lines 4 to 11 in 2b85e4c

# Cache dependencies between test runs,
# See https://blog.mgattozzi.dev/caching-rust-docker-builds/
# And https://github.com/rust-lang/cargo/issues/2644
RUN mkdir -p ./plan/src/
RUN echo "fn main() { println!(\"If you see this message, you may want to clean up the target directory or the Docker build cache.\") }" > ./plan/src/main.rs
COPY ./plan/Cargo.* ./plan/
RUN cd ./plan/ && cargo build

If so, would you mind proposing a pull request to this repository @divagant-martian? That would make the discussion more concrete and thus easier.

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

No branches or pull requests

4 participants