8000 Action incompatible with self-hosted runners · Issue #58 · erlef/setup-beam · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Action incompatible with self-hosted runners #58

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.

Sign up for GitHub

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

Jump to bottom
Closed
jeremylvln opened this issue Jul 9, 2021 · 32 comments · Fixed by #59
Closed

Action incompatible with self-hosted runners #58

jeremylvln opened this issue Jul 9, 2021 · 32 comments · Fixed by #59

Comments

@jeremylvln
Copy link
jeremylvln commented Jul 9, 2021 •
edited
Loading

Hi,

This action is now incompatible with self-hosted runners, where the ImageOS environment variable is not provided. To let it work, the environment variable has to be injected manually:

      - uses: erlef/setup-beam@v1
        with:
          otp-version: ${{ env.TARGET_ERLANG_VERSION }}
          elixir-version: ${{ env.TARGET_ELIXIR_VERSION }}
        env:
          # See:
          # https://github.com/erlef/setup-beam/blob/master/src/setup-beam.js#L340
          ImageOS: ubuntu20

Honestly, I do not know what to think about this 😞 , maybe you should rework this part by asking the system directly the version with uname maybe :).
@paulo-ferraz-oliveira
Copy link
Collaborator

Hi, @IamBlueSlime.

Was this working until recently but suddenly stopped working?

@jeremylvln
Copy link
Author

Hi, @IamBlueSlime.

Was this working until recently but suddenly stopped working?

Hi! Exactly, it stopped working since v1.8.0. I decided to pin the v1.7.2 in my workflows rather than using this tweak, personal preference.

@jeremylvln
Copy link
Author
jeremylvln commented Jul 9, 2021 •
edited
Loading

After investigating the diff, I'm pretty confident by saying that removing the fallback value here cause the issue, see https://github.com/erlef/setup-beam/compare/v1.7.2...v1.8.0#diff-6406ae11684199b17d2cdca1efe2cfa29fb4587c3b142f1d33ad5f81a5acf7adR332

The OTP download URL contains undefined in its path, the result of dereferencing an array with undefined.

@paulo-ferraz-oliveira
Copy link
Collaborator

The code to "guess" this, before, was

return mapToUbuntuVersion[process.env.ImageOS] || 'ubuntu-18.04'

now changed to

return mapToUbuntuVersion[process.env.ImageOS]

(since we now support Windows too)

but:

  1. it gets easily outdated (20.04 is out, for example),
  2. we're randomly guessing ubuntu-18.04 (what if it's a self-hosted Windows runner?)

I don't mind restoring the ubuntu-18.04 and keep previous behaviour, but it'll be a time bomb.

On the other hand, I could try as you propose uname but it'd have to work for Windows also, and macOS soon.

Let's wait on other maintainers to give their opinion on whether we should use the env or not.

@paulo-ferraz-oliveira
Copy link
Collaborator

After investigating the diff, I'm pretty confident by saying that removing the fallback value here cause the issue, see v1.7.2...v1.8.0diff-6406ae11684199b17d2cdca1efe2cfa29fb4587c3b142f1d33ad5f81a5acf7adR332

The OTP download URL contains undefined in its path, the result of dereferencing an array with undefined.

This link doesn't work for me.

@jeremylvln
Copy link
Author

Restoring the default value is clearly not an option here, as you said it is a time bomb. You can read the following file /etc/os-release for Ubuntu:

$ cat /etc/os-release
NAME="Ubuntu"
VERSION="20.10 (Groovy Gorilla)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.10"
VERSION_ID="20.10"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=groovy
UBUNTU_CODENAME=groovy

I suggest you to:

  1. Check if ImageOS is present in the environment, then use it
  2. Check if process.platform is Linux then try to read the file above
  3. Otherwise, throw an error

Still not perfect, but better than nothing :)

@paulo-ferraz-oliveira
Copy link
Collaborator

Check if process.platform is Linux then try to read the file above

That'd put a block on Windows self-hosted, right? I'm not sure we want that.

@jeremylvln
Copy link
Author

I do not have a Windows machine nearby, but there should be something more-or-less equivalent I guess. Meanwhile, it will be a good compromise don't you think?

@paulo-ferraz-oliveira
Copy link
Collaborator

I'll look at this in the evening, if I have time, because I need to think about Windows too. It should be considered a bug and not a purposeful breaking change, so expect 1.8.2. In the meantime, yeah, you'll have to pin.

@paulo-ferraz-oliveira
Copy link
Collaborator
paulo-ferraz-oliveira commented Jul 9, 2021 •
edited
Loading

Meanwhile, it will be a good compromise don't you think?

What will? The compromise, for me, at the moment, is to pin the version to 1.7 until there's a fix. We added this info. (Action versioning) to the README in the latest release, so consumers should not rely on a moving tag (which might introduce bugs, even if unexpectedly).

@paulo-ferraz-oliveira
Copy link
Collaborator
paulo-ferraz-oliveira commented Jul 9, 2021 •
edited
Loading

Leaving this here for future reference (this evening hopefully, for me), which should be combined with NodeJS' os.platform().

$ cat /etc/issue | head -n1 | awk '{print $2}' | awk -F. '{print "ubuntu-"$1"."$2}'
ubuntu-20.04

(works for Ubuntu 16.04, 18.04 and 20.04)

Merged
@paulo-ferraz-oliveira
Copy link
Collaborator

#59 was created to address this issue (hopefully it solves the reported bug). We try to guess the version as best as possible, especially for the containers we support (Ubuntu and Windows - made explicit in the README). If we can't do that, we exit with an explicit error (and you'll have to use ImageOS, in that case). Exceptions to this might require further external collaboration/pull requests.

@ericmj
Copy link
Collaborator
ericmj commented Jul 10, 2021

I don't know how self-hosted runners work but shouldn't they replicate the environment of hosted runners as closely as possible? It is impossible for us to test against every possible self-hosted runner, we can only reasonably test against the runners GitHub provides.

If other runners deviate so they no longer work I think it is up to the runner maintainers to fix it on their end and add the ImageOS env var.

@starbelly
Copy link
Member
starbelly commented Jul 10, 2021 •
edited
Loading

If other runners deviate so they no longer work I think it is up to the runner maintainers to fix it on their end and add the ImageOS env var.

I whole heartily agree with this. I'd rather us not start doing guess work around operating systems when it's quite easy to specify the os. We have to keep in mind that we'll be supporting mac os and other *BSDs as well, in addition to linux distros and windows.

Edit:

Maybe it's counter-intuitive for some cases of self-hosted runners to specify ImageOS per a self-hosted runner perhaps not even using docker or the like. If that's the case, we could support an alternative variable name. I think that's as far as I'd be willing to go myself though.

@ericmj
Copy link
Collaborator
ericmj commented Jul 11, 2021

Maybe it's counter-intuitive for some cases of self-hosted runners to specify ImageOS per a self-hosted runner perhaps not even using docker or the like. If that's the case, we could support an alternative variable name. I think that's as far as I'd be willing to go myself though.

I agree, we should document ImageOS or perhaps another setting that you can set to select the OS that OTP was precompiled on. We can also point you in this direction in the error message if ImageOS was not set. Does that sound good @IamBlueSlime @paulo-ferraz-oliveira?

@paulo-ferraz-oliveira
Copy link
Collaborator
paulo-ferraz-oliveira commented Jul 12, 2021 •
edited
Loading

@ericmj, #59 introduces changes to README.md, and states needs for ImageOS in the error message, to make sure the consumer knows where to look when we can't guess properly.

@bryannaegele
Copy link

This does not seem to be working properly with act.

https://github.com/nektos/act

[Elixir Unit Tests/Test on Elixir 1.11.4 (OTP 21.3.8.24) and ubuntu-18.04-7]   ❌  Failure - erlef/setup-beam@v1
[Elixir Unit Tests/Test on Elixir 1.12.2 (OTP 22.3.4.20) and ubuntu-18.04-5]   ❗  ::error::Tried to map a target OS from env. variable 'ImageOS', but failed. If you're using a self-hosted runner, you should set 'env': 'ImageOS': ... to one of the following: ['ubuntu16', 'ubuntu18', 'ubuntu20', 'win16', 'win19']
[Elixir Unit Tests/Test on Elixir 1.12.2 (OTP 22.3.4.20) and ubuntu-18.04-5]   ❌  Failure - erlef/setup-beam@v1
Error: exit with `FAILURE`: 1

name: Elixir Unit Tests

on:
  pull_request:
    branches:
      - 'main'
  push:
    branches:
      - 'main'

jobs:
  tests:
    runs-on: ${{ matrix.os }}
    name: Test on Elixir ${{ matrix.elixir_version }} (OTP ${{ matrix.otp_version }}) and ${{ matrix.os }}
    strategy:
      matrix:
        otp_version: ['24.0.4', '23.3.4.2', '22.3.4.20']
        elixir_version: ['1.12.2', '1.11.4']
        rebar3_version: ['3.16.1']
        os: [ubuntu-18.04]
        app: |
          instrumentation/opentelemetry_phoenix
        include:
          - otp_version: '21.3.8.24'
            elixir_version: '1.11.4'
            rebar3_version: '3.15.2'
            os: ubuntu-18.04
        exclude:
          - otp_version: '21.3.8.24'
            elixir_version: '1.12.2'
    env:
      OTP_VERSION: ${{ matrix.otp_version }}
      ELIXIR_VERSION: ${{ matrix.elixir_version }}
      ImageOS: ubuntu18
    steps:
      - uses: actions/checkout@v2
      - uses: erlef/setup-beam@v1
        with:
          otp-version: ${{ matrix.otp_version }}
          elixir-version: ${{ matrix.elixir_version }}
          rebar3-version: ${{ matrix.rebar3_version }}
      - uses: actions/cache@v2
        name: Cache
        with:
          path: |
            ${{ matrix.app }}/deps
            ${{ matrix.app }}/_build
          key: ${{ runner.os }}-build-${{ matrix.otp_version }}-${{ matrix.elixir_version }}-v3-${{ hashFiles(format('{0}{1}', github.workspace, '/${{ matrix.app }}/mix.lock')) }}
          restore-keys: |
            ${{ runner.os }}-build-${{ matrix.otp_version }}-${{ matrix.elixir_version }}-
      - run: mix deps.get
        working-directory: ${{ matrix.app }}
        name: Deps
      - run: mix test --warnings-as-errors
        working-directory: ${{ matrix.app }}
        name: Test

@ericmj
Copy link
Collaborator
ericmj commented Jul 25, 2021

@bryannaegele Can you ask the act developers why they do not add the ImageOS environment variable? Isn't the idea of that project to create an environment as close to hosted runners as possible?

@paulo-ferraz-oliveira
Copy link
Collaborator

Hm... this is odd. (apart from what @ericmj wrote). You're actually passing ImageOS and the action seems to react poorly to that.

@bryannaegele
Copy link

It says it's in there 🤷🏻

https://github.com/nektos/act/pull/571/files

@paulo-ferraz-oliveira
Copy link
Collaborator

It seems to be failing for that include. Is it not passing for the other ones, too? Could you try with strategy fail-fast: false?

@bryannaegele
Copy link

Hm... this is odd. (apart from what @ericmj wrote). You're actually passing ImageOS and the action seems to react poorly to that.

Yeah. It's not getting it whether set explicitly or not.

Tried changing the strategy and it didn't work.

@paulo-ferraz-oliveira
Copy link
Collaborator

The change to the strategy was just to try to understand if all runs were failing. I've pushed https://github.com/paulo-ferraz-oliveira/setup-beam/tree/more-debug-info and will pull request soon. Could you change your stuff to paulo-ferraz-oliveira/setup-beam@more-debug-info and re-run? We'll see what the action is getting as ImageOS from the env.

@bryannaegele
Copy link

Did a verbose run and the env has two os vars available IMAGE_OS and the one being manually set ImageOS

[Elixir Unit Tests/Test on Elixir 1.12.2 (OTP 24.0.4)-1   ] setupEnv => map[ACT:true AGENT_TOOLSDIRECTORY:/opt/hostedtoolcache CI:true DEPLOYMENT_BASEPATH:/opt/runner ELIXIR_VERSION:1.12.2 GITHUB_ACTION:0 GITHUB_ACTIONS:true GITHUB_ACTION_REF: GITHUB_ACTION_REPOSITORY: GITHUB_ACTOR:nektos/act GITHUB_API_URL:https://api.github.com GITHUB_BASE_REF: GITHUB_ENV:/var/run/act/workflow/envs.txt GITHUB_EVENT_NAME:push GITHUB_EVENT_PATH:/var/run/act/workflow/event.json GITHUB_GRAPHQL_URL:https://api.github.com/graphql GITHUB_HEAD_REF: GITHUB_JOB:Test on Elixir 1.12.2 (OTP 24.0.4) GITHUB_PATH:/var/run/act/workflow/paths.txt GITHUB_REF:refs/heads/otel-phoenix GITHUB_REPOSITORY:open-telemetry/opentelemetry-erlang-contrib GITHUB_REPOSITORY_OWNER:open-telemetry GITHUB_RETENTION_DAYS:0 GITHUB_RUN_ID:1 GITHUB_RUN_NUMBER:1 GITHUB_SERVER_URL:https://github.com GITHUB_SHA:ea784490278a85ff01feccc29c2648ecfe6d0aff GITHUB_TOKEN: GITHUB_WORKFLOW:Elixir Unit Tests GITHUB_WORKSPACE:/Users/bryan/dev/otel/opentelemetry-erlang-contrib IMAGE_OS:ubuntu18 ImageOS:ubuntu18.04 LSB_OS_VERSION:1804 LSB_RELEASE:18.04 OTP_VERSION:24.0.4 PATH:/opt/hostedtoolcache/node/14.17.3/x64/bin:/usr

@paulo-ferraz-oliveira
Copy link
Collaborator

What I'd like to see would be the output of the action when using my branch (it seems somehow the env.var isn't obtained by the action itself, which is the odd bit).

@paulo-ferraz-oliveira
Copy link
Collaborator

Oh, wait. ImageOS is wrong, there. It says ImageOS:ubuntu18.04, when it should be ubuntu18 only, as per the README and the error message.

@bryannaegele
Copy link
bryannaegele commented Jul 25, 2021 •
edited
Loading

Oh, wait. ImageOS is wrong, there. It says ImageOS:ubuntu18.04, when it should be ubuntu18 only, as per the README and the error message.

Act is setting it to that, so we have an impasse? 😆

Maybe we can add that format also, or honor the IMAGE_OS env var?

@bryannaegele
Copy link

Actually, I'm not sure how it's getting the ubuntu18.04 according to this line

https://github.com/nektos/act/blob/dcbd5837afc22b5d9beb79834771cd6a63d82bee/pkg/runner/run_context.go#L710

@paulo-ferraz-oliveira
Copy link
Collaborator
paulo-ferraz-oliveira commented Jul 25, 2021 •
edited
Loading

It seems the Go code you linked to might be wrong (but I don't have further context). SplitN's last argument is 1 when it probably should be 2 (as in "split the stuff into two strings and then get the first one - the [0]").

@paulo-ferraz-oliveira
Copy link
Collaborator

Act is setting it to that, so we have an impasse? 😆

No impasse for me 😄. The action works for whatever GitHub Actions is itself setting, which is our reference. This is where it got added: actions/runner-images#345.

Maybe we can add that format also, or honor the IMAGE_OS env var?

Where does this IMAGE_OS come from? Is it GitHub Actions -specific, or otherwise? Could you point to a reference?

Merged
@bryannaegele
Copy link

I'm not sure. I think it's set by actions.

I tried the split change you pointed out in a repl and it is wrong. I can submit a PR to them and that should fix it. It does appear to be incorrect according to that link.

@bryannaegele
Copy link

nektos/act#761

rtorrero added a commit to rtorrero/web that referenced this issue Apr 11, 2022
@rtorrero
apply erlef/setup-beam#58
7765c21
chukinas added a commit to chukinas/statechart that referenced this issue Dec 12, 2022
@chukinas
fix ci
b9b9a05 
see
```
Error: Tried to map a target OS from env. variable 'ImageOS' (got ubuntu22), but failed. If you're using a self-hosted runner, you should set 'env': 'ImageOS': ... to one of the following: ['ubuntu16', 'ubuntu18', 'ubuntu20', 'win16', 'win19']
```

erlef/setup-beam#58
chukinas added a commit to chukinas/statechart that referenced this issue Dec 27, 2022
@chukinas
[workflow] fixes, updates, deploy
4408f49 
Part 1: fix the self-hosted runner warnings
see
```
Error: Tried to map a target OS from env. variable 'ImageOS' (got ubuntu22), but failed. If you're using a self-hosted runner, you should set 'env': 'ImageOS': ... to one of the following: ['ubuntu16', 'ubuntu18', 'ubuntu20', 'win16', 'win19']
```

erlef/setup-beam#58

Part 2: add publish-to-hex workflow
chukinas added a commit to chukinas/statechart that referenced this issue Dec 27, 2022
@chukinas
[workflow] fixes, updates, deploy
89dceed 
Part 1: fix the self-hosted runner warnings
see
```
Error: Tried to map a target OS from env. variable 'ImageOS' (got ubuntu22), but failed. If you're using a self-hosted runner, you should set 'env': 'ImageOS': ... to one of the following: ['ubuntu16', 'ubuntu18', 'ubuntu20', 'win16', 'win19']
```

erlef/setup-beam#58

Part 2: add publish-to-hex workflow
@fabjan fabjan mentioned this issue Feb 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Improve warning message for missing ImageOS env. variable (self-hosted runners)
5 participants
@ericmj @bryannaegele @jeremylvln @paulo-ferraz-oliveira @starbelly
0