From bbb823306d7a632cf8f235133be125d32d622ed0 Mon Sep 17 00:00:00 2001
From: Drew Walters <andrew.walters@att.com>
Date: Fri, 1 May 2020 18:14:40 +0000
Subject: [PATCH] Add mutual exclusivity to manager host selectors

Using a label selector and name when creating a manager will produce the
union of all host documents that match the criteria. While this is not
inherently a problem, it's not expected behavior. This change makes
using the label selector and name property mutually exclusive. When
creating a manger using both selectors, the manager will contain the
intersection of hosts, rather than the union.

Change-Id: I791e0460cea2ae3d60ea2cc24f900b925c36d9af
Signed-off-by: Drew Walters <andrew.walters@att.com>
---
 pkg/remote/management.go      | 29 +++++++++++++++++++++++++++--
 pkg/remote/management_test.go | 20 ++++++++++++++++++++
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/pkg/remote/management.go b/pkg/remote/management.go
index c7e77c598..2c84521d0 100644
--- a/pkg/remote/management.go
+++ b/pkg/remote/management.go
@@ -78,15 +78,18 @@ func ByLabel(label string) HostSelector {
 			return document.ErrDocNotFound{Selector: selector}
 		}
 
+		var matchingHosts []baremetalHost
 		for _, doc := range docs {
 			host, err := newBaremetalHost(mgmtCfg, doc, docBundle)
 			if err != nil {
 				return err
 			}
 
-			a.Hosts = append(a.Hosts, host)
+			matchingHosts = append(matchingHosts, host)
 		}
 
+		a.Hosts = reconcileHosts(a.Hosts, matchingHosts...)
+
 		return nil
 	}
 }
@@ -105,7 +108,7 @@ func ByName(name string) HostSelector {
 			return err
 		}
 
-		a.Hosts = append(a.Hosts, host)
+		a.Hosts = reconcileHosts(a.Hosts, host)
 
 		return nil
 	}
@@ -202,3 +205,25 @@ func newBaremetalHost(mgmtCfg config.ManagementConfiguration,
 
 	return host, nil
 }
+
+// reconcileHosts produces the intersection of two baremetal host arrays.
+func reconcileHosts(existingHosts []baremetalHost, newHosts ...baremetalHost) []baremetalHost {
+	if len(existingHosts) == 0 {
+		return newHosts
+	}
+
+	// Create a map of host document names for efficient filtering
+	hostMap := make(map[string]bool)
+	for _, host := range existingHosts {
+		hostMap[host.HostName] = true
+	}
+
+	var reconciledHosts []baremetalHost
+	for _, host := range newHosts {
+		if _, exists := hostMap[host.HostName]; exists {
+			reconciledHosts = append(reconciledHosts, host)
+		}
+	}
+
+	return reconciledHosts
+}
diff --git a/pkg/remote/management_test.go b/pkg/remote/management_test.go
index 2804d0923..bc4f1983f 100644
--- a/pkg/remote/management_test.go
+++ b/pkg/remote/management_test.go
@@ -95,6 +95,26 @@ func TestNewManagerMultipleNodes(t *testing.T) {
 	assert.Equal(t, "node-master-2", manager.Hosts[1].NodeID())
 }
 
+func TestNewManagerMultipleSelectors(t *testing.T) {
+	settings := initSettings(t, withTestDataPath("base"))
+
+	manager, err := NewManager(settings, config.BootstrapPhase, ByName("master-1"),
+		ByLabel("airshipit.org/test-node=true"))
+	require.NoError(t, err)
+	require.Equal(t, 1, len(manager.Hosts))
+
+	assert.Equal(t, "node-master-1", manager.Hosts[0].NodeID())
+}
+
+func TestNewManagerMultipleSelectorsNoMatch(t *testing.T) {
+	settings := initSettings(t, withTestDataPath("base"))
+
+	manager, err := NewManager(settings, config.BootstrapPhase, ByName("master-2"),
+		ByLabel(document.EphemeralHostSelector))
+	require.Equal(t, 0, len(manager.Hosts))
+	assert.Error(t, err)
+}
+
 func TestNewManagerByNameNoHostFound(t *testing.T) {
 	settings := initSettings(t, withTestDataPath("base"))