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

ci(build-image): remove ghcr user login #1760

Merged
merged 2 commits into from
Oct 31, 2023

Conversation

aali309
Copy link
Contributor

@aali309 aali309 commented Oct 31, 2023

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits using a GPG signature

To recreate commits with GPG signature git fetch upstream && git rebase --force --gpg-sign upstream/main


Fixes: #1755

Description of the change:

This change allows an environment variable to be configured so that...

Motivation for the change:

This change is helpful because users may want to...

How to manually test:

  1. Run CRYOSTAT_IMAGE=quay.io... sh smoketest.sh...
  2. ...

@aali309
Copy link
Contributor Author

aali309 commented Oct 31, 2023

with podman login

/build_test LINK: https://github.com/aali309/cryostat/actions/runs/6700846180

image

retest LINK: https://github.com/aali309/cryostat/actions/runs/6700928824

image

Push to quay fails is expected to fail I assume because of the access restrictions (i.e username+password).
push-ci workflow LINK: https://github.com/aali309/cryostat/actions/runs/6700718429
image

@aali309
Copy link
Contributor Author

aali309 commented Oct 31, 2023

Option to explore suggested by @andrewazores: remove podman-login

@andrewazores
Copy link
Member

push-to-quay probably fails on your fork because it requires the correct token ("registry password") to be provided as a secret, which your fork would not have access to.

I'm still not seeing/understanding how this addresses the part where the mvn build fails to download the cryostat-core dependency. Being logged in to ghcr.io with podman would not make Maven able to authorize to the mvn pkg repository, so if the build starts to complain about being unable to download -core when not logged in through podman, then I suspect that the build is actually not even building an image, but rather pulling an existing one.

I do see that you have provided screenshots illustrating that removing the podman login step causes the cryostat-core authorization issues/download failures, but the reason for that is where I'm getting lost.

@andrewazores
Copy link
Member

This is the relevant part of the workflow in question:

  build-image:
    runs-on: ubuntu-latest
    permissions:
      pull-requests: write
    needs: [get-pom-properties]
    steps:
    - name: Install qemu
      if: ${{ inputs.build-arch != 'amd64' }}
      continue-on-error: true
      run: |
        sudo apt-get update
        sudo apt-get install -y qemu-user-static
    - uses: actions/checkout@v4
      with:
        repository: ${{ inputs.checkout-repo }}
        ref: ${{ inputs.checkout-ref }}
        submodules: true
        fetch-depth: 0
    - uses: actions/setup-java@v3
      with:
        java-version: '17'
        distribution: 'temurin'
    - name: maven-settings
      uses: s4u/maven-settings-action@v2
      with:
        githubServer: false
    - name: ghcr login
      uses: redhat-actions/podman-login@v1
      with:
        registry: ghcr.io/${{ github.repository_owner }}
        username: ${{ github.actor }}
        password: ${{ secrets.GITHUB_TOKEN }}
    - uses: skjolber/maven-cache-github-action@v1
      with:
        step: restore
    - run: git submodule init
    - run: git submodule update
    - run: mvn -B -U -Dbuild.arch=${{ inputs.build-arch }} clean package
      env:
        GITHUB_TOKEN_REF: ${{ secrets.GITHUB_TOKEN }}
    - name: Save cryostat image
      run: podman save -o cryostat-${{ inputs.build-arch }}.tar --format oci-archive quay.io/cryostat/cryostat
    - uses: actions/upload-artifact@v3
      with:
        name: cryostat-${{ inputs.build-arch }}
        path: /home/runner/work/cryostat/cryostat/cryostat-${{ inputs.build-arch }}.tar
    - uses: skjolber/maven-cache-github-action@v1
      with:
        step: save

There are a bunch of uses: pointing to other actions/reusable workflows, but all of them are quite simple and none are our own reusable workflows that should be doing any image pushing or pulling. They just do tasks like checking out the repo, or dealing with cached files.

None of the steps here should require an active login session with ghcr.io (ie the podman-login step), I think. So how can it be that removing that step breaks this overall workflow?

@aali309
Copy link
Contributor Author

aali309 commented Oct 31, 2023

I'm still not seeing/understanding how this addresses the part where the mvn build fails to download the cryostat-core dependency. Being logged in to ghcr.io with podman would not make Maven able to authorize to the mvn pkg repository, so if the build starts to complain about being unable to download -core when not logged in through podman, then I suspect that the build is actually not even building an image, but rather pulling an existing one.

From my understanding, I thought Maven build, will attempt to download dependencies from repositories or container registries hosted on the ghcr.io registry, and would require the proper authentication (which you get from the ghcr login step) to download those dependencies.
But at the same time, what you are saying makes sense that logging in with podman should not affect maven's ability to download dependencies from a different repo

@andrewazores
Copy link
Member

andrewazores commented Oct 31, 2023

From my understanding, I thought Maven build, will attempt to download dependencies from repositories or container registries

mvn package will attempt to download Java dependencies from Maven repositories (maven.pkg.github.com), but it will not attempt to pull container images from registries like ghcr.io - the only exception is that the build will attempt to pull one container base image:

https://github.com/cryostatio/cryostat/blob/09b010aca2b1809f910f1583b22d081582194800/src/container/Dockerfile#L1

but this is not from ghcr.io, and in fact this image can be pulled anonymously anyway. If it did require auth then logging in to ghcr.io wouldn't help because these are separate services with separate logins.

@aali309
Copy link
Contributor Author

aali309 commented Oct 31, 2023

with the updated changes

Link to PR: https://github.com/aali309/cryostat/pull/130

image

/build_test workflow LINK:

image

/retest workflow LINK

image

merge LINK

image

@andrewazores
Copy link
Member

Rebase doesn't look right - the merge conflict markers are in the patch.

@andrewazores andrewazores changed the title fix(ci): fix ghcr user login ci(build-image): remove ghcr user login Oct 31, 2023
@andrewazores andrewazores merged commit a012cb5 into cryostatio:main Oct 31, 2023
10 checks passed
@aali309 aali309 deleted the fixStartUpFailure branch October 31, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] CI startup failure
2 participants