Bug 38345 - Remove support for OpenSearch
Summary: Remove support for OpenSearch
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Bugs List
QA Contact: Testopia
URL: https://gitlab.com/koha-community/koh...
Keywords:
Depends on:
Blocks:
 
Reported: 2024-11-04 14:38 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2024-11-26 22:49 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2024-11-04 14:38:58 UTC
Truth is our test suite has been exploding consistently when trying to launch OpenSearch:

* https://jenkins.koha-community.org/job/Koha_Main_OS2/
* https://jenkins.koha-community.org/job/Koha_Main_OS1/

This has some precedent discussion on [issue 429](https://gitlab.com/koha-community/koha-testing-docker/-/issues/429) in `KTD` project issues.

We are not going to remove it, I guess. But some noise is needed as people using OpenSearch in production don't seem to care we are not being able to run the test suite, so probably promising some 'OpenSearch support' that we cannot guarantee in any way. And if there's a problem with OpenSearch, we (devs) cannot launch a working environment to fix/test.

My two cents on this matter.
Comment 1 Tomás Cohen Arazi (tcohen) 2024-11-04 19:21:43 UTC
This is the last Jenkins run before it got borked and never worked again:

https://jenkins.koha-community.org/job/Koha_Main_OS2/38
Comment 2 David Cook 2024-11-04 22:32:49 UTC
(In reply to Tomás Cohen Arazi (tcohen) from comment #0)
> We are not going to remove it, I guess. But some noise is needed as people
> using OpenSearch in production don't seem to care we are not being able to
> run the test suite, so probably promising some 'OpenSearch support' that we
> cannot guarantee in any way. And if there's a problem with OpenSearch, we
> (devs) cannot launch a working environment to fix/test.

As someone using OpenSearch in production, I wouldn't say that I don't care. I totally forgot that this was an issue, so thanks for making the noise and drawing my attention. 

I thought that I provided some advice on this one previously, but I can't recall where now.

With my koha-testing-docker setup on Windows, I don't think the "ktd" command works, so I don't/can't use it.

I'll look at figuring out my own test plan.
Comment 3 David Cook 2024-11-04 22:35:47 UTC
Based off a comment on bug 33563 maybe it was just something Magnus and I discussed privately for a prod install...
Comment 4 David Cook 2024-11-04 22:55:14 UTC Comment hidden (obsolete)
Comment 5 David Cook 2024-11-04 23:17:35 UTC Comment hidden (obsolete)
Comment 6 David Cook 2024-11-05 01:18:13 UTC
As I note on the koha-testing-docker Gitlab issue, it appears that version 8 of the Search::Elasticsearch Perl library arbitrarily prevents connections to Opensearch (and any other Elasticsearch-based product that doesn't provide a particular HTTP header in its HTTP response).

Technically speaking, due to the lack of encapsulation in Perl code, it's trivial to turn off this product checking from Koha code.

I suppose the question is whether or not people want to workaround this feature of the Search::Elasticsearch Perl library.

As I note on the koha-testing-docker Gitlab issue, we can get the Opensearch integration working again, but do we want to?
Comment 7 David Cook 2024-11-05 01:19:25 UTC
This is how you'd do it if you wanted to:

diff --git a/Koha/SearchEngine/Elasticsearch.pm b/Koha/SearchEngine/Elasticsearch.pm
index 5ef5235720..1cee774d41 100644
--- a/Koha/SearchEngine/Elasticsearch.pm
+++ b/Koha/SearchEngine/Elasticsearch.pm
@@ -38,6 +38,8 @@ use Clone qw( clone );
 use Modern::Perl;
 use Readonly qw( Readonly );
 use Search::Elasticsearch;
+use Search::Elasticsearch::Role::Cxn;
+$Search::Elasticsearch::Role::Cxn::PRODUCT_CHECK_VALUE = '';
 use Try::Tiny qw( catch try );
 use YAML::XS;
Comment 8 David Cook 2024-11-05 01:20:58 UTC
Keeping in mind that in the future the variable scoping of that variable could change and even the looseness of Perl wouldn't prevent this workaround from failing.
Comment 9 David Cook 2024-11-05 01:23:00 UTC
Of course, you could put a proxy in front of your Opensearch and inject the HTTP header.

Or you could clone Search::Elasticsearch::Cxn::HTTPTiny, write a hack into a new version, and then update the config in koha-conf.xml to point to that new Cxn library.

Lots of possible workarounds, but it would be going against the intention of the library.

I'll leave it up to you folk to determine what you'd like to do.
Comment 10 Tomás Cohen Arazi (tcohen) 2024-11-20 03:53:36 UTC
(In reply to David Cook from comment #7)
> This is how you'd do it if you wanted to:
> 
> diff --git a/Koha/SearchEngine/Elasticsearch.pm
> b/Koha/SearchEngine/Elasticsearch.pm
> index 5ef5235720..1cee774d41 100644
> --- a/Koha/SearchEngine/Elasticsearch.pm
> +++ b/Koha/SearchEngine/Elasticsearch.pm
> @@ -38,6 +38,8 @@ use Clone qw( clone );
>  use Modern::Perl;
>  use Readonly qw( Readonly );
>  use Search::Elasticsearch;
> +use Search::Elasticsearch::Role::Cxn;
> +$Search::Elasticsearch::Role::Cxn::PRODUCT_CHECK_VALUE = '';
>  use Try::Tiny qw( catch try );
>  use YAML::XS;

Let's go with this. That said, I launched an OpenSearch instance on DigitalOcean and didn't manage to make this work.
Comment 11 David Cook 2024-11-20 22:47:43 UTC
(In reply to Tomás Cohen Arazi (tcohen) from comment #10)
> (In reply to David Cook from comment #7)
> > +use Search::Elasticsearch::Role::Cxn;
> > +$Search::Elasticsearch::Role::Cxn::PRODUCT_CHECK_VALUE = '';
> >  use Try::Tiny qw( catch try );
> >  use YAML::XS;
> 
> Let's go with this. That said, I launched an OpenSearch instance on
> DigitalOcean and didn't manage to make this work.

Were you able to get it working with koha-testing-docker?

Sometime in the next year, I'll probably be doing a series of upgrades, which will expose me to the problem. That might not be helpful, but I'd have more real-world info then...
Comment 12 David Cook 2024-11-25 23:44:37 UTC
Just looking at my history... and it looks like I used "koha/elasticsearch-icu:opensearch-2.x" for my testing.
Comment 13 David Cook 2024-11-25 23:48:12 UTC
If you couldn't get it to work, you might also need to add the following:

$Search::Elasticsearch::Role::Cxn::PRODUCT_CHECK_HEADER= 'NONEXISTENT_HEADER';

--

If we patch this like I described before, it would disable the product check for all ES/OS. Is that what we'd want?
Comment 14 Jake Deery 2024-11-26 11:24:05 UTC
(In reply to David Cook from comment #7)
> This is how you'd do it if you wanted to:
> 
> diff --git a/Koha/SearchEngine/Elasticsearch.pm
> b/Koha/SearchEngine/Elasticsearch.pm
> index 5ef5235720..1cee774d41 100644
> --- a/Koha/SearchEngine/Elasticsearch.pm
> +++ b/Koha/SearchEngine/Elasticsearch.pm
> @@ -38,6 +38,8 @@ use Clone qw( clone );
>  use Modern::Perl;
>  use Readonly qw( Readonly );
>  use Search::Elasticsearch;
> +use Search::Elasticsearch::Role::Cxn;
> +$Search::Elasticsearch::Role::Cxn::PRODUCT_CHECK_VALUE = '';
>  use Try::Tiny qw( catch try );
>  use YAML::XS;

Just applied this patch and had a go, and it seemed to work for me. There are a few failing tests, but it launches and is searchable. I'll see if I can figure out why the test is failing. For reference:

> [10:04:49] t/db_dependent/Search.t ........................................
> Dubious, test returned 255 (wstat 65280, 0xff00)
> Failed 3/3 subtests
Comment 15 David Cook 2024-11-26 22:49:41 UTC
(In reply to Jake Deery from comment #14)
> Just applied this patch and had a go, and it seemed to work for me. There
> are a few failing tests, but it launches and is searchable. I'll see if I
> can figure out why the test is failing. For reference:
> 
> > [10:04:49] t/db_dependent/Search.t ........................................
> > Dubious, test returned 255 (wstat 65280, 0xff00)
> > Failed 3/3 subtests

I'm taking a look at t/db_dependent/Search.t and I don't think it actually connects to Elasticsearch/OS, so that's interesting.

What are the tests that are failing?