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

Workflows target docker hub by Org/Account. #6

Merged
merged 2 commits into from
Feb 11, 2020
Merged

Workflows target docker hub by Org/Account. #6

merged 2 commits into from
Feb 11, 2020

Conversation

rbellamy
Copy link
Contributor

@rbellamy rbellamy commented Feb 11, 2020

Install maven & npm. Workflows target docker hub by Org/Account.

  • Make workflows target docker hub by organization/account.
  • Modified sample function to support different organization/account when pulling docker image. This assumes the same value in both GitHub and Docker Hub.
  • Modified the names of the workflows to reflect their purpose.

WRT the installing npm or maven or any other tools, in reading through the code for the linux runner virtual environments, it seems the next logical step would be to leverage those same shell scripts. The fact that GitHub is using packer adds a wrinkle of complexity. I think that could be overcome with some jq scripting.

IT-4071 #comment Install maven and npm - should go through the list of default utilities and tools installed into `ubuntu-latest` for parity with GitHub runners. Many actions will rely on and expect them.
IT-4071 #comment Because of https://bugs.launchpad.net/ubuntu/+source/nodejs/+bug/1794589, both libcurl4-openssl-dev and npm cannot be installed in bionic using the package. There are two options: 1) Use libcurl4-gnutls-dev rather than libcurl4-openssl-dev, 2) Install npm using the installation script from github. I choose option #2.
IT-4071 #comment Make workflows target docker hub by organization/account.
@myoung34
Copy link
Owner

myoung34 commented Feb 11, 2020

i like all these changes except the maven and npm.

mostly because they're non-tunable by version, they install whats latest (npm) or whatever is in the upstream repo (unpredictable across OS flavors)

The better way to handle this is to use your runner to set up matrix testing for your language(s) similar to this

Point being: if you want to test different versions of npm (or node), or maven etc this will likely cause more issues than solutions.
Actually you already see the issue:

&& [[ $(lsb_release -cs) == "bionic" ]] && ( apt-get install -y nodejs && curl -L https://www.npmjs.com/install.sh | sh ) || ( apt-get install -y npm ) \

Id like to keep all the nuances out of the image where possible, which is why this makes sure docker works, you can use the shared socket to run npm/maven/etc as a docker container within this container

If you were to leave that out and run this on your self hosted node it should technically work because of docker:

name: Package

on:
  release:
    types: [created]

jobs:
  test:
    runs-on: self-hosted
    strategy:
      matrix:
        node: [ '10', '8' ]
    name: Node ${{ matrix.node }} sample
    steps:
      - uses: actions/checkout@v2
      - name: Setup node
        uses: actions/setup-node@v1
        with:
          node-version: ${{ matrix.node }}
      - run: npm install
      - run: npm test

or maven/java:

name: Package

on:
  release:
    types: [created]

jobs:
  test:
    runs-on: self-hosted
    strategy:
      matrix:
        java: [ 11, 12, 13 ]
    name: Java ${{ matrix.java }} compile
    steps:
      - uses: actions/checkout@master
      - name: Setup java
        uses: actions/setup-java@v1
        with:
          java-version: ${{ matrix.java }}
      - run: mvn -f github-actions-java-maven/pom.xml clean compile

The rest those are 💯

@rbellamy
Copy link
Contributor Author

rbellamy commented Feb 11, 2020

Id like to keep all the nuances out of the image where possible, which is why this makes sure docker works, you can use the shared socket to run npm/maven/etc as a docker container within this container

I hadn't considered this point. Very interesting. And I can definitely see the value in not having to "fight" the installed tools in order to build matrix-based builds.

Where my head went was that this runner should have parity with the GitHub hosted runners, since most, if not all, actions will assume they are running in GitHub hosted runners. The downfall to diverging from the GitHub hosted runners is that builds will have to manage almost all their tool installations - and therefore will be markedly different than those workflows created for GitHub hosted runners.

All that said, I'll remove maven and npm and rebase.

IT-4071 #comment Make workflows target docker hub by organization/account.
@rbellamy rbellamy changed the title Install maven & npm. Workflows target docker hub by Org/Account. Workflows target docker hub by Org/Account. Feb 11, 2020
@myoung34 myoung34 merged commit f7405e5 into myoung34:master Feb 11, 2020
@myoung34
Copy link
Owner

Nice add and thanks!

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.

2 participants