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

Action does not relay that host has docker installed #5

Closed
wants to merge 4 commits into from
Closed

Action does not relay that host has docker installed #5

wants to merge 4 commits into from

Conversation

kfirpeled
Copy link

Changed implementation such that the action would run on docker
This allows the step to run and succeed to copy when docker is not installed on the hosting container

@shrink
Copy link
Owner

shrink commented Nov 12, 2020

Thank you very much for the contribution.

My understanding is that an action that runs using Docker is only compatible with Linux runners, and therefore this change would remove Windows and macOS compatibility from the action.

About actions > Docker container actions
Docker container actions can only execute on runners with a Linux operating system. Self-hosted runners must use a Linux operating system and have Docker installed to run Docker container actions.

Perhaps if you can describe your use-case we can come up with a solution that is cross-platform: is there a reason you're using a runner that doesn't have docker installed (and can't have docker installed)?

@shrink shrink self-assigned this Nov 12, 2020
@shrink shrink self-requested a review November 12, 2020 21:27
@shrink shrink added the enhancement New feature or request label Nov 12, 2020
@kfirpeled
Copy link
Author

kfirpeled commented Nov 15, 2020

:)
So I haven't tested it cross platform, so I cannot say whether it works or not.
On my project the job which executes the tests is running under alpine container. and that container doesn't have docker installed on it.

It looks something like:

jobs:
  api_test:
    runs-on: ubuntu-latest
    timeout-minutes: 30
    container: mcr.microsoft.com/dotnet/core/sdk:3.1-alpine3.12
    steps:
    - name: Checkout
      uses: actions/checkout@v2
    - name: Copy dependencies
      id: extract
      uses: buildsecurity-kfir/actions-docker-extract@v1.0.4
      with:
        image: 'image'
        path: 'path/file'
    - name: Run tests
      env:
        FILE_PATH: '${{ steps.extract.outputs.destination }}/file'
      run: |
        echo "Run tests"

@shrink
Copy link
Owner

shrink commented Nov 15, 2020

@buildsecurity-kfir Thank you very much for the follow-up with clarity on your use-case.

According to the GitHub documentation:

Each step runs in its own process in the runner environment and has access to the workspace and filesystem.
[...]
A container to run any steps in a job that don't already specify a container. If you have steps that use both script and container actions, the container actions will run as sibling containers on the same network with the same volume mounts. If you do not set a container, all steps will run directly on the host specified by runs-on unless a step refers to an action configured to run in a container.

Therefore your example is equivalent to this:

jobs:
  api_test:
    runs-on: ubuntu-latest
    timeout-minutes: 30
-  container: mcr.microsoft.com/dotnet/core/sdk:3.1-alpine3.12
    steps:
    - name: Checkout
+     container: mcr.microsoft.com/dotnet/core/sdk:3.1-alpine3.12
      uses: actions/checkout@v2
    - name: Copy dependencies
+     container: mcr.microsoft.com/dotnet/core/sdk:3.1-alpine3.12
      id: extract
      uses: buildsecurity-kfir/actions-docker-extract@v1.0.4
      with:
        image: 'image'
        path: 'path/file'
    - name: Run tests
+     container: mcr.microsoft.com/dotnet/core/sdk:3.1-alpine3.12
      env:
        FILE_PATH: '${{ steps.extract.outputs.destination }}/file'
      run: |
        echo "Run tests"

This means that each of your step is running inside of the dotnet alpine container, unless your step uses a Docker action which forces GitHub to ignore your container and run a new container (for the action) on the host (runs-on). Although I can't speak to any nuance associated with your use-case, in general terms: you should minimise the surface area between your pipeline and specific containers, you should aim to run as much of your pipeline on the host as possible.

Jobs and dependencies between jobs are a great way to achieve this, for example, you could have a dependencies job running on the host which extracts the dependencies from your docker image, and then in a separate job you can use your dotnet container for just the steps that need dotnet.

jobs:
  dependencies:
    runs-on: ubuntu-latest
    outputs:
      extracted: ${{ steps.artifacts.outputs.destination }}
    steps:
    - uses: shrink/actions-docker-extract@v1
      id: artifacts
      with:
        image: 'ghost:alpine'
        path: '/var/lib/ghost/current/core/built/assets/.'
  test:
    runs-on: ubuntu-latest
    container: mcr.microsoft.com/dotnet/core/sdk:3.1-alpine3.12
    needs: [dependencies]
    steps:
    - uses: actions/checkout@v2
    - name: Run Tests
      env:
        FILE_PATH: '${{ needs.dependencies.outputs.extracted }}/file'
      run: |
        echo "Run tests"

This approach will give you the flexibility to grow your workflows over time so that you can continue to use actions that do not run in Docker :-)

As an aside, I'd recommend containerising your application: running tests inside your application container as part of the build process will greatly improve developer experience. I have an example project called Laravel Strict which implements the pattern (see Dockerfile and build.yaml).

Let me know if you need any more guidance, or if any aspect of what I've described here is unclear: if there's an aspect of this use-case that I've missed that requires Docker, we could look at publishing two versions of this action.

@kfirpeled
Copy link
Author

Thanks a lot for the detailed respond
I've changed my workflow and now it works without my version of the action

It worth to mention that in order to share artifacts I used actions/upload-artifact@v2 and actions/download-artifact@v2.

Thanks a lot.
I think there's no need for the docker version of the action.
Maybe it worth to add an example in the readme for what you've suggested. For newbies like me :)

@shrink
Copy link
Owner

shrink commented Nov 16, 2020

I'm glad to hear that it's working for you now! I'll close this Pull Request now, but please feel free to create a new issue if you encounter any further issues.

Maybe it worth to add an example in the readme for what you've suggested. For newbies like me :)

That's a great idea, I'll add some additional documentation covering your use-case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants