From 5d1bc77c09d62b2dcbc5336fa260292c59472ed7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20Wanzenb=C3=B6ck?= Date: Mon, 21 Oct 2024 15:51:02 +0200 Subject: [PATCH] satellites: delete unneeded interfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CHANGELOG.md | 4 + go.mod | 1 + pkg/linstorhelper/client.go | 70 ++++++++++++++++ pkg/linstorhelper/client_test.go | 133 +++++++++++++++++++++++++++++++ pkg/vars/vars.go | 1 + 5 files changed, 209 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc47824e..0888f94e 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/go.mod b/go.mod index 54da7336..50c951d8 100644 --- a/go.mod +++ b/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 diff --git a/pkg/linstorhelper/client.go b/pkg/linstorhelper/client.go index 0d7cda75..fa6dd77f 100644 --- a/pkg/linstorhelper/client.go +++ b/pkg/linstorhelper/client.go @@ -4,6 +4,7 @@ import ( "context" "crypto/tls" "crypto/x509" + "encoding/json" "fmt" "net/http" "net/url" @@ -11,8 +12,10 @@ import ( "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 +} diff --git a/pkg/linstorhelper/client_test.go b/pkg/linstorhelper/client_test.go index e754c7d2..067efecc 100644 --- a/pkg/linstorhelper/client_test.go +++ b/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) + }) + } +} diff --git a/pkg/vars/vars.go b/pkg/vars/vars.go index 07ddb9bb..b8852227 100644 --- a/pkg/vars/vars.go +++ b/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"