[gha] pull_docker_image fails with latest master

[fmessmer]

we have been using our own docker image in github actions with our ici branch pinned to db261d7 (+ pylint feature from #477, i.e. https://github.com/fmessmer/industrial_ci/tree/master_pylint)

the config is (ref #574):

name: CI

on:
  push:
  pull_request:

jobs:
  industrial_ci:
    timeout-minutes: 60
    env:
      ADDITIONAL_DEBS: 'apt-utils curl dialog git-core openssh-client openssh-server wget'
      AFTER_INIT: mkdir -p ~/.ssh && touch ~/.ssh/known_hosts && ssh-keyscan github.com >> ~/.ssh/known_hosts && git config --global url."https://token:${{secrets.GITHUBPAT}}@github.com/".insteadOf "[email protected]:" && curl -s https://packagecloud.io/install/repositories/github/git-lfs/script.deb.sh | bash && apt-get install git-lfs -y && cd $HOME && git lfs install && cd -
      BEFORE_PREPARE_DOCKER_IMAGE: echo ${{secrets.GITHUBPAT}} | docker login ghcr.io -u mojin-robotics-ci --password-stdin
      CATKIN_LINT: pedantic
      CATKIN_LINT_ARGS: '--ignore description_boilerplate --ignore target_name_collision'
      CMAKE_ARGS: -DCMAKE_BUILD_TYPE=Release
      NOT_TEST_DOWNSTREAM: true
      PYLINT_ARGS: "--output-format=parseable --errors-only"
      PYLINT2_CHECK: true  # pylint2 for kinetic, pylint3 for noetic
      PYLINT3_CHECK: false
      ROS_REPO: main
      SECRET_AVAILABLE: ${{secrets.GITHUBPAT}}
      VERBOSE_OUTPUT: false
      VERBOSE_TESTS: false
    strategy:
      matrix:
        env:
          - { ROS_DISTRO: kinetic,
              DOCKER_IMAGE: 'ghcr.io/mojin-robotics/mojin-ros:kinetic-full'
            }
          - { ROS_DISTRO: noetic,
              DOCKER_IMAGE: 'ghcr.io/mojin-robotics/mojin-ros:noetic-full'
            }
    name: ${{matrix.env.ROS_DISTRO}}
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
        with:
          lfs: true
      - name: Checkout LFS objects
        run: git lfs checkout
      - uses: 'fmessmer/industrial_ci@master_pylint'
        if: ${{ env.SECRET_AVAILABLE }} # prevents failure notifications if secret GITHUBPAT is not available
        env: ${{matrix.env}}

I rebased my fmessmer/industrial_ci@master_pylint branch to latest ros-industrial/industrial_ci@master branch to get the latest features, but with latest ros-industrial/industrial_ci@master we cannot pull our docker image anymore:

  Starting function 'pull_docker_image'
  Error response from daemon: Get https://ghcr.io/v2/mojin-robotics/mojin-ros/manifests/kinetic-full: unauthorized
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Function 'pull_docker_image' returned with code '1' after 0 min 1 sec
Error: Process completed with exit code 1.

I quickly investigated what has changed since db261d7, but could not find an obvious solution

[mathias-luedtke]

Did you try it with the regular master version?

[fmessmer]

yes, same thing!
so it's nothing related to our additional (pylint) commits - or a wrongly resolved merge conflict...

I'll try to iterate through the commit history trying to identify where things get broken...

[fmessmer]

as far as I can tell things get broken with the commits from "Commits on Jan 16, 2021" - see https://github.com/ros-industrial/industrial_ci/commits/master
everything up to "Commits on Jan 13, 2021" still works

trying to find the specific commit...

[mathias-luedtke]

I think I just renamed "prepare_docker_image" to "pull_docker_image" step, so BEFORE_PREPARE_DOCKER_IMAGE has to be renamed.

[gavanderhoorn]

@ipa-mdl: might be nice to provide some bw-compatibility by supporting the old env-var for a while?

[mathias-luedtke]

@fmessmer: Apart from the rename, it might be better (and simpler) to run the login outside of industrial_ci.
@gavanderhoorn: it is not a single environment variable.. not sure how we can check it properly. The semantics of the hook changes slightly (thus the rename), so we cannot make it 100% backward compatible. However, we can show a warning and try to handle the compatibility..

[mathias-luedtke]

However, we can show a warning and try to handle the compatibility..

Here we go: #611

@fmessmer: please test

[fmessmer]

thanks!
renaming BEFORE_PREPARE_DOCKER_IMAGEto BEFORE_PULL_DOCKER_IMAGE solves this issue

[mathias-luedtke]

@fmessmer: Your config contains some superfluous settings, you might want to clean this up. And I strongly recommend against calling ssh-keyscan in the script.

[fmessmer]

@fmessmer: Your config contains some superfluous settings, you might want to clean this up. And I strongly recommend against calling ssh-keyscan in the script.

could you point me to something specific, please?
I'd also be happy to hear an alternative to ssh-keyscan...
maybe you could post suggestions to #574 or #590

I'm just getting started with GithubAction and appreciate any guidance...😉

[mathias-luedtke]

could you point me to something specific, please?

NOT_TEST_DOWNSTREAM and VERBOSE_* can be removed.
Not sure what you do with SECRET_AVAILABLE ^^

The docker login can be added as a step (no need for a hook script).
And I believe that the known_hosts fix is not needed, because you rewrite the URL to https.

[mathias-luedtke]

And it seems that you are not using git lfs inside (?), so AFTER_INIT could be removed as well.

[fmessmer]

to come back to this:
thanks for the feedback. indeed we could simplify things a lot 😉
just a comment on the SECRET_AVAILABLE magic: we use this trick to prevent gha builds on forks that do not have a secret.GITHUBPAT set up for their repository (because that would always fail). unfortunately, the if condition only works on env variables on not on secrets directly