Recover geoip.region_iso_code field in test expectations

[jsoriano]

This field appears on ES 6.5.X

This reverts part of commit e229852.

[exekias]

LGTM, I guess we should see green on these tests

[jsoriano]

jenkins, test this again

[jsoriano]

Tests fixed in travis, affected tests also passed on jenkins but other things failed, I think they are unrelated but lets wait for another run.

[jsoriano]

"Fixed" tests failed now in jenkins, @ruflin do you know if it is possible that different geoip plugin versions are used in jenkins and travis? Maybe we need to do these tests less strict with the comparison of objects.

[ruflin]

It could be that a different "snapshot" of Elasticsearch is used and we have to invalidate the cache in Jenkins. I thought it does it checks automatically every time because we use the --pull flag.

What happens if you run it locally?

[jsoriano]

Locally it works, it also worked on travis, and it seemed to work in the first jenkins build too.

[jsoriano]

I'm worried that this kind of plugins that inject fields could make us to need to revisit these checks from time to time, what about completely ignoring the geoip fields for this test?

[jsoriano]

I have added a whitelist for fields that may change, though the geoip fields could be placed on any target field that doesn't need to contain geoip, so I'm not sure of this change.
If accepted with the whitelist this should be merged also in master.

Opinions?

[ruflin]

I think there are 2 parts here:

• Validating that the fields are there
• Checking if the fields are identical

I agree that the identical part is not optimal for geoip to run tests on as it might change over time. At the same time I think we should check that the fields are there and if some are added / removed we should know as it could be that our dashboards rely on it.

Could we modify it that for the whitelist fields we don't do a comparison but still a check if they are there?

If it was working first and then stopped working I would assume it's a different jenkins worker and they have different versions of the docker image.

[jsoriano]

I agree that the identical part is not optimal for geoip to run tests on as it might change over time. At the same time I think we should check that the fields are there and if some are added / removed we should know as it could be that our dashboards rely on it.

Good point on dashboards.

Could we modify it that for the whitelist fields we don't do a comparison but still a check if they are there?

In the case of geoip.region_iso_code it wouldn't be enough, as the problem was that in some versions it was present and in others it wasn't.

I'm going to try to make it work as it was before, without the whitelist thing, this hasn't been so problematic till now, and I can open another PR to discuss how to do the separation of checking equality and validating that fields are there.

[jsoriano]

Affected test passed after adding an explicit pull.

[jsoriano]

jenkins, test this again

[ruflin]

Should we forward port this change?

[jsoriano]

Yes, I will open a PR to master with the second commit here.