satellites: delete unneeded interfaces

It is now possible to reconfigure the LINSTOR control traffic to only use IPv4
or IPv6. To enforce this even on existing clusters, we need to be able to
delete network interfaces.

This commit adds management of network interfaces, ensuring only expected
network interface are actually configured. We apply the same strategy as
with other properties, in that we only try to manage interfaces that where
configured by the operator.

Since we did not store this information up to now, we simply assume that
the Operator managed all interfaces named "default-ipv4" or "default-ipv6".

This also fixes an issue when upgrading from Operator v1 to v2, as Operator v1
uses "default" as interface name: After the upgrade, the interface was left
unchanged, leaving behind a potentially wrong interface configuration what was
not updated by the Operator. We also consider "default" interfaces to be
managed by the Operator.

Signed-off-by: Moritz Wanzenböck <[email protected]>

CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Option to select IP Family to use for LINSTOR control traffic.
 
+### Changed
+
+- Reconciliation of a Satellites network interfaces now also deletes unneeded interfaces.
+
 ## [v2.6.0] - 2024-09-04
 
 ### Added

go.mod

@@ -71,6 +71,7 @@ require (
 	github.com/prometheus/procfs v0.15.1 // indirect
 	github.com/sirupsen/logrus v1.9.3 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
+	github.com/stretchr/objx v0.5.2 // indirect
 	github.com/x448/float16 v0.8.4 // indirect
 	github.com/xlab/treeprint v1.2.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect

pkg/linstorhelper/client.go

@@ -4,15 +4,18 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
+	"encoding/json"
 	"fmt"
 	"net/http"
 	"net/url"
 	"strings"
 	"sync"
 	"time"
 
+	linstor "github.com/LINBIT/golinstor"
 	lapicache "github.com/LINBIT/golinstor/cache"
 	lapi "github.com/LINBIT/golinstor/client"
+	"golang.org/x/exp/slices"
 	"golang.org/x/time/rate"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -218,6 +221,13 @@ func caReferenceToCert(ctx context.Context, caRef *piraeusv1.CAReference, namesp
 
 // CreateOrUpdateNode ensures a node in LINSTOR matches the given node object.
 func (c *Client) CreateOrUpdateNode(ctx context.Context, node lapi.Node) (*lapi.Node, error) {
+	props, err := appliedInterfaceAnnotation(&node)
+	if err != nil {
+		return nil, err
+	}
+
+	node.Props[NodeInterfaceProperty] = props
+
 	existingNode, err := c.Nodes.Get(ctx, node.Name)
 	if err != nil {
 		// For 404
@@ -254,6 +264,26 @@ func (c *Client) CreateOrUpdateNode(ctx context.Context, node lapi.Node) (*lapi.
 		}
 	}
 
+	managedNics := managedInterfaces(&existingNode)
+	for _, existingNic := range existingNode.NetInterfaces {
+		if !slices.Contains(managedNics, existingNic.Name) {
+			// Not managed by Operator
+			continue
+		}
+
+		if slices.ContainsFunc(node.NetInterfaces, func(netInterface lapi.NetInterface) bool {
+			return netInterface.Name == existingNic.Name
+		}) {
+			// Interface should exist, do not delete
+			continue
+		}
+
+		err := c.Nodes.DeleteNetinterface(ctx, node.Name, existingNic.Name)
+		if err != nil {
+			return nil, fmt.Errorf("failed to delete network interface %s: %w", existingNic.Name, err)
+		}
+	}
+
 	return &existingNode, nil
 }
 
@@ -276,3 +306,43 @@ func (c *Client) ensureWantedInterface(ctx context.Context, node lapi.Node, want
 	// Interface was not found, creating it now
 	return c.Nodes.CreateNetInterface(ctx, node.Name, wanted)
 }
+
+func appliedInterfaceAnnotation(node *lapi.Node) (string, error) {
+	result := make([]string, 0, len(node.NetInterfaces))
+
+	for _, iface := range node.NetInterfaces {
+		result = append(result, iface.Name)
+	}
+
+	slices.Sort(result)
+	b, err := json.Marshal(result)
+	if err != nil {
+		return "", fmt.Errorf("failed to encode node interfaces: %w", err)
+	}
+
+	return string(b), nil
+}
+
+var (
+	// defaultManagedInterfaces is the interface names used by the Operator, used if no current node property is found.
+	// Operator v1 used "default"
+	// Operator v2 uses "default-ipv4" and "default-ipv6"
+	defaultManagedInterfaces = []string{"default", "default-ipv4", "default-ipv6"}
+
+	NodeInterfaceProperty = linstor.NamespcAuxiliary + "/" + vars.NodeInterfaceAnnotation
+)
+
+func managedInterfaces(node *lapi.Node) []string {
+	val, ok := node.Props[NodeInterfaceProperty]
+	if !ok {
+		return defaultManagedInterfaces
+	}
+
+	var result []string
+	err := json.Unmarshal([]byte(val), &result)
+	if err != nil {
+		return defaultManagedInterfaces
+	}
+
+	return result
+}

pkg/linstorhelper/client_test.go

@@ -9,14 +9,18 @@ import (
 	"crypto/x509/pkix"
 	"encoding/pem"
 	"math/big"
+	"net"
 	"net/http"
 	"net/url"
 	"reflect"
 	"testing"
 
+	linstor "github.com/LINBIT/golinstor"
 	lapi "github.com/LINBIT/golinstor/client"
 	"github.com/google/go-cmp/cmp"
+	"github.com/piraeusdatastore/linstor-csi/pkg/client/mocks"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -247,3 +251,132 @@ func testTlsConfig(t *testing.T) (*tls.Config, map[string][]byte) {
 			"tls.crt": pemCert,
 		}
 }
+
+func TestCreateOrUpdateNode(t *testing.T) {
+	t.Parallel()
+
+	sampleNode := lapi.Node{
+		Name: "node1",
+		Props: map[string]string{
+			"ExampleProp1":                      "val1",
+			linstorhelper.LastApplyProperty:     `["` + linstorhelper.NodeInterfaceProperty + `","ExampleProp1"]`,
+			linstorhelper.NodeInterfaceProperty: `["default-ipv4"]`,
+		},
+		NetInterfaces: []lapi.NetInterface{{
+			Name:          "default-ipv4",
+			Address:       net.IPv4(127, 0, 0, 1),
+			SatellitePort: linstor.DfltStltPortPlain,
+		}},
+	}
+
+	testcases := []struct {
+		name       string
+		node       lapi.Node
+		setupCalls func(t *testing.T) lapi.NodeProvider
+	}{
+		{
+			name: "new-node",
+			node: sampleNode,
+			setupCalls: func(t *testing.T) lapi.NodeProvider {
+				m := mocks.NewNodeProvider(t)
+				m.On("Get", mock.Anything, "node1").Return(lapi.Node{}, lapi.NotFoundError).Once()
+				m.On("Create", mock.Anything, sampleNode).Return(nil)
+				m.On("Get", mock.Anything, "node1").Return(sampleNode, nil)
+				return m
+			},
+		},
+		{
+			name: "existing-node",
+			node: sampleNode,
+			setupCalls: func(t *testing.T) lapi.NodeProvider {
+				m := mocks.NewNodeProvider(t)
+				m.On("Get", mock.Anything, "node1").Return(sampleNode, nil)
+				return m
+			},
+		},
+		{
+			name: "existing-node-with-updated-props-and-interfaces",
+			node: lapi.Node{
+				Name: "node1",
+				Props: map[string]string{
+					"ExampleProp1": "val2",
+				},
+				NetInterfaces: []lapi.NetInterface{{
+					Name:                    "default-ipv6",
+					Address:                 net.IPv6loopback,
+					SatelliteEncryptionType: "SSL",
+					SatellitePort:           linstor.DfltStltPortSsl,
+				}},
+			},
+			setupCalls: func(t *testing.T) lapi.NodeProvider {
+				m := mocks.NewNodeProvider(t)
+				m.On("Get", mock.Anything, "node1").Return(sampleNode, nil)
+				m.On("Modify", mock.Anything, "node1", lapi.NodeModify{
+					GenericPropsModify: lapi.GenericPropsModify{
+						OverrideProps: map[string]string{
+							"ExampleProp1":                      "val2",
+							linstorhelper.NodeInterfaceProperty: `["default-ipv6"]`,
+						},
+					},
+				}).Return(nil)
+				m.On("CreateNetInterface", mock.Anything, "node1", lapi.NetInterface{
+					Name:                    "default-ipv6",
+					Address:                 net.IPv6loopback,
+					SatelliteEncryptionType: "SSL",
+					SatellitePort:           linstor.DfltStltPortSsl,
+				}).Return(nil)
+				m.On("DeleteNetinterface", mock.Anything, "node1", "default-ipv4").Return(nil)
+				return m
+			},
+		},
+		{
+			name: "existing-node-without-interface-props",
+			node: sampleNode,
+			setupCalls: func(t *testing.T) lapi.NodeProvider {
+				m := mocks.NewNodeProvider(t)
+				m.On("Get", mock.Anything, "node1").Return(lapi.Node{
+					Name:  "node1",
+					Props: nil,
+					NetInterfaces: []lapi.NetInterface{
+						{
+							Name:          "default-ipv4",
+							Address:       net.IPv4(127, 0, 0, 1),
+							SatellitePort: linstor.DfltStltPortPlain,
+						},
+						{
+							Name:                    "default-ipv6",
+							Address:                 net.IPv6loopback,
+							SatelliteEncryptionType: "SSL",
+							SatellitePort:           linstor.DfltStltPortSsl,
+						},
+					},
+				}, nil)
+				m.On("Modify", mock.Anything, "node1", lapi.NodeModify{
+					GenericPropsModify: lapi.GenericPropsModify{
+						OverrideProps: map[string]string{
+							"ExampleProp1":                      "val1",
+							linstorhelper.LastApplyProperty:     `["` + linstorhelper.NodeInterfaceProperty + `","ExampleProp1"]`,
+							linstorhelper.NodeInterfaceProperty: `["default-ipv4"]`,
+						},
+					},
+				}).Return(nil)
+				m.On("DeleteNetinterface", mock.Anything, "node1", "default-ipv6").Return(nil)
+				return m
+			},
+		},
+	}
+
+	for i := range testcases {
+		test := &testcases[i]
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			lc := linstorhelper.Client{Client: lapi.Client{
+				Nodes: test.setupCalls(t),
+			}}
+
+			_, err := lc.CreateOrUpdateNode(context.Background(), test.node)
+			assert.NoError(t, err)
+		})
+	}
+}

pkg/vars/vars.go

@@ -17,6 +17,7 @@ var (
 const (
 	FieldOwner              = Domain + "/operator"
 	ApplyAnnotation         = Domain + "/last-applied"
+	NodeInterfaceAnnotation = Domain + "/configured-interfaces"
 	ManagedByLabel          = Domain + "/managed-by"
 	SatelliteNodeLabel      = Domain + "/linstor-satellite"
 	SatelliteFinalizer      = Domain + "/satellite-protection"