Skip to content

Commit ac567c2

Browse files
ZetaoZhuangtimraymond
authored and
sivakami
committed
update for new home az contract from nma (#3151)
* update for new home az contract * fix lint * address comment * fix lint * address comment * Add "AppliedFixes" to HomeAzResponse The NMAgent team has added an API version field to the HomeAZ response as a means to indicate whether or not certain bugfixes have been applied. Since this API Version field conveys no meaningful information, this adds an "AppliedFixes" field to the response to make it meaningful to users. * Implement fmt.Stringer for HomeAZFix This is to ensure that these are prett-printed properly for clients that log them. * Use nmagent.(HomeAZResponse).ContainsFixes in CNS This alters the response to use the ContainsFixes method added to the HomeAzResponse. Rather than using API Version directly, the more semantically-meaningful constants from the nmagent package are used instead to indicate whether or not some bugfixes have been applied. * add nma client test * fix lint * fix UTs --------- Co-authored-by: Timothy J. Raymond <[email protected]>
1 parent 83392e9 commit ac567c2

File tree

7 files changed

+257
-15
lines changed

7 files changed

+257
-15
lines changed

‎cns/api.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,8 +354,9 @@ type NmAgentSupportedApisResponse struct {
354354
}
355355

356356
type HomeAzResponse struct {
357-
IsSupported bool `json:"isSupported"`
358-
HomeAz uint `json:"homeAz"`
357+
IsSupported bool `json:"isSupported"`
358+
HomeAz uint `json:"homeAz"`
359+
NmaAppliedTheIPV6Fix bool `json:"NmaAppliedTheIPV6Fix"`
359360
}
360361

361362
type GetHomeAzResponse struct {

‎cns/restserver/homeazmonitor.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ func (h *HomeAzMonitor) Populate(ctx context.Context) {
154154
h.update(returnCode, returnMessage, cns.HomeAzResponse{IsSupported: true})
155155
return
156156
}
157-
h.update(types.Success, "Get Home Az succeeded", cns.HomeAzResponse{IsSupported: true, HomeAz: azResponse.HomeAz})
157+
h.update(types.Success, "Get Home Az succeeded", cns.HomeAzResponse{IsSupported: true, HomeAz: azResponse.HomeAz, NmaAppliedTheIPV6Fix: azResponse.ContainsFixes(nmagent.HomeAZFixIPv6)})
158158
}
159159

160160
// update constructs a GetHomeAzResponse entity and update its cache

‎cns/restserver/homeazmonitor_test.go‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,23 +24,23 @@ func TestHomeAzMonitor(t *testing.T) {
2424
{
2525
"happy path",
2626
&fakes.NMAgentClientFake{
27-
SupportedAPIsF: func(ctx context.Context) ([]string, error) {
27+
SupportedAPIsF: func(_ context.Context) ([]string, error) {
2828
return []string{"GetHomeAz"}, nil
2929
},
30-
GetHomeAzF: func(ctx context.Context) (nmagent.AzResponse, error) {
31-
return nmagent.AzResponse{HomeAz: uint(1)}, nil
30+
GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) {
31+
return nmagent.AzResponse{HomeAz: uint(1), AppliedFixes: []nmagent.HomeAZFix{nmagent.HomeAZFixIPv6}}, nil
3232
},
3333
},
34-
cns.HomeAzResponse{IsSupported: true, HomeAz: uint(1)},
34+
cns.HomeAzResponse{IsSupported: true, HomeAz: uint(1), NmaAppliedTheIPV6Fix: true},
3535
false,
3636
},
3737
{
3838
"getHomeAz is not supported in nmagent",
3939
&fakes.NMAgentClientFake{
40-
SupportedAPIsF: func(ctx context.Context) ([]string, error) {
40+
SupportedAPIsF: func(_ context.Context) ([]string, error) {
4141
return []string{"dummy"}, nil
4242
},
43-
GetHomeAzF: func(ctx context.Context) (nmagent.AzResponse, error) {
43+
GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) {
4444
return nmagent.AzResponse{}, nil
4545
},
4646
},
@@ -50,23 +50,36 @@ func TestHomeAzMonitor(t *testing.T) {
5050
{
5151
"api supported but home az value is not valid",
5252
&fakes.NMAgentClientFake{
53-
SupportedAPIsF: func(ctx context.Context) ([]string, error) {
53+
SupportedAPIsF: func(_ context.Context) ([]string, error) {
5454
return []string{GetHomeAzAPIName}, nil
5555
},
56-
GetHomeAzF: func(ctx context.Context) (nmagent.AzResponse, error) {
56+
GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) {
5757
return nmagent.AzResponse{HomeAz: 0}, nil
5858
},
5959
},
6060
cns.HomeAzResponse{IsSupported: true},
6161
true,
6262
},
63+
{
64+
"api supported but apiVersion value is not valid",
65+
&fakes.NMAgentClientFake{
66+
SupportedAPIsF: func(_ context.Context) ([]string, error) {
67+
return []string{GetHomeAzAPIName}, nil
68+
},
69+
GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) {
70+
return nmagent.AzResponse{HomeAz: uint(1), AppliedFixes: []nmagent.HomeAZFix{nmagent.HomeAZFixInvalid}}, nil
71+
},
72+
},
73+
cns.HomeAzResponse{IsSupported: true, HomeAz: uint(1), NmaAppliedTheIPV6Fix: false},
74+
false,
75+
},
6376
{
6477
"api supported but got unexpected errors",
6578
&fakes.NMAgentClientFake{
66-
SupportedAPIsF: func(ctx context.Context) ([]string, error) {
79+
SupportedAPIsF: func(_ context.Context) ([]string, error) {
6780
return []string{GetHomeAzAPIName}, nil
6881
},
69-
GetHomeAzF: func(ctx context.Context) (nmagent.AzResponse, error) {
82+
GetHomeAzF: func(_ context.Context) (nmagent.AzResponse, error) {
7083
return nmagent.AzResponse{}, errors.New("unexpected error")
7184
},
7285
},

‎nmagent/client_test.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -751,14 +751,25 @@ func TestGetHomeAz(t *testing.T) {
751751
}{
752752
{
753753
"happy path",
754-
nmagent.AzResponse{HomeAz: uint(1)},
754+
nmagent.AzResponse{HomeAz: uint(1), AppliedFixes: nil},
755755
"/machine/plugins?comp=nmagent&type=GetHomeAz%2Fapi-version%2F1",
756756
map[string]interface{}{
757757
"httpStatusCode": "200",
758758
"HomeAz": 1,
759759
},
760760
false,
761761
},
762+
{
763+
"happy path with new version",
764+
nmagent.AzResponse{HomeAz: uint(1), AppliedFixes: []nmagent.HomeAZFix{nmagent.HomeAZFixIPv6}},
765+
"/machine/plugins?comp=nmagent&type=GetHomeAz%2Fapi-version%2F1",
766+
map[string]interface{}{
767+
"httpStatusCode": "200",
768+
"HomeAz": 1,
769+
"APIVersion": 2,
770+
},
771+
false,
772+
},
762773
{
763774
"empty response",
764775
nmagent.AzResponse{},

‎nmagent/error.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,14 @@ import (
1111
pkgerrors "github.com/pkg/errors"
1212
)
1313

14+
type HomeAzAPIVersionError struct {
15+
ReceivedAPIVersion uint
16+
}
17+
18+
func (h HomeAzAPIVersionError) Error() string {
19+
return fmt.Sprintf("invalid homeaz api version (must be 0 or 2): received %d", h.ReceivedAPIVersion)
20+
}
21+
1422
var deleteNetworkPattern = regexp.MustCompile(`/NetworkManagement/joinedVirtualNetworks/[^/]+/api-version/\d+/method/DELETE`)
1523

1624
// ContentError is encountered when an unexpected content type is obtained from

‎nmagent/responses.go‎

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
package nmagent
22

3+
import (
4+
"encoding/json"
5+
6+
"github.com/pkg/errors"
7+
)
8+
39
type VirtualNetwork struct {
410
CNetSpace string `json:"cnetSpace"`
511
DefaultGateway string `json:"defaultGateway"`
@@ -37,8 +43,74 @@ type NCVersionList struct {
3743
Containers []NCVersion `json:"networkContainers"`
3844
}
3945

46+
// HomeAZFix is an indication that a particular bugfix has been applied to some
47+
// HomeAZ.
48+
type HomeAZFix int
49+
50+
func (h HomeAZFix) String() string {
51+
switch h {
52+
case HomeAZFixInvalid:
53+
return "HomeAZFixInvalid"
54+
case HomeAZFixIPv6:
55+
return "HomeAZFixIPv6"
56+
default:
57+
return "Unknown HomeAZ Fix"
58+
}
59+
}
60+
61+
const (
62+
HomeAZFixInvalid HomeAZFix = iota
63+
HomeAZFixIPv6
64+
)
65+
4066
type AzResponse struct {
41-
HomeAz uint `json:"homeAz"`
67+
HomeAz uint
68+
AppliedFixes []HomeAZFix
69+
}
70+
71+
func (az *AzResponse) UnmarshalJSON(in []byte) error {
72+
type resp struct {
73+
HomeAz uint `json:"homeAz"`
74+
APIVersion uint `json:"apiVersion"`
75+
}
76+
77+
var rsp resp
78+
err := json.Unmarshal(in, &rsp)
79+
if err != nil {
80+
return errors.Wrap(err, "unmarshaling raw home az response")
81+
}
82+
83+
if rsp.APIVersion != 0 && rsp.APIVersion != 2 {
84+
return HomeAzAPIVersionError{
85+
ReceivedAPIVersion: rsp.APIVersion,
86+
}
87+
}
88+
89+
az.HomeAz = rsp.HomeAz
90+
91+
if rsp.APIVersion == 2 { // nolint:gomnd // ignore magic number 2
92+
az.AppliedFixes = append(az.AppliedFixes, HomeAZFixIPv6)
93+
}
94+
95+
return nil
96+
}
97+
98+
// ContainsFixes reports whether all fixes requested are present in the
99+
// AzResponse returned.
100+
func (az AzResponse) ContainsFixes(requestedFixes ...HomeAZFix) bool {
101+
for _, requested := range requestedFixes {
102+
found := false
103+
for _, present := range az.AppliedFixes {
104+
if requested == present {
105+
found = true
106+
}
107+
}
108+
109+
if !found {
110+
return false
111+
}
112+
}
113+
return true
42114
}
43115

44116
type NodeIP struct {

‎nmagent/responses_test.go‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
package nmagent_test
2+
3+
import (
4+
"encoding/json"
5+
"testing"
6+
7+
"github.com/Azure/azure-container-networking/nmagent"
8+
"github.com/google/go-cmp/cmp"
9+
)
10+
11+
func TestContainsFixes(t *testing.T) {
12+
tests := []struct {
13+
name string
14+
resp nmagent.AzResponse
15+
fixes []nmagent.HomeAZFix
16+
exp bool
17+
}{
18+
{
19+
"empty",
20+
nmagent.AzResponse{},
21+
[]nmagent.HomeAZFix{},
22+
true,
23+
},
24+
{
25+
"one present",
26+
nmagent.AzResponse{
27+
AppliedFixes: []nmagent.HomeAZFix{
28+
nmagent.HomeAZFixIPv6,
29+
},
30+
},
31+
[]nmagent.HomeAZFix{nmagent.HomeAZFixIPv6},
32+
true,
33+
},
34+
{
35+
"one absent",
36+
nmagent.AzResponse{
37+
AppliedFixes: []nmagent.HomeAZFix{},
38+
},
39+
[]nmagent.HomeAZFix{nmagent.HomeAZFixIPv6},
40+
false,
41+
},
42+
{
43+
"one with empty request",
44+
nmagent.AzResponse{
45+
AppliedFixes: []nmagent.HomeAZFix{
46+
nmagent.HomeAZFixIPv6,
47+
},
48+
},
49+
[]nmagent.HomeAZFix{},
50+
true,
51+
},
52+
}
53+
54+
for _, test := range tests {
55+
test := test
56+
t.Run(test.name, func(t *testing.T) {
57+
t.Parallel()
58+
got := test.resp.ContainsFixes(test.fixes...)
59+
60+
exp := test.exp
61+
if got != exp {
62+
t.Error("unexpected response from ContainsFixes: exp:", exp, "got:", got)
63+
}
64+
})
65+
}
66+
}
67+
68+
func TestUnmarshalAzResponse(t *testing.T) {
69+
tests := []struct {
70+
name string
71+
in string
72+
exp nmagent.AzResponse
73+
shouldErr bool
74+
}{
75+
{
76+
"empty",
77+
"{}",
78+
nmagent.AzResponse{},
79+
false,
80+
},
81+
{
82+
"only homeaz",
83+
`{"homeAz": 42}`,
84+
nmagent.AzResponse{
85+
HomeAz: 42,
86+
},
87+
false,
88+
},
89+
{
90+
"valid apiversion",
91+
`{"homeAz": 42, "apiVersion": 0}`,
92+
nmagent.AzResponse{
93+
HomeAz: 42,
94+
},
95+
false,
96+
},
97+
{
98+
"valid apiversion ipv6",
99+
`{"homeAz": 42, "apiVersion": 2}`,
100+
nmagent.AzResponse{
101+
HomeAz: 42,
102+
AppliedFixes: []nmagent.HomeAZFix{
103+
nmagent.HomeAZFixIPv6,
104+
},
105+
},
106+
false,
107+
},
108+
{
109+
"invalid apiversion",
110+
`{"homeAz": 42, "apiVersion": 42}`,
111+
nmagent.AzResponse{},
112+
true,
113+
},
114+
}
115+
116+
for _, test := range tests {
117+
test := test
118+
t.Run(test.name, func(t *testing.T) {
119+
t.Parallel()
120+
121+
var got nmagent.AzResponse
122+
err := json.Unmarshal([]byte(test.in), &got)
123+
if err != nil && !test.shouldErr {
124+
t.Fatal("unexpected error unmarshaling JSON: err:", err)
125+
}
126+
127+
if err == nil && test.shouldErr {
128+
t.Fatal("expected error but received none")
129+
}
130+
131+
exp := test.exp
132+
if !cmp.Equal(got, exp) {
133+
t.Error("received response differs from expected: diff:", cmp.Diff(got, exp))
134+
}
135+
})
136+
}
137+
}

0 commit comments

Comments
 (0)