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

Allow separating build steps #26

Closed
davidchisnall opened this issue Feb 8, 2023 · 13 comments
Closed

Allow separating build steps #26

davidchisnall opened this issue Feb 8, 2023 · 13 comments
Labels
enhancement New feature or request

Comments

@davidchisnall
Copy link

It would be nice to create the VM on the first step and then be able to reuse it for different steps so that we can see whether we failed in build, test, or whatever stages from the dashboard, rather than having to read the ouput.

@jacob-carlborg
Copy link
Contributor

Hmm, how do you envision this to look like, from API point of view?

@jacob-carlborg jacob-carlborg added the enhancement New feature or request label Feb 8, 2023
@davidchisnall
Copy link
Author

Ideally, being able to just specify multiple steps, with each one in the same job checking for the existence of the VM and reusing it if it exists. I'm not sure whether the APIs for plugins have the right hooks for this.

@jacob-carlborg
Copy link
Contributor

jacob-carlborg commented Feb 9, 2023

The main problem I see is knowing when to stop the VM. It's easy, at least in theory, if it's required to specify a flag when the VM should not be stopped and when it should be stopped. Another problem is that all the existing arguments that currently are required would need to be optional. Perhaps it's possible to make them conditionally required.

Something like:

steps:
  - uses: actions/checkout@v2

  - name: Build
    uses: cross-platform-actions/action@v0.10.0
    with:
      operating_system: 'freebsd'
      version: '13.1'
      stop: false
      run: ./build.sh

  - name: Link
    uses: cross-platform-actions/action@v0.10.0
    with:
      stop: false
      run: ./link.sh

  - name: Test
    uses: cross-platform-actions/action@v0.10.0
    with:
      stop: true
      run: ./test.sh

If the stop flag is not specified the action would behave as it does currently. Then the currently required arguments could still be required. If the stop flag is specified, then I guess all arguments except for run would be optional. I guess I could store a PID file to indicate if the VM is currently running. Then the action could require the currently required arguments if the VM is not running.

Another problem, more of a theoretical problem, is if the user specifies different versions of the action at the different steps.

If a flag, as in the example above, is acceptable then I think it's at least technically doable.

@davidchisnall
Copy link
Author

That would certainly work for me. I thought the actions APIs let you register a cleanup phase. Using this would let each step add a refcount (in a templ file somewhere if there isn't some shared variable space) and each matching cleanup could then drop the reference count and the last one actually shut down the VM (is that actually needed? Doesn't it get killed when the job finishes?).

I could also imagine an alternative form like this:

steps:
  - uses: actions/checkout@v3

  - name: Start VM
    uses: cross-platform-actions/action-start-vm@v0.10.0
    with:
      operating_system: 'freebsd'
      version: '13.1'

  - name: Build
    uses: cross-platform-actions/action-run@v0.10.0
    with:
      run: ./build.sh

  - name: Link
    uses: cross-platform-actions/action-run@v0.10.0
    with:
      run: ./link.sh

  - name: Test
    uses: cross-platform-actions/action-run@v0.10.0
    with:
      run: ./test.sh

Here, the cleanup for action-start-vm would do the shutdown of that vm.

This would also let you later extend it to doing something like:

steps:
  - uses: actions/checkout@v3

  - name: Start server
    uses: cross-platform-actions/action-start-vm@v0.10.0
    with:
      operating_system: 'freebsd'
      version: '13.1'
      memory: '2G'
      name: server

  - name: Start client
    uses: cross-platform-actions/action-start-vm@v0.10.0
    with:
      operating_system: 'freebsd'
      version: '13.1'
      memory: '2G'
      name: client
      
  - name: Build Server
    uses: cross-platform-actions/action-run@v0.10.0
    with:
      run: ./build-server.sh
      on: server

  - name: Build client
    uses: cross-platform-actions/action-run@v0.10.0
    with:
      run: ./build-client.sh
      on: client

  - name: Start Server
    uses: cross-platform-actions/action-run@v0.10.0
    with:
      run: ./start-server.sh
      on: server

  - name: Run tests
    uses: cross-platform-actions/action-run@v0.10.0
    with:
      run: ./test-client.sh
      on: client

  - name: Stop server
    uses: cross-platform-actions/action-run@v0.10.0
    with:
      run: ./stop-server.sh
      on: server

Or other things that want to run multiple jobs in different VMs on the same host.

@jacob-carlborg
Copy link
Contributor

I see now that GitHub actions support a post step. I'll give that a try and see how it behaves.

@albinahlback
Copy link

Any progress on this?

@jacob-carlborg
Copy link
Contributor

Any progress on this?

No, unfortunately there's no progress on this yet.

@Consolatis
Copy link

Consolatis commented Nov 6, 2023

We currently evaluate switching over to cross-platform-actions.

Having multiple run actions would help a lot when used with a OS matrix (all containers and one FreeBSD entry) + multiple shared build steps.

Currently it seems when we'd switch over to the cross-platform-actions we'd have to have all the build steps in one for the FreeBSD run. Currently we use multiple steps (which run on the host in the case of our FreeBSD runner) and pipe the commands to either sh -xe for the usual containers or ssh freebsd /bin/sh -xe in the FreeBSD case (set as env var in the matrix). It is a bit verbose but gets the job done.

Is there a way to make something like that work with cross-platform-actions as well, e.g.

  • preventing the VM to destroy until all build steps are finished
  • having something like ssh freebsd on the host which just works without a key

Supporting multiple run actions that directly run on the VM would match better with the usual container cases but as long as some kind of ssh freebsd workaround is available that would be enough for us.

@jacob-carlborg
Copy link
Contributor

preventing the VM to destroy until all build steps are finished

@Consolatis no, that's unfortunately not possible right now. The VM will be shut down when the run command has finished.

@manxorist
Copy link

@jacob-carlborg

I tried using this feature, and it works, mostly.

I am using the following yml file: (https://github.com/manxorist/openmpt/blob/52bf0cef350967befab4fc109311da81fd5715c1/.github/workflows/CPA-Test.yml#L1)

name: CPA-Multistep test

on:
  push:
    branches: [ cpa-test ]
  pull_request:
    branches: [ cpa-test ]

concurrency:
  group: ${{ github.ref }}-${{ github.workflow }}
  cancel-in-progress: true

jobs:
  build:

    runs-on: macos-12

    steps:
    - name: Checkout
      uses: actions/checkout@v3
    - name: Startup VM
      uses: cross-platform-actions/action@7f2fab9c5601e3f6f057283dbac16f581ded8a8b
      with:
        architecture: x86_64
        hypervisor: qemu
        memory: 4G
        operating_system: freebsd
        version: '13.2'
        sync_files: runner-to-vm
        shutdown_vm: false
        shell: bash
        run: true
    - name: Install dependencies
      uses: cross-platform-actions/action@7f2fab9c5601e3f6f057283dbac16f581ded8a8b
      with:
        architecture: x86_64
        hypervisor: qemu
        memory: 4G
        operating_system: freebsd
        version: '13.2'
        sync_files: false
        shutdown_vm: false
        shell: bash
        run: sudo pkg install -y subversion p5-XML-XPath git mawk gmake pkgconf autoconf autoconf-archive automake libtool help2man mpg123 libogg libvorbis flac libsndfile pulseaudio portaudio sdl2
    - name: Build
      uses: cross-platform-actions/action@7f2fab9c5601e3f6f057283dbac16f581ded8a8b
      with:
        architecture: x86_64
        hypervisor: qemu
        memory: 4G
        operating_system: freebsd
        version: '13.2'
        sync_files: false
        shutdown_vm: false
        shell: bash
        run: gmake -j$(sysctl -n hw.ncpu) STRICT=1 VERBOSE=1 AUTO_DEPS=1
    - name: Test
      uses: cross-platform-actions/action@7f2fab9c5601e3f6f057283dbac16f581ded8a8b
      with:
        architecture: x86_64
        hypervisor: qemu
        memory: 4G
        operating_system: freebsd
        version: '13.2'
        sync_files: false
        shutdown_vm: false
        shell: bash
        run: gmake -j$(sysctl -n hw.ncpu) STRICT=1 VERBOSE=1 AUTO_DEPS=1 check
    - name: Shutdown VM
      uses: cross-platform-actions/action@7f2fab9c5601e3f6f057283dbac16f581ded8a8b
      with:
        architecture: x86_64
        hypervisor: qemu
        memory: 4G
        operating_system: freebsd
        version: '13.2'
        sync_files: false
        shutdown_vm: true
        shell: bash
        run: true

which results in VM shutdown failing: (https://github.com/manxorist/openmpt/actions/runs/7220212842/job/19672686943)

 Error: Cannot read properties of undefined (reading 'pid')

Am I doing something wrong with the last step of shutting down the VM? I do not actually need the VM shutdown in this particular case, but I tend to prefer to properly clean things up, even if not strictly required.

Also, should we be repeating the VM parameters in every build step or is there a more elegant solution available? (I do not consider myself an expert on Github Actions in general, so I may be missing something obvious/trivial)

@jacob-carlborg
Copy link
Contributor

Error: Cannot read properties of undefined (reading 'pid')

@manxorist That's a bug.

Also, should we be repeating the VM parameters in every build step or is there a more elegant solution available?

I did that on purpose. I thought it might be confusing if one would try to use different input arguments for the different steps. For example, if you first specify freebsd and then specify openbsd. You would still get FreeBSD for the second step.

@jacob-carlborg
Copy link
Contributor

@manxorist the bug should be fixed now.

@manxorist
Copy link

@jacob-carlborg

@manxorist the bug should be fixed now.

Thanks, works for me.

korli added a commit to korli/action that referenced this issue Mar 15, 2024
Added
- Added support for using the action in multiple steps in the same job ([cross-platform-actions#26](cross-platform-actions#26)).
    All the inputs need to be the same for all steps, except for the following
    inputs: `sync_files`, `shutdown_vm` and `run`.

- Added support for specifying that the VM should not shutdown after the action
    has run. This adds a new input parameter: `shutdown_vm`. When set to `false`,
    this will hopefully mitigate very frequent freezing of VM during teardown ([cross-platform-actions#61](cross-platform-actions#61), [cross-platform-actions#72](cross-platform-actions#72)).

Changed
- Always terminate VM instead of shutting down. This is more efficient and this
    will hopefully mitigate very frequent freezing of VM during teardown
    ([cross-platform-actions#61](cross-platform-actions#61),
    [cross-platform-actions#72](cross-platform-actions#72)).

- Use `unsafe` as the cache mode for QEMU disks. This should improve performance ([cross-platform-actions#67](cross-platform-actions#67)).
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

No branches or pull requests

5 participants