Metricbeat: Integration tests for haproxy 1.7 and 1.8 #6793
Conversation
jsoriano
force-pushed
the
haproxy-versions-test
branch
2 times, most recently
from
7154f33
to
871ba0f
Compare
jsoriano
force-pushed
the
haproxy-versions-test
branch
from
871ba0f
to
53e5ef7
Compare
| @@ -6,7 +6,7 @@ | |||
| HAPROXY_FIELDS = metricbeat.COMMON_FIELDS + ["haproxy"] | |||
|
|
|||
|
|
|||
| class Test(metricbeat.BaseTest): | |||
| class HaproxyTest(metricbeat.BaseTest): | |||
There was a problem hiding this comment.
The reason we aways call it test is that it makes it very easy to run tests manually: nosetest test_haproxy.py:Test.test_*. What about not postfixing this by Test but just call it Haproxy for example and then have
16Test
17Test
18Test
like you have at the bottom.
There was a problem hiding this comment.
Yeah, indeed haproxy is already in the name of the file, I'll change it when I revisit this :)
| "period": "5s" | ||
| }]) | ||
| self._test_stat() | ||
|
|
||
|
|
||
| class Haproxy_1_6_Test(HaproxyTest): |
There was a problem hiding this comment.
Not for now, but what if we could autogenerate this part? Meaning we would separate the test code from the versions against we want to test. This is getting interesting :-)
There was a problem hiding this comment.
There are ways to generate classes, I could give a try, but I'd wait to see more cases, for example in apache we had to override some parts of the test because each version provides different metrics, it was not only changing the scenario.
There was a problem hiding this comment.
+1 on waiting for some more cases to see how we can generalise it.
| @@ -32,7 +32,7 @@ def test_info_socket(self): | |||
| self.render_config_template(modules=[{ | |||
| "name": "haproxy", | |||
| "metricsets": ["info"], | |||
| "hosts": ["tcp://%s:%d" % (os.getenv('HAPROXY_HOST', 'localhost'), 14567)], | |||
| "hosts": ["tcp://%s:%d" % (self.compose_hosts()[0], 14567)], | |||
There was a problem hiding this comment.
What happens if more then 1 service is running? I think we are fine in this case as there will be only 1 haproxy instance but there are tests which require more then 1 service to be running.
There was a problem hiding this comment.
This method could return a hash with the service name as key, I'll refactor this. It could also return the hash with a list as values, in case some service has multiple replicas, e.g:
{
"proxy": ["172.0.18.1"],
"server": ["172.0.18.2", "172.0.18.3"],
}
I don't know if we have any case with multiple replicas.
There was a problem hiding this comment.
Although if we have different service names for different versions this is not going to work as I'd want, I have to think more about this...
| @@ -43,7 +43,7 @@ The default metricsets are `info`and `stat`. | |||
| [float] | |||
| === Compatibility | |||
|
|
|||
| The HAProxy metricsets were tested with HAProxy 1.6 and are expected to work with all 1.6 versions. | |||
| The HAProxy metricsets are tested with HAProxy versions from 1.6, 1.7 to 1.8. | |||
There was a problem hiding this comment.
Should we find a way to define the supported versions in fields.yml so we can generate this?
There was a problem hiding this comment.
We could have a central place where to define the versions matrix for the tests and use it also to generate doc, yes, this will require further thinking.
There was a problem hiding this comment.
I wonder if it would be enough if we start with an array `version: ["1.7", "2.1", "5.2-alpha"] or if this will not be enough, as we should also have the supported OS in there and for some versions only a subset of metricset works? Perhaps we should also allow to overwrite these values per metricset?
Test haproxy with versions 1.7 and 1.8 apart of 1.5.
It also adds a method to base compose mixin for tests to obtain hosts for tests using docker inspect.