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.

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

Remove DAX plugin #163

Merged
merged 9 commits into from
Feb 4, 2025
Merged

Remove DAX plugin #163

merged 9 commits into from
Feb 4, 2025

Conversation

p-j-smith
Copy link
Contributor

@p-j-smith p-j-smith commented Jan 29, 2025 •
edited
Loading

Fixes #95

  • remove DAX, Container Service, and Batch Launch plugins from list of default XNAT plugin
  • add these plugins as server-specific plugins in molecule tests

@p-j-smith p-j-smith requested a review from a team January 29, 2025 14:18
ruaridhg
ruaridhg previously approved these changes Jan 29, 2025
View reviewed changes
Copy link
Contributor

@ruaridhg ruaridhg left a comment

Choose a reason for hiding this comment

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

LGTM

@p-j-smith
Copy link
Contributor Author

Lots of tests are failing.

Issue with Rocky images in GitHub actions

The following tests are failing with this issue:

  • Test provision / molecule-provision (rocky9)
  • Test PostgreSQL / molecule-postgresql (rocky9)

Issue with latest version of molecule

The following tests are failing due to molecule / ansible not finding roles and playbooks that are part of this collection:

  • Test install_monitoring playbook / molecule-monitoring (centos7_monitoring)
  • Test install_omero playbook / molecule-install-omero
  • Test install_xnat playbook / molecule-install-xnat (centos7_xnat)
  • Test install_monitoring playbook / molecule-monitoring (rocky9_monitoring)
  • Test install_xnat playbook / molecule-install-xnat (rocky9_xnat)

There are various issues related to this:

And there was a PR merged recently that may have introduced this issue

@p-j-smith
Copy link
Contributor Author

We no longer run into the Rocky9 issue if we run CI jobs on Ubuntu<24. And the issue with molecule / ansible not finding the roles is fixed by explicitly setting ANSIBLE_ROLES_PATH and ANSIBLE_COLLECTIONS_PATH in the molecule configs (molecule previously set these automatically, not sure why it doesn't anymore)

p-j-smith
p-j-smith commented Feb 4, 2025
View reviewed changes
.renovaterc.json5
Comment on lines +22 to +24
"xnatdev/container-service",
"xnatx/xnatx-batch-launch-plugin",
"VUIIS/dax",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved these to the bottom as they're no longer in the default list of plugins (they're only used in testing with molecule)

.github/workflows/molecule-docker.yaml
@@ -17,7 +17,7 @@ concurrency:

jobs:
molecule-docker:
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we run into errors running sudo commands on Rocky 9 containers if we use ubuntu-latest

.renovaterc.json5
@@ -46,7 +46,7 @@
"VUIIS-dax": {
defaultRegistryUrlTemplate: "https://api.github.com/repos/VUIIS/dax/contents/misc/xnat-plugins",
transformTemplates: [
'{"releases": $.{"version": $join(["v", $match(name, /(\\d+\\.)?(\\d+\\.)?(\\*|\\d+)/ ).match ])}}',
'{"releases": $.{"version": $match(name, /(\\d+\\.)?(\\d+\\.)?(\\*|\\d+)/ ).match }}',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removing the v in the version name (i.e. 1.4.2 rather than v1.4.1) to match the format of currentValue

@p-j-smith p-j-smith requested a review from a team February 4, 2025 13:18
drmatthews
drmatthews approved these changes Feb 4, 2025
View reviewed changes
Copy link
Contributor

@drmatthews drmatthews left a comment

Choose a reason for hiding this comment

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

👍

@p-j-smith p-j-smith merged commit d5ab1fc into main Feb 4, 2025
27 checks passed
@p-j-smith p-j-smith deleted the paul/remove-dax branch February 4, 2025 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drmatthews drmatthews drmatthews approved these changes

@ruaridhg ruaridhg ruaridhg left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove Container Service, Batch Launch, and DAX plugins from default list
3 participants
@p-j-smith @drmatthews @ruaridhg