ansible-community / molecule-hetznercloud

A molecule driver for Hetzner Cloud
https://ansible.readthedocs.io/projects/molecule/
GNU Lesser General Public License v3.0
27 stars 14 forks source link

Instance name length limit #34

Closed tumbl3w33d closed 1 year ago

tumbl3w33d commented 3 years ago

When creating a hcloud VM you are limited in length when choosing a name. Usually there's enough space when just manually giving names via platforms definitions, however, when you run your scenarios in --parallel then molecule adds a UUID to the instance name. This happens here (util.py around line 329):

def _parallelize_platforms(config, run_uuid):
    def parallelize(platform):
        platform["name"] = "{}-{}".format(platform["name"], run_uuid)
        return platform

    return [parallelize(platform) for platform in config["platforms"]]

So I was thinking whether I should ask for this in the molecule repo, but maybe you have an idea how we could override this behavior in the driver to limit the name to whatever the hcloud API accepts. Shortening to the first part of the UUID would probably do the trick in most cases.

The actual error:

TASK [Wait for instance(s) creation to complete] *******************************
FAILED - RETRYING: Wait for instance(s) creation to complete (300 retries left).
failed: [localhost] (item={'started': 1, 'finished': 0, 'ansible_job_id': '856426607467.349', 'results_file': '/home/jenkins/agent/workspace/myproject_master@2/.ansible_async/856426607467.349', 'changed': True, 'failed': False, 'item': {'image': 'debian-10', 'name': 'master-73-firewalld-blocked-7e316d97-359d-4d32-8c83-04d2123b39e9', 'server_type': 'cx11'}, 'ansible_loop_var': 'item'}) => {"ansible_job_id": "856426607467.349", "ansible_loop_var": "item", "attempts": 2, "changed": false, "finished": 1, "item": {"ansible_job_id": "856426607467.349", "ansible_loop_var": "item", "changed": true, "failed": false, "finished": 0, "item": {"image": "debian-10", "name": "master-73-firewalld-blocked-7e316d97-359d-4d32-8c83-04d2123b39e9", "server_type": "cx11"}, "results_file": "/home/jenkins/agent/workspace/myproject_master@2/.ansible_async/856426607467.349", "started": 1}, "msg": "invalid input in field 'name'"}

My platforms definition:

platforms:
  - name: "${VM_PREFIX:-$USER}-firewalld-blocked"
    server_type: cx11
    image: debian-10
ssbarnea commented 3 years ago

I agree that this is a bug, one that would require changing at least molecule core and maybe even the plugin API, so they could detect if given configuration is not compatible with their own requirements. Here are few steps to to

On the other hand I am not sure if adding a suffix to the instance names is the best approach for isolating molecule resources. Most clouds support use of tagging and they also allow duplicate instance names. We may end-up not having to alternate the instance names and just use other means to identify the resources.

tumbl3w33d commented 3 years ago

Your suggestions are certainly the right things to do from a long-term perspective. Since this would require some greater effort and is unlikely to happen in the near future… would you be severely disgusted if I suggested a naive approach that would provide a direct way to influence the name generation immediately?

def _parallelize_platforms(config, run_uuid):
    def parallelize(platform):
        platform["name"] = "{}-{}".format(platform["name"], run_uuid)
        if "name_length_limit" in platform:
            platform["name"] = platform["name"][0: platform["name_length_limit"] - 1]
        return platform

    return [parallelize(platform) for platform in config["platforms"]]
platforms:
  - name: "${VM_PREFIX:-$USER}-firewalld-exposed"
    name_length_limit: 64
    server_type: cx11
    image: debian-10

It does the trick in my scenario and I guess it wouldn't hurt anyone.

Edit: nevermind, it didn't work yet for some reason which I will probably figure out in a moment. But I guess you get the idea. Edit2: updated my snippet with actually working code.

decentral1se commented 3 years ago

Thanks for weighing in you both. I do agree there is something to do here. I can sympathise with wanting to get something working real quick but I don't really think we should add an additional option for something that core should probably be doing anyway. https://github.com/ansible-community/molecule-hetznercloud/issues/34#issuecomment-873361739 seems pretty sensible. To push that forward:

So @ssbarnea if we got those 3 steps done here downstream, could you prepare a change upstream to use this new constant to ensure names are generated within that limit?

tumbl3w33d commented 3 years ago

I tested the Hetzner web UI a little and it seems that the instance name must look like ^\w[a-zA-Z0-9-.]{0,62}?\w?$ or in words:

P.S.: the regex isn't perfect. For example, it won't catch trailing -/.. Not sure it's possible to reflect that in a regex. Simple string check for startswith and endswith is probably the better choice.

decentral1se commented 3 years ago

Nice @tumbl3w33d, we don't need to worry about matching the contents right now, just the length. So a < 65 check should do it.