From f7f12ecf9976b0250c2e2deb4d7f7a61c3720403 Mon Sep 17 00:00:00 2001
From: Travis Nielsen <tnielsen@redhat.com>
Date: Wed, 29 May 2024 09:53:15 -0600
Subject: [PATCH] network: improve the IsHost function

This change is a follow-up to PR https://github.com/rook/rook/pull/13651 .

It is also meant to provide a continuation of the closed, stale
PR https://github.com/rook/rook/pull/13878 .

It is  furthermore  intended as a preparation for the introduction of
a way to enforce host networking in the next round of updates to PR
    https://github.com/rook/rook/pull/13651 .

This change improves the documentation,  implementation,  and unit testing for the   method of the
 spec.

Signed-off-by: Michael Adam <obnox@samba.org>
Co-authored-by: Blaine Gardner <b.blaine.gardner@gmail.com>
Co-authored-by: Travis Nielsen <tnielsen@redhat.com>
---
 pkg/apis/ceph.rook.io/v1/network.go      |  9 +++++--
 pkg/apis/ceph.rook.io/v1/network_test.go | 34 ++++++++++++++++++++++--
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/pkg/apis/ceph.rook.io/v1/network.go b/pkg/apis/ceph.rook.io/v1/network.go
index 629f30a06..c950eed22 100644
--- a/pkg/apis/ceph.rook.io/v1/network.go
+++ b/pkg/apis/ceph.rook.io/v1/network.go
@@ -32,8 +32,13 @@ func (n *NetworkSpec) IsMultus() bool {
 	return n.Provider == NetworkProviderMultus
 }
 
-// IsHost get whether to use host network provider. This method also preserve
-// compatibility with the old HostNetwork field.
+// IsHost is intended to be used to determine if the rook operator should configure
+// managed pods to use host networking.
+// This behavior is enabled by configuring the cephCluster with the "host" network provider.
+// This method also maintains compatibility with the old HostNetwork setting
+// which is incompatible with other network providers: HostNetwork set to true
+// together with an empty or unset network provider has the same effect as
+// network.Provider set to "host"
 func (n *NetworkSpec) IsHost() bool {
 	return (n.HostNetwork && n.Provider == NetworkProviderDefault) || n.Provider == NetworkProviderHost
 }
diff --git a/pkg/apis/ceph.rook.io/v1/network_test.go b/pkg/apis/ceph.rook.io/v1/network_test.go
index 3e3446257..6c6980f2c 100644
--- a/pkg/apis/ceph.rook.io/v1/network_test.go
+++ b/pkg/apis/ceph.rook.io/v1/network_test.go
@@ -72,10 +72,40 @@ func TestValidateNetworkSpec(t *testing.T) {
 	assert.NoError(t, err)
 }
 
-func TestNetworkCephIsHostLegacy(t *testing.T) {
-	net := NetworkSpec{HostNetwork: true}
+// test the NetworkSpec.IsHost method with different network providers
+// Also test it in combination with the legacy
+// "HostNetwork" setting.
+func TestNetworkCephIsHost(t *testing.T) {
+	net := NetworkSpec{HostNetwork: false}
 
+	net.Provider = NetworkProviderHost
 	assert.True(t, net.IsHost())
+
+	net.Provider = NetworkProviderDefault
+	net.HostNetwork = true
+	assert.True(t, net.IsHost())
+
+	net = NetworkSpec{}
+	net.Provider = NetworkProviderDefault
+	net.HostNetwork = false
+	assert.False(t, net.IsHost())
+
+	net = NetworkSpec{}
+	net.Provider = NetworkProviderMultus
+	net.HostNetwork = false
+	assert.False(t, net.IsHost())
+
+	net = NetworkSpec{}
+	net.Provider = NetworkProviderMultus
+	net.HostNetwork = true
+	assert.False(t, net.IsHost())
+
+	// test with  nonempty but invalid provider
+	net = NetworkSpec{}
+	net.HostNetwork = true
+	net.Provider = "foo"
+	assert.False(t, net.IsHost())
+
 }
 
 func TestNetworkSpec(t *testing.T) {