[salt-cloud]Defining `script_args: -j json_object` at provider level does not work with openstack driver

sblaisot commented 5 years ago

Description of Issue/Question

Under salt-cloud 2019.2, using script_args at provider level to pass options -j json_object to bootstrap-salt fails because openstacksdk tries to apply a python formatting to all config options and curly braces in json object fails python formatting. The culprit line of openstacksdk code is here

Escaping curly-braces does not work. Doubling them to escape python-formatting let you instanciate VM but this fails later on when executing bootstrap-salt because this is not a valid json object anymore.

Workaround is to move the script: and script_args configuration at profile level but, hey, who wants to duplicate configuration in each profile if they are the same everywhere?

Setup

Note: I don't use /etc/openstack/cloud.yml but rather define all provider onfiguration in /etc/salt/cloud.provider.d/<provider>.conf Note2: If relevant, I use OVH public cloud, based on openstack

Using salt-cloud 2019.2 on Debian 9.

/etc/salt/cloud.providers.d/ovh-gra3.conf:

ovh-gra3:
  driver: openstack
  region_name: GRA3
  profile: ovh
  auth:
    username: REDACTED
    password: 'REDACTED'
    project_id: REDACTED
    project_name: 'REDATED'

  ssh_interface: private_ips
  ssh_key_name: my_ssh_key
  ssh_key_file: /path/to/keyfile

  minion:
    master: salt-master

  script: bootstrap-salt
  script_args: |
    -j '{"master": "salt-master"}' stable 2019.2

  security_groups:
    - no-input

  nics:
    - net-id: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx  
    - net-id: yyyyyyyy-yyyy-yyyy-yyyy-yyyyyyyyyyyy

/etc/salt/cloud.providers.d/ovh-gra3.conf:

ovh-gra3_s1-2_deb9base:
  provider: ovh-gra3
  size: s1-2
  image: 'Debian 9'
  ssh_username: 'debian'

Steps to Reproduce Issue

(Include debug logs if possible and relevant.)

# cat my-cloud.map 
ovh-gra3_s1-2_deb9base:
  - test-saltcloud.company.tld

# salt-cloud -m my-cloud.map 
The following virtual machines are set to be created:
  test-saltcloud.company.tld

Proceed? [N/y] y
... proceeding
[ERROR   ] Failed to create VM test-saltcloud.company.tld. Configuration value u'"master"' needs to be set
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/cloud/__init__.py", line 1288, in create
    output = self.clouds[func](vm_)
  File "/usr/lib/python2.7/dist-packages/salt/cloud/clouds/openstack.py", line 682, in create
    conn = get_conn()
  File "/usr/lib/python2.7/dist-packages/salt/cloud/clouds/openstack.py", line 320, in get_conn
    conn = shade.openstackcloud.OpenStackCloud(cloud_config=None, **vm_)
  File "/usr/local/lib/python2.7/dist-packages/shade/openstackcloud.py", line 146, in __init__
    cloud_config = config.get_one_cloud(**kwargs)
  File "/usr/local/lib/python2.7/dist-packages/openstack/config/loader.py", line 1066, in get_one
    config[key] = value.format(**config)
KeyError: u'"master"'
test-saltcloud.company.tld:
    ----------

Versions Report

# salt-cloud --versions-report
Salt Version:
            Salt: 2019.2.0

Dependency Versions:
 Apache Libcloud: 1.5.0
            cffi: 1.12.3
        cherrypy: Not Installed
        dateutil: 2.5.3
       docker-py: Not Installed
           gitdb: Not Installed
       gitpython: Not Installed
           ioflo: Not Installed
          Jinja2: 2.9.4
         libgit2: 0.24.5
         libnacl: Not Installed
        M2Crypto: 0.24.0
            Mako: Not Installed
    msgpack-pure: Not Installed
  msgpack-python: 0.4.8
    mysql-python: Not Installed
       pycparser: 2.19
        pycrypto: 2.6.1
    pycryptodome: Not Installed
          pygit2: 0.24.2
          Python: 2.7.13 (default, Sep 26 2018, 18:42:22)
    python-gnupg: Not Installed
          PyYAML: 3.12
           PyZMQ: 16.0.2
            RAET: Not Installed
           smmap: Not Installed
         timelib: Not Installed
         Tornado: 4.4.3
             ZMQ: 4.2.1

System Versions:
            dist: debian 9.9 
          locale: UTF-8
         machine: x86_64
         release: 4.9.0-8-amd64
          system: Linux
         version: debian 9.9

garethgreenaway commented 5 years ago

@saltstack/team-cloud Thoughts?

gtmanfred commented 5 years ago

  script_args: |
    -j '{{"master": "salt-master"}}' stable 2019.2

The solution would be to inform people to pass information through like this, and then just call .format() on all strings in the config before passing them to arguments on the minions.

Can probably be done piece mail, but without handling that, the openstack values will fail.

Alternatively, a patch could be pushed upstream to configure disabling the .format() calls.

sblaisot commented 5 years ago

I don't think it's a good idea to have the same argument with the same value configured two different ways depending on the backend provider driver used.

sblaisot commented 5 years ago

I see this bug is thill in "pending discussion" and "block". Do you still need something from me?

waynew commented 5 years ago

I think there probably hasn't been discussion on this, as far as what would be the best approach here. Sounds like maybe the best approach would be to push the upstream patch to allow disabling the formatting, and then adding that flag when we call it.

That would satisfy your desires @sblaisot? If I understand, you don't want to have to repeat things, like have script_args everywhere like gtmanfred proposed, right?

I'm not super familiar with the underlying code, but it seems like that would be the least intrusive change that does the most amount of good.

The other fix I suppose would be for us to detect this backend and just do something like script_args = script_args.replace('{', '{{').replace('}', '}}'), but that seems less desirable as well :suspect: