openshift inventory plugin: fix exception when auth fails #45826

pilou- · 2018-09-19T02:09:23Z

SUMMARY

Fix ForbiddenError object has no attribute message:

[WARNING]:  * Failed to parse test.yml with openshift plugin: 'ForbiddenError' object has no attribute 'message'
 File "ansible/lib/ansible/inventory/manager.py", line 270, in parse_source
   plugin.parse(self._inventory, self._loader, source, cache=cache)
 File "ansible/lib/ansible/plugins/inventory/openshift.py", line 122, in parse
   self.setup(config_data, cache, cache_key)
 File "ansible/lib/ansible/module_utils/k8s/inventory.py", line 58, in setup
   self.fetch_objects(connections)
 File "ansible/lib/ansible/module_utils/k8s/inventory.py", line 250, in fetch_objects
   super(OpenShiftInventoryHelper, self).fetch_objects(connections)
 File "ansible/lib/ansible/module_utils/k8s/inventory.py", line 81, in fetch_objects
   namespaces = self.get_available_namespaces(client)
 File "ansible/lib/ansible/module_utils/k8s/inventory.py", line 95, in get_available_namespaces
   raise K8sInventoryException('Error fetching Namespace list: {0}'.format(exc.message))

Don't try to get message attribute from:

K8sInventoryException instances
Exception instances
KubernetesException instances (because KubernetesException can be Exception)

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

lib/ansible/plugins/inventory/openshift.py

ANSIBLE VERSION

ansible 2.8.0.dev0 (devel af40d8c2a5) last updated 2018/09/19 01:46:21 (GMT +200)

ansibot · 2018-09-19T02:23:38Z

cc @chouseknecht @fabianvf @flaper87 @maxamillion @willthames
click here for bot help

willthames · 2018-09-19T04:02:06Z

I'd rather not use str.format(exc) for DynamicApiException, it's very verbose - can we use json.loads(exc.body)['message'] instead?

(would be good to have a helper function to do that, I want to make the same change in k8s/raw.py too)

pilou- · 2018-09-19T10:26:16Z

Done.

Note that we loose the original backtrace. Once merged, I will ask if backports (2.7 and 2.6) are allowed (if so, i will create backport PRs) and create another PR in order to use format_dynamic_api_exc helper in k8s/raw.py.

Fix 'ForbiddenError' object has no attribute 'message': [WARNING]: * Failed to parse test.yml with openshift plugin: 'ForbiddenError' object has no attribute 'message' File "ansible/lib/ansible/inventory/manager.py", line 270, in parse_source plugin.parse(self._inventory, self._loader, source, cache=cache) File "ansible/lib/ansible/plugins/inventory/openshift.py", line 122, in parse self.setup(config_data, cache, cache_key) File "ansible/lib/ansible/module_utils/k8s/inventory.py", line 58, in setup self.fetch_objects(connections) File "ansible/lib/ansible/module_utils/k8s/inventory.py", line 250, in fetch_objects super(OpenShiftInventoryHelper, self).fetch_objects(connections) File "ansible/lib/ansible/module_utils/k8s/inventory.py", line 81, in fetch_objects namespaces = self.get_available_namespaces(client) File "ansible/lib/ansible/module_utils/k8s/inventory.py", line 95, in get_available_namespaces raise K8sInventoryException('Error fetching Namespace list: {0}'.format(exc.message)) Don't try to get 'message' attribute from: - K8sInventoryException instances - Exception instances - KubernetesException instances (because KubernetesException can be Exception)

willthames · 2018-09-19T10:44:01Z

@pilou- that looks great. I'm not clear why we lose the backtrace though, and if there's anything we can do to get it back (is it just that it was part of e.message?)

We can presumably use the traceback library and maybe put that in as a keyword arg to the exception (a bit like how fail_json_aws works - perhaps a fail_json_k8s would look pretty similar?)

inventory plugin specific code should not be located in lib/ansible/module_utils directory. Then ansible.utils methods can be reused (for example Display).

pilou- · 2018-09-20T01:31:30Z

About the traceback: it is not possible to use something like fail_json_aws because there is no AnsibleModule instance there. In fact module.utils.Display should be used in order to be more verbose when debug is enabled. That's why I moved all code specific to inventory only from ansible.module_utils.k8s to ansible.plugins.inventory.k8s.

Now Display can be used and traceback is displayed when -vvv switch is used.
Note there isn't any license problem since ansible.module_utils.k8s is GPL.

fabianvf · 2018-09-24T15:15:34Z

lib/ansible/plugins/inventory/openshift.py

-        cache_key = self._get_cache_prefix(path)
-        config_data = self._read_config_data(path)
-        self.setup(config_data, cache, cache_key)
+    helper = None


what is this for?

It comes from:

ansible/lib/ansible/module_utils/k8s/inventory.py

Line 246 in db5b65f

helper = None

Not used since 4d77878: I removed it.

unused since 4d77878.

fabianvf · 2018-09-24T17:46:02Z

willthames · 2018-09-26T02:06:08Z

lib/ansible/plugins/inventory/k8s.py

    NAME = 'k8s'

+    transport = 'kubectl'


What is this for? kubectl is not used at all by ansible, afaik

I didnt' add it, it comes from:

ansible/lib/ansible/module_utils/k8s/inventory.py

Line 40 in f73996e

transport = 'kubectl'

It is used there in order to define connection plugin (kubectl or oc) which will be used with the generated inventory:

ansible/lib/ansible/module_utils/k8s/inventory.py

Lines 161 to 167 in f73996e

self.inventory.set_variable(container_name, 'ansible_connection', self.transport)

self.inventory.set_variable(container_name, 'ansible_{0}_pod'.format(self.transport),

pod_name)

self.inventory.set_variable(container_name, 'ansible_{0}_container'.format(self.transport),

container.name)

self.inventory.set_variable(container_name, 'ansible_{0}_namespace'.format(self.transport),

namespace)

kubectl is a valid connection plugin, without looking into the code further I assume that's being used as the transport backend in that inventory plugin.

https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/connection/kubectl.py

Yeah, ansible_connection selects the connection plugin :)

fabianvf · 2018-09-26T18:32:16Z

bot_status

ansibot · 2018-09-26T18:36:41Z

Components

lib/ansible/module_utils/k8s/common.py
support: community
maintainers: chouseknecht fabianvf flaper87 maxamillion willthames

lib/ansible/module_utils/k8s/inventory.py
support: community
maintainers: chouseknecht fabianvf flaper87 maxamillion willthames

lib/ansible/module_utils/k8s/scale.py
support: community
maintainers: chouseknecht fabianvf flaper87 maxamillion willthames

lib/ansible/plugins/inventory/k8s.py
support: community
maintainers:

lib/ansible/plugins/inventory/openshift.py
support: community
maintainers:

Metadata

waiting_on: maintainer
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 0
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 0
shipit_actors (maintainer or core team member): []
shipit_actors_other: []
automerge: automerge shipit test failed

click here for bot help

fabianvf · 2018-09-26T18:39:58Z

shipit

maxamillion · 2018-09-26T18:55:01Z

rebuild_merge

ansibot removed the needs_triage Needs a first human triage before being processed. label Sep 19, 2018

pilou- force-pushed the oc_plugin_inventory_object_has_no_attribute_message branch from 9152b23 to 065bf81 Compare September 19, 2018 10:22

pilou- force-pushed the oc_plugin_inventory_object_has_no_attribute_message branch 2 times, most recently from 15af713 to e804f24 Compare September 19, 2018 10:37

pilou- force-pushed the oc_plugin_inventory_object_has_no_attribute_message branch from e804f24 to 9d06668 Compare September 19, 2018 10:40

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Sep 19, 2018

move k8s/OpenShift inventory plugin dedicated code

87d02cd

inventory plugin specific code should not be located in lib/ansible/module_utils directory. Then ansible.utils methods can be reused (for example Display).

fabianvf reviewed Sep 24, 2018

View reviewed changes

Remove unused class variables 'helper'

ea8342e

unused since 4d77878.

pilou- mentioned this pull request Sep 26, 2018

OpenShift and k8s inventory plugins: fix "resource field is not json serializable" #46145

Merged

willthames reviewed Sep 26, 2018

View reviewed changes

ansibot added the inventory Inventory category label Sep 26, 2018

ansibot merged commit 2fd18c7 into ansible:devel Sep 26, 2018

ansible locked and limited conversation to collaborators Jul 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

openshift inventory plugin: fix exception when auth fails #45826

openshift inventory plugin: fix exception when auth fails #45826

pilou- commented Sep 19, 2018

ansibot commented Sep 19, 2018

willthames commented Sep 19, 2018 •

edited

pilou- commented Sep 19, 2018

willthames commented Sep 19, 2018 •

edited

pilou- commented Sep 20, 2018

fabianvf Sep 24, 2018

pilou- Sep 24, 2018

fabianvf commented Sep 24, 2018

willthames Sep 26, 2018

pilou- Sep 26, 2018

maxamillion Sep 26, 2018 •

edited

pilou- Sep 26, 2018

fabianvf commented Sep 26, 2018

ansibot commented Sep 26, 2018

fabianvf commented Sep 26, 2018

maxamillion commented Sep 26, 2018

	self.inventory.set_variable(container_name, 'ansible_connection', self.transport)
	self.inventory.set_variable(container_name, 'ansible_{0}_pod'.format(self.transport),
	pod_name)
	self.inventory.set_variable(container_name, 'ansible_{0}_container'.format(self.transport),
	container.name)
	self.inventory.set_variable(container_name, 'ansible_{0}_namespace'.format(self.transport),
	namespace)

openshift inventory plugin: fix exception when auth fails #45826

openshift inventory plugin: fix exception when auth fails #45826

Conversation

pilou- commented Sep 19, 2018

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ansibot commented Sep 19, 2018

willthames commented Sep 19, 2018 • edited

pilou- commented Sep 19, 2018

willthames commented Sep 19, 2018 • edited

pilou- commented Sep 20, 2018

fabianvf Sep 24, 2018

Choose a reason for hiding this comment

pilou- Sep 24, 2018

Choose a reason for hiding this comment

fabianvf commented Sep 24, 2018

willthames Sep 26, 2018

Choose a reason for hiding this comment

pilou- Sep 26, 2018

Choose a reason for hiding this comment

maxamillion Sep 26, 2018 • edited

Choose a reason for hiding this comment

pilou- Sep 26, 2018

Choose a reason for hiding this comment

fabianvf commented Sep 26, 2018

ansibot commented Sep 26, 2018

Components

Metadata

fabianvf commented Sep 26, 2018

maxamillion commented Sep 26, 2018

willthames commented Sep 19, 2018 •

edited

willthames commented Sep 19, 2018 •

edited

maxamillion Sep 26, 2018 •

edited