Repository: saltstack/salt
Pull request: Gate elasticsearch runner requirements
### What does this PR do?
Ensure that elasticsearch.index_exists is available before loading the elasticsearch returner.

### What issues does this PR fix or reference?
Fixes: #58015 

### Merge requirements satisfied?
**[NOTICE] Bug fixes or features added to Salt require tests.**
<!-- Please review the [test documentation](https://docs.saltstack.com/en/master/topics/tutorials/writing_tests.html) for details on how to implement tests into Salt's test suite. -->
- [ ] Docs
- [x] Changelog - https://docs.saltstack.com/en/master/topics/development/changelog.html
- [x] Tests written/updated

### Commits signed with GPG?
Yes/No

Please review [Salt's Contributing Guide](https://docs.saltstack.com/en/master/topics/development/contributing.html) for best practices.

See GitHub's [page on GPG signing](https://help.github.com/articles/signing-commits-using-gpg/) for more information about signing commits with GPG.


Problem statement:
Elasticsearch returner fails when index exists.
### Description of Issue
When I trigger the elasticsearch returner the first time an index is created, on subsequent calls it just fails. 

### Setup
```
elasticsearch:
    hosts:
      - "elastic7:9200"
      - "elastic8:9200"
    index_date: False
    number_of_shards: 3
    number_of_replicas: 1
    debug_returner_payload: True
    states_count: True
    states_order_output: True
    states_single_index: True
```

### Steps to Reproduce Issue
Trigger the returner. 

Here's the logs
```
2020-07-23 16:19:52,469 [salt.minion      :2022][ERROR   ][29262] The return failed for job 20200723141934627314: 'elasticsearch.index_exists'
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/minion.py", line 2011, in _thread_return
    minion_instance.returners[returner_str](ret)
  File "/usr/lib/python3/dist-packages/salt/returners/elasticsearch_return.py", line 301, in returner
    _ensure_index(index)
  File "/usr/lib/python3/dist-packages/salt/returners/elasticsearch_return.py", line 177, in _ensure_index
    index_exists = __salt__["elasticsearch.index_exists"](index)
  File "/usr/lib/python3/dist-packages/salt/loader.py", line 1278, in __getitem__
    func = super(LazyLoader, self).__getitem__(item)
  File "/usr/lib/python3/dist-packages/salt/utils/lazy.py", line 108, in __getitem__
    raise KeyError(key)
KeyError: 'elasticsearch.index_exists'
```


### Versions Report

```
salt --versions-report
Salt Version:
           Salt: 3001
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: unknown
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: 2.0.3
      gitpython: 2.1.8
         Jinja2: 2.10
        libgit2: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: 1.3.10
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: 3.4.7
         pygit2: Not Installed
         Python: 3.6.9 (default, Apr 18 2020, 01:56:04)
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 17.1.2
          smmap: 2.0.3
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.1
 
System Versions:
           dist: ubuntu 18.04 Bionic Beaver
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-101-generic
         system: Linux
        version: Ubuntu 18.04 Bionic Beaver

```

Hints from issue discussion:
@waynegemmell thanks for the report, could you please provide elastic version you're using?
Elastic 7.8

On Thu, Jul 23, 2020, 21:01 Kirill Ponomarev <notifications@github.com>
wrote:

> @waynegemmell <https://github.com/waynegemmell> thanks for the report,
> could you please provide elastic version you're using?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/saltstack/salt/issues/58015#issuecomment-663178332>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAFORIWYOMW4V3FVN6JN6QDR5CCHVANCNFSM4PFZOI3A>
> .
>

@waynegemmell thanks for the report. Salt can find index_exists and it exists. I don’t know why it cant find it but this is a bug.
@waynegemmell Looking into this I saw the exception that you mentioned because I didn't have the Python `elasticsearch` library installed which is needed by the elastichsearch Salt module, which the returner is calling when checking that the index exists.  Can you verify that you have that library installed?  Thanks!

Commit message: Adding changelog.

Patch to review:
diff --git a/changelog/58851.fixed b/changelog/58851.fixed
new file mode 100644
index 000000000000..961fd2a7bd61
--- /dev/null
+++ b/changelog/58851.fixed
@@ -0,0 +1 @@
+Ensure that elasticsearch.index_exists is available before loading the elasticsearch returner.
diff --git a/salt/returners/elasticsearch_return.py b/salt/returners/elasticsearch_return.py
index b0f67d97432b..3f85504204bb 100644
--- a/salt/returners/elasticsearch_return.py
+++ b/salt/returners/elasticsearch_return.py
@@ -1,4 +1,3 @@
-# -*- coding: utf-8 -*-
 """
 Return data to an elasticsearch server for indexing.
 
@@ -94,22 +93,16 @@
           - saltutil.find_job
 """
 
-# Import Python libs
-from __future__ import absolute_import, print_function, unicode_literals
 
 import datetime
 import logging
 import uuid
 from datetime import timedelta, tzinfo
 
-# Import Salt libs
 import salt.returners
 import salt.utils.jid
 import salt.utils.json
 
-# Import 3rd-party libs
-from salt.ext import six
-
 __virtualname__ = "elasticsearch"
 
 log = logging.getLogger(__name__)
@@ -122,6 +115,11 @@
 
 
 def __virtual__():
+    if "elasticsearch.index_exists" not in __salt__:
+        log.debug(
+            "Elasticsearch module not availble.  Check that the elasticsearch library is installed."
+        )
+        return False
     return __virtualname__
 
 
@@ -184,8 +182,8 @@ def _ensure_index(index):
                 "number_of_replicas": options["number_of_replicas"],
             }
         }
-        __salt__["elasticsearch.index_create"]("{0}-v1".format(index), index_definition)
-        __salt__["elasticsearch.alias_create"]("{0}-v1".format(index), index)
+        __salt__["elasticsearch.index_create"]("{}-v1".format(index), index_definition)
+        __salt__["elasticsearch.alias_create"]("{}-v1".format(index), index)
 
 
 def _convert_keys(data):
@@ -238,12 +236,12 @@ def returner(ret):
 
     # Build the index name
     if options["states_single_index"] and job_fun in STATE_FUNCTIONS:
-        index = "salt-{0}".format(STATE_FUNCTIONS[job_fun])
+        index = "salt-{}".format(STATE_FUNCTIONS[job_fun])
     else:
-        index = "salt-{0}".format(job_fun_escaped)
+        index = "salt-{}".format(job_fun_escaped)
 
     if options["index_date"]:
-        index = "{0}-{1}".format(index, datetime.date.today().strftime("%Y.%m.%d"))
+        index = "{}-{}".format(index, datetime.date.today().strftime("%Y.%m.%d"))
 
     counts = {}
 
@@ -262,23 +260,23 @@ def returner(ret):
         # index to be '<index>-ordered' so as not to clash with the unsorted
         # index data format
         if options["states_order_output"] and isinstance(ret["return"], dict):
-            index = "{0}-ordered".format(index)
-            max_chars = len(six.text_type(len(ret["return"])))
+            index = "{}-ordered".format(index)
+            max_chars = len(str(len(ret["return"])))
 
-            for uid, data in six.iteritems(ret["return"]):
+            for uid, data in ret["return"].items():
                 # Skip keys we've already prefixed
                 if uid.startswith(tuple("0123456789")):
                     continue
 
                 # Store the function being called as it's a useful key to search
                 decoded_uid = uid.split("_|-")
-                ret["return"][uid]["_func"] = "{0}.{1}".format(
+                ret["return"][uid]["_func"] = "{}.{}".format(
                     decoded_uid[0], decoded_uid[-1]
                 )
 
                 # Prefix the key with the run order so it can be sorted
-                new_uid = "{0}_|-{1}".format(
-                    six.text_type(data["__run_num__"]).zfill(max_chars), uid,
+                new_uid = "{}_|-{}".format(
+                    str(data["__run_num__"]).zfill(max_chars), uid,
                 )
 
                 ret["return"][new_uid] = ret["return"].pop(uid)
@@ -344,7 +342,7 @@ def event_return(events):
     doc_type = options["master_event_doc_type"]
 
     if options["index_date"]:
-        index = "{0}-{1}".format(index, datetime.date.today().strftime("%Y.%m.%d"))
+        index = "{}-{}".format(index, datetime.date.today().strftime("%Y.%m.%d"))
 
     _ensure_index(index)
 
diff --git a/tests/unit/returners/test_elasticsearch_return.py b/tests/unit/returners/test_elasticsearch_return.py
new file mode 100644
index 000000000000..0f407a709bf7
--- /dev/null
+++ b/tests/unit/returners/test_elasticsearch_return.py
@@ -0,0 +1,37 @@
+"""
+Test the elasticsearch returner
+"""
+import salt.returners.elasticsearch_return as elasticsearch_return
+from tests.support.mixins import LoaderModuleMockMixin
+from tests.support.mock import MagicMock, patch
+from tests.support.unit import TestCase
+
+
+class ElasticSearchReturnerTestCase(TestCase, LoaderModuleMockMixin):
+    """
+    Test the functions in the elasticsearch returner
+    """
+
+    def setup_loader_modules(self):
+        return {elasticsearch_return: {}}
+
+    def test__virtual_no_elasticsearch(self):
+        """
+        Test __virtual__ function when elasticsearch is not installed
+        and the elasticsearch module is not available
+        """
+        result = elasticsearch_return.__virtual__()
+        expected = False
+        self.assertEqual(expected, result)
+
+    def test__virtual_with_elasticsearch(self):
+        """
+        Test __virtual__ function when elasticsearch
+        and the elasticsearch module is not available
+        """
+        with patch.dict(
+            elasticsearch_return.__salt__, {"elasticsearch.index_exists": MagicMock()}
+        ):
+            result = elasticsearch_return.__virtual__()
+            expected = "elasticsearch"
+            self.assertEqual(expected, result)


Code review comments:
diff_hunk: @@ -122,6 +115,11 @@
 
 
 def __virtual__():
+    if "elasticsearch.index_exists" not in __salt__:
+        log.debug(
+            "Elasticsearch module not availble.  Check that the elasticsearch library is installed."
+        )
+        return False
path: salt/returners/elasticsearch_return.py
text: @user1:
```suggestion
        return False, "Elasticsearch module not availble.  Check that the elasticsearch library is installed."
```