diff --git a/clab/clab.go b/clab/clab.go index 10c188d41..5e15f87f1 100644 --- a/clab/clab.go +++ b/clab/clab.go @@ -559,21 +559,13 @@ func (c *CLab) scheduleNodes(ctx context.Context, maxWorkers int, skipPostDeploy dn.EnterStage(types.WaitForCreateLinks) - err = node.DeployLinks(ctx) + // Deploy the Nodes link endpoints + err = node.DeployEndpoints(ctx) if err != nil { log.Errorf("failed deploy links for node %q: %v", node.Config().ShortName, err) continue } - // DeployLinks does not necessarily create all the links already. - // veth links might be created by the peer node, if it was not already - // deployed properly. Hence we need to wait for all the links to be created. - // we just wait if there is actually a dependency on this state, otherwise - // we head on. - if dn.MustWait(types.WaitForCreateLinks) { - node.WaitForAllLinksCreated() - } - dn.Done(types.WaitForCreateLinks) // start config stage @@ -589,12 +581,11 @@ func (c *CLab) scheduleNodes(ctx context.Context, maxWorkers int, skipPostDeploy dn.Done(types.WaitForConfigure) - // HOTFIX: EXECS are for now being run after all nodes are deployed, see: deploy.go - // // run execs - // err = node.RunExecFromConfig(ctx, execCollection) - // if err != nil { - // log.Errorf("failed to run exec commands for %s: %v", node.GetShortName(), err) - // } + // run execs + err = node.RunExecFromConfig(ctx, execCollection) + if err != nil { + log.Errorf("failed to run exec commands for %s: %v", node.GetShortName(), err) + } // health state processing if dn.MustWait(types.WaitForHealthy) { diff --git a/cmd/deploy.go b/cmd/deploy.go index af827b260..e1f809e35 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -221,24 +221,21 @@ func deployFn(_ *cobra.Command, _ []string) error { return err } - if nodesWg != nil { - nodesWg.Wait() - } - - // HOTFIX: originally execs were executed by the node, but because - // execs were run before all the links are connected between the nodes they could fail. - // we couldn't wait for all links to be created/connected since this would cause a deadlock - // either when a nodeA depends on nodeB and there is a link between them, or when - // there is more links than concurrent node worker. - // For the time being the execs would be executed only after the nodes are created - // with a more elegant fix in the works. - for _, n := range c.Nodes { - err = n.RunExecFromConfig(ctx, execCollection) + // also call deploy on the special nodes endpoints (only host is required for the + // vxlan stitched endpoints) + eps := c.GetSpecialLinkNodes()["host"].GetEndpoints() + for _, ep := range eps { + err = ep.Deploy(ctx) if err != nil { - return err + log.Warnf("failed deploying endpoint %s", ep) } } + // wait for all the containers to get created + if nodesWg != nil { + nodesWg.Wait() + } + // write to log execCollection.Log() diff --git a/cmd/tools_veth.go b/cmd/tools_veth.go index d403ce007..d74d1fd62 100644 --- a/cmd/tools_veth.go +++ b/cmd/tools_veth.go @@ -111,9 +111,9 @@ var vethCreateCmd = &cobra.Command{ return err } - err = link.Deploy(ctx) - if err != nil { - return err + // deploy the endpoints of the Link + for _, ep := range link.GetEndpoints() { + ep.Deploy(ctx) } log.Info("veth interface successfully created!") diff --git a/cmd/vxlan.go b/cmd/vxlan.go index 36530da4c..504f2ef15 100644 --- a/cmd/vxlan.go +++ b/cmd/vxlan.go @@ -108,10 +108,10 @@ var vxlanCreateCmd = &cobra.Command{ return fmt.Errorf("not a VxlanStitched link") } - err = vxl.DeployWithExistingVeth(ctx) - if err != nil { - return err + for _, ep := range vxl.GetEndpoints() { + ep.Deploy(ctx) } + return nil }, } diff --git a/docs/manual/vrnetlab.md b/docs/manual/vrnetlab.md index 40024983f..8eabb85a9 100644 --- a/docs/manual/vrnetlab.md +++ b/docs/manual/vrnetlab.md @@ -152,12 +152,6 @@ topology: stage: healthy ``` -/// admonition | Warning! - type: warning -When using VM-based nodes and creating the dependencies for the heatlhy stage, it is important to ensure that no links exist between the nodes that depend on each other. This is because the VM-based node do not support link hot plugging and wait till all the links are attached to the container before starting the boot process. -Pay attention, as it may lead to a deadlock situation where the nodes are waiting for each other to boot. -/// - ### Boot delay A predecessor of the Boot Order is the boot delay that can be set with `BOOT_DELAY` environment variable that the supported VM-based nodes will respect. diff --git a/links/endpoint.go b/links/endpoint.go index f4877d109..40b08de33 100644 --- a/links/endpoint.go +++ b/links/endpoint.go @@ -1,6 +1,7 @@ package links import ( + "context" "fmt" "net" @@ -31,6 +32,15 @@ type Endpoint interface { // has the same node and interface name as the given endpoint. HasSameNodeAndInterface(ept Endpoint) bool Remove() error + // Deploy deploys the endpoint by calling the Deploy method of the link it is assigned to + // and passing the endpoint as an argument so that the link that consists of A and B endpoints + // can deploy them independently. + Deploy(context.Context) error + // IsNodeless returns true for the endpoints that has no explicit node defined in the topology. + // E.g. host endpoints, mgmt bridge endpoints. + // Because there is no node that would deploy this side of the link they should be deployed along + // with the A side of the veth link. + IsNodeless() bool } // EndpointGeneric is the generic endpoint struct that is used by all endpoint types. diff --git a/links/endpoint_bridge.go b/links/endpoint_bridge.go index 328849b26..7f304a9e8 100644 --- a/links/endpoint_bridge.go +++ b/links/endpoint_bridge.go @@ -1,6 +1,7 @@ package links import ( + "context" "errors" "fmt" @@ -11,11 +12,13 @@ import ( type EndpointBridge struct { EndpointGeneric + isMgmtBridgeEndpoint bool } -func NewEndpointBridge(eg *EndpointGeneric) *EndpointBridge { +func NewEndpointBridge(eg *EndpointGeneric, isMgmtBridgeEndpoint bool) *EndpointBridge { return &EndpointBridge{ - EndpointGeneric: *eg, + isMgmtBridgeEndpoint: isMgmtBridgeEndpoint, + EndpointGeneric: *eg, } } @@ -41,6 +44,16 @@ func (e *EndpointBridge) Verify(p *VerifyLinkParams) error { return nil } +func (e *EndpointBridge) Deploy(ctx context.Context) error { + return e.GetLink().Deploy(ctx, e) +} + +func (e *EndpointBridge) IsNodeless() bool { + // the mgmt bridge is nodeless. + // If this is a regular bridge, then it should trigger BEnd deployment. + return e.isMgmtBridgeEndpoint +} + // CheckBridgeExists verifies that the given bridge is present in the // network namespace referenced via the provided nspath handle. func CheckBridgeExists(n Node) error { diff --git a/links/endpoint_host.go b/links/endpoint_host.go index 943ef460e..93b8b158e 100644 --- a/links/endpoint_host.go +++ b/links/endpoint_host.go @@ -1,6 +1,9 @@ package links -import "errors" +import ( + "context" + "errors" +) type EndpointHost struct { EndpointGeneric @@ -12,6 +15,10 @@ func NewEndpointHost(eg *EndpointGeneric) *EndpointHost { } } +func (e *EndpointHost) Deploy(ctx context.Context) error { + return e.GetLink().Deploy(ctx, e) +} + func (e *EndpointHost) Verify(_ *VerifyLinkParams) error { var errs []error err := CheckEndpointUniqueness(e) @@ -27,3 +34,7 @@ func (e *EndpointHost) Verify(_ *VerifyLinkParams) error { } return nil } + +func (e *EndpointHost) IsNodeless() bool { + return true +} diff --git a/links/endpoint_macvlan.go b/links/endpoint_macvlan.go index 997c512dd..2811e0da0 100644 --- a/links/endpoint_macvlan.go +++ b/links/endpoint_macvlan.go @@ -1,5 +1,7 @@ package links +import "context" + type EndpointMacVlan struct { EndpointGeneric } @@ -10,7 +12,15 @@ func NewEndpointMacVlan(eg *EndpointGeneric) *EndpointMacVlan { } } +func (e *EndpointMacVlan) Deploy(ctx context.Context) error { + return e.GetLink().Deploy(ctx, e) +} + // Verify runs verification to check if the endpoint can be deployed. func (e *EndpointMacVlan) Verify(_ *VerifyLinkParams) error { return CheckEndpointExists(e) } + +func (e *EndpointMacVlan) IsNodeless() bool { + return false +} diff --git a/links/endpoint_raw.go b/links/endpoint_raw.go index 08ff1a4f4..e51e258a6 100644 --- a/links/endpoint_raw.go +++ b/links/endpoint_raw.go @@ -58,7 +58,7 @@ func (er *EndpointRaw) Resolve(params *ResolveParams, l Link) (Endpoint, error) switch node.GetLinkEndpointType() { case LinkEndpointTypeBridge: - e = NewEndpointBridge(genericEndpoint) + e = NewEndpointBridge(genericEndpoint, false) case LinkEndpointTypeHost: e = NewEndpointHost(genericEndpoint) diff --git a/links/endpoint_veth.go b/links/endpoint_veth.go index 50c89d08f..51c0e7bef 100644 --- a/links/endpoint_veth.go +++ b/links/endpoint_veth.go @@ -1,5 +1,7 @@ package links +import "context" + type EndpointVeth struct { EndpointGeneric } @@ -14,3 +16,11 @@ func NewEndpointVeth(eg *EndpointGeneric) *EndpointVeth { func (e *EndpointVeth) Verify(_ *VerifyLinkParams) error { return CheckEndpointUniqueness(e) } + +func (e *EndpointVeth) Deploy(ctx context.Context) error { + return e.GetLink().Deploy(ctx, e) +} + +func (e *EndpointVeth) IsNodeless() bool { + return false +} diff --git a/links/endpoint_vxlan.go b/links/endpoint_vxlan.go index cc305456c..9d0db9b32 100644 --- a/links/endpoint_vxlan.go +++ b/links/endpoint_vxlan.go @@ -1,6 +1,7 @@ package links import ( + "context" "fmt" "net" ) @@ -32,3 +33,11 @@ func (e *EndpointVxlan) String() string { func (e *EndpointVxlan) Verify(*VerifyLinkParams) error { return CheckEndpointUniqueness(e) } + +func (e *EndpointVxlan) Deploy(ctx context.Context) error { + return e.GetLink().Deploy(ctx, e) +} + +func (e *EndpointVxlan) IsNodeless() bool { + return false +} diff --git a/links/generic_link_node.go b/links/generic_link_node.go index 6fe10e688..6a00ebf76 100644 --- a/links/generic_link_node.go +++ b/links/generic_link_node.go @@ -10,7 +10,6 @@ import ( type GenericLinkNode struct { shortname string - links []Link endpoints []Endpoint nspath string } @@ -39,10 +38,6 @@ func (g *GenericLinkNode) ExecFunction(f func(ns.NetNS) error) error { return netns.Do(f) } -func (g *GenericLinkNode) AddLink(l Link) { - g.links = append(g.links, l) -} - func (g *GenericLinkNode) AddEndpoint(e Endpoint) { g.endpoints = append(g.endpoints, e) } @@ -62,8 +57,8 @@ func (*GenericLinkNode) GetState() state.NodeState { } func (g *GenericLinkNode) Delete(ctx context.Context) error { - for _, l := range g.links { - err := l.Remove(ctx) + for _, e := range g.endpoints { + err := e.GetLink().Remove(ctx) if err != nil { return err } diff --git a/links/link.go b/links/link.go index 2598ccf89..d9941bb98 100644 --- a/links/link.go +++ b/links/link.go @@ -20,7 +20,10 @@ const ( DefaultLinkMTU = 9500 LinkDeploymentStateNotDeployed = iota - LinkDeploymentStateDeployed + // LinkDeploymentStateHalfDeployed is a state in which one of the endpoints + // of the links finished deploying and the other one is not yet deployed. + LinkDeploymentStateHalfDeployed + LinkDeploymentStateFullDeployed LinkDeploymentStateRemoved ) @@ -295,8 +298,8 @@ type RawLink interface { // Link is an interface that all concrete link types must implement. // Concrete link types are resolved from raw links and become part of CLab.Links. type Link interface { - // Deploy deploys the link. - Deploy(context.Context) error + // Deploy deploys the link. Endpoint is the endpoint that triggers the creation of the link. + Deploy(context.Context, Endpoint) error // Remove removes the link. Remove(context.Context) error // GetType returns the type of the link. @@ -353,7 +356,6 @@ type Node interface { // In case of a bridge node (ovs or regular linux bridge) it will take the interface and make the bridge // the master of the interface and bring the interface up. AddLinkToContainer(ctx context.Context, link netlink.Link, f func(ns.NetNS) error) error - AddLink(l Link) // AddEndpoint adds the Endpoint to the node AddEndpoint(e Endpoint) GetLinkEndpointType() LinkEndpointType diff --git a/links/link_host.go b/links/link_host.go index 596bc029f..072447ea5 100644 --- a/links/link_host.go +++ b/links/link_host.go @@ -79,9 +79,7 @@ func (r *LinkHostRaw) Resolve(params *ResolveParams) (Link, error) { link.Endpoints = []Endpoint{ep, hostEp} // add the link to the endpoints node - hostEp.GetNode().AddLink(link) hostEp.GetNode().AddEndpoint(hostEp) - ep.GetNode().AddLink(link) // set default link mtu if MTU is unset if link.MTU == 0 { diff --git a/links/link_macvlan.go b/links/link_macvlan.go index 362518523..c7a13cecb 100644 --- a/links/link_macvlan.go +++ b/links/link_macvlan.go @@ -103,9 +103,6 @@ func (r *LinkMacVlanRaw) Resolve(params *ResolveParams) (Link, error) { return nil, err } - // add endpoint links to nodes - link.NodeEndpoint.GetNode().AddLink(link) - return link, nil } @@ -128,7 +125,7 @@ func (l *LinkMacVlan) GetParentInterfaceMTU() (int, error) { return hostLink.Attrs().MTU, nil } -func (l *LinkMacVlan) Deploy(ctx context.Context) error { +func (l *LinkMacVlan) Deploy(ctx context.Context, _ Endpoint) error { // lookup the parent host interface parentInterface, err := utils.LinkByNameOrAlias(l.HostEndpoint.GetIfaceName()) if err != nil { diff --git a/links/link_mgmt-net.go b/links/link_mgmt-net.go index f0b928aa4..c2a20f31e 100644 --- a/links/link_mgmt-net.go +++ b/links/link_mgmt-net.go @@ -47,9 +47,7 @@ func (r *LinkMgmtNetRaw) Resolve(params *ResolveParams) (Link, error) { mgmtBridgeNode := GetMgmtBrLinkNode() - bridgeEp := &EndpointBridge{ - EndpointGeneric: *NewEndpointGeneric(mgmtBridgeNode, r.HostInterface, link), - } + bridgeEp := NewEndpointBridge(NewEndpointGeneric(mgmtBridgeNode, r.HostInterface, link), true) var err error bridgeEp.MAC, err = utils.GenMac(ClabOUI) @@ -66,9 +64,7 @@ func (r *LinkMgmtNetRaw) Resolve(params *ResolveParams) (Link, error) { link.Endpoints = []Endpoint{bridgeEp, contEp} // add link to respective endpoint nodes - bridgeEp.GetNode().AddLink(link) bridgeEp.GetNode().AddEndpoint(bridgeEp) - contEp.GetNode().AddLink(link) // set default link mtu if MTU is unset if link.MTU == 0 { @@ -114,7 +110,7 @@ func (*mgmtBridgeLinkNode) GetLinkEndpointType() LinkEndpointType { return LinkEndpointTypeBridge } -func (b *mgmtBridgeLinkNode) AddLinkToContainer(ctx context.Context, link netlink.Link, f func(ns.NetNS) error) error { +func (b *mgmtBridgeLinkNode) AddLinkToContainer(_ context.Context, link netlink.Link, f func(ns.NetNS) error) error { // retrieve the namespace handle ns, err := ns.GetCurrentNS() if err != nil { diff --git a/links/link_veth.go b/links/link_veth.go index 204334083..c904774e3 100644 --- a/links/link_veth.go +++ b/links/link_veth.go @@ -3,10 +3,10 @@ package links import ( "context" "fmt" + "strings" "sync" log "github.com/sirupsen/logrus" - "github.com/srl-labs/containerlab/nodes/state" "github.com/srl-labs/containerlab/utils" "github.com/vishvananda/netlink" ) @@ -57,8 +57,6 @@ func (r *LinkVEthRaw) Resolve(params *ResolveParams) (Link, error) { } // add endpoint to the link endpoints l.Endpoints = append(l.Endpoints, ep) - // add link to endpoint node - ep.GetNode().AddLink(l) } // set default link mtu if MTU is unset @@ -109,31 +107,24 @@ func (*LinkVEth) GetType() LinkType { return LinkTypeVEth } -func (l *LinkVEth) Deploy(ctx context.Context) error { - // since each node calls deploy on its links, we need to make sure that we only deploy - // the link once, even if multiple nodes call deploy on the same link. - l.deployMutex.Lock() - defer l.deployMutex.Unlock() - if l.DeploymentState == LinkDeploymentStateDeployed { - return nil - } - - for _, ep := range l.GetEndpoints() { - if ep.GetNode().GetState() != state.Deployed { - return nil - } - } +func (l *LinkVEth) deployAEnd(ctx context.Context, idx int) error { + ep := l.Endpoints[idx] + // the peer Endpoint is the other of the two endpoints in the + // Endpoints slice. So do a +1 on the index and modulo operation + // to take care of the wrap around. + peerIdx := (idx + 1) % 2 + peerEp := l.Endpoints[peerIdx] - log.Infof("Creating link: %s <--> %s", l.GetEndpoints()[0], l.GetEndpoints()[1]) + log.Infof("Creating Endpoint: %s ( --> %s )", ep, peerEp) // build the netlink.Veth struct for the link provisioning linkA := &netlink.Veth{ LinkAttrs: netlink.LinkAttrs{ - Name: l.Endpoints[0].GetRandIfaceName(), + Name: ep.GetRandIfaceName(), MTU: l.MTU, // Mac address is set later on }, - PeerName: l.Endpoints[1].GetRandIfaceName(), + PeerName: peerEp.GetRandIfaceName(), // PeerMac address is set later on } @@ -143,36 +134,109 @@ func (l *LinkVEth) Deploy(ctx context.Context) error { return err } - // retrieve the netlink.Link for the B / Peer side of the link - linkB, err := utils.LinkByNameOrAlias(l.Endpoints[1].GetRandIfaceName()) + // disable TXOffloading + if err := utils.EthtoolTXOff(ep.GetRandIfaceName()); err != nil { + return err + } + + // the link needs to be moved to the relevant network namespace + // and enabled (up). This is done via linkSetupFunc. + // based on the endpoint type the link setup function is different. + // linkSetupFunc is executed in a netns of a node. + // if the node is a regular namespace node + // add link to node, rename, set mac and Up + err = ep.GetNode().AddLinkToContainer(ctx, linkA, + SetNameMACAndUpInterface(linkA, ep)) if err != nil { return err } - // once veth pair is created, disable tx offload for the veth pair - for _, ep := range l.Endpoints { - if err := utils.EthtoolTXOff(ep.GetRandIfaceName()); err != nil { - return err - } + l.DeploymentState = LinkDeploymentStateHalfDeployed + + // e.g. host endpoints are nodeless, and therefore the B end of the veth link should + // be deployed right after the A end is deployed. + if peerEp.IsNodeless() { + return l.deployBEnd(ctx, peerIdx) + } + + return nil +} + +func (l *LinkVEth) deployBEnd(ctx context.Context, idx int) error { + ep := l.Endpoints[idx] + peerEp := l.Endpoints[(idx+1)%2] + + log.Infof("Assigning Endpoint: %s ( --> %s )", ep, peerEp) + + // retrieve the netlink.Link for the provided Endpoint + link, err := utils.LinkByNameOrAlias(ep.GetRandIfaceName()) + if err != nil { + return err } - // both ends of the link need to be moved to the relevant network namespace + // disable TXOffloading + if err := utils.EthtoolTXOff(ep.GetRandIfaceName()); err != nil { + return err + } + + // the link needs to be moved to the relevant network namespace // and enabled (up). This is done via linkSetupFunc. // based on the endpoint type the link setup function is different. // linkSetupFunc is executed in a netns of a node. - for idx, link := range []netlink.Link{linkA, linkB} { - // if the node is a regular namespace node - // add link to node, rename, set mac and Up - err = l.Endpoints[idx].GetNode().AddLinkToContainer(ctx, link, - SetNameMACAndUpInterface(link, l.Endpoints[idx])) - if err != nil { - return err + // if the node is a regular namespace node + // add link to node, rename, set mac and Up + err = ep.GetNode().AddLinkToContainer(ctx, link, + SetNameMACAndUpInterface(link, ep)) + if err != nil { + return err + } + + l.DeploymentState = LinkDeploymentStateFullDeployed + return nil +} + +// getEndpointIndex returns the index of the ep endpoint belonging to l link. +// An error is returned when the ep is not part of the l's endpoints. +func (l *LinkVEth) getEndpointIndex(ep Endpoint) (int, error) { + for idx, e := range l.Endpoints { + if e == ep { + return idx, nil } } - l.DeploymentState = LinkDeploymentStateDeployed + // if the endpoint is not part of the link + // build a string list of endpoints and return a meaningful error + epStrings := []string{} + for _, e := range l.Endpoints { + epStrings = append(epStrings, e.String()) + } - return nil + return -1, fmt.Errorf("endpoint %s does not belong to link [ %s ]", ep.String(), strings.Join(epStrings, ", ")) + +} + +// Deploy deploys the veth link by creating the A and B sides of the veth pair independently +// based on the calling endpoint. +func (l *LinkVEth) Deploy(ctx context.Context, ep Endpoint) error { + // since each node calls deploy on its links, we need to make sure that we only deploy + // the link once, even if multiple nodes call deploy on the same link. + l.deployMutex.Lock() + defer l.deployMutex.Unlock() + + // first we need to check that the provided ep is part of this link + idx, err := l.getEndpointIndex(ep) + if err != nil { + return err + } + + // The first node to trigger the link creation will call deployAEnd, + // subsequent (the second) call will end up in deployBEnd. + switch l.DeploymentState { + case LinkDeploymentStateHalfDeployed: + return l.deployBEnd(ctx, idx) + default: + return l.deployAEnd(ctx, idx) + } } func (l *LinkVEth) Remove(_ context.Context) error { diff --git a/links/link_vxlan.go b/links/link_vxlan.go index 7cb710997..08ce4b413 100644 --- a/links/link_vxlan.go +++ b/links/link_vxlan.go @@ -73,7 +73,7 @@ func (lr *LinkVxlanRaw) resolveStitchedVEthComponent(params *ResolveParams) (*Li vethLink := hl.(*LinkVEth) - // host endpoint is always a 2nd element in the Endpoints slice + // host endpoint is always the 2nd element in the Endpoints slice return vethLink, vethLink.Endpoints[1], nil } @@ -94,9 +94,6 @@ func (lr *LinkVxlanRaw) resolveStitchedVxlan(params *ResolveParams) (Link, error // return the stitched vxlan link stitchedLink := NewVxlanStitched(vxlanLink, vethLink, stitchEp) - // add stitched link to node - params.Nodes[lr.Endpoint.Node].AddLink(stitchedLink) - return stitchedLink, nil } @@ -172,9 +169,6 @@ func (lr *LinkVxlanRaw) resolveVxlan(params *ResolveParams, stitched bool) (*Lin link.remoteEndpoint.MAC = hwaddr } - // add link to local endpoints node - link.localEndpoint.GetNode().AddLink(link) - return link, nil } @@ -211,7 +205,7 @@ type LinkVxlan struct { remoteEndpoint *EndpointVxlan } -func (l *LinkVxlan) Deploy(ctx context.Context) error { +func (l *LinkVxlan) Deploy(ctx context.Context, _ Endpoint) error { err := l.deployVxlanInterface() if err != nil { return err diff --git a/links/link_vxlan_stitched.go b/links/link_vxlan_stitched.go index a86e43202..de1328aef 100644 --- a/links/link_vxlan_stitched.go +++ b/links/link_vxlan_stitched.go @@ -37,25 +37,25 @@ func NewVxlanStitched(vxlan *LinkVxlan, veth *LinkVEth, vethStitchEp Endpoint) * // DeployWithExistingVeth provisons the stitched vxlan link whilst the // veth interface does already exist, hence it is not created as part of this // deployment. -func (l *VxlanStitched) DeployWithExistingVeth(ctx context.Context) error { - return l.internalDeploy(ctx, true) +func (l *VxlanStitched) DeployWithExistingVeth(ctx context.Context, ep Endpoint) error { + return l.internalDeploy(ctx, ep, true) } // Deploy provisions the stitched vxlan link with all its underlaying sub-links. -func (l *VxlanStitched) Deploy(ctx context.Context) error { - return l.internalDeploy(ctx, false) +func (l *VxlanStitched) Deploy(ctx context.Context, ep Endpoint) error { + return l.internalDeploy(ctx, ep, false) } -func (l *VxlanStitched) internalDeploy(ctx context.Context, skipVethCreation bool) error { +func (l *VxlanStitched) internalDeploy(ctx context.Context, ep Endpoint, skipVethCreation bool) error { // deploy the vxlan link - err := l.vxlanLink.Deploy(ctx) + err := l.vxlanLink.Deploy(ctx, ep) if err != nil { return err } // the veth creation might be skipped if it already exists if !skipVethCreation { - err = l.vethLink.Deploy(ctx) + err = l.vethLink.Deploy(ctx, ep) if err != nil { return err } diff --git a/mocks/dependency_manager.go b/mocks/dependency_manager.go index 7b7ef4999..3188c183c 100644 --- a/mocks/dependency_manager.go +++ b/mocks/dependency_manager.go @@ -95,21 +95,6 @@ func (mr *MockDependencyManagerMockRecorder) GetNode(name any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNode", reflect.TypeOf((*MockDependencyManager)(nil).GetNode), name) } -// MustWait mocks base method. -func (m *MockDependencyManager) MustWait(nodeName string, stage types.WaitForStage) (bool, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "MustWait", nodeName, stage) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// MustWait indicates an expected call of MustWait. -func (mr *MockDependencyManagerMockRecorder) MustWait(nodeName, stage any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MustWait", reflect.TypeOf((*MockDependencyManager)(nil).MustWait), nodeName, stage) -} - // String mocks base method. func (m *MockDependencyManager) String() string { m.ctrl.T.Helper() diff --git a/mocks/mocknodes/node.go b/mocks/mocknodes/node.go index eaae11c00..ae25b5c12 100644 --- a/mocks/mocknodes/node.go +++ b/mocks/mocknodes/node.go @@ -59,18 +59,6 @@ func (mr *MockNodeMockRecorder) AddEndpoint(e any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddEndpoint", reflect.TypeOf((*MockNode)(nil).AddEndpoint), e) } -// AddLink mocks base method. -func (m *MockNode) AddLink(l links.Link) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "AddLink", l) -} - -// AddLink indicates an expected call of AddLink. -func (mr *MockNodeMockRecorder) AddLink(l any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddLink", reflect.TypeOf((*MockNode)(nil).AddLink), l) -} - // AddLinkToContainer mocks base method. func (m *MockNode) AddLinkToContainer(ctx context.Context, link netlink.Link, f func(ns.NetNS) error) error { m.ctrl.T.Helper() @@ -169,18 +157,18 @@ func (mr *MockNodeMockRecorder) Deploy(arg0, arg1 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Deploy", reflect.TypeOf((*MockNode)(nil).Deploy), arg0, arg1) } -// DeployLinks mocks base method. -func (m *MockNode) DeployLinks(ctx context.Context) error { +// DeployEndpoints mocks base method. +func (m *MockNode) DeployEndpoints(ctx context.Context) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeployLinks", ctx) + ret := m.ctrl.Call(m, "DeployEndpoints", ctx) ret0, _ := ret[0].(error) return ret0 } -// DeployLinks indicates an expected call of DeployLinks. -func (mr *MockNodeMockRecorder) DeployLinks(ctx any) *gomock.Call { +// DeployEndpoints indicates an expected call of DeployEndpoints. +func (mr *MockNodeMockRecorder) DeployEndpoints(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeployLinks", reflect.TypeOf((*MockNode)(nil).DeployLinks), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeployEndpoints", reflect.TypeOf((*MockNode)(nil).DeployEndpoints), ctx) } // ExecFunction mocks base method. @@ -483,18 +471,6 @@ func (mr *MockNodeMockRecorder) VerifyStartupConfig(topoDir any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "VerifyStartupConfig", reflect.TypeOf((*MockNode)(nil).VerifyStartupConfig), topoDir) } -// WaitForAllLinksCreated mocks base method. -func (m *MockNode) WaitForAllLinksCreated() { - m.ctrl.T.Helper() - m.ctrl.Call(m, "WaitForAllLinksCreated") -} - -// WaitForAllLinksCreated indicates an expected call of WaitForAllLinksCreated. -func (mr *MockNodeMockRecorder) WaitForAllLinksCreated() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WaitForAllLinksCreated", reflect.TypeOf((*MockNode)(nil).WaitForAllLinksCreated)) -} - // WithMgmtNet mocks base method. func (m *MockNode) WithMgmtNet(arg0 *types.MgmtNet) { m.ctrl.T.Helper() diff --git a/nodes/default_node.go b/nodes/default_node.go index 66e07def5..9e4ddd34a 100644 --- a/nodes/default_node.go +++ b/nodes/default_node.go @@ -42,15 +42,11 @@ type DefaultNode struct { // OverwriteNode stores the interface used to overwrite methods defined // for DefaultNode, so that particular nodes can provide custom implementations. OverwriteNode NodeOverwrites - // List of links that reference the node. - Links []links.Link // List of link endpoints that are connected to the node. Endpoints []links.Endpoint // State of the node state state.NodeState statemutex sync.RWMutex - // links created wg - linksCreated *sync.WaitGroup } // NewDefaultNode initializes the DefaultNode structure and receives a NodeOverwrites interface @@ -62,7 +58,6 @@ func NewDefaultNode(n NodeOverwrites) *DefaultNode { OverwriteNode: n, LicensePolicy: types.LicensePolicyNone, SSHConfig: types.NewSSHConfig(), - linksCreated: &sync.WaitGroup{}, } return dn @@ -74,10 +69,6 @@ func (d *DefaultNode) GetRuntime() runtime.ContainerRuntime { r func (d *DefaultNode) Config() *types.NodeConfig { return d.Cfg } func (*DefaultNode) PostDeploy(_ context.Context, _ *PostDeployParams) error { return nil } -func (d *DefaultNode) WaitForAllLinksCreated() { - d.linksCreated.Wait() -} - // PreDeploy is a common method for all nodes that is called before the node is deployed. func (d *DefaultNode) PreDeploy(_ context.Context, params *PreDeployParams) error { _, err := d.LoadOrGenerateCertificate(params.Cert, params.TopologyName) @@ -153,8 +144,8 @@ func (d *DefaultNode) Deploy(ctx context.Context, _ *DeployParams) error { } func (d *DefaultNode) Delete(ctx context.Context) error { - for _, l := range d.Links { - err := l.Remove(ctx) + for _, e := range d.Endpoints { + err := e.GetLink().Remove(ctx) if err != nil { return err } @@ -478,8 +469,6 @@ func (d *DefaultNode) AddLinkToContainer(_ context.Context, link netlink.Link, f if err != nil { return err } - // indicate this link is created - d.linksCreated.Done() return nil } @@ -508,11 +497,6 @@ func (d *DefaultNode) ExecFunction(f func(ns.NetNS) error) error { return netns.Do(f) } -func (d *DefaultNode) AddLink(l links.Link) { - d.Links = append(d.Links, l) - d.linksCreated.Add(1) -} - func (d *DefaultNode) AddEndpoint(e links.Endpoint) { d.Endpoints = append(d.Endpoints, e) } @@ -531,15 +515,15 @@ func (d *DefaultNode) GetShortName() string { return d.Cfg.ShortName } -// DeployLinks deploys links associated with the node. -func (d *DefaultNode) DeployLinks(ctx context.Context) error { - for _, l := range d.Links { - err := l.Deploy(ctx) +// DeployEndpoints deploys endpoints associated with the node. +// The deployment of endpoints is done by deploying a link with the endpoint triggering it. +func (d *DefaultNode) DeployEndpoints(ctx context.Context) error { + for _, ep := range d.Endpoints { + err := ep.Deploy(ctx) if err != nil { return err } } - return nil } diff --git a/nodes/node.go b/nodes/node.go index f6f47b8ab..825efe5dc 100644 --- a/nodes/node.go +++ b/nodes/node.go @@ -96,20 +96,17 @@ type Node interface { // Adds the given link to the Node (container). After adding the Link to the node, // the given function f is called within the Nodes namespace to setup the link. AddLinkToContainer(ctx context.Context, link netlink.Link, f func(ns.NetNS) error) error - AddLink(l links.Link) AddEndpoint(e links.Endpoint) GetEndpoints() []links.Endpoint GetLinkEndpointType() links.LinkEndpointType GetShortName() string - // DeployLinks deploys the links for the node. - DeployLinks(ctx context.Context) error + // DeployEndpoints deploys the links for the node. + DeployEndpoints(ctx context.Context) error // ExecFunction executes the given function within the nodes network namespace ExecFunction(func(ns.NetNS) error) error GetState() state.NodeState SetState(state.NodeState) GetSSHConfig() *types.SSHConfig - // WaitForAllLinksCreated will block until all the nodes links are created - WaitForAllLinksCreated() // RunExecFromConfig executes the topologyfile defined exec commands RunExecFromConfig(context.Context, *exec.ExecCollection) error IsHealthy(ctx context.Context) (bool, error) diff --git a/tests/08-vxlan/02-vxlan-stitch.robot b/tests/08-vxlan/02-vxlan-stitch.robot index 9cce4097e..aface523b 100644 --- a/tests/08-vxlan/02-vxlan-stitch.robot +++ b/tests/08-vxlan/02-vxlan-stitch.robot @@ -29,7 +29,7 @@ Check VxLAN interface parameters on the host for srl1 node Should Contain ${output} mtu 9050 - Should Contain ${output} vxlan id 100 remote 172.20.25.22 dev clab-vxlan-br srcport 0 0 dstport 14788 + Should Contain ${output} vxlan id 100 remote 172.20.25.22 dev ${vxlan-br} srcport 0 0 dstport 14788 Should Not Contain ${output} nolearning