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"