From 65acda821ecb89b54fd6d3b6478c8438042c0532 Mon Sep 17 00:00:00 2001
From: Vincent Untz <vuntz@suse.com>
Date: Thu, 26 Apr 2018 18:36:34 +0200
Subject: [PATCH] crowbar: Fix Node.get_network_by_type for overridden
 attributes

We were looking for attributes overriding the network proposal only on
the node role, but this is not consistent with what the cookbook does
(it looks at attributes on the node itself), and customer using this
feature are more likely to edit the node directly.

So reflect that by looking at the node attributes also. This is slightly
tricky, as the merge of node attributes and node role attributes occur
on a chef run, and in the rails app, this can be out of sync. So we do
our own merge to always see the real values.
---
 .../app/models/network_service.rb             |  4 +--
 crowbar_framework/app/models/node.rb          | 33 +++++++++++++++++--
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/crowbar_framework/app/models/network_service.rb b/crowbar_framework/app/models/network_service.rb
index e278886b7e..48f57b68c4 100644
--- a/crowbar_framework/app/models/network_service.rb
+++ b/crowbar_framework/app/models/network_service.rb
@@ -434,8 +434,8 @@ def enable_interface(bc_instance, network, name)
 
   def build_net_info(role, network, node)
     net_info = role.default_attributes["network"]["networks"][network].to_hash
-    unless node.nil? || node.crowbar.nil?
-      net_info.merge!(node["crowbar"]["network"][network] || {})
+    unless node.nil?
+      net_info.merge!(node.crowbar_network[network] || {})
     end
     net_info
   end
diff --git a/crowbar_framework/app/models/node.rb b/crowbar_framework/app/models/node.rb
index d63959f7c7..1bcfd3743f 100644
--- a/crowbar_framework/app/models/node.rb
+++ b/crowbar_framework/app/models/node.rb
@@ -634,9 +634,36 @@ def destroy
     Rails.logger.debug("Done with removal of node: #{@node.name} - #{crowbar_revision}")
   end
 
+  def crowbar_network
+    # This is slightly annoying: we store data in the node role, but since we
+    # allow overriding the network data on a per-node basis, users may also put
+    # some data in the node directly.
+    # Within a chef cookbook, it's fine because it's all merged and accessible
+    # directly through the attribute, but we can't rely on this here as the
+    # merge of the node attributes and the node role attributes only occurs on
+    # a chef run. That means that if the rails app is changing the node role
+    # attribute and then trying to read the data before a chef run, it cannot
+    # rely solely on the the node attributes.
+    # Therefore we manually merge here the two sets of attributes...
+
+    role_attrs = crowbar["crowbar"]["network"].to_hash
+
+    # We don't take default attributes, as we would also include the node role
+    # attributes as they exists on the last chef run, therefore always
+    # overriding the possibly different node role attributes that exist now. As
+    # a result, the attribute path may not exist and we need some care when
+    # accessing what we look for.
+    node_normal_crowbar = @node.normal_attrs["crowbar"]
+    node_attrs = unless node_normal_crowbar.nil? || node_normal_crowbar["network"].nil?
+      @node.normal_attrs["crowbar"]["network"].to_hash
+    end
+
+    role_attrs.merge(node_attrs)
+  end
+
   def networks
     networks = {}
-    crowbar["crowbar"]["network"].each do |name, data|
+    crowbar_network.each do |name, data|
       # note that node might not be part of network proposal yet (for instance:
       # if discovered, and IP got allocated by user)
       next if @node["network"]["networks"].nil? || !@node["network"]["networks"].key?(name)
@@ -647,11 +674,11 @@ def networks
 
   def get_network_by_type(type)
     return nil if @role.nil?
-    return nil unless crowbar["crowbar"]["network"].key?(type)
+    return nil unless crowbar_network.key?(type)
     # note that node might not be part of network proposal yet (for instance:
     # if discovered, and IP got allocated by user)
     return nil if @node["network"]["networks"].nil? || !@node["network"]["networks"].key?(type)
-    @node["network"]["networks"][type].to_hash.merge(crowbar["crowbar"]["network"][type].to_hash)
+    @node["network"]["networks"][type].to_hash.merge(crowbar_network[type].to_hash)
   end
 
   def set_network_attribute(network, attribute, value)