Promote HEAT_ENV
At the moment we're in a conflicted situation where certain config values must be specified in the HEAT_ENV file, but others must be set via env vars. This spec is intended to start a discussion around resolving that. Change-Id: Idb49477f4b37474aac11c680a837be17b696a18d
This commit is contained in:
Alexis Lee
parent
35c975a2b3
commit
2b4152d686
|
|
@ -0,0 +1,287 @@
|
|||
..
|
||||
This work is licensed under a Creative Commons Attribution 3.0 Unported
|
||||
License.
|
||||
|
||||
http://creativecommons.org/licenses/by/3.0/legalcode
|
||||
|
||||
================
|
||||
Promote HEAT_ENV
|
||||
================
|
||||
|
||||
https://blueprints.launchpad.net/tripleo/+spec/promote-heat-env
|
||||
|
||||
Promote values set in the Heat environment file to take precedence over
|
||||
input environment variables.
|
||||
|
||||
Problem Description
|
||||
===================
|
||||
|
||||
Historically TripleO scripts have consulted the environment for many items of
|
||||
configuration. This raises risks of scope leakage and the number of env vars
|
||||
required often forces users to manage their environment with scripts.
|
||||
Consequently, there's a push to prefer data files like the Heat environment
|
||||
file (HEAT_ENV) which may be set by passing --heat-env to Heat. To allow this
|
||||
file to provide an unambiguous source of truth, the environment must not be
|
||||
allowed to override the values from this file. That is to say, precedence must
|
||||
be transferred.
|
||||
|
||||
Note env vars may also be used as locals, by overwriting them without reference
|
||||
to their current value before they are used. In this case, no information is
|
||||
transferred from the environment so this use is allowed and new locals may be
|
||||
introduced without issue. To avoid confusion, the two kinds of env vars will
|
||||
be referred to as "input vars" and "local vars".
|
||||
|
||||
|
||||
Proposed Change
|
||||
===============
|
||||
|
||||
Since changes I5b7c8a27a9348d850d1a6e4ab79304cf13697828 and
|
||||
I42a9d4b85edcc99d13f7525e964baf214cdb7cbf, ENV_JSON (the contents of the file
|
||||
named by HEAT_ENV) is constructed in devtest_undercloud.sh like so::
|
||||
|
||||
ENV_JSON=$(jq '.parameters = {
|
||||
"MysqlInnodbBufferPoolSize": 100
|
||||
} + .parameters + {
|
||||
"AdminPassword": "'"${UNDERCLOUD_ADMIN_PASSWORD}"'",
|
||||
"AdminToken": "'"${UNDERCLOUD_ADMIN_TOKEN}"'",
|
||||
"CeilometerPassword": "'"${UNDERCLOUD_CEILOMETER_PASSWORD}"'",
|
||||
"GlancePassword": "'"${UNDERCLOUD_GLANCE_PASSWORD}"'",
|
||||
"HeatPassword": "'"${UNDERCLOUD_HEAT_PASSWORD}"'",
|
||||
"NovaPassword": "'"${UNDERCLOUD_NOVA_PASSWORD}"'",
|
||||
"NeutronPassword": "'"${UNDERCLOUD_NEUTRON_PASSWORD}"'",
|
||||
"NeutronPublicInterface": "'"${NeutronPublicInterface}"'",
|
||||
"undercloudImage": "'"${UNDERCLOUD_ID}"'",
|
||||
"BaremetalArch": "'"${NODE_ARCH}"'",
|
||||
"PowerSSHPrivateKey": "'"${POWER_KEY}"'",
|
||||
"NtpServer": "'"${UNDERCLOUD_NTP_SERVER}"'"
|
||||
}' <<< $ENV_JSON)
|
||||
|
||||
This is broadly equivalent to "A + B + C", where:
|
||||
|
||||
* A is a collection of default values
|
||||
* B is the contents of the HEAT_ENV file
|
||||
* C is a collection of override values
|
||||
|
||||
Values from B override those from A, while values from C override those from
|
||||
either. Currently section C contains a mix of input vars and local vars. It is
|
||||
proposed that the following be moved into section A, as they are input vars::
|
||||
|
||||
NeutronPublicInterface (default 'eth0')
|
||||
UNDERCLOUD_NTP_SERVER (default '')
|
||||
|
||||
So the result would look like::
|
||||
|
||||
ENV_JSON=$(jq '.parameters = {
|
||||
"MysqlInnodbBufferPoolSize": 100,
|
||||
"NeutronPublicInterface": "eth0",
|
||||
"NtpServer": ""
|
||||
} + .parameters + {
|
||||
"AdminPassword": "'"${UNDERCLOUD_ADMIN_PASSWORD}"'",
|
||||
"AdminToken": "'"${UNDERCLOUD_ADMIN_TOKEN}"'",
|
||||
"BaremetalArch": "'"${NODE_ARCH}"'",
|
||||
"CeilometerPassword": "'"${UNDERCLOUD_CEILOMETER_PASSWORD}"'",
|
||||
"GlancePassword": "'"${UNDERCLOUD_GLANCE_PASSWORD}"'",
|
||||
"HeatPassword": "'"${UNDERCLOUD_HEAT_PASSWORD}"'",
|
||||
"NeutronPassword": "'"${UNDERCLOUD_NEUTRON_PASSWORD}"'",
|
||||
"NovaPassword": "'"${UNDERCLOUD_NOVA_PASSWORD}"'",
|
||||
"PowerSSHPrivateKey": "'"${POWER_KEY}"'",
|
||||
"undercloudImage": "'"${UNDERCLOUD_ID}"'"
|
||||
}' <<< $ENV_JSON)
|
||||
|
||||
Note the input vars have been removed, this choice is discussed in
|
||||
Alternatives_.
|
||||
|
||||
Moving on to devtest_overcloud.sh::
|
||||
|
||||
ENV_JSON=$(jq '.parameters = {
|
||||
"MysqlInnodbBufferPoolSize": 100
|
||||
} + .parameters + {
|
||||
"AdminPassword": "'"${OVERCLOUD_ADMIN_PASSWORD}"'",
|
||||
"AdminToken": "'"${OVERCLOUD_ADMIN_TOKEN}"'",
|
||||
"CinderPassword": "'"${OVERCLOUD_CINDER_PASSWORD}"'",
|
||||
"CloudName": "'"${OVERCLOUD_NAME}"'",
|
||||
"GlancePassword": "'"${OVERCLOUD_GLANCE_PASSWORD}"'",
|
||||
"HeatPassword": "'"${OVERCLOUD_HEAT_PASSWORD}"'",
|
||||
"HypervisorNeutronPhysicalBridge":
|
||||
"'"${OVERCLOUD_HYPERVISOR_PHYSICAL_BRIDGE}"'",
|
||||
"HypervisorNeutronPublicInterface":
|
||||
"'"${OVERCLOUD_HYPERVISOR_PUBLIC_INTERFACE}"'",
|
||||
"NeutronBridgeMappings": "'"${OVERCLOUD_BRIDGE_MAPPINGS}"'",
|
||||
"NeutronFlatNetworks": "'"${OVERCLOUD_FLAT_NETWORKS}"'",
|
||||
"NeutronPassword": "'"${OVERCLOUD_NEUTRON_PASSWORD}"'",
|
||||
"NeutronPublicInterface": "'"${NeutronPublicInterface}"'",
|
||||
"NovaComputeLibvirtType": "'"${OVERCLOUD_LIBVIRT_TYPE}"'",
|
||||
"NovaPassword": "'"${OVERCLOUD_NOVA_PASSWORD}"'",
|
||||
"NtpServer": "'"${OVERCLOUD_NTP_SERVER}"'",
|
||||
"SwiftHashSuffix": "'"${OVERCLOUD_SWIFT_HASH}"'",
|
||||
"SwiftPassword": "'"${OVERCLOUD_SWIFT_PASSWORD}"'",
|
||||
"NovaImage": "'"${OVERCLOUD_COMPUTE_ID}"'",
|
||||
"SSLCertificate": "'"${OVERCLOUD_SSL_CERT}"'",
|
||||
"SSLKey": "'"${OVERCLOUD_SSL_KEY}"'"
|
||||
}' <<< $ENV_JSON)
|
||||
|
||||
It is proposed the following be moved into section A::
|
||||
|
||||
OVERCLOUD_NAME (default '')
|
||||
OVERCLOUD_HYPERVISOR_PHYSICAL_BRIDGE (default '')
|
||||
OVERCLOUD_HYPERVISOR_PUBLIC_INTERFACE (default '')
|
||||
OVERCLOUD_BRIDGE_MAPPINGS (default '')
|
||||
OVERCLOUD_FLAT_NETWORKS (default '')
|
||||
NeutronPublicInterface (default 'eth0')
|
||||
OVERCLOUD_LIBVIRT_TYPE (default 'qemu')
|
||||
OVERCLOUD_NTP_SERVER (default '')
|
||||
OVERCLOUD_SSL_CERT (default ${SSLBASE:+$(<$SSLBASE.crt)})
|
||||
OVERCLOUD_SSL_KEY (default ${SSLBASE:+$(<$SSLBASE.key)})
|
||||
|
||||
Resulting in::
|
||||
|
||||
ENV_JSON=$(jq '.parameters = {
|
||||
"CloudName": "",
|
||||
"HypervisorNeutronPhysicalBridge": "",
|
||||
"HypervisorNeutronPublicInterface": "",
|
||||
"MysqlInnodbBufferPoolSize": 100,
|
||||
"NeutronBridgeMappings": "",
|
||||
"NeutronFlatNetworks": "",
|
||||
"NeutronPublicInterface": "eth0",
|
||||
"NovaComputeLibvirtType": "qemu",
|
||||
"NtpServer": "",
|
||||
"SSLCertificate": "'"${SSLBASE:+$(<$SSLBASE.crt)}"'",
|
||||
"SSLKey": "'"${SSLBASE:+$(<$SSLBASE.key)}"'"
|
||||
} + .parameters + {
|
||||
"AdminPassword": "'"${OVERCLOUD_ADMIN_PASSWORD}"'",
|
||||
"AdminToken": "'"${OVERCLOUD_ADMIN_TOKEN}"'",
|
||||
"CinderPassword": "'"${OVERCLOUD_CINDER_PASSWORD}"'",
|
||||
"GlancePassword": "'"${OVERCLOUD_GLANCE_PASSWORD}"'",
|
||||
"HeatPassword": "'"${OVERCLOUD_HEAT_PASSWORD}"'",
|
||||
"NeutronPassword": "'"${OVERCLOUD_NEUTRON_PASSWORD}"'",
|
||||
"NovaImage": "'"${OVERCLOUD_COMPUTE_ID}"'",
|
||||
"NovaPassword": "'"${OVERCLOUD_NOVA_PASSWORD}"'",
|
||||
"SwiftHashSuffix": "'"${OVERCLOUD_SWIFT_HASH}"'",
|
||||
"SwiftPassword": "'"${OVERCLOUD_SWIFT_PASSWORD}"'"
|
||||
}' <<< $ENV_JSON)
|
||||
|
||||
Discussion: Note SSLBASE=${11:-''}. I think the positional args are
|
||||
deprecated but still in use. I'm open to debate on whether the SSL keys should
|
||||
be moved or not.
|
||||
|
||||
Only one of these variables is used outside devtest_{under,over}cloud.sh and
|
||||
consequently the rest are safe to remove.
|
||||
|
||||
The exception is OVERCLOUD_LIBVIRT_TYPE. This is used in the script
|
||||
'write-tripleorc'. Comments are requested on whether the variable needs to be
|
||||
saved, indeed whether that script is still desired at all.
|
||||
|
||||
Alternatives
|
||||
------------
|
||||
|
||||
Instead of removing the input vars entirely, an interim form could be used::
|
||||
|
||||
ENV_JSON=$(jq '.parameters = {
|
||||
"MysqlInnodbBufferPoolSize": 100,
|
||||
"NeutronPublicInterface": "'"${NeutronPublicInterface}"'",
|
||||
"NtpServer": "'"${UNDERCLOUD_NTP_SERVER}"'"
|
||||
} + .parameters + {
|
||||
...
|
||||
}
|
||||
|
||||
However, the env vars would only have an effect if the keys they affect are
|
||||
not present in HEAT_ENV. As HEAT_ENV is written each time devtest runs, the
|
||||
keys will usually be present unless the file is deleted each time (rendering
|
||||
it pointless). So this form is more likely to cause confusion than aid
|
||||
transition.
|
||||
|
||||
----
|
||||
|
||||
If we were able to rely on all users having jq 1.3, we could use //. jq 1.2,
|
||||
still included by Ubuntu 12 (Saucy), has a bug which means // does not work.
|
||||
The rough form would be::
|
||||
|
||||
ENV_JSON=$(jq '.parameters = {
|
||||
"AdminPassword": .parameters.AdminPassword
|
||||
// "'"${UNDERCLOUD_ADMIN_PASSWORD}"'",
|
||||
...
|
||||
}' <<< $ENV_JSON)
|
||||
|
||||
This is substantially more verbose for the same effect. Unfortunately we can't
|
||||
simply do '.parameters //= {...}'.
|
||||
|
||||
----
|
||||
|
||||
More future work than alternative, another spec could change
|
||||
setup-{under,over}cloud-passwords to write directly to HEAT_ENV.
|
||||
|
||||
Security Impact
|
||||
---------------
|
||||
|
||||
None.
|
||||
|
||||
Other End User Impact
|
||||
---------------------
|
||||
|
||||
Ideally, users would be notified that future changes must be applied to
|
||||
HEAT_ENV not env vars. Whether any announcement is in fact made and what form
|
||||
it takes is up for debate. TripleO-incubator is still undergoing rapid change.
|
||||
|
||||
As HEAT_ENV is rewritten every time devtest executes, we can safely assume it
|
||||
matches the last environment used. However users who use scripts to switch
|
||||
their environment may be surprised. Overall the change should be a benefit to
|
||||
these users, as they can use two separate HEAT_ENV files (passing --heat-env to
|
||||
specify which to activate) instead of needing to maintain scripts to set up
|
||||
their environment and risking settings leaking from one to the other.
|
||||
|
||||
Performance Impact
|
||||
------------------
|
||||
|
||||
None.
|
||||
|
||||
Other Deployer Impact
|
||||
---------------------
|
||||
|
||||
None.
|
||||
|
||||
Developer Impact
|
||||
----------------
|
||||
|
||||
None.
|
||||
|
||||
|
||||
Implementation
|
||||
==============
|
||||
|
||||
Assignee(s)
|
||||
-----------
|
||||
|
||||
lxsli
|
||||
|
||||
Work Items
|
||||
----------
|
||||
|
||||
* Change devtest_undercloud.sh.
|
||||
* Change devtest_overcloud.sh.
|
||||
* Resolve OVERCLOUD_LIBVIRT_TYPE in write-tripleorc.
|
||||
|
||||
|
||||
Dependencies
|
||||
============
|
||||
|
||||
None.
|
||||
|
||||
|
||||
Testing
|
||||
=======
|
||||
|
||||
The change will be tested by the existing CI infrastructure without
|
||||
modification.
|
||||
|
||||
|
||||
Documentation Impact
|
||||
====================
|
||||
|
||||
None.
|
||||
|
||||
|
||||
References
|
||||
==========
|
||||
|
||||
#. http://stedolan.github.io/jq/manual/ - JQ manual
|
||||
#. http://jqplay.herokuapp.com/ - Try JQ
|
||||
Loading…
Reference in New Issue