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

Update omero_ms_image_region version #443

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

khaledk2
Copy link
Contributor

This PR updates the omero_ms_image_region ansible role version

Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

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

Running this against either test125 (where the change has been applied manually?) or prod126 in dry-run mode, Ansible does not plan to apply any change to the NGINX configuration.

This is a problem in the logic of the Ansible role which skips the NGINX configuration modification if an upstream block for the micro-service is found - https://github.com/ome/ansible-role-omero-ms-image-region/blob/f9629d454d802f4fe614cd2aeb3a67cb60f43e35/tasks/update_nginx_config.yml#L22.
In practice, this ome/ansible-role-omero-ms-image-region#6 (or any change to the NGINX configuration) will not apply out of the box on existing deployments.

@khaledk2
Copy link
Contributor Author

The microservice is integrated with the OMERO-Web, it updates the existing Web Nginx configuration file(omero-web.conf).
If any changes are required in the Nginx configuration for the microservice, we should first run the OMERO-web role to reset the configuration file. After that, we should execute the ms role, which will update the Nginx configuration file with the desired changes.

@sbesson
Copy link
Member

sbesson commented Jan 21, 2025

Thanks, this is consistent with the issue described in #436 i.e. the roles are effectively coupled and cannot be run in isolation.

Another issue highlighted by this testing is that we cannot reliably use the output of the dry-run command since the relevant changes are skipped. To test the change, we need to apply it outside dry-run first to override the NGINX configuration in the ome.omero-web and then patch this configuration with the new ome.omero_ms_image_region. Has the configuration in idr-testing been modified by hand or using these changes? Otherwise, I would propose to run the changes against test125.

@khaledk2
Copy link
Contributor Author

khaledk2 commented Jan 21, 2025

The configuration changes on the idr-testing have been done manually before modifying the ms role.

@sbesson
Copy link
Member

sbesson commented Jan 21, 2025

Tried to deploy https://github.com/IDR/deployment/blob/master/ansible/idr-omero-web.yml followed by https://github.com/IDR/deployment/blob/master/ansible/idr-omero-readonly.yml against test125 as discussed above. While the first playbook completed successfully, the second one failed to restart the omero-server service.

After investigation this is related to the fact Java 17 was installed on these systems on January 9th

2025-01-09T09:08:02+0000 DDEBUG Command: yum install -y gcc-c++ make ant 
...
2025-01-09T09:08:09+0000 DEBUG ---> Package javapackages-tools.noarch 6.0.0-7.el9_5 will be installed
2025-01-09T09:08:09+0000 DEBUG ---> Package java-17-openjdk-headless.x86_64 1:17.0.13.0.11-4.el9 will be installed
2025-01-09T09:08:09+0000 DEBUG ---> Package java-17-openjdk-devel.x86_64 1:17.0.13.0.11-4.el9 will be installed
2025-01-09T09:08:09+0000 DEBUG ---> Package java-17-openjdk.x86_64 1:17.0.13.0.11-4.el9 will be installed
2025-01-09T09:08:09+0000 DEBUG ---> Package javapackages-filesystem.noarch 6.0.0-4.el9 will be upgraded
2025-01-09T09:08:09+0000 DEBUG ---> Package javapackages-filesystem.noarch 6.0.0-7.el9_5 will be an upgrade

but the server configuration was not patched to work with this version (see ome/openmicroscopy#6383 for more details).

This issue will need to be resolved in order to retest the deployment

Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

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

Having heard no objection, JDK 17 was uninstalled from the OMERO servers of the test125 environment

sbesson@test125-proxy ~]$ for i in omeroreadwrite omeroreadonly-1 omeroreadonly-2 omeroreadonly-3 omeroreadonly-4; do ssh $i sudo dnf remove  java-17-openjdk* -y; done

The playbooks were re-executed including the changes from this PR

changed: [cd535951-ff64-4325-a847-d10bd38a0889]
--- before: /etc/nginx/conf.d/omero-web.conf
+++ after: /Users/sbesson/.ansible/tmp/ansible-local-23196kjbz15lm/tmpy6_hi6c_/omero-web.conf.j2
@@ -1,3 +1,9 @@
+proxy_cache_path /var/cache/nginx/omero-ms-image-region levels=1:2 keys_zone=ms_cache:10m max_size=25m inactive=120m use_temp_path=off;
+
+upstream image_region_backend {
+    server 127.0.0.1:8081 fail_timeout=0 max_fails=0;
+}
+

As there is no good way to have a minimal diff of the changes created by this PR, I have no objection to seeing this deployed in prod126. In the meantime, if any testing must be performed, this can be scheduled. I'll add this for discussion at next Monday's IDR meeting.

@sbesson
Copy link
Member

sbesson commented Jan 27, 2025

Discussed earlier at the weekly IDR meeting. NO objection was raised against deploying these changes against prod126

@sbesson sbesson merged commit 76c1ecb into IDR:master Jan 27, 2025
3 checks passed
@sbesson
Copy link
Member

sbesson commented Jan 27, 2025

Now deployed on prod126

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants