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

add another automatic group by tag name (which can easily set when... #53165

Closed
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
5 participants
@desmap
Copy link

desmap commented Mar 1, 2019

…creating Vultr instances via Ansible)

SUMMARY

Since the Vultr plugin seems not to support features like keyed_groups or group like the aws_ec2 plugin it would be great to have some option to group individually.

With this feature you can group servers dynamically and individually. E.g. you want to group a region-wide cluster, I give all those servers the tag name tag: k8s-cluster on creation or on updating. Then, I can access those easily with hosts: k8s-cluster

ISSUE TYPE
  • Bug
COMPONENT NAME

vultr plugin

Edit: I changed the type to bug, see below for more info; tl;dr without the tag field, this plugin is a nice toy but not useful for production

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 1, 2019

@bcoca
Copy link
Member

bcoca left a comment

to support those features you just need to inherit from Constructable, seems a lot more useful than updating the code each time you want new groups (also it forces these groups on other users that might already be happy with current output).

@desmap

This comment has been minimized.

Copy link
Author

desmap commented Mar 2, 2019

you just need to inherit from Constructable

Could you pls elaborate on this? Thx.

seems a lot more useful than updating the code each time you want new groups

I think you might missed the point. The 'tag' field at Vultr is exactly for this use case, to just add any kind of group/filter/whatever you like without updating the code each time you want new groups. It gives you full freedom/flexibility and you can set it to any value you want. Leaving this important field out feels even more like a bug than a feature. I'll edit my PR on this regard.

I could say that for all my use case a default hard-coded region-based group setting like it is now implemented, is completely useless. All my systems are global/CDN-like and I never change their state based on a region. So, what's the point and we don't need to bikeshed which group filter should go into the code. Thus, we need a field which can be set by the user => 'tag' and the rest is nice-to-have and should not be hard-coded like it is now.

So, I think that this Vultr plugin without 'tag'-support, without all the nice things the AWS-EC2-plugin has (keyed groups, groups, etc. => hint => user-defined groups are built-in there) is a nice toy but not really for production. This is super sad since Vultr offers the fastest VPS'. Hence, I'd love to see Vultr's 'tag' field supported and the region-field deprecated with a warning.

BTW, don't confuse Vultr's tag field with Ansible's 'tag'.

cc @Spredzy @resmo

@ansibot ansibot added the bug label Mar 2, 2019

@chopraaa

This comment has been minimized.

Copy link
Contributor

chopraaa commented Mar 2, 2019

@desmap He's not against it, he's just advising that you use Constructable: https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/inventory/__init__.py#L295

This allows you to specify how you want to list your inventory instead of hardcoding it. Unfortunately, I don't know much about this and there's not a lot to go by in the docs for me but you can give it a shot.

@desmap

This comment has been minimized.

Copy link
Author

desmap commented Mar 2, 2019

@chopraaa, thanks for your comment. Ok, maybe I got @bcoca wrong but I am not a Python-native and still don't understand his idea. So, I would be happy if he or whoever could make a suggestion. Thx

@bcoca

This comment has been minimized.

Copy link
Member

bcoca commented Mar 4, 2019

other plugins (ec2) also use this to allow user to use any 'cloud field' to create groups according to user logic. tags are also available on ec2 and a good example for keyed_groups feature.

@desmap

This comment has been minimized.

Copy link
Author

desmap commented Mar 5, 2019

other plugins (ec2) also use this to allow user to use any 'cloud field' to create groups according to user logic. tags are also available on ec2 and a good example for keyed_groups feature.

@bcoca hm... this is exactly what I am saying: the AWS-EC2 has all this fancy stuff while the Vultr plugin does not have any of them just OS and region hard-coded and that we need to change that, adding the 'tag' field would be just a first super urgent step. You guys are welcome to make it even better then. Or I didn't get your message again...

So what's next? I am not a Python guy and should not mess more with that code base, I mean I can try if you tell what should be exactly done beyond my extra 'tag' field. Btw, what's with @Spredzy? Any opinions, you wrote the plugin, would be awesome if you sent a sign of life!

Or should I switch to Terraform, I mean look what full-fledged solution they have => https://github.com/squat/terraform-provider-vultr? Compared to them, the Ansible-Vultr-plugin feels like...

@bcoca, @Spredzy, c'mon let's either fix this asap (merge my PR or make a better suggestion) or just decide against, then I know where I am at and can move on. A weeks long discussion wouldn't be appropriate here.

Looking forward to next steps!

@resmo

This comment has been minimized.

Copy link
Member

resmo commented Mar 15, 2019

@desmap Thanks for your time, because we run out of time for 2.8, I implemented this in

Superseded by #53869

@resmo resmo closed this in #53869 Mar 19, 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.