Merge pull request #478 from twelho/renew-return-new-lease

dhcpv4: return a new lease from Renew()

3 files changed, 13 insertions, 11 deletions

diff --git a/dhcpv4/nclient4/lease.go b/dhcpv4/nclient4/lease.go
index 1895dd0..9ee3109 100644
--- a/dhcpv4/nclient4/lease.go
+++ b/dhcpv4/nclient4/lease.go
@@ -42,15 +42,15 @@ func (c *Client) Release(lease *Lease, modifiers ...dhcpv4.Modifier) error {
 // sourced from the initial offer in the lease, and the ACK of the lease is updated to the ACK of
 // the latest renewal. This avoids issues with DHCP servers that omit information needed to build a
 // completely new lease from their renewal ACK (such as the Windows DHCP Server).
-func (c *Client) Renew(ctx context.Context, lease *Lease, modifiers ...dhcpv4.Modifier) error {
+func (c *Client) Renew(ctx context.Context, lease *Lease, modifiers ...dhcpv4.Modifier) (*Lease, error) {
 	if lease == nil {
-		return fmt.Errorf("lease is nil")
+		return nil, fmt.Errorf("lease is nil")
 	}
 
 	request, err := dhcpv4.NewRenewFromOffer(lease.Offer, dhcpv4.PrependModifiers(modifiers,
 		dhcpv4.WithOption(dhcpv4.OptMaxMessageSize(MaxMessageSize)))...)
 	if err != nil {
-		return fmt.Errorf("unable to create a request: %w", err)
+		return nil, fmt.Errorf("unable to create a request: %w", err)
 	}
 
 	// Servers are supposed to only respond to Requests containing their server identifier,
@@ -61,17 +61,19 @@ func (c *Client) Renew(ctx context.Context, lease *Lease, modifiers ...dhcpv4.Mo
 		IsCorrectServer(lease.Offer.ServerIdentifier()),
 		IsMessageType(dhcpv4.MessageTypeAck, dhcpv4.MessageTypeNak)))
 	if err != nil {
-		return fmt.Errorf("got an error while processing the request: %w", err)
+		return nil, fmt.Errorf("got an error while processing the request: %w", err)
 	}
 	if response.MessageType() == dhcpv4.MessageTypeNak {
-		return &ErrNak{
+		return nil, &ErrNak{
 			Offer: lease.Offer,
 			Nak:   response,
 		}
 	}
 
-	// Update the ACK of the lease with the ACK of the latest renewal
-	lease.ACK = response
-
-	return nil
+	// Return a new lease with the latest ACK and updated creation time
+	return &Lease{
+		Offer:        lease.Offer,
+		ACK:          response,
+		CreationTime: time.Now(),
+	}, nil
 }
diff --git a/dhcpv4/nclient4/lease_test.go b/dhcpv4/nclient4/lease_test.go
index d27eeca..d9377e7 100644
--- a/dhcpv4/nclient4/lease_test.go
+++ b/dhcpv4/nclient4/lease_test.go
@@ -238,7 +238,7 @@ func (sll *testServerLeaseList) runTest(t *testing.T) {
 		sll.lastTestSvrErrLock.RUnlock()
 
 		if keepgoing {
-			err = clnt.Renew(context.Background(), lease)
+			lease, err = clnt.Renew(context.Background(), lease)
 			sll.lastTestSvrErrLock.RLock()
 			keepgoing = chkerr(err, sll.lastTestSvrErr, l.ShouldFail, t)
 			sll.lastTestSvrErrLock.RUnlock()
diff --git a/dhcpv4/server4/server_test.go b/dhcpv4/server4/server_test.go
index 7be868c..b0336db 100644
--- a/dhcpv4/server4/server_test.go
+++ b/dhcpv4/server4/server_test.go
@@ -116,7 +116,7 @@ func TestServer(t *testing.T) {
 		require.Equal(t, ifaces[0].HardwareAddr, p.ClientHWAddr)
 	}
 
-	err = c.Renew(context.Background(), lease, modifiers...)
+	lease, err = c.Renew(context.Background(), lease, modifiers...)
 	require.NoError(t, err)
 	require.NotNil(t, lease.Offer, lease.ACK)
 	for _, p := range []*dhcpv4.DHCPv4{lease.Offer, lease.ACK} {