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

Add support in CNS NMAgent Client to fetch secondary IPs #3017

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
dec11b3
feat(CNS): Changes for fetching secondary IPs from NMAgent in CNS
santhoshmprabhu Sep 13, 2024
210577f
test(CNS): Add UT for case where no fetch happens
santhoshmprabhu Sep 13, 2024
0b6f2e0
style: Cleanup some comments, address some linting issues
santhoshmprabhu Sep 13, 2024
651848f
style: address some linting issues
santhoshmprabhu Sep 13, 2024
2221e69
style:Run gofumpt
santhoshmprabhu Sep 14, 2024
bf4ba09
Merge remote-tracking branch 'origin/master' into sanprabhu/cilium-no…
santhoshmprabhu Sep 16, 2024
1ec48f7
test: Update test
santhoshmprabhu Sep 16, 2024
1b4e0d3
refactor: Address comments to move business logic out of nmagent client
santhoshmprabhu Sep 16, 2024
3e01a7f
chore: Add missed files
santhoshmprabhu Sep 16, 2024
3b5a24b
style: Better naming, comments
santhoshmprabhu Sep 16, 2024
0d41328
style: Better naming
santhoshmprabhu Sep 16, 2024
d16f8c3
style: Better naming, comments
santhoshmprabhu Sep 16, 2024
83d9913
chore: undo accidental edit
santhoshmprabhu Sep 16, 2024
4d7d6fc
style: comments
santhoshmprabhu Sep 16, 2024
de70e6e
style: naming
santhoshmprabhu Sep 16, 2024
65181b7
style: linting issues
santhoshmprabhu Sep 16, 2024
821ec3d
feat: Address comments. Add MacAddress and IPAddress as types, refact…
santhoshmprabhu Sep 18, 2024
17db28d
style: linting issues
santhoshmprabhu Sep 18, 2024
651bdf3
style: linting issues
santhoshmprabhu Sep 18, 2024
200d46c
chore: remove accidental edits
santhoshmprabhu Sep 18, 2024
659679e
style: lower case in error messages
santhoshmprabhu Sep 18, 2024
6768a80
chore: add missed file
santhoshmprabhu Sep 18, 2024
33fcb4f
Merge remote-tracking branch 'origin/master' into sanprabhu/cilium-no…
santhoshmprabhu Sep 18, 2024
fd432e6
style: Rename MACAddress
santhoshmprabhu Sep 18, 2024
90c4ae3
style: Address comments
santhoshmprabhu Sep 18, 2024
e05b073
refactor: Address comments
santhoshmprabhu Sep 18, 2024
c53101e
chore: ip_fetcher changes
santhoshmprabhu Sep 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions cns/nodesubnet/helper_for_ip_fetcher_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package nodesubnet

import "time"

// This method is in this file (_test.go) because it is a test helper method.
// The following method is built during tests, and is not part of the main code.
func (c *IPFetcher) SetSecondaryIPQueryInterval(interval time.Duration) {
c.secondaryIPQueryInterval = interval
}
77 changes: 77 additions & 0 deletions cns/nodesubnet/ip_fetcher.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package nodesubnet

import (
"context"
"log"
"net/netip"
"time"

"github.com/Azure/azure-container-networking/nmagent"
"github.com/pkg/errors"
)

var ErrRefreshSkipped = errors.New("refresh skipped due to throttling")

// InterfaceRetriever is an interface is implemented by the NMAgent Client, and also a mock client for testing.
type InterfaceRetriever interface {
GetInterfaceIPInfo(ctx context.Context) (nmagent.Interfaces, error)
}

type IPFetcher struct {
// Node subnet state
secondaryIPQueryInterval time.Duration // Minimum time between secondary IP fetches
secondaryIPLastRefreshTime time.Time // Time of last secondary IP fetch

ipFectcherClient InterfaceRetriever
}

func NewIPFetcher(nmaClient InterfaceRetriever, queryInterval time.Duration) *IPFetcher {
return &IPFetcher{
ipFectcherClient: nmaClient,
secondaryIPQueryInterval: queryInterval,
}
}

func (c *IPFetcher) RefreshSecondaryIPsIfNeeded(ctx context.Context) (ips []netip.Addr, err error) {
// If secondaryIPQueryInterval has elapsed since the last fetch, fetch secondary IPs
if time.Since(c.secondaryIPLastRefreshTime) < c.secondaryIPQueryInterval {
return nil, ErrRefreshSkipped
}

c.secondaryIPLastRefreshTime = time.Now()
response, err := c.ipFectcherClient.GetInterfaceIPInfo(ctx)
if err != nil {
return nil, errors.Wrap(err, "getting interface IPs")
}

res := flattenIPListFromResponse(&response)
return res, nil
}

// Get the list of secondary IPs from fetched Interfaces
func flattenIPListFromResponse(resp *nmagent.Interfaces) (res []netip.Addr) {
// For each interface...
for _, intf := range resp.Entries {
if !intf.IsPrimary {
continue
}

// For each subnet on the interface...
for _, s := range intf.InterfaceSubnets {
addressCount := 0
// For each address in the subnet...
for _, a := range s.IPAddress {
// Primary addresses are reserved for the host.
if a.IsPrimary {
continue
}

res = append(res, netip.Addr(a.Address))
addressCount++
}
log.Printf("Got %d addresses from subnet %s", addressCount, s.Prefix)
}
}

return res
}
86 changes: 86 additions & 0 deletions cns/nodesubnet/ip_fetcher_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package nodesubnet_test

import (
"context"
"errors"
"testing"
"time"

"github.com/Azure/azure-container-networking/cns/nodesubnet"
"github.com/Azure/azure-container-networking/nmagent"
)

// Mock client that simply tracks if refresh has been called
type TestClient struct {
fetchCalled bool
}

// Mock refresh
func (c *TestClient) GetInterfaceIPInfo(_ context.Context) (nmagent.Interfaces, error) {
c.fetchCalled = true
return nmagent.Interfaces{}, nil
}

func TestRefreshSecondaryIPsIfNeeded(t *testing.T) {
getTests := []struct {
name string
shouldCall bool
interval time.Duration
}{
{
"fetch called",
true,
-1 * time.Second, // Negative timeout to force refresh
},
{
"no refresh needed",
false,
10 * time.Hour, // High timeout to avoid refresh
},
}

clientPtr := &TestClient{}
fetcher := nodesubnet.NewIPFetcher(clientPtr, 0)

for _, test := range getTests {
test := test
t.Run(test.name, func(t *testing.T) { // Do not parallelize, as we are using a shared client
fetcher.SetSecondaryIPQueryInterval(test.interval)
ctx, cancel := testContext(t)
defer cancel()
clientPtr.fetchCalled = false
_, err := fetcher.RefreshSecondaryIPsIfNeeded(ctx)

if test.shouldCall {
if err != nil && errors.Is(err, nodesubnet.ErrRefreshSkipped) {
t.Error("refresh expected, but didn't happen")
}

checkErr(t, err, false)
} else if err == nil || !errors.Is(err, nodesubnet.ErrRefreshSkipped) {
t.Error("refresh not expected, but happened")
}
})
}
}

// testContext creates a context from the provided testing.T that will be
// canceled if the test suite is terminated.
func testContext(t *testing.T) (context.Context, context.CancelFunc) {
if deadline, ok := t.Deadline(); ok {
return context.WithDeadline(context.Background(), deadline)
}
return context.WithCancel(context.Background())
}

// checkErr is an assertion of the presence or absence of an error
func checkErr(t *testing.T, err error, shouldErr bool) {
t.Helper()
if err != nil && !shouldErr {
t.Fatal("unexpected error: err:", err)
}

if err == nil && shouldErr {
t.Fatal("expected error but received none")
}
}
36 changes: 33 additions & 3 deletions nmagent/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,8 @@ type Client struct {
httpClient *http.Client

// config
host string
port uint16

host string
port uint16
enableTLS bool

retrier interface {
Expand Down Expand Up @@ -284,6 +283,37 @@ func (c *Client) GetHomeAz(ctx context.Context) (AzResponse, error) {
return homeAzResponse, nil
}

// GetInterfaceIPInfo fetches the node's interface IP information from nmagent
func (c *Client) GetInterfaceIPInfo(ctx context.Context) (Interfaces, error) {
santhoshmprabhu marked this conversation as resolved.
Show resolved Hide resolved
req, err := c.buildRequest(ctx, &GetSecondaryIPsRequest{})
var out Interfaces

if err != nil {
return out, errors.Wrap(err, "building request")
}

resp, err := c.httpClient.Do(req)
if err != nil {
return out, errors.Wrap(err, "submitting request")
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
return out, die(resp.StatusCode, resp.Header, resp.Body, req.URL.Path)
}

if resp.StatusCode != http.StatusOK {
return out, die(resp.StatusCode, resp.Header, resp.Body, req.URL.Path)
}

err = xml.NewDecoder(resp.Body).Decode(&out)
santhoshmprabhu marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return out, errors.Wrap(err, "decoding response")
}

return out, nil
}

func die(code int, headers http.Header, body io.ReadCloser, path string) error {
// nolint:errcheck // make a best effort to return whatever information we can
// returning an error here without the code and source would
Expand Down
85 changes: 85 additions & 0 deletions nmagent/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ package nmagent_test
import (
"context"
"encoding/json"
"encoding/xml"
"fmt"
"net/http"
"net/http/httptest"
"net/netip"
"strings"
"testing"

Expand Down Expand Up @@ -809,3 +811,86 @@ func TestGetHomeAz(t *testing.T) {
})
}
}

func TestGetInterfaceIPInfo(t *testing.T) {
tests := []struct {
name string
expURL string
response nmagent.Interfaces
respStr string
}{
{
"happy path",
"/machine/plugins?comp=nmagent&type=getinterfaceinfov1",
nmagent.Interfaces{
Entries: []nmagent.Interface{
{
MacAddress: nmagent.MACAddress{0x00, 0x0D, 0x3A, 0xF9, 0xDC, 0xA6},
IsPrimary: true,
InterfaceSubnets: []nmagent.InterfaceSubnet{
{
Prefix: "10.240.0.0/16",
IPAddress: []nmagent.NodeIP{
{
Address: nmagent.IPAddress(netip.AddrFrom4([4]byte{10, 240, 0, 5})),
IsPrimary: true,
},
{
Address: nmagent.IPAddress(netip.AddrFrom4([4]byte{10, 240, 0, 6})),
IsPrimary: false,
},
},
},
},
},
},
},
"<Interfaces><Interface MacAddress=\"000D3AF9DCA6\" IsPrimary=\"true\"><IPSubnet Prefix=\"10.240.0.0/16\">" +
"<IPAddress Address=\"10.240.0.5\" IsPrimary=\"true\"/><IPAddress Address=\"10.240.0.6\" IsPrimary=\"false\"/>" +
"</IPSubnet></Interface></Interfaces>",
},
}

for _, test := range tests {
test := test
t.Run(test.name, func(t *testing.T) {
t.Parallel()

var gotURL string
client := nmagent.NewTestClient(&TestTripper{
RoundTripF: func(req *http.Request) (*http.Response, error) {
gotURL = req.URL.RequestURI()
rr := httptest.NewRecorder()
rr.WriteHeader(http.StatusOK)
err := xml.NewEncoder(rr).Encode(test.response)
if err != nil {
t.Fatal("unexpected error encoding response: err:", err)
}
return rr.Result(), nil
},
})

ctx, cancel := testContext(t)
defer cancel()

resp, err := client.GetInterfaceIPInfo(ctx)
checkErr(t, err, false)

if gotURL != test.expURL {
t.Error("received URL differs from expected: got:", gotURL, "exp:", test.expURL)
}

if got := resp; !cmp.Equal(got, test.response) {
t.Error("response differs from expectation: diff:", cmp.Diff(got, test.response))
}

var unmarshaled nmagent.Interfaces
err = xml.Unmarshal([]byte(test.respStr), &unmarshaled)
checkErr(t, err, false)

if !cmp.Equal(resp, unmarshaled) {
t.Error("response differs from expected decoded string: diff:", cmp.Diff(resp, unmarshaled))
}
})
}
}
52 changes: 52 additions & 0 deletions nmagent/ipaddress.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package nmagent

import (
"encoding/xml"
"net/netip"

"github.com/pkg/errors"
)

type IPAddress netip.Addr

func (h IPAddress) Equal(other IPAddress) bool {
return netip.Addr(h).Compare(netip.Addr(other)) == 0
}

func (h *IPAddress) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
var ipStr string
if err := d.DecodeElement(&ipStr, &start); err != nil {
return errors.Wrap(err, "decoding IP address")
}

ip, err := netip.ParseAddr(ipStr)
if err != nil {
return errors.Wrap(err, "parsing IP address")
}

*h = IPAddress(ip)
return nil
}

func (h *IPAddress) UnmarshalXMLAttr(attr xml.Attr) error {
ipStr := attr.Value
ip, err := netip.ParseAddr(ipStr)
if err != nil {
return errors.Wrap(err, "parsing IP address")
}

*h = IPAddress(ip)
return nil
}

func (h IPAddress) MarshalXML(e *xml.Encoder, start xml.StartElement) error {
err := e.EncodeElement(netip.Addr(h).String(), start)
return errors.Wrap(err, "encoding IP address")
}

func (h IPAddress) MarshalXMLAttr(name xml.Name) (xml.Attr, error) {
return xml.Attr{
Name: name,
Value: netip.Addr(h).String(),
}, nil
}
Loading
Loading