Skip to content

Smoke test and PR#21277

Merged
nuwang merged 8 commits intogalaxyproject:devfrom
ksuderman:pr-and-smoke-test
Nov 18, 2025
Merged

Smoke test and PR#21277
nuwang merged 8 commits intogalaxyproject:devfrom
ksuderman:pr-and-smoke-test

Conversation

@ksuderman
Copy link
Copy Markdown
Contributor

@ksuderman ksuderman commented Nov 12, 2025

This PR adds two new jobs to the build_container_image.yaml workflow.

  1. smoke-test installs the newly built image to a k8s cluster and then calls /api/version
  2. pr opens a PR to the Galaxy Helm chart repository to update the version. Only triggered when a release is tagged

Outstanding issues

  • ensure the GITHUB_TOKEN used to create a PR in the Galaxy Helm repository has permission.
  • updating the version with sed to generate the PR is fragile

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@ksuderman ksuderman requested review from afgane and nuwang November 12, 2025 01:40
@ksuderman ksuderman added this to the 26.0 milestone Nov 12, 2025
@ksuderman
Copy link
Copy Markdown
Contributor Author

I have tested the smoke-test and pr jobs in a separate repository and this "should work" ™️. However, as I suspected the standard secrets.GITHUB_TOKEN is scoped to the current repository and doesn't have permission to open a pull request in a different repository. A separate token (PAT or similar) will need to be created (I've called it secrets.PULL_REQUEST_TOKEN here) before this PR can be accepted.

Copy link
Copy Markdown
Member

@nuwang nuwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, looks great to me! Let me know when it's ready for testing.

Comment thread .github/workflows/build_container_image.yaml Outdated
Use a better name for the pull request token.

Co-authored-by: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com>
@ksuderman
Copy link
Copy Markdown
Contributor Author

Thanks for this, looks great to me! Let me know when it's ready for testing.

I think this is ready for testing. At least I think I've done all the testing that I can as is. Do you have suggestions for testing beyond releasing this into the wild and see how it does?

@nuwang
Copy link
Copy Markdown
Member

nuwang commented Nov 17, 2025

I think this is ready for testing. At least I think I've done all the testing that I can as is. Do you have suggestions for testing beyond releasing this into the wild and see how it does?

I should have said - can it be taken out of review? I agree with what you said about testing

@ksuderman
Copy link
Copy Markdown
Contributor Author

I was going to take it out of Draft once we had the token set up. Maybe @afgane can do that?

@nuwang
Copy link
Copy Markdown
Member

nuwang commented Nov 17, 2025

I've added the token @ksuderman. It only has PR read-write permissions on the galaxy-helm repo. Let me know if it needs more perms than that.

@ksuderman
Copy link
Copy Markdown
Contributor Author

That should be enough. I guess we will find out when it attempts to create a PR.

@ksuderman ksuderman marked this pull request as ready for review November 18, 2025 16:18
@nuwang nuwang merged commit 58829dd into galaxyproject:dev Nov 18, 2025
57 of 60 checks passed
with:
repository: galaxyproject/galaxy-helm
token: ${{ secrets.GITHUB_TOKEN }}
persist-credentials: true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ksuderman I'm reviewing our use of persist-credentials and I think it should be set to false here, as you are using a different token when creating the pull request?
Also the token line can be removed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

3 participants