fix: prefer logical on merging link specs

This solves a case when lower layer (platform) defines `bond0` as logical interface properly, and upper layer (configuration) defines only some part of the config (e.g. VIP). Signed-off-by: Andrey Smirnov <[email protected]>
siderolabs · Feb 16, 2022 · 95a564b · 95a564b
1 parent 8b7091a
commit 95a564b
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 2 deletions.
diff --git a/internal/app/machined/pkg/controllers/network/link_merge_test.go b/internal/app/machined/pkg/controllers/network/link_merge_test.go
@@ -24,6 +24,7 @@ import (
 
 	netctrl "github.com/talos-systems/talos/internal/app/machined/pkg/controllers/network"
 	"github.com/talos-systems/talos/pkg/logging"
+	"github.com/talos-systems/talos/pkg/machinery/nethelpers"
 	"github.com/talos-systems/talos/pkg/machinery/resources/network"
 )
 
@@ -186,6 +187,47 @@ func (suite *LinkMergeSuite) TestMerge() {
 		}))
 }
 
+func (suite *LinkMergeSuite) TestMergeLogicalLink() {
+	bondPlatform := network.NewLinkSpec(network.ConfigNamespaceName, "platform/bond0")
+	*bondPlatform.TypedSpec() = network.LinkSpecSpec{
+		Name:    "bond0",
+		Logical: true,
+		Up:      true,
+		BondMaster: network.BondMasterSpec{
+			Mode: nethelpers.BondMode8023AD,
+		},
+		ConfigLayer: network.ConfigPlatform,
+	}
+
+	bondMachineConfig := network.NewLinkSpec(network.ConfigNamespaceName, "config/bond0")
+	*bondMachineConfig.TypedSpec() = network.LinkSpecSpec{
+		Name:        "bond0",
+		MTU:         1450,
+		Up:          true,
+		ConfigLayer: network.ConfigMachineConfiguration,
+	}
+
+	for _, res := range []resource.Resource{bondPlatform, bondMachineConfig} {
+		suite.Require().NoError(suite.state.Create(suite.ctx, res), "%v", res.Spec())
+	}
+
+	suite.Assert().NoError(retry.Constant(3*time.Second, retry.WithUnits(100*time.Millisecond)).Retry(
+		func() error {
+			return suite.assertLinks([]string{
+				"bond0",
+			}, func(r *network.LinkSpec) error {
+				if r.TypedSpec().MTU != 1450 {
+					return retry.ExpectedErrorf("not merged yet")
+				}
+
+				suite.Assert().True(r.TypedSpec().Logical)
+				suite.Assert().EqualValues(1450, r.TypedSpec().MTU)
+
+				return nil
+			})
+		}))
+}
+
 //nolint:gocyclo
 func (suite *LinkMergeSuite) TestMergeFlapping() {
 	// simulate two conflicting link definitions which are getting removed/added constantly

diff --git a/pkg/machinery/resources/network/link_spec.go b/pkg/machinery/resources/network/link_spec.go
@@ -66,8 +66,9 @@ var (
 //
 //nolint:gocyclo
 func (spec *LinkSpecSpec) Merge(other *LinkSpecSpec) error {
-	if spec.Logical != other.Logical {
-		return fmt.Errorf("mismatch on logical for %q: %v != %v", spec.Name, spec.Logical, other.Logical)
+	// prefer Logical, as it is defined for bonds/vlans, etc.
+	if other.Logical {
+		spec.Logical = other.Logical
 	}
 
 	if other.Up {