Skip to content
This repository was archived by the owner on Jan 24, 2018. It is now read-only.

Conversation

@lasley
Copy link
Contributor

@lasley lasley commented Dec 14, 2016

This provides a base Elasticsearch 5.0 template.

The config file is pretty blank, but we can hit that in another ticket after I finish tuning it on my dev install.

Regarding the backups - we have to use another tool called Elasticdump. It's best to run that in a container of its own though, so I just added to the readme.

Did I miss anything else? I feel like I missed something.

@lasley lasley force-pushed the feature/master/elasticsearch branch from fedbe0b to 1f7621b Compare December 14, 2016 00:44
@codecov-io
Copy link

codecov-io commented Dec 14, 2016

Current coverage is 31.01% (diff: 37.03%)

Merging #179 into master will increase coverage by 0.02%

@@             master       #179   diff @@
==========================================
  Files            72         73     +1   
  Lines          5626       5653    +27   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1743       1753    +10   
- Misses         3883       3900    +17   
  Partials          0          0          

Powered by Codecov. Last update 5b44438...c868b64

@@ -0,0 +1,41 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- Copyright 2016 LasLabs Inc.
License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used wrong license, switch to LGPL

<record id="application_elasticsearch" model="clouder.application">
<field name="name">Elasticsearch</field>
<field name="code">elasticsearch</field>
<field name="type_id" ref="application_type_elasticsearch" />
Copy link
Owner

Choose a reason for hiding this comment

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

Since there is a web access you'll probably want to inherit the base_www template to get the link to dns/proxy/monitoring and allow base creation.

Look at the old shinken configuration here : https://github.com/clouder-community/clouder/blob/0.9.0/clouder_template_shinken/template.xml#L70

<field name="name">elasticsearch</field>
<field name="system_user">elasticsearch</field>
<field name="tag_ids"
eval="[(6, 0, [ref('clouder.tag_database')])]"
Copy link
Owner

Choose a reason for hiding this comment

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

I don't believe the tag_database is required for this application

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm but doesn't Elasticsearch meet the definition of a database? I admittedly have no idea what this is actually doing, but it seemed a good idea to make this a database interface.

From there we would attach the Logstash instance (for example) via similar mechanisms used for attaching databases to, say, an Odoo instance. Am I overthinking it?

Copy link
Owner

Choose a reason for hiding this comment

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

Maybe the definition is the same, but we're not gonna use it the same way than postgresql or mysql (like, the place where the database of clouder.base is stored).

The database tag trigger some specific code like https://github.com/clouder-community/clouder/blob/master/clouder/models/service.py#L135, so it's best not to mix them. We'll see later if the database tag is really needed.

<field name="name">listen</field>
<field name="type">service</field>
<field name="default">*</field>
</record>
Copy link
Owner

Choose a reason for hiding this comment

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

Theses options, probably copied from postgres template, are probably not needed

>
<field name="template_id" ref="image_template_elasticsearch_exec" />
<field name="name">elasticsearch-http</field>
<field name="local_port">9200</field>
Copy link
Owner

Choose a reason for hiding this comment

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

The port which need to be redirected by the proxy container need to be named "http"

Copy link
Contributor Author

@lasley lasley Dec 14, 2016

Choose a reason for hiding this comment

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

Is my assumption that switching to https would identify it as an SSL only port? That's not the case here yet, but just good to know.

Copy link
Owner

Choose a reason for hiding this comment

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

@@ -0,0 +1,6 @@
FROM yannickburon/clouder:base
Copy link
Owner

Choose a reason for hiding this comment

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

Are you sure we shall have the versioning number (5.0) in the path ? According to me we with clouder branches and configuration in docker hub we already have what we need.

Moreover, I'd like to keep the same name in path image than the name which will appears on Docker hub, thus clouder_template_elasticsearch/images/elasticsearch-data/Dockerfile

Copy link
Contributor Author

@lasley lasley Dec 14, 2016

Choose a reason for hiding this comment

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

I think the explicit versioning will be good here - Elastic has historically not been very loving to old API between major versions, so the difference between them is fairly drastic.

An upgrade of Elastic sometimes requires a full cluster restart or a reindex, and sometimes things simply don't work after the fact. A great example is the current fracturing of Kibana dashboards - almost nothing works out of the box, because they're for 2.x. This could force users into a version lock while proprietary things are being upgraded, but we need a way to also support bleeding edge.

Due to this, I think we need to manage Elastic major versions in the same respect we would an Odoo version. Should this instead be clouder_template_elasticsearch_5, then drop versioning on inner folder hierarchy?

Copy link
Owner

Choose a reason for hiding this comment

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

Ok I see the point and the comparison with Odoo template. Then, I suggest the path
clouder_template_elasticsearch/images/elasticsearch-data/Dockerfile clouder_template_elasticsearch/images/elasticsearch5-exec/Dockerfile

Exec need to be versionned but not data IMO (same for Odoo template, data isn't versionned)

Copy link
Contributor Author

@lasley lasley Dec 14, 2016

Choose a reason for hiding this comment

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

Hmm but in this case, the data itself could become invalidated by the upgrade. One main thing I've been running into with the Kibana Dashboards has been the removal of support for a specific data type in Elastic v5.

I guess this is similar to the Odoo data though, which would become invalidated with a new exec.

I'm curious then - what is the upgrade path for the non-versioned data, say in the case of Odoo v9 to v10? Some of these also contain configs, which could change by a version.

Copy link
Owner

Choose a reason for hiding this comment

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

That's what I was thinking also, we still need to think about how to manage major migration in Clouder. For example I'd love to have automatic Odoo migration using OpenUpgrade (and I guess I'm not the only one..).

I suggest to stick with current version for now, and get back to it when we would be able to manage major Odoo migration in Clouder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's a definite must. Ok yeah so I'm going to version both the data and exec on this one in order to avoid future changes then. Will use your recommended naming of elasticsearch5-*

@@ -0,0 +1,6 @@
FROM yannickburon/clouder:base
Copy link
Owner

Choose a reason for hiding this comment

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

FROM clouder/base:3.4

@@ -0,0 +1,65 @@
FROM yannickburon/clouder:base
Copy link
Owner

Choose a reason for hiding this comment

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

FROM clouder/base:3.4

_inherit = 'clouder.backup'

@api.multi
def backup_database(self):
Copy link
Owner

Choose a reason for hiding this comment

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

We'll probably call deploy_base function like in https://github.com/clouder-community/clouder/blob/0.9.0/clouder_template_odoo/template.py#L561 and not backup_database.

Elasticdump will need to put files in /base-backup like we do for odoo filestore. Except if the basic container save already do the trick, maybe we don't need elacticdump.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The container save should work now that you say it. I have both the config and the db data stored on the data container, so simply adding another exec onto a copy of it should yield an identical installation assuming that the exec versions are the same.

Copy link
Owner

Choose a reason for hiding this comment

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

If the data are stored in the filesystem it shall be ok. I'll carefully review backup when it'll be merge, let's not worry about it for now.

'category': 'Clouder',
'depends': [
'clouder',
'clouder_template_proxy',
Copy link
Owner

Choose a reason for hiding this comment

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

Since it's a web application, some dependences are probably missing, like dns

@lasley lasley force-pushed the feature/master/elasticsearch branch from 1de2bcf to 8c91737 Compare December 14, 2016 22:05
@lasley lasley force-pushed the feature/master/elasticsearch branch from 27921ef to 1f028ae Compare December 15, 2016 00:11
* Bump version
* Change openerp references to odoo
* Rename manifest
@lasley
Copy link
Contributor Author

lasley commented Dec 19, 2016

@YannickB Alright this is good for a second review and possible merge.

I have another ticket open to add SSL between Elastic and the Proxy. That's dependent on the CA #180 though, so best to just leave as a task IMO.

Did I need to add any proxy configs, or are those all automatic assuming the use of the http tag on the port?

@YannickB
Copy link
Owner

OK LGTM.

The proxy will automatically use the http tag, so no configuration needed here.

@YannickB YannickB merged commit f97985c into YannickB:master Dec 26, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants