artefactual-labs / am

Development environment for Archivematica
https://archivematica.org
GNU Affero General Public License v3.0
10 stars 20 forks source link

Upgrade Elasticsearch image to 6.5.4 #71

Closed jraddaoui closed 5 years ago

jraddaoui commented 5 years ago

Needed after https://github.com/artefactual/archivematica/pull/1285 gets merged and its submodule updated in here.

Connects to https://github.com/artefactual/archivematica/issues/1171.

jraddaoui commented 5 years ago

Hi @sevein,

I think there is no need to use expose when the port is included in ports, so we could remove that from all the services. But I wonder why was the privileged option in the ES service, is that needed using a named volume?

sevein commented 5 years ago

I think there is no need to use expose when the port is included in ports, so we could remove that from all the services.

I didn't know that, cool. So from other containers, the original port would remain right? E.g. 9200 from a container, 62002 from the host?

But I wonder why was the privileged option in the ES service, is that needed using a named volume?

Probably that was a workaround to make it work in early versions of Docker for Mac, if I remember correctly. I think it's a good idea to remove it, if any problem comes up we can find a better solution.

jraddaoui commented 5 years ago

I didn't know that, cool. So from other containers, the original port would remain right? E.g. 9200 from a container, 62002 from the host?

That's right ;)

jraddaoui commented 5 years ago

I'll update the archivematica sub-module when artefactual/archivematica#1171 is merged and merge this PR.

sevein commented 5 years ago

@jraddaoui will the flush-elasticsearch-indices rule need to be updated?

https://github.com/artefactual-labs/am/blob/f8596df7e1a4a5ea73728b4c76ab6d1552a9a2e7/compose/Makefile#L133-L134

jraddaoui commented 5 years ago

Yes, good point!

jraddaoui commented 5 years ago

I've fixed the flush ES indexes rule and changed the image again to a more recent 6.x version. I think it's okay to merge this PR with --ff-only once I update the sub-modules.

ross-spencer commented 5 years ago

@sevein @jraddaoui would it be prudent to cut a stable/1.8.x branch for am.git before merging into master? I suspect the additional indices will break the Makefile for 1.8. It might be good to have one for 1.8 anyway, but what do y'all think?

sevein commented 5 years ago

@sevein @jraddaoui would it be prudent to cut a stable/1.8.x branch for am.git before merging into master?

That's a good idea. But there would be more work needed, e.g. tweak the project name, submodules, volumes, etc... I'd say that it almost deserves a new ticket maybe? I would personally be happy not making it a priority nor a blocker since we haven't really made a promise about what's supported other than the latest development tip of the project. Or maybe we did?

ross-spencer commented 5 years ago

Definitely its own ticket. Might be one to discuss Monday -in hindsight, we can probably cut it from a commit anyway so this shouldn't be a blocker.

The impact, i think, will be measured in how many users we're supporting that we have on 1.7, 1.8, and 1.9 eventually, and easily being able to switch between those.

jraddaoui commented 5 years ago

I like the idea of supporting stable versions too. So far, I've never trusted anything behind the submodules commits in here and I use the Vagrant Boxes for that matter.