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

Dependency compilation tests should run in a docker container #596

Open
joshuatcasey opened this issue Oct 25, 2022 · 18 comments
Open

Dependency compilation tests should run in a docker container #596

joshuatcasey opened this issue Oct 25, 2022 · 18 comments

Comments

@joshuatcasey
Copy link
Contributor

joshuatcasey commented Oct 25, 2022

Currently dependency compilation tests are run directly on the github actions virtual environment by running make test.

This means that any compilation or dynamic linking that is incompatible with the virtual environment will fail.

One solution is to run the dependency tests in the same way that the dependency compilation is run: As an action. It would be up to the buildpack authors to determine whether the compile and test steps should share dockerfiles. It's quite likely that the test dockerfile could be simpler than the compile docker file.

Another approach is for the workflow to run the docker build and docker run steps so that the buildpack author only need specify dockerfile and entrypoint. This could look like so:

  - name: docker build
    shell: bash
    run: |
      docker build \
        --tag test \
        --file dependency/actions/compile/${{ <some.step.outputs>.target }}.Dockerfile \
        dependency/actions/compile

  - name: docker run
    shell: bash
    run: |
      docker run \
        --volume ${{ <some.step.outputs>.outputDir }}:/home \
        test \
        --version ${{ <some.step.outputs>.version }} \
        --outputDir /home \
        --target ${{ <some.step.outputs>.target }}
@joshuatcasey
Copy link
Contributor Author

See paketo-buildpacks/cpython#450 for context.

@fg-j
Copy link

fg-j commented Oct 31, 2022

Personally, I'd rather see the test step defined as a Github Action in each dependency repo. One fewer bespoke interface to manage!

@ryanmoran
Copy link
Member

ryanmoran commented Oct 31, 2022

I think we'd want the test Dockerfile to base itself off of the most minimal stack run image it supports since that's how it might appear in an actual application. Does this seem right?

@joshuatcasey
Copy link
Contributor Author

@ryanmoran that definitely makes sense to me. At the workflow level we may still require the buildpack author to specify how the tests are run, but it sounds like we do need the workflow to allow that to happen. Likely the most flexible way is through the actions interface.

@sophiewigmore
Copy link
Member

If/when we move to this model, should we remove the make test requirement all together and just have test be a standalone dependency-specific action like compile is? This would be a breaking change for the workflow and we'd need to update the RFC?

The alternative is to leave the make test requirement in the RFC and implementers can just decide how to implement testing, still adhering to a Makefile target.

@fg-j
Copy link

fg-j commented Oct 31, 2022

I'm currently working on bundler, and about to start on the tests. In this case, my compile step only has one target, since I have reason to believe the same dependency will work well for both bionic and jammy. But I want to validate that in the test phase. Ideally, I want a way for the workflow to run my tests for both bionic and jammy stacks. Maybe this means another matrix is needed in the workflows to cover testing across stacks?

@sophiewigmore
Copy link
Member

sophiewigmore commented Nov 1, 2022

@fg-j right I think so, it would be smart to test against multiple stacks.

I think we just need to align on whether we want to change how the test code is set up entirely to move to actions, or if we want to support both using an action-style test code as well as the strategy we have laid out now. I want it to be clear for people what their choices are

@fg-j
Copy link

fg-j commented Nov 1, 2022

If given the choice between a Github Action and a make target that runs locally, I think I'd always opt for the Github Action. But am I missing relevant cases? When would it be better to not run a dependency's tests in containers?

@sophiewigmore
Copy link
Member

I think mostly it's just ever so slightly easier to write a make target than an action - but everyone on our team has experience writing actions so probably not that much of a big deal. It might be a pain to mandate an action specifically though since all the dependencies that have already been implemented will have to change.

@fg-j
Copy link

fg-j commented Nov 1, 2022

Since I haven't yet implemented any dependency tests, happy to hear from folks who have on this! Probably @TisVictress, @joshuatcasey, @robdimsdale and @ForestEckhardt?

@joshuatcasey
Copy link
Contributor Author

The idea behind make initially was that running the tasks locally would necessarily have first-class support.

That being said, an up-to-date README with steps for how to emulate the action locally using docker run is probably just as good.

Both actions and make can use the docker daemon available on the GHA virtual environments to run the testing step on any variety of docker images: custom, stack run images, etc. I can definitely see a testing solution for bundler that runs tests on bionic and jammy in series, hidden inside a testing action.

@TisVictress
Copy link

I like the make approach mainly because I don't like messing with GHA configurations. On the other hand, I can see how updating github-config could prevent authors from not testing on both stacks by accident. I think it would forcibly make people more aware of testing on different stacks while getting those workflows to go green.

@robdimsdale
Copy link
Member

Setting aside the question of adhering to the RFC because we can edit it after the fact if we find we don't like the direction we originally proposed, I honestly don't have strong opinions either way.

I'd lean slightly towards GitHub Actions over make for two reasons:

  1. It's more idiomatic for what the Paketo org currently does (i.e. we use GHA, we don't use make)
  2. I am unlikely to ever want to run the compilation/test steps locally. I'd generally always run the action from a branch

@TisVictress
Copy link

2. I am unlikely to ever want to run the compilation/test steps locally. I'd generally always run the action from a branch

@robdimsdale That's interesting. I find testing locally makes things easier for debugging. This could also be my bias against github in general 😆

@robdimsdale
Copy link
Member

That's interesting. I find testing locally makes things easier for debugging. This could also be my bias against github in general 😆

Great point @TisVictress - I haven't actually iterated much on the new dependency workflow - I've just tried to run it to get the new versions of a dependency.

Do you think clear instructions in a README (e.g. docker run ...) would be sufficient for your local debugging?

@TisVictress
Copy link

Do you think clear instructions in a README (e.g. docker run ...) would be sufficient for your local debugging?

Yea I think a good README will suffice.

@sophiewigmore
Copy link
Member

sophiewigmore commented Nov 3, 2022

Alright, it sounds like the general consensus is that moving to an actions-based approach for the tests would be favourable in solving this issue if:

  • its well documented about how to run locally in a README
  • Dockerfile is based on the most minimal stack run image allowed

Implementing this for our dependencies will include:

  • Modifying RFC to specify that we will take an actions-based approach for the tests
  • Modifications to the workflow to accommodate for looking for/running test code as an action instead of a make target
  • Updating any already-implemented dependencies to use the new actions approach

While this change will be breaking for dependency code that has already been implemented, I don't think its worth supporting both options just for consistency's sake and keeping things simple to maintain. Happy to concede this point if people feel strongly about supporting both. Give me a 👍 if you're good with this proposal and we can get the ball rolling.

@sophiewigmore
Copy link
Member

Update on this: we will eventually do this, but at this point our first priority will be rolling out the original dependency workflows/approach to all of the dependencies so that we can enable Jammy stack support ASAP. The changes outlined here are useful but will be breaking.

In the meantime, if anyone needs to test against multiple stack images, you can still do this with the existing set-up, sort of like what @joshuatcasey did in paketo-buildpacks/passenger#296

@sophiewigmore sophiewigmore removed their assignment Dec 9, 2022
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

6 participants