HEAT incorrect assignment/submitting of optional parameters

IMO this is a neutronclient bug - we can probably work around it in heat, but all clients should always treat None values the same as not passing any value.

Hi Steven, if this isn't Heat's issue, nothing to be fixed. Do you suggest to reopen this bug in Neutron?

Thank you.

I think you're changing(removing) the device_owner property of Port resource when doing a stack update. So actually you are removing the device_owner from the existing port. Neutron's handling of None values is weird. Actually neutron is expecting 'None'(quoted). We've done number of these hacks for other resources and can probably do one more.

Hi Rabi,

to be more precise - I create stack with the following configuration:

  e-secgroup:
    type: OS::Neutron::SecurityGroup
    properties:
      name: SSH_ICMP
      rules:
        - direction: ingress
# - protocol: tcp
# remote_ip_prefix: 0.0.0.0/0
# port_range_min: 22
# port_range_max: 22
        - protocol: icmp
          remote_ip_prefix: 0.0.0.0/0

  node1-wan:
    type: OS::Neutron::Port
    properties:
      name: jadm-node1-wan
      network: e-net
      port_security_enabled: True
      security_groups: [ default, { get_resource: e-secgroup } ]

and, then, in order to allow SSH (port 22) in this security group, just uncomment commented rows in e-secgroup statement (and nothing else!) launch update and get:

Resource UPDATE failed: BadRequest: resources.e-secgroup: Invalid input for description. Reason: 'None' is not a valid string. Neutron server returns request_ids: ['req-287db7f6-06be-4bc8-a11b-94be203c67da']

The snippet you've pasted and the changes for update should work fine.

From the error "Invalid input for description. Reason: 'None' is not a valid string" it's clear that you've removed the 'description' property of 'e-secgroup' before update and neutron is not able to handle that.

Rabi,

no, definitely no. I'm doing exactly as I described: before update, I uncomment in template _ONLY_ the following rows:

# - protocol: tcp
# remote_ip_prefix: 0.0.0.0/0
# port_range_min: 22
# port_range_max: 22

there was no 'description' parameter during start and, thus, it can't be changed before update.

I reproduce this problem as many times as I need to check it again.

FYI there is an answer from Neutron team re this case - https://bugs.launchpad.net/neutron/+bug/1681784/ .

I just checked. The snippets and steps (adding ssh rule before update)you provided in comment #4 seem to work fine on master. AFAICT, there has not been any change to these resources since ocata. Please provide your template and the heat engine log.

Although, there are some issues with properties like description, device_owner etc when they are removed before update. We'll fix those.

Hi Rabi,

please, find attached all requested information:

heat-engine-create.log.gz
deb-jadm-create.log -- output of 'openstack stack create -vvv'

heat-engine-update.log.hz
deb-jadm-update.log -- output of 'openstack stack update -vvv'

deb-jadm.yaml:
1) creation of stack was with commented tcp/22 entries in e-secgroup resource
2) updating of stack was with uncommented these entries

Thank you.

Hi Rabi, if for some reasons you will need access to my lab - just drop an e-mail at doka@<email address hidden> and I will provide you with access credentials.

Actually heat should not be calling update_security_group if there is nothing else changed other than the rules for SecurityGroup resource. The fact that it's calling it with None value for 'description' is absurd. I don't know why it's happening in your setup.

May be this is something distribution-specific? I'm using Ubuntu and
their Openstack:

ii heat-api 1:8.0.0-0ubuntu1~cloud0 all
OpenStack orchestration service - ReST API
ii heat-api-cfn 1:8.0.0-0ubuntu1~cloud0 all
OpenStack orchestration service - CFN API
ii heat-common 1:8.0.0-0ubuntu1~cloud0 all
OpenStack orchestration service - common files
ii heat-engine 1:8.0.0-0ubuntu1~cloud0 all
OpenStack orchestration service - engine
ii python-heat 1:8.0.0-0ubuntu1~cloud0 all
OpenStack orchestration service - Python files
ii python-heatclient 1.8.0-0ubuntu2~cloud0 all
client library and CLI for OpenStack Heat - Python 2.7

Which one distro do you use to reproduce this issue? Again, I can
provide access to my lab in order to investigate it.

I installed Heat in full accordance to Ocata installation guide. To be
frank, these templates were working on Newton; problems appeared after
we reinstalled (from scratch) Ocata.

Another change is openvswitch instead of linux bridges and OVS's
firewall (we stopped using iptables). But AFAIU this change shouldn't
affect orchestration.

On 4/12/17 6:50 AM, Rabi Mishra wrote:
> Actually heat should not be calling update_security_group if there is
> nothing else changed other than the rules for SecurityGroup resource.
> The fact that it's calling it with None value for 'description' is
> absurd. I don't know why it's happening in your setup.
>

--
Volodymyr Litovka
   "Vision without Execution is Hallucination." -- Thomas Edison

Hi Rabi,

let me clarify a bit. I don't see issues with Heat engine itself. I'm creating stack with the following security group:

  e-secgroup:
    type: OS::Neutron::SecurityGroup
    properties:
      name: SSH_ICMP
      rules:
        - direction: ingress
        - protocol: icmp
          remote_ip_prefix: 0.0.0.0/0

and then update stack with changed rules in the security group:

  e-secgroup:
    type: OS::Neutron::SecurityGroup
    properties:
      name: SSH_ICMP
      rules:
        - direction: ingress
        - protocol: tcp
          remote_ip_prefix: 0.0.0.0/0
          port_range_min: 22
          port_range_max: 22
        - protocol: icmp
          remote_ip_prefix: 0.0.0.0/0

so, since group changed, Heat tries to update it.

The only problem with update is that Heat sends values for parameters, which neutronclient/neutron can't work with, particularly 'description': None or 'device_owner': None.

As soon as this problem will be fixed either in Heat, Neutron or neutronclient, everything will be ok.

Hi,

I commented on https://bugs.launchpad.net/heat/+bug/1681784 as well, but there is no way to fix this in Neutron because None is an API value used by Neutron. If you pass None to the client, it needs to pass None to the server for validation. In this particular case None is not allowed for these fields so it gets rejected.

If you don't explicitly want a value of None for a field, you can't request it to be None.

Dear colleagues,

I've prepared a fix for the problem, which works _for me_ but this is rather suggestion than fix, which requires your review and can be the first step to general fix.

So, creation of object is ok because there is check for 'None' parameters before sending them to Neutron:

port.py/handle_create() calls neutron.py/prepare_properties() which in turn validates key-value pairs:

props = dict((k, v) for k, v in properties.items()
             if v is not None)

while, for example, port.py/handle_update() doesn't.

I've changed two files:

===== port.py =====

$ diff -u port.py.orig port.py
--- port.py.orig 2017-04-21 08:55:19.033207478 +0000
+++ port.py 2017-04-21 08:59:29.259608748 +0000
@@ -519,8 +519,11 @@
                 prop_diff['qos_policy_id'] = self.client_plugin(
                     ).get_qos_policy_id(qos_policy) if qos_policy else None
             self._prepare_port_properties(prop_diff, prepare_for_update=True)
- LOG.debug('updating port with %s' % prop_diff)
- self.client().update_port(self.resource_id, {'port': prop_diff})
+ #-- doka -- https://bugs.launchpad.net/bugs/1681769
+ prop_diff_cleaned = dict((k, v) for k, v in prop_diff.items()
+ if v is not None)
+ LOG.debug('updating port with %s' % prop_diff_cleaned)
+ self.client().update_port(self.resource_id, {'port': prop_diff_cleaned})

     def check_update_complete(self, *args):
         attributes = self._show_resource()

===== security_group.py =====

# diff -u security_group.py.orig security_group.py
--- security_group.py.orig 2017-04-21 09:08:52.492005906 +0000
+++ security_group.py 2017-04-21 10:11:53.083743117 +0000
@@ -11,6 +11,8 @@
 # License for the specific language governing permissions and limitations
 # under the License.

+from oslo_log import log as logging
+
 from heat.common import exception
 from heat.common.i18n import _
 from heat.engine import constraints
@@ -18,6 +20,8 @@
 from heat.engine.resources.openstack.neutron import neutron
 from heat.engine import support

+LOG = logging.getLogger(__name__)
+

 class SecurityGroup(neutron.NeutronResource):
     """A resource for managing Neutron security groups.
@@ -254,8 +258,12 @@

         if prop_diff:
             self.prepare_update_properties(prop_diff)
+ #-- doka -- https://bugs.launchpad.net/bugs/1681769
+ prop_diff_cleaned = dict((k, v) for k, v in prop_diff.items()
+ if v is not None)
+ LOG.debug('updating security group with %s' % prop_diff_cleaned)
             self.client().update_security_group(
- self.resource_id, {'security_group': prop_diff})
+ self.resource_id, {'security_group': prop_diff_cleaned})
         if rules:
             self._create_rules(rules)

This change removes empty (None) keys from dictionary before it passed to Neutron and latter accepts this update. Probably, there are other places in Heat where such validation is required and that's why I don't see it as a final fix, but rather informational message on how this problem can be so...

Volodymor,

As I mentioned earlier, we don't need to clean prop_diff for None values. There would not be any None value in prop_diff, if you've not removed a property from the template before update(which was there for create). Your template works fine for me(on fedora). We also tests for port update at the gate(ubuntu xenial) that works fine.

So there is something wrong with your setup. Is this a clean install or an upgarde? May be you can reinstall and check.

The reason this bug is tracked is because there are scenarios when some properties like device_owner is removed from the template before update and neutron throws an error.

Hi Rabi,

It's good that you don't experience this error in your installations but
it seems I pulled a lucky ticket and won the lottery. I want to return
this ticket. How it's possible? :)

To make check one more time I've reinstalled Heat

<stopped all heat services>
apt purge heat* python-heat*

cd /usr/lib/python2.7/dist-packages/
rm -rf heat*
find . -name '*heat*' -- return nothing

then put configs back to /etc/heat ('purge' remove configs) and
installed heat back. Sometimes ago on these serverswas Newton, but
before installing Ocata we apt-removed/purged all packages of Openstack,
removed all MySQL databases and rebooted servers (so no information left
in memcached), installed Ocata and wrote all config files from scratch
(no copy/paste from old installation). At the moment, there is

# apt show heat-engine
Package: heat-engine
Version: 1:8.0.0-0ubuntu1~cloud0
Priority: optional
Section: web
Source: heat
Maintainer: Ubuntu Developers <email address hidden>
Original-Maintainer: PKG OpenStack <email address hidden>
Installed-Size: 38.9 kB
Depends: adduser, heat-common (= 1:8.0.0-0ubuntu1~cloud0),
init-system-helpers (>= 1.18~), python:any
Supported: 48m
Download-Size: 5,360 B
APT-Manual-Installed: yes
APT-Sources: http://ubuntu-cloud.archive.canonical.com/ubuntu
xenial-updates/ocata/main amd64 Packages
Description: OpenStack orchestration service - engine
  Heat is a service to orchestrate multiple composite cloud applications
using
  templates, through both an OpenStack-native ReST API and a
  CloudFormation-compatible Query API.
  .
  This package contains the heat engine, which is the core service of
heat, and
  which the API servers will use.

Then, I simplified template and removed any mentions about other
resources like security groups in port definition:

   node1:
     type: OS::Nova::Server
     properties:
       [ ... ]
       networks:
         - port: { get_resource: node1-wan }

   node1-wan:
     type: OS::Neutron::Port
     properties:
       name: jadm-node1-wan
       network: e-net
       port_security_enabled: False

as usual, creation of port is successful:

2017-04-21 12:29:15.898 26265 DEBUG
heat.engine.resources.openstack.neutron.port
[req-29d762a3-e6b5-4215-9fcd-4b9d3a8fd28c - bush - - -] creating port
with {'admin_state_up': True, 'network_id':
u'260a32d6-2a2b-4e7e-b809-b504bb8c2587', 'port_security_enabled': False,
'name': u'jadm-node1-wan'} handle_create
/usr/lib/python2.7/dist-packages/heat/engine/resources/openstack/neutron/port.py:424

$ openstack port show jadm-node1-wan
[ ... ]
| port_security_enabled | False
| security_groups |
+-----------------------+-----------

and since there is no port security, I can ping it:

$ ping x.x.x.253
PING x.x.x.253 (x.x.x.253): 56 data bytes
64 bytes from x.x.x.253: icmp_seq=0 ttl=52 time=42.507 ms
64 bytes from x.x.x.253: icmp_seq=1 ttl=52 time=40.990 ms

then I change JUST and ONLY single parameter: set
'port_security_enabled' to 'True' (without quotes). No other changes,
additions or deletions to other parameters and sections of template.
Update causes an error from Neutron since there are keys with None...

Here is what I see in the logs:

2017-04-21 19:56:20.518 19364 DEBUG heat.engine.resources.openstack.neutron.port [req-9a049d55-e189-49ad-8cd5-dc32a9cc962d - demo - - -] updating port with {'port_security_enabled': True} handle_updat
e /opt/stack/heat/heat/engine/resources/openstack/neutron/port.py:522

It seems random properties are populated for you in prop_diff. I can't say anything for the ubuntu packages thought. The code seems different from upstream stable/ocata[1] though. The above debug log is at L520[1] upstream whereas it's L525 for you.

[1] https://github.com/openstack/heat/blob/stable/ocata/heat/engine/resources/openstack/neutron/port.py#L520

Anyway, we determine the prop_diff here[1]
https://github.com/openstack/heat/blob/stable/ocata/heat/engine/resource.py#L576-L590

May be you can add a debug log to see what's there in 'before_props' and 'after_props' dicts.

I checked your setup. You had the 'observe_on_update' flag set to true in your heat.conf. I reset it to false and the update works fine now.

'observe_on_update' is not a production ready feature yet and has number of bugs. It would be good to advise users not to use it.

Thank you, Rabi. I highly appreciate.

May be it makes sense to mention in documentation and in heat.conf, that this option is buggy and experimental, to avoid confusing. Because I see it as very useful and switched it on as soon as explored it.