Code review comment for ~redriver/cloud-init:frbsd-azure-branch

Hongjiang Zhang (redriver) wrote :

> I'm confused because Ubuntu's cloud.cfg is exactly what is in
> config/cloud.cfg in trunk. which has:
>
> users:
> - default
> ...
>
> system_info:
> # This will affect which distro class gets used
> distro: ubuntu
> # Default user name + that default users groups (if added/used)
> default_user:
> name: ubuntu
> lock_passwd: True
> gecos: Ubuntu
> groups: [adm, audio, cdrom, dialout, dip, floppy, lxd, netdev, plugdev,
> sudo, video]
> sudo: ["ALL=(ALL) NOPASSWD:ALL"]
> shell: /bin/bash
>
> Thats almost identical to what config/cloud.cfg-freebsd has in your
> branch.
>
> I think that comment is actually wrong. If you look at how that method
> works, it basically builds an array of configs (dictionaries), with
> environment config (from environment CFG_ENV_NAME) first, then isntance
> configs, then datasource, then base. Then calls mergemanydict with that.
>
> So basically:
> util.mergemanydict([
> {'foo': 'from enviroment'},
> {'foo': 'from instance, no user override'},
> {'system_info': {'default_user': {'name': 'smoser-from-ds'}}},
> {'system_info': {'default_user': {'name': 'ubuntu'}}},
> ])
>
> Calling that returns:
> {'foo': 'from enviroment',
> 'system_info': {'default_user': {'name': 'smoser-from-ds'}}}
>
> So I'm pretty sure the comment is just wrong. Its probably because
> 'mergemanydict' has very odd usage in that precedence goes to the *first*
> value, not the last.
Ok. Let's ignore that comment. I debug this code snippet and still found it there.
I added a test function in helper.py, and specify two input files '/usr/local/etc/cloud/cloud.cfg' and 'ds.dat'
The content of ds.dat is as following.

{'disk_setup': {'ephemeral0': {'table_type': 'gpt', 'layout': [100], 'overwrite': True}}, 'fs_setup': [{'device': 'ephemeral0.1', 'replace_fs': 'ntfs', 'filesystem': 'freebsd-ufs'}], 'system_info': {'default_user': {'passwd': '$6$WSLBpwonNkC0Yi2b$w0xih6LR1X.TSyjn2mYSNiHITd.2oqD/j6CdoskUoXhH04/N8bvi6GB3smdZ4JT5hMrbT1svqtkkQeYY/B2fZ/', 'name': u'azureuser', 'lock_passwd': False}}, 'ssh_pwauth': True}

The output indicates that the username is still "freebsd" even though the "passwd" field was updated. So, in my environment, there is something different from the environment on Ubuntu.

{"ssh_pwauth": true, "users": ["default"], "disable_root": false, "disk_setup": {"ephemeral0": {"table_type": "gpt", "layout": [100], "overwrite": true}}, "system_info": {"default_user": {"shell": "/bin/tcsh", "gecos": "FreeBSD", "name": "freebsd", "groups": ["wheel"], "passwd": "$6$WSLBpwonNkC0Yi2b$w0xih6LR1X.TSyjn2mYSNiHITd.2oqD/j6CdoskUoXhH04/N8bvi6GB3smdZ4JT5hMrbT1svqtkkQeYY/B2fZ/", "sudo": ["ALL=(ALL) NOPASSWD:ALL"], "lock_passwd": true}, "distro": "freebsd"}, "syslog_fix_perms": "root:wheel", "fs_setup": [{"device": "ephemeral0.1", "replace_fs": "ntfs", "filesystem": "freebsd-ufs"}], "cloud_init_modules": ["seed_random", "bootcmd", "growpart", "resizefs", "set_hostname", "update_hostname", "users-groups", "ssh"], "preserve_hostname": false, "cloud_final_modules": ["rightscale_userdata", "scripts-vendor", "scripts-per-once", "scripts-per-boot", "scripts-per-instance", "scripts-user", "ssh-authkey-fingerprints", "keys-to-console", "phone-home", "final-message", "power-state-change"], "datasource_list": ["ConfigDrive", "OpenStack", "Ec2"], "cloud_config_modules": ["ssh-import-id", "locale", "set-passwords", "package-update-upgrade-install", "timezone", "disable-ec2-metadata", "runcmd"]}

def test_read_cfg():
        # Input config files override
        # env config files which
        # override instance configs
        # which override datasource
        # configs which override
        # base configuration
        cfgs = []
        env_config = []
        env_config.append(util.read_conf('/usr/local/etc/cloud/cloud.cfg'))
        datasource_config = []
        datasource_config.append(util.read_conf('ds.dat'))

        cfgs.extend(env_config)
        cfgs.extend({})
        cfgs.extend(datasource_config)
        print(json.dumps(util.mergemanydict(cfgs)))

« Back to merge proposal