Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

veth creation: make first node create the veth endpoints #1914

Merged
merged 16 commits into from
Feb 29, 2024
23 changes: 7 additions & 16 deletions clab/clab.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
25 changes: 11 additions & 14 deletions cmd/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
6 changes: 3 additions & 3 deletions cmd/tools_veth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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!")
Expand Down
6 changes: 3 additions & 3 deletions cmd/vxlan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
}
Expand Down
9 changes: 9 additions & 0 deletions links/endpoint.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package links

import (
"context"
"fmt"
"net"

Expand Down Expand Up @@ -31,6 +32,14 @@ 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
// AutoDeployWithAEnd indicates if this endpoint is to be deployed
// along with the peer Endpoint. This should be true for e.g. host endpoints
// because there is no host node that would bring the interface and rename it.
AutoDeployWithAEnd() bool
}

// EndpointGeneric is the generic endpoint struct that is used by all endpoint types.
Expand Down
17 changes: 15 additions & 2 deletions links/endpoint_bridge.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package links

import (
"context"
"errors"
"fmt"

Expand All @@ -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,
}
}

Expand All @@ -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) AutoDeployWithAEnd() bool {
// the mgmt bridge needs AutoDeployment. Otherwise the regular bridge node
// should trigger BEnd deployment seperately
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 {
Expand Down
13 changes: 12 additions & 1 deletion links/endpoint_host.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package links

import "errors"
import (
"context"
"errors"
)

type EndpointHost struct {
EndpointGeneric
Expand All @@ -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)
Expand All @@ -27,3 +34,7 @@ func (e *EndpointHost) Verify(_ *VerifyLinkParams) error {
}
return nil
}

func (e *EndpointHost) AutoDeployWithAEnd() bool {
return true
}
10 changes: 10 additions & 0 deletions links/endpoint_macvlan.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package links

import "context"

type EndpointMacVlan struct {
EndpointGeneric
}
Expand All @@ -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) AutoDeployWithAEnd() bool {
return false
}
2 changes: 1 addition & 1 deletion links/endpoint_raw.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 10 additions & 0 deletions links/endpoint_veth.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package links

import "context"

type EndpointVeth struct {
EndpointGeneric
}
Expand All @@ -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)
}

hellt marked this conversation as resolved.
Show resolved Hide resolved
func (e *EndpointVeth) AutoDeployWithAEnd() bool {
return false
}
9 changes: 9 additions & 0 deletions links/endpoint_vxlan.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package links

import (
"context"
"fmt"
"net"
)
Expand Down Expand Up @@ -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) AutoDeployWithAEnd() bool {
return false
}
9 changes: 2 additions & 7 deletions links/generic_link_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (

type GenericLinkNode struct {
shortname string
links []Link
endpoints []Endpoint
nspath string
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
}
Expand Down
5 changes: 2 additions & 3 deletions links/link.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,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.
Expand Down Expand Up @@ -353,7 +353,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
Expand Down
2 changes: 0 additions & 2 deletions links/link_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 1 addition & 4 deletions links/link_macvlan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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, ep Endpoint) error {
// lookup the parent host interface
parentInterface, err := utils.LinkByNameOrAlias(l.HostEndpoint.GetIfaceName())
if err != nil {
Expand Down
6 changes: 1 addition & 5 deletions links/link_mgmt-net.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
Loading
Loading