From 46494a18bde79b6df2691e797da76df7ba5e731b Mon Sep 17 00:00:00 2001 From: abikouo <79859644+abikouo@users.noreply.github.com> Date: Tue, 15 Jun 2021 14:32:21 +0200 Subject: [PATCH] Revert "k8s ability to wait on arbitrary property (#105)" (#133) This reverts commit 4ccb15d4ad21c379dfca24cfc799f9baf6ab97f9. --- changelogs/fragments/105-wait_property.yaml | 3 - molecule/default/tasks/waiter.yml | 87 ------------ plugins/doc_fragments/k8s_wait_options.py | 18 --- plugins/module_utils/args_common.py | 8 -- plugins/module_utils/common.py | 47 +++---- plugins/module_utils/jsonpath_extractor.py | 142 -------------------- plugins/modules/k8s.py | 26 ---- plugins/modules/k8s_info.py | 1 - tests/unit/module_utils/test_jsonpath.py | 76 ----------- 9 files changed, 15 insertions(+), 393 deletions(-) delete mode 100644 changelogs/fragments/105-wait_property.yaml delete mode 100644 plugins/module_utils/jsonpath_extractor.py delete mode 100644 tests/unit/module_utils/test_jsonpath.py diff --git a/changelogs/fragments/105-wait_property.yaml b/changelogs/fragments/105-wait_property.yaml deleted file mode 100644 index 4ecabd1a..00000000 --- a/changelogs/fragments/105-wait_property.yaml +++ /dev/null @@ -1,3 +0,0 @@ ---- -minor_changes: - - k8s - add new option ``wait_property`` to support ability to wait on arbitrary property (https://github.com/ansible-collections/kubernetes.core/pull/105). diff --git a/molecule/default/tasks/waiter.yml b/molecule/default/tasks/waiter.yml index 12920aa2..44fc42b3 100644 --- a/molecule/default/tasks/waiter.yml +++ b/molecule/default/tasks/waiter.yml @@ -364,93 +364,6 @@ that: - short_wait_remove_pod is failed - - name: add a simple crashing pod and wait until container is running - k8s: - definition: - apiVersion: v1 - kind: Pod - metadata: - name: pod-crash-0 - namespace: "{{ wait_namespace }}" - spec: - containers: - - name: crashing-container - image: busybox - command: ['/dummy/dummy-shell', '-c', 'sleep 2000'] - wait: yes - wait_timeout: 10 - wait_property: - property: status.containerStatuses[*].state.running - ignore_errors: true - register: crash_pod - - - name: assert that task failed - assert: - that: - - crash_pod is failed - - crash_pod is changed - - '"Resource creation timed out" in crash_pod.msg' - - - name: add a valid pod and wait until container is running - k8s: - definition: - apiVersion: v1 - kind: Pod - metadata: - name: pod-valid-0 - namespace: "{{ wait_namespace }}" - spec: - containers: - - name: crashing-container - image: busybox - command: ['/bin/sh', '-c', 'sleep 10000'] - wait: yes - wait_timeout: 10 - wait_property: - property: status.containerStatuses[*].state.running - ignore_errors: true - register: valid_pod - - - name: assert that task failed - assert: - that: - - valid_pod is successful - - valid_pod.changed - - valid_pod.result.status.containerStatuses[0].state.running is defined - - - name: create pod (waiting for container.ready set to false) - k8s: - definition: - apiVersion: v1 - kind: Pod - metadata: - name: redis-pod - namespace: "{{ wait_namespace }}" - spec: - containers: - - name: redis-container - image: redis - volumeMounts: - - name: test - mountPath: "/etc/test" - readOnly: true - volumes: - - name: test - configMap: - name: redis-config - wait: yes - wait_timeout: 10 - wait_property: - property: status.containerStatuses[0].ready - value: "false" - register: wait_boolean - - - name: assert that pod was created but not running - assert: - that: - - wait_boolean.changed - - wait_boolean.result.status.phase == 'Pending' - always: - name: Remove namespace k8s: diff --git a/plugins/doc_fragments/k8s_wait_options.py b/plugins/doc_fragments/k8s_wait_options.py index 7cc74e55..06600564 100644 --- a/plugins/doc_fragments/k8s_wait_options.py +++ b/plugins/doc_fragments/k8s_wait_options.py @@ -64,22 +64,4 @@ options: - The possible reasons in a condition are specific to each resource type in Kubernetes. - See the API documentation of the status field for a given resource to see possible choices. type: dict - wait_property: - description: - - Specifies a property on the resource to wait for. - - Ignored if C(wait) is not set or is set to I(False). - type: dict - version_added: '2.1.0' - suboptions: - property: - type: str - required: True - description: - - The property name to wait for. - value: - type: str - description: - - The expected value of the C(property). - - The value is not case-sensitive. - - If this is missing, we will check only that the attribute C(property) is present. ''' diff --git a/plugins/module_utils/args_common.py b/plugins/module_utils/args_common.py index 10171ae9..67c183db 100644 --- a/plugins/module_utils/args_common.py +++ b/plugins/module_utils/args_common.py @@ -70,14 +70,6 @@ WAIT_ARG_SPEC = dict( status=dict(default=True, choices=[True, False, "Unknown"]), reason=dict() ) - ), - wait_property=dict( - type='dict', - default=None, - options=dict( - property=dict(required=True), - value=dict() - ) ) ) diff --git a/plugins/module_utils/common.py b/plugins/module_utils/common.py index 0c094d27..e3a48cca 100644 --- a/plugins/module_utils/common.py +++ b/plugins/module_utils/common.py @@ -29,7 +29,6 @@ from distutils.version import LooseVersion from ansible_collections.kubernetes.core.plugins.module_utils.args_common import (AUTH_ARG_MAP, AUTH_ARG_SPEC, AUTH_PROXY_HEADERS_SPEC) from ansible_collections.kubernetes.core.plugins.module_utils.hashes import generate_hash -from ansible_collections.kubernetes.core.plugins.module_utils.jsonpath_extractor import validate_with_jsonpath from ansible.module_utils.basic import missing_required_lib from ansible.module_utils.six import iteritems, string_types @@ -37,7 +36,6 @@ from ansible.module_utils._text import to_native, to_bytes, to_text from ansible.module_utils.common.dict_transformations import dict_merge from ansible.module_utils.parsing.convert_bool import boolean - K8S_IMP_ERR = None try: import kubernetes @@ -232,7 +230,7 @@ class K8sAnsibleMixin(object): self.fail(msg='Failed to find exact match for {0}.{1} by [kind, name, singularName, shortNames]'.format(api_version, kind)) def kubernetes_facts(self, kind, api_version, name=None, namespace=None, label_selectors=None, field_selectors=None, - wait=False, wait_sleep=5, wait_timeout=120, state='present', condition=None, property=None): + wait=False, wait_sleep=5, wait_timeout=120, state='present', condition=None): resource = self.find_resource(kind, api_version) api_found = bool(resource) if not api_found: @@ -288,7 +286,7 @@ class K8sAnsibleMixin(object): for resource_instance in resource_list: success, res, duration = self.wait(resource, resource_instance, sleep=wait_sleep, timeout=wait_timeout, - state=state, condition=condition, property=property) + state=state, condition=condition) if not success: self.fail(msg="Failed to gather information about %s(s) even" " after waiting for %s seconds" % (res.get('kind'), duration)) @@ -351,7 +349,7 @@ class K8sAnsibleMixin(object): def fail(self, msg=None): self.fail_json(msg=msg) - def _wait_for(self, resource, name, namespace, predicates, sleep, timeout, state): + def _wait_for(self, resource, name, namespace, predicate, sleep, timeout, state): start = datetime.now() def _wait_for_elapsed(): @@ -361,7 +359,7 @@ class K8sAnsibleMixin(object): while _wait_for_elapsed() < timeout: try: response = resource.get(name=name, namespace=namespace) - if all([predicate(response) for predicate in predicates]): + if predicate(response): if response: return True, response.to_dict(), _wait_for_elapsed() return True, {}, _wait_for_elapsed() @@ -373,7 +371,7 @@ class K8sAnsibleMixin(object): response = response.to_dict() return False, response, _wait_for_elapsed() - def wait(self, resource, definition, sleep, timeout, state='present', condition=None, property=None): + def wait(self, resource, definition, sleep, timeout, state='present', condition=None): def _deployment_ready(deployment): # FIXME: frustratingly bool(deployment.status) is True even if status is empty @@ -424,29 +422,19 @@ class K8sAnsibleMixin(object): def _resource_absent(resource): return not resource - def _wait_for_property(resource): - return validate_with_jsonpath(self, resource.to_dict(), property.get('property'), property.get('value', None)) - waiter = dict( Deployment=_deployment_ready, DaemonSet=_daemonset_ready, Pod=_pod_ready ) kind = definition['kind'] - predicates = [] - if state == 'present': - if condition is None and property is None: - predicates.append(waiter.get(kind, lambda x: x)) - else: - if condition: - # add waiter on custom condition - predicates.append(_custom_condition) - if property: - # json path predicate - predicates.append(_wait_for_property) + if state == 'present' and not condition: + predicate = waiter.get(kind, lambda x: x) + elif state == 'present' and condition: + predicate = _custom_condition else: - predicates = [_resource_absent] - return self._wait_for(resource, definition['metadata']['name'], definition['metadata'].get('namespace'), predicates, sleep, timeout, state) + predicate = _resource_absent + return self._wait_for(resource, definition['metadata']['name'], definition['metadata'].get('namespace'), predicate, sleep, timeout, state) def set_resource_definitions(self, module): resource_definition = module.params.get('resource_definition') @@ -589,7 +577,6 @@ class K8sAnsibleMixin(object): continue_on_error = self.params.get('continue_on_error') if self.params.get('wait_condition') and self.params['wait_condition'].get('type'): wait_condition = self.params['wait_condition'] - wait_property = self.params.get('wait_property') def build_error_msg(kind, name, msg): return "%s %s: %s" % (kind, name, msg) @@ -699,8 +686,7 @@ class K8sAnsibleMixin(object): success = True result['result'] = k8s_obj if wait and not self.check_mode: - success, result['result'], result['duration'] = self.wait(resource, definition, wait_sleep, wait_timeout, - condition=wait_condition, property=wait_property) + success, result['result'], result['duration'] = self.wait(resource, definition, wait_sleep, wait_timeout, condition=wait_condition) if existing: existing = existing.to_dict() else: @@ -760,8 +746,7 @@ class K8sAnsibleMixin(object): success = True result['result'] = k8s_obj if wait and not self.check_mode: - success, result['result'], result['duration'] = self.wait(resource, definition, wait_sleep, wait_timeout, - condition=wait_condition, property=wait_property) + success, result['result'], result['duration'] = self.wait(resource, definition, wait_sleep, wait_timeout, condition=wait_condition) result['changed'] = True result['method'] = 'create' if not success: @@ -796,8 +781,7 @@ class K8sAnsibleMixin(object): success = True result['result'] = k8s_obj if wait and not self.check_mode: - success, result['result'], result['duration'] = self.wait(resource, definition, wait_sleep, wait_timeout, - condition=wait_condition, property=wait_property) + success, result['result'], result['duration'] = self.wait(resource, definition, wait_sleep, wait_timeout, condition=wait_condition) match, diffs = self.diff_objects(existing.to_dict(), result['result']) result['changed'] = not match result['method'] = 'replace' @@ -831,8 +815,7 @@ class K8sAnsibleMixin(object): success = True result['result'] = k8s_obj if wait and not self.check_mode: - success, result['result'], result['duration'] = self.wait(resource, definition, wait_sleep, wait_timeout, - condition=wait_condition, property=wait_property) + success, result['result'], result['duration'] = self.wait(resource, definition, wait_sleep, wait_timeout, condition=wait_condition) match, diffs = self.diff_objects(existing.to_dict(), result['result']) result['changed'] = not match result['method'] = 'patch' diff --git a/plugins/module_utils/jsonpath_extractor.py b/plugins/module_utils/jsonpath_extractor.py deleted file mode 100644 index 5e34583e..00000000 --- a/plugins/module_utils/jsonpath_extractor.py +++ /dev/null @@ -1,142 +0,0 @@ -# Copyright [2021] [Red Hat, Inc.] -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - - -from __future__ import absolute_import, division, print_function -__metaclass__ = type - -from ansible.module_utils._text import to_native - - -class JsonPathException(Exception): - """ Error while parsing Json path structure """ - - -def find_element(value, start): - dot_idx = value.find(".", start) - end = dot_idx if dot_idx != -1 else len(value) - arr_idx = value.find("[", start, end) - - if dot_idx == -1 and arr_idx == -1: - # last element of the json path - if value[start] == '[': - raise JsonPathException("unable to find array end string for array starting at index {0} '{1}'".format(start, value[start:])) - return value[start:], None - - elif arr_idx != -1: - if arr_idx == start: - # array element (ex: "[0]" or "[*].ready" or "[*][0].ready" ) - arr_end = value.find("]", arr_idx) - if arr_end == -1: - raise JsonPathException("unable to find array end string for array starting at index {0} '{1}'".format(arr_idx, value[arr_idx:])) - data = value[arr_idx + 1:arr_end] - if data != "*" and not data.isnumeric(): - raise JsonPathException("wrong value specified into array starting at index {0} => '{1}'".format(arr_idx, data)) - return int(data) if data != "*" else -1, arr_end + 1 if arr_end < len(value) - 1 else None - elif arr_idx > start: - # single value found (ex: "containers[0]") - return value[start:arr_idx], arr_idx - - else: # dot_idx != -1 - return value[start:dot_idx], dot_idx + 1 - - -def parse(expr): - result = [] - if expr[0] == ".": - expr = expr[1:] - start = 0 - while start is not None: - elt, next = find_element(expr, start) - if elt == '': - if not isinstance(result[-1], int): - raise JsonPathException("empty element following non array element at index {0} '{1}'".format(start, expr[start:])) - else: - result.append(elt) - start = next - return result - - -def search_json_item(jsonpath_expr, json_doc): - json_idx = 0 - json_item = jsonpath_expr[json_idx] - if isinstance(json_item, int): - if not isinstance(json_doc, list): - # trying to parse list items, but current document is not a list - return None - elements = json_doc - if json_item != -1: - # looking for specific index from the list - if json_item >= len(json_doc): - return None - else: - elements = json_doc[json_item] - - # when we reach the end of the json path - if len(jsonpath_expr) == 1: - return elements - elif json_item != -1 and (isinstance(elements, dict) or isinstance(elements, list)): - return search_json_item(jsonpath_expr[1:], elements) - elif json_item == -1: - result = [] - for elt in elements: - ret = search_json_item(jsonpath_expr[1:], elt) - if ret is not None: - result.append(ret) - return result if result != [] else None - else: - # looking for a specific field into the json document - if not isinstance(json_doc, dict): - return None - if json_item not in json_doc: - return None - if len(jsonpath_expr) == 1: - return json_doc.get(json_item) - else: - return search_json_item(jsonpath_expr[1:], json_doc.get(json_item)) - - -def search(expr, data): - jsonpath_expr = parse(expr) - return search_json_item(jsonpath_expr, data) - - -def validate_with_jsonpath(module, data, expr, value=None): - def _raise_or_fail(err, **kwargs): - if module and hasattr(module, "fail_json"): - module.fail_json(error=to_native(err), **kwargs) - raise err - - def _match_value(buf, v): - if isinstance(buf, list): - # convert all values from bool to str and lowercase them - return all([str(i).lower() == v.lower() for i in buf]) - elif isinstance(buf, str) or isinstance(buf, int) or isinstance(buf, float): - return v.lower() == str(buf).lower() - elif isinstance(buf, bool): - return v.lower() == str(buf).lower() - else: - # unable to test single value against dict - return False - - try: - content = search(expr, data) - if content is None or content == []: - return False - if value is None or _match_value(content, value): - # looking for state present - return True - return False - except Exception as err: - _raise_or_fail(err, msg="Failed to extract path from Json: {0}".format(expr)) diff --git a/plugins/modules/k8s.py b/plugins/modules/k8s.py index fe47c71f..5bcb51cf 100644 --- a/plugins/modules/k8s.py +++ b/plugins/modules/k8s.py @@ -252,32 +252,6 @@ EXAMPLES = r''' status: Unknown reason: DeploymentPaused -# Wait for this service to have acquired an External IP -- name: Create ingress and wait for ip to be assigned - kubernetes.core.k8s: - template: dash-service.yaml - wait: yes - wait_property: - property: status.loadBalancer.ingress[*].ip - -# Wait for container inside a pod to be ready -- name: Create Pod and wait for containers to be ready - kubernetes.core.k8s: - template: pod.yaml - wait: yes - wait_property: - property: status.containerStatuses[*].ready - value: "true" - -# Wait for first container inside a pod to be ready -- name: Create Pod and wait for first containers to be ready - kubernetes.core.k8s: - template: pod.yaml - wait: yes - wait_property: - property: status.containerStatuses[0].ready - value: "true" - # Patch existing namespace : add label - name: add label to existing namespace kubernetes.core.k8s: diff --git a/plugins/modules/k8s_info.py b/plugins/modules/k8s_info.py index 5871ec32..50059e41 100644 --- a/plugins/modules/k8s_info.py +++ b/plugins/modules/k8s_info.py @@ -164,7 +164,6 @@ def execute_module(module, k8s_ansible_mixin): wait_sleep=module.params["wait_sleep"], wait_timeout=module.params["wait_timeout"], condition=module.params["wait_condition"], - property=module.params["wait_property"] ) module.exit_json(changed=False, **facts) diff --git a/tests/unit/module_utils/test_jsonpath.py b/tests/unit/module_utils/test_jsonpath.py deleted file mode 100644 index 7dca2a47..00000000 --- a/tests/unit/module_utils/test_jsonpath.py +++ /dev/null @@ -1,76 +0,0 @@ -# Copyright [2021] [Red Hat, Inc.] -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - - -from __future__ import absolute_import, division, print_function -__metaclass__ = type - -from ansible_collections.kubernetes.core.plugins.module_utils.jsonpath_extractor import validate_with_jsonpath - - -def test_property_present(): - data = { - "containers": [ - {"name": "t0", "image": "nginx"}, - {"name": "t1", "image": "python"}, - {"name": "t2", "image": "mongo", "state": "running"} - ] - } - assert validate_with_jsonpath(None, data, "containers[*].state") - assert not validate_with_jsonpath(None, data, "containers[*].status") - - -def test_property_value(): - data = { - "containers": [ - {"name": "t0", "image": "nginx"}, - {"name": "t1", "image": "python"}, - {"name": "t2", "image": "mongo", "state": "running"} - ] - } - assert validate_with_jsonpath(None, data, "containers[*].state", "running") - assert validate_with_jsonpath(None, data, "containers[*].state", "Running") - assert not validate_with_jsonpath(None, data, "containers[*].state", "off") - - -def test_boolean_value(): - data = { - "containers": [ - {"image": "nginx", "poweron": False}, - {"image": "python"}, - {"image": "mongo", "connected": True} - ] - } - assert validate_with_jsonpath(None, data, "containers[*].connected", "true") - assert validate_with_jsonpath(None, data, "containers[*].connected", "True") - assert validate_with_jsonpath(None, data, "containers[*].connected", "TRUE") - assert validate_with_jsonpath(None, data, "containers[0].poweron", "false") - - data = { - "containers": [ - {"image": "nginx", "ready": False}, - {"image": "python", "ready": False}, - {"image": "mongo", "ready": True} - ] - } - assert not validate_with_jsonpath(None, data, "containers[*].ready", "true") - - data = { - "containers": [ - {"image": "nginx", "ready": True}, - {"image": "python", "ready": True}, - {"image": "mongo", "ready": True} - ] - } - assert validate_with_jsonpath(None, data, "containers[*].ready", "true")