New module: management of users in ManageIQ (monitoring/manageiq/manageiq_user) #25106

dkorn · 2017-05-28T07:06:10Z

SUMMARY

ManageIQ is an open source management platform for Hybrid IT.

This change is adding:

manageiq_user module, responsible for user management in ManageIQ
manageiq utils
manageiq doc_fragment

All the modules, including docs, tests and usage examples can be found here

Currently, the only requirement for the modules is manageiq-api-client-python

ISSUE TYPE

New Module Pull Request

COMPONENT NAME

manageiq_user.py (module)
manageiq.py (module utils and doc_fragement)

ANSIBLE VERSION

ansible 2.3.0.0

ansibot · 2017-05-28T07:10:02Z

@dkorn Greetings! Thanks for taking the time to open this pullrequest. In order for the community to handle your pullrequest effectively, we need a bit more information.

Here are the items we could not find in your description:

issue type

Please set the description of this pullrequest with this template:
https://raw.githubusercontent.com/ansible/ansible/devel/.github/PULL_REQUEST_TEMPLATE.md

click here for bot help

ansibot · 2017-05-28T07:10:03Z

cc @dharmabumstead
click here for bot help

dkorn · 2017-05-28T07:22:56Z

@thaumos please review
cc @cben @simon3z @enoodle @zgalor

gundalow · 2017-05-30T11:18:14Z

lib/ansible/utils/module_docs_fragments/manageiq.py

+    description:
+      - manageiq password
+    default: MIQ_PASSWORD env var if set. otherwise, it is required to pass it
+  miq_verify_ssl:


To be consistent with other modules please use:

validate_certs: description: - If C(no), SSL certificates will not be validated. This should only be used on personally controlled sites using self-signed certificates.

Done, thanks!

gundalow · 2017-05-30T11:19:14Z

lib/ansible/modules/monitoring/manageiq/manageiq_user.py

+
+module: manageiq_user
+
+short_description: management of users in ManageIQ


All description should start with a capital letter, and end with a fullstop.

Done, thanks!

gundalow · 2017-05-30T11:21:10Z

lib/ansible/module_utils/manageiq.py

+#
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify


module_utils files should be licences under BSD

https://docs.ansible.com/ansible/dev_guide/developing_modules_in_groups.html#before-you-start-coding

Done, thanks!

thaumos · 2017-05-30T14:14:49Z

@dkorn, please provide Github usernames of who you would like listed as maintainers of the manageiq namespace.

gundalow · 2017-05-30T15:47:13Z

Number of CI issues: https://app.shippable.com/github/ansible/ansible/runs/23524/1/tests

dkorn · 2017-06-04T13:38:10Z

@dkorn, please provide Github usernames of who you would like listed as maintainers of the manageiq namespace.

Maintainers: @cben @dkorn @simon3z

@simon3z - anyone else?

ansibot · 2017-06-05T09:26:10Z

The test ansible-test sanity --test validate-modules failed with the following error:

lib/ansible/modules/monitoring/manageiq/manageiq_user.py:0:0: E312 No RETURN provided

click here for bot help

dkorn · 2017-06-05T12:48:43Z

The test ansible-test sanity --test validate-modules failed with the following error:

lib/ansible/modules/monitoring/manageiq/manageiq_user.py:0:0: E312 No RETURN provided

@gundalow @thaumos the only return values in the manageiq_user module are msg and changed, which are common to all modules (https://docs.ansible.com/ansible/common_return_values.html#common-return-values).

do you want me to leave the RETURN documentation empty or mention them?

dkorn · 2017-06-11T10:36:19Z

@thaumos passed CI, can you approve/merge?

thaumos

lgtm

thaumos · 2017-06-13T19:53:02Z

@dkorn, could you attend the Public Core meeting on 6/15 @ 1500 UTC and raise the PR there? Make sure to add this PR as a comment in ansible/community issue number 162.

simon3z · 2017-06-13T21:13:11Z

@dkorn, could you attend the Public Core meeting on 6/15 @ 1500 UTC and raise the PR there? Make sure to add this PR as a comment in ansible/community issue number 162.

cc @cben

ryansb · 2017-06-15T15:16:05Z

bot_status

thaumos · 2017-06-15T15:24:08Z

lib/ansible/module_utils/manageiq.py

+
+
+import os
+from manageiq_client.api import ManageIQClient


handle import error

gundalow · 2017-06-15T15:18:38Z

lib/ansible/modules/monitoring/manageiq/manageiq_user.py

+    fullname: 'Daniel Korn'
+    password: '******'
+    group: 'EvmGroup-user'
+    email: 'dkorn@redhat.com'


someone@example.com

gundalow · 2017-06-15T15:20:28Z

lib/ansible/utils/module_docs_fragments/manageiq.py

+options:
+  miq_url:
+    description:
+      - The ManageIQ environment url


Full sentences + full stops
Capitals for acronyms

The ManageIQ environment URL.

gundalow · 2017-06-15T15:21:13Z

lib/ansible/utils/module_docs_fragments/manageiq.py

+    default: null
+
+requirements:
+  - 'manageiq-client (source: https://github.com/ManageIQ/manageiq-api-client-python/)'


To make this a clickable link please do
'manageiq-client U(https://github.com/ManageIQ/manageiq-api-client-python/)'

gundalow · 2017-06-15T15:25:36Z

lib/ansible/utils/module_docs_fragments/manageiq.py

+    default: MIQ_URL env var if set. otherwise, it is required to pass it
+  miq_username:
+    description:
+      - ManageIQ username


ManageIQ username. C(MIQ_USERNAME) env var if set. otherwise, it is required to pass it.

and delete the default: line.

This applies to the remainder of this file

ansibot · 2017-06-15T15:28:08Z

waiting_on: dkorn
module: manageiq_user
changes_requested_by: thaumos
needs_info: False
needs_revision: True
needs_rebase: False
merge_commits: []
mergeable_state: clean
shippable_status: success
maintainer_shipits: 0
community_shipits: 0
ansible_shipits: 1
shipit_actors: thaumos

click here for bot help

pilou- · 2017-06-15T15:20:47Z

lib/ansible/modules/monitoring/manageiq/manageiq_user.py

+  state:
+    description:
+      - The state of the user
+      - On present, it will create the user if it does not exist or update the


Use formatting options: for example C(present) instead of present.

pilou- · 2017-06-15T15:26:42Z

lib/ansible/modules/monitoring/manageiq/manageiq_user.py

+    """
+
+    def __init__(self, manageiq):
+        self.changed = False


changed doesn't need to be an attribute.

pilou- · 2017-06-15T15:49:28Z

lib/ansible/modules/monitoring/manageiq/manageiq_user.py

+            Whether or not a change took place and a message describing the
+            operation executed.
+        """
+        group = self.manageiq.find_collection_resource_by('groups', description=group)


group parameter is flagged as not required in argument_spec, what returns the ManageIQ API when group is None ?

pilou- · 2017-06-15T15:51:12Z

lib/ansible/modules/monitoring/manageiq/manageiq_user.py

+                changed=self.changed,
+                msg="User {userid} already exist, no need for updates".format(userid=userid))
+        url = '{api_url}/users/{user_id}'.format(api_url=self.manageiq.api_url, user_id=user.id)
+        resource = {'userid': userid, 'name': username, 'password': password,


I am not sure it the resource dictionary should contain all these keys or only the ones with value which isn't None ? I mean, when password parameter is set andemail parameter is not set, will the API unset the email or keep this attribute unmodified ?

cben · 2017-06-19T12:15:00Z

cc @abraverm who's working on another ManageIQ module.

@abraverm you're adding in modules/remote_management/manageiq/, this PR adds in modules/monitoring/manageiq/ — let's converge on one.

simon3z · 2017-06-19T12:19:32Z

@cben can you bring forward this? Pick up and address the comments?

cben · 2017-06-19T13:28:34Z

=> A couple manageiq devs say Remote Management sounds more appropriate than Monitoring.
@thaumos @gundalow @pilou- any opinion on that?

gundalow · 2017-06-19T13:35:04Z

@cben Good idea asking others.
I've happy with "Remote management"

thaumos · 2017-06-19T16:20:37Z

@cben, I would agree with Remote Management... I didn't like monitoring to begin with, personally. I just didn't want to say anything.

simon3z · 2017-06-19T16:27:13Z

👎 -1 for Monitoring
(Remote Management could be OK)

gundalow · 2017-07-12T11:19:51Z

Closing as this has moved to #26641

dkorn force-pushed the manageiq_utild_docs_and_user_module branch from 14586ea to 5465cb3 Compare May 28, 2017 07:07

dkorn force-pushed the manageiq_utild_docs_and_user_module branch from 5465cb3 to 1b0823c Compare May 28, 2017 07:31

gundalow requested changes May 30, 2017

View reviewed changes

gundalow added the ci_verified Changes made in this PR are causing tests to fail. label May 31, 2017

bcoca removed the needs_triage Needs a first human triage before being processed. label May 31, 2017

dkorn force-pushed the manageiq_utild_docs_and_user_module branch from 1b0823c to abcf80c Compare June 4, 2017 13:36

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Jun 4, 2017

dkorn force-pushed the manageiq_utild_docs_and_user_module branch from abcf80c to d4326b5 Compare June 5, 2017 09:02

ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Jun 5, 2017

dkorn force-pushed the manageiq_utild_docs_and_user_module branch from d4326b5 to da3a0b6 Compare June 5, 2017 12:58

ansibot added the community_review In order to be merged, this PR must follow the community review workflow. label Jun 10, 2017

thaumos self-requested a review June 13, 2017 19:50

thaumos approved these changes Jun 13, 2017

View reviewed changes

cben mentioned this pull request Jun 14, 2017

Public Core Meeting Agenda - June, July 2017 ansible/community#162

Closed

thaumos suggested changes Jun 15, 2017

View reviewed changes

lib/ansible/module_utils/manageiq.py

import os

from manageiq_client.api import ManageIQClient

Copy link

Contributor

thaumos Jun 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handle import error

gundalow reviewed Jun 15, 2017

View reviewed changes

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Jun 15, 2017

pilou- reviewed Jun 15, 2017

View reviewed changes

ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jun 19, 2017

abraverm mentioned this pull request Jun 19, 2017

New module: database and key management for ManageIQ (monitoring/manageiq/manageiq_{database,key}) #25865

Closed

ansibot added the support:community This issue/PR relates to code supported by the Ansible community. label Jun 29, 2017

cben mentioned this pull request Jul 5, 2017

Add manageiq_fetch_key and manage_join_region modules dkorn/manageiq-ansible-module#79

Closed

yaacov mentioned this pull request Jul 11, 2017

New model manageiq manageiq user #26641

Merged

gundalow closed this Jul 12, 2017

ansibot added docs This issue/PR relates to or includes documentation. and removed docs_pull_request labels Mar 4, 2018

ansible locked and limited conversation to collaborators Apr 26, 2019


		module: manageiq_user

		short_description: management of users in ManageIQ

New module: management of users in ManageIQ (monitoring/manageiq/manageiq_user) #25106

New module: management of users in ManageIQ (monitoring/manageiq/manageiq_user) #25106

Conversation

dkorn commented May 28, 2017 • edited

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ansibot commented May 28, 2017

ansibot commented May 28, 2017

dkorn commented May 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thaumos commented May 30, 2017

gundalow commented May 30, 2017

dkorn commented Jun 4, 2017 • edited

ansibot commented Jun 5, 2017

dkorn commented Jun 5, 2017

dkorn commented Jun 11, 2017

thaumos left a comment

Choose a reason for hiding this comment

thaumos commented Jun 13, 2017

simon3z commented Jun 13, 2017

ryansb commented Jun 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ansibot commented Jun 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cben commented Jun 19, 2017

simon3z commented Jun 19, 2017

cben commented Jun 19, 2017

gundalow commented Jun 19, 2017

thaumos commented Jun 19, 2017

simon3z commented Jun 19, 2017

gundalow commented Jul 12, 2017

dkorn commented May 28, 2017 •

edited

dkorn commented Jun 4, 2017 •

edited