From 3161bb52e0aa444fe8ff0c34466b918bb1ad8a25 Mon Sep 17 00:00:00 2001 From: toby lorne Date: Mon, 11 Jan 2021 20:53:45 +0000 Subject: [PATCH 1/3] dhcp: timeout value is set in DHCP daemon Eventually the timeout value will become a CLI argument The default timeout was nestled all the way in the lease constructor This commit is the first step in making the timeout configurable by moving it to the DHCPLease constructor Signed-off-by: toby lorne --- plugins/ipam/dhcp/daemon.go | 3 ++- plugins/ipam/dhcp/lease.go | 20 ++++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/plugins/ipam/dhcp/daemon.go b/plugins/ipam/dhcp/daemon.go index 7ec2818bd..a5c1e095d 100644 --- a/plugins/ipam/dhcp/daemon.go +++ b/plugins/ipam/dhcp/daemon.go @@ -26,6 +26,7 @@ import ( "path/filepath" "runtime" "sync" + "time" "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" @@ -63,7 +64,7 @@ func (d *DHCP) Allocate(args *skel.CmdArgs, result *current.Result) error { clientID := generateClientID(args.ContainerID, conf.Name, args.IfName) hostNetns := d.hostNetnsPrefix + args.Netns - l, err := AcquireLease(clientID, hostNetns, args.IfName) + l, err := AcquireLease(clientID, hostNetns, args.IfName, 5*time.Second) if err != nil { return err } diff --git a/plugins/ipam/dhcp/lease.go b/plugins/ipam/dhcp/lease.go index b014a0b65..16848fcc6 100644 --- a/plugins/ipam/dhcp/lease.go +++ b/plugins/ipam/dhcp/lease.go @@ -56,6 +56,7 @@ type DHCPLease struct { renewalTime time.Time rebindingTime time.Time expireTime time.Time + timeout time.Duration stopping uint32 stop chan struct{} wg sync.WaitGroup @@ -64,11 +65,15 @@ type DHCPLease struct { // AcquireLease gets an DHCP lease and then maintains it in the background // by periodically renewing it. The acquired lease can be released by // calling DHCPLease.Stop() -func AcquireLease(clientID, netns, ifName string) (*DHCPLease, error) { +func AcquireLease( + clientID, netns, ifName string, + timeout time.Duration, +) (*DHCPLease, error) { errCh := make(chan error, 1) l := &DHCPLease{ clientID: clientID, stop: make(chan struct{}), + timeout: timeout, } log.Printf("%v: acquiring lease", clientID) @@ -115,7 +120,7 @@ func (l *DHCPLease) Stop() { } func (l *DHCPLease) acquire() error { - c, err := newDHCPClient(l.link, l.clientID) + c, err := newDHCPClient(l.link, l.clientID, l.timeout) if err != nil { return err } @@ -242,7 +247,7 @@ func (l *DHCPLease) downIface() { } func (l *DHCPLease) renew() error { - c, err := newDHCPClient(l.link, l.clientID) + c, err := newDHCPClient(l.link, l.clientID, l.timeout) if err != nil { return err } @@ -273,7 +278,7 @@ func (l *DHCPLease) renew() error { func (l *DHCPLease) release() error { log.Printf("%v: releasing lease", l.clientID) - c, err := newDHCPClient(l.link, l.clientID) + c, err := newDHCPClient(l.link, l.clientID, l.timeout) if err != nil { return err } @@ -361,7 +366,10 @@ func backoffRetry(f func() (*dhcp4.Packet, error)) (*dhcp4.Packet, error) { return nil, errNoMoreTries } -func newDHCPClient(link netlink.Link, clientID string) (*dhcp4client.Client, error) { +func newDHCPClient( + link netlink.Link, clientID string, + timeout time.Duration, +) (*dhcp4client.Client, error) { pktsock, err := dhcp4client.NewPacketSock(link.Attrs().Index) if err != nil { return nil, err @@ -369,7 +377,7 @@ func newDHCPClient(link netlink.Link, clientID string) (*dhcp4client.Client, err return dhcp4client.New( dhcp4client.HardwareAddr(link.Attrs().HardwareAddr), - dhcp4client.Timeout(5*time.Second), + dhcp4client.Timeout(timeout), dhcp4client.Broadcast(false), dhcp4client.Connection(pktsock), ) From 25fc741e37c9ba5e30a0657a9593dd78e42dca79 Mon Sep 17 00:00:00 2001 From: toby lorne Date: Mon, 11 Jan 2021 21:27:42 +0000 Subject: [PATCH 2/3] dhcp: daemon dhcp client timeout is configurable Fixes #470 Signed-off-by: toby lorne --- plugins/ipam/dhcp/daemon.go | 15 ++++++++++----- plugins/ipam/dhcp/main.go | 5 ++++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/plugins/ipam/dhcp/daemon.go b/plugins/ipam/dhcp/daemon.go index a5c1e095d..021edc30e 100644 --- a/plugins/ipam/dhcp/daemon.go +++ b/plugins/ipam/dhcp/daemon.go @@ -42,11 +42,13 @@ type DHCP struct { mux sync.Mutex leases map[string]*DHCPLease hostNetnsPrefix string + clientTimeout time.Duration } -func newDHCP() *DHCP { +func newDHCP(clientTimeout time.Duration) *DHCP { return &DHCP{ - leases: make(map[string]*DHCPLease), + leases: make(map[string]*DHCPLease), + clientTimeout: clientTimeout, } } @@ -64,7 +66,7 @@ func (d *DHCP) Allocate(args *skel.CmdArgs, result *current.Result) error { clientID := generateClientID(args.ContainerID, conf.Name, args.IfName) hostNetns := d.hostNetnsPrefix + args.Netns - l, err := AcquireLease(clientID, hostNetns, args.IfName, 5*time.Second) + l, err := AcquireLease(clientID, hostNetns, args.IfName, d.clientTimeout) if err != nil { return err } @@ -157,7 +159,10 @@ func getListener(socketPath string) (net.Listener, error) { } } -func runDaemon(pidfilePath string, hostPrefix string, socketPath string) error { +func runDaemon( + pidfilePath, hostPrefix, socketPath string, + dhcpClientTimeout time.Duration, +) error { // since other goroutines (on separate threads) will change namespaces, // ensure the RPC server does not get scheduled onto those runtime.LockOSThread() @@ -177,7 +182,7 @@ func runDaemon(pidfilePath string, hostPrefix string, socketPath string) error { return fmt.Errorf("Error getting listener: %v", err) } - dhcp := newDHCP() + dhcp := newDHCP(dhcpClientTimeout) dhcp.hostNetnsPrefix = hostPrefix rpc.Register(dhcp) rpc.HandleHTTP() diff --git a/plugins/ipam/dhcp/main.go b/plugins/ipam/dhcp/main.go index 08b148ca4..514e4f5b9 100644 --- a/plugins/ipam/dhcp/main.go +++ b/plugins/ipam/dhcp/main.go @@ -22,6 +22,7 @@ import ( "net/rpc" "os" "path/filepath" + "time" "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" @@ -37,17 +38,19 @@ func main() { var pidfilePath string var hostPrefix string var socketPath string + var timeout time.Duration daemonFlags := flag.NewFlagSet("daemon", flag.ExitOnError) daemonFlags.StringVar(&pidfilePath, "pidfile", "", "optional path to write daemon PID to") daemonFlags.StringVar(&hostPrefix, "hostprefix", "", "optional prefix to host root") daemonFlags.StringVar(&socketPath, "socketpath", "", "optional dhcp server socketpath") + daemonFlags.DurationVar(&timeout, "timeout", 5*time.Second, "optional dhcp client timeout duration") daemonFlags.Parse(os.Args[2:]) if socketPath == "" { socketPath = defaultSocketPath } - if err := runDaemon(pidfilePath, hostPrefix, socketPath); err != nil { + if err := runDaemon(pidfilePath, hostPrefix, socketPath, timeout); err != nil { log.Printf(err.Error()) os.Exit(1) } From a8d1f5cd1b132f10e5d86746ab82de1cd3c2b616 Mon Sep 17 00:00:00 2001 From: toby lorne Date: Wed, 13 Jan 2021 19:24:41 +0000 Subject: [PATCH 3/3] dhcp: default dhcp clien timeout is 10s Consistent with https://github.com/d2g/dhcp4client/blob/ef524ad9cb076fe235ecc3c55913ef8445fa35fa/client.go#L39 Signed-off-by: toby lorne Co-authored-by: bruce ma --- plugins/ipam/dhcp/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/ipam/dhcp/main.go b/plugins/ipam/dhcp/main.go index 514e4f5b9..bbb2c6d4d 100644 --- a/plugins/ipam/dhcp/main.go +++ b/plugins/ipam/dhcp/main.go @@ -43,7 +43,7 @@ func main() { daemonFlags.StringVar(&pidfilePath, "pidfile", "", "optional path to write daemon PID to") daemonFlags.StringVar(&hostPrefix, "hostprefix", "", "optional prefix to host root") daemonFlags.StringVar(&socketPath, "socketpath", "", "optional dhcp server socketpath") - daemonFlags.DurationVar(&timeout, "timeout", 5*time.Second, "optional dhcp client timeout duration") + daemonFlags.DurationVar(&timeout, "timeout", 10*time.Second, "optional dhcp client timeout duration") daemonFlags.Parse(os.Args[2:]) if socketPath == "" {