Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VMware: New httpapi connection plugin for VMware #57832

Open
wants to merge 1 commit into
base: devel
from

Conversation

@Akasurde
Copy link
Member

commented Jun 14, 2019

SUMMARY
  • Initial draft for httpapi connection plugin
  • New module vmware_get_info

Signed-off-by: Abhijeet Kasurde akasurde@redhat.com

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

.github/BOTMETA.yml
lib/ansible/modules/cloud/vmware/vmware_get_info.py
lib/ansible/plugins/httpapi/vmware.py

ADDITIONAL INFORMATION

Documentation and test PR will be add in a separate PR.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2019

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/cloud/vmware/vmware_get_info.py:158:29: mixed-format-string Mixing named and unnamed conversion specifiers in format string

click here for bot help

@Akasurde Akasurde force-pushed the Akasurde:vmware_httpapi branch from 3295d3a to 9a61400 Jun 14, 2019

@ansibot ansibot removed the ci_verified label Jun 14, 2019

@Akasurde Akasurde force-pushed the Akasurde:vmware_httpapi branch 2 times, most recently from 36727ce to ce91d58 Jun 17, 2019

@ansibot ansibot added core_review and removed needs_revision labels Jun 17, 2019

@ansibot ansibot removed the needs_triage label Jun 20, 2019

Show resolved Hide resolved lib/ansible/modules/cloud/vmware/vmware_get_info.py Outdated
Show resolved Hide resolved lib/ansible/plugins/httpapi/vmware.py Outdated

return response.getcode(), self._response_to_json(response_value)
except AnsibleConnectionFailure as e:
return 404, 'Object not found'

This comment has been minimized.

Copy link
@goneri

goneri Jun 20, 2019

Contributor

AnsibleConnectionFailure is a generic error, it does not mean the resource is missing. It should be more like an err 500.

def login(self, username, password):
if username and password:
payload = {}
url = '/rest/com/vmware/cis/session'

This comment has been minimized.

Copy link
@goneri

goneri Jun 20, 2019

Contributor

you don't use username and password?

This comment has been minimized.

Copy link
@goneri

goneri Jul 11, 2019

Contributor

I suggest the following change, to make clear the two parameters are ignored:

diff --git a/lib/ansible/plugins/httpapi/vmware.py b/lib/ansible/plugins/httpapi/vmware.py
index 1e20d667a1..a4164a6fd3 100644
--- a/lib/ansible/plugins/httpapi/vmware.py
+++ b/lib/ansible/plugins/httpapi/vmware.py
@@ -31,13 +31,16 @@ BASE_HEADERS = {
 
 
 class HttpApi(HttpApiBase):
-    def login(self, username, password):
-        if username and password:
-            payload = {}
-            url = '/rest/com/vmware/cis/session'
-            response, response_data = self.send_request(url, payload)
-        else:
-            raise AnsibleConnectionFailure('Username and password are required for login')
+    def login(self, *args):
+        if not self.connection.get_option('remote_user'):
+            raise AnsibleConnectionFailure('remote_user option is required for login')
+
+        if not self.connection.get_option('password'):
+            raise AnsibleConnectionFailure('password option is required for login')
+
+        payload = {}
+        url = '/rest/com/vmware/cis/session'
+        response, response_data = self.send_request(url, payload)
 
         if response == 404:
             raise ConnectionError(response_data)

@ansibot ansibot added needs_revision and removed core_review labels Jun 20, 2019

@Akasurde Akasurde force-pushed the Akasurde:vmware_httpapi branch from ce91d58 to 5c13ba8 Jun 22, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2019

@Akasurde Akasurde force-pushed the Akasurde:vmware_httpapi branch from 5c13ba8 to 0ebda37 Jun 27, 2019

VMware: New httpapi connection plugin for VMware
* Initial draft for httpapi connection plugin
* New module vmware_get_info
* Added n3pjk as maintainer in vmware httpapi

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

@Akasurde Akasurde force-pushed the Akasurde:vmware_httpapi branch from 0ebda37 to 23efbed Jun 27, 2019

host=('/vcenter/host', ['folders', 'datacenters', 'names', 'hosts', 'clusters', 'connection_states']),
network=('/vcenter/network', ['folders', 'datacenters', 'names', 'types', 'networks']),
resource_pool=('/vcenter/resource-pool', ['datacenters', 'names', 'hosts', 'parent_resource_pools', 'clusters', 'resource_pools']),
virtual_machine=('/vcenter/vm', ['folders', 'datacenters', 'names', 'hosts', 'power_states', 'clusters', 'resource_pools', 'vms']),

This comment has been minimized.

Copy link
@goneri

goneri Jun 27, 2019

Contributor

Could we use vm too, maybe as an alias. This is shorter, clear enough IMO and it's actually already VMware naming convention.

filters = module.params.get('filters')
if filters:
first = True
for filter in filters:

This comment has been minimized.

Copy link
@goneri

goneri Jun 27, 2019

Contributor

could you please isolate the lines from 130 to 146 in a new function.

first = False
# Escape characters
if '/' in filter[key]:
filter[key].replace('/', '%2F')

This comment has been minimized.

Copy link
@goneri

goneri Jun 27, 2019

Contributor

We can use urlparse.urlunsplit() and urlparse.urljoin() here.

@ansibot ansibot added the stale_ci label Jul 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.