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

Create vultr_lb module #159

Closed
wants to merge 2 commits into from

Conversation

jstoja
Copy link

@jstoja jstoja commented Apr 13, 2020

SUMMARY

Implement Vultr Load Balancer API (https://www.vultr.com/api/#loadbalancer)

Fixes ansible/ansible#68900

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

vulr_lb

ADDITIONAL INFORMATION

Goal is something like this:

- vultr_lb:
    dcid: 1
    algorithm: Leastconn
    label: web
    state: present
    forwarding_rules:
      - frontend_protocol: https
        frontend_port: 81
        backend_protocol: https
        backend_port: 81

@felixfontein
Copy link
Collaborator

You need to add a relative symlink from plugins/modules/ to your module.

CC @resmo @Spredzy

@jstoja
Copy link
Author

jstoja commented Apr 13, 2020

I've added the symlink now. Please let me know if you have some feedback, this is my first module.

@jstoja jstoja changed the title [WIP] Create vultr_lb module Create vultr_lb module Apr 13, 2020
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Some generic comments.

DOCUMENTATION = r'''
---
module: vultr_lb
short_description: Manages Load Balanders on Vultr.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
short_description: Manages Load Balanders on Vultr.
short_description: Manages Load Balancers on Vultr

dcid:
description:
- DCID integer Location in which to create the load balancer.
required: true
Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to add type: xxx as well. (For all options.)

ssl_certificate:
description:
- The SSL Certificate.
ssl_chain
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ssl_chain
ssl_chain:

local_action:
module: vultr_lb
dcid: 1
algorithm: Leastconn
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
algorithm: Leastconn
balancing_algorithm: leastconn


EXAMPLES = r'''
- name: Ensure a Load Balancer exists
local_action:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
local_action:
community.general.vultr_lb:

EXAMPLES = r'''
- name: Ensure a Load Balancer exists
local_action:
module: vultr_lb
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
module: vultr_lb

- frontend_protocol: https
frontend_port: 81
backend_protocol: https
backend_port: 81
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
backend_port: 81
backend_port: 81
delegate_to: localhost

from __future__ import (absolute_import, division, print_function)
__metaclass__ = type

ANSIBLE_METADATA = {'metadata_version': '0.1',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ANSIBLE_METADATA = {'metadata_version': '0.1',
ANSIBLE_METADATA = {'metadata_version': '1.1',

argument_spec = vultr_argument_spec()
argument_spec.update({
'name': {
'required': True,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
'required': True,
'type': str,
'required': True,

You need to specify the type for every option.

module = AnsibleModule(
argument_spec=argument_spec,
required_if=[
('state', 'present', ['name', 'dcid']),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't make sense, since you marked both options name and dcid as always required.

@resmo
Copy link
Member

resmo commented Apr 13, 2020

We actively maintain the vultr modules in the collection https://github.com/ngine-io/ansible-collection-vultr. The PR for cleanup vultr related content in this repo is currently in the making.

@felixfontein
Copy link
Collaborator

See #172.

@jstoja
Copy link
Author

jstoja commented Apr 13, 2020 via email

@resmo
Copy link
Member

resmo commented Apr 13, 2020

No problems :) Would you like me to open the same PR, but on your repository? Otherwise, I’ll close this one.

yes, please create a PR targeting the new repository, feel free to close this PR.

@jstoja
Copy link
Author

jstoja commented Apr 14, 2020

Closing this one in favor of ngine-io/ansible-collection-vultr#3 .

@jstoja jstoja closed this Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vultr: implement load balancer module
3 participants