Skip to content

Commit ada4eac

Browse files
dlipovetskydlipovetsky
authored
feat: Support context for all Nutanix client calls (#1234)
**What problem does this PR solve?**: This allows us to handle context cancellation when using the Nutanix v4 go client, which itself does not support context. That, together with #1235, allows us to return check results before the API server webhook timeout. This means the client sees results, instead of an uninformative (and misleading) "failed to call webhook" error. ## Before Notice that no results are returned. ``` │ kubectl apply --dry-run=server -f invalid-bad-prismcentral-url.yaml -v=4 I0723 12:12:17.523594 2625398 envvar.go:172] "Feature gate default state" feature="ClientsPreferCBOR" enabled=false I0723 12:12:17.523624 2625398 envvar.go:172] "Feature gate default state" feature="InformerResourceVersion" enabled=false I0723 12:12:17.523627 2625398 envvar.go:172] "Feature gate default state" feature="InOrderInformers" enabled=true I0723 12:12:17.523630 2625398 envvar.go:172] "Feature gate default state" feature="WatchListClient" enabled=false I0723 12:12:17.523633 2625398 envvar.go:172] "Feature gate default state" feature="ClientsAllowCBOR" enabled=false I0723 12:12:27.582271 2625398 helpers.go:246] server response object: %s[{ "kind": "Status", "apiVersion": "v1", "metadata": {}, "status": "Failure", "message": "error when creating \"invalid-bad-prismcentral-url.yaml\": Internal error occurred: failed calling webhook \"preflight.cluster.caren.nutanix.com\": failed to call webhook: Post \"https://cluster-api-runtime-extensions-nutanix-admission.default.svc:443/preflight-v1beta1-cluster?timeout=10s\": context deadline exceeded", "reason": "InternalError", "details": { "causes": [ { "message": "failed calling webhook \"preflight.cluster.caren.nutanix.com\": failed to call webhook: Post \"https://cluster-api-runtime-extensions-nutanix-admission.default.svc:443/preflight-v1beta1-cluster?timeout=10s\": context deadline exceeded" } ] }, "code": 500 }] Error from server (InternalError): error when creating "invalid-bad-prismcentral-url.yaml": Internal error occurred: failed calling webhook "preflight.cluster.caren.nutanix.com": failed to call webhook: Post "https://cluster-api-runtime-extensions-nutanix-admission.default.svc:443/preflight-v1beta1-cluster?timeout=10s": context deadline exceeded ``` ## After Notice that the results are returned. ``` │ kubectl apply --dry-run=server -f invalid-bad-prismcentral-url.yaml -v=4 I0723 12:06:59.458667 2603987 envvar.go:172] "Feature gate default state" feature="ClientsAllowCBOR" enabled=false I0723 12:06:59.458703 2603987 envvar.go:172] "Feature gate default state" feature="ClientsPreferCBOR" enabled=false I0723 12:06:59.458706 2603987 envvar.go:172] "Feature gate default state" feature="InformerResourceVersion" enabled=false I0723 12:06:59.458709 2603987 envvar.go:172] "Feature gate default state" feature="InOrderInformers" enabled=true I0723 12:06:59.458711 2603987 envvar.go:172] "Feature gate default state" feature="WatchListClient" enabled=false I0723 12:07:07.583019 2603987 helpers.go:246] server response object: %s[{ "kind": "Status", "apiVersion": "v1", "metadata": {}, "status": "Failure", "message": "error when creating \"invalid-bad-prismcentral-url.yaml\": admission webhook \"preflight.cluster.caren.nutanix.com\" denied the request: preflight checks failed due to an internal error", "reason": "InternalError", "details": { "causes": [ { "reason": "NutanixCredentials", "message": "Failed to validate credentials: Get \"https://example.com:9441/api/nutanix/v3/users/me\": context deadline exceeded. This is usually a temporary error. Please retry.", "field": "$.spec.topology.variables[?@.name==\"clusterConfig\"].value.nutanix.prismCentralEndpoint" } ] }, "code": 500 }] Error from server (InternalError): error when creating "invalid-bad-prismcentral-url.yaml": admission webhook "preflight.cluster.caren.nutanix.com" denied the request: preflight checks failed due to an internal error ``` **Which issue(s) this PR fixes**: Fixes # **How Has This Been Tested?**: <!-- Please describe the tests that you ran to verify your changes. Provide output from the tests and any manual steps needed to replicate the tests. --> **Special notes for your reviewer**: <!-- Use this to provide any additional information to the reviewers. This may include: - Best way to review the PR. - Where the author wants the most review attention on. - etc. -->
1 parent 55455f5 commit ada4eac

11 files changed

+647
-379
lines changed

‎pkg/webhook/preflight/nutanix/clients.go

Lines changed: 274 additions & 86 deletions
Large diffs are not rendered by default.

‎pkg/webhook/preflight/nutanix/clients_test.go

Lines changed: 88 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -5,74 +5,13 @@ package nutanix
55

66
import (
77
"context"
8+
"errors"
9+
"testing"
10+
"time"
811

9-
clustermgmtv4 "github.com/nutanix/ntnx-api-golang-clients/clustermgmt-go-client/v4/models/clustermgmt/v4/config"
10-
netv4 "github.com/nutanix/ntnx-api-golang-clients/networking-go-client/v4/models/networking/v4/config"
11-
vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content"
1212
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
13-
14-
prismv3 "github.com/nutanix-cloud-native/prism-go-client/v3"
1513
)
1614

17-
var _ = client(&mocknclient{})
18-
19-
// mocknclient is a mock implementation of the client interface for testing purposes.
20-
type mocknclient struct {
21-
user *prismv3.UserIntentResponse
22-
err error
23-
24-
getImageByIdFunc func(
25-
uuid *string,
26-
) (
27-
*vmmv4.GetImageApiResponse, error,
28-
)
29-
30-
listImagesFunc func(
31-
page,
32-
limit *int,
33-
filter,
34-
orderby,
35-
select_ *string,
36-
args ...map[string]interface{},
37-
) (
38-
*vmmv4.ListImagesApiResponse,
39-
error,
40-
)
41-
42-
getClusterByIdFunc func(id *string) (*clustermgmtv4.GetClusterApiResponse, error)
43-
44-
listClustersFunc func(
45-
page,
46-
limit *int,
47-
filter,
48-
orderby,
49-
apply,
50-
select_ *string,
51-
args ...map[string]interface{},
52-
) (*clustermgmtv4.ListClustersApiResponse, error)
53-
54-
listStorageContainersFunc func(
55-
page,
56-
limit *int,
57-
filter,
58-
orderby,
59-
select_ *string,
60-
args ...map[string]interface{},
61-
) (*clustermgmtv4.ListStorageContainersApiResponse, error)
62-
63-
GetSubnetByIdFunc func(id *string) (*netv4.GetSubnetApiResponse, error)
64-
65-
ListSubnetsFunc func(
66-
page_ *int,
67-
limit_ *int,
68-
filter_ *string,
69-
orderby_ *string,
70-
expand_ *string,
71-
select_ *string,
72-
args ...map[string]interface{},
73-
) (*netv4.ListSubnetsApiResponse, error)
74-
}
75-
7615
type mockKubeClient struct {
7716
ctrlclient.Client
7817
SubResourceClient ctrlclient.SubResourceClient
@@ -97,54 +36,89 @@ func (m *mockKubeClient) SubResource(subResource string) ctrlclient.SubResourceC
9736
return m.SubResourceClient
9837
}
9938

100-
func (m *mocknclient) GetCurrentLoggedInUser(ctx context.Context) (*prismv3.UserIntentResponse, error) {
101-
return m.user, m.err
102-
}
103-
104-
func (m *mocknclient) GetImageById(uuid *string) (*vmmv4.GetImageApiResponse, error) {
105-
return m.getImageByIdFunc(uuid)
106-
}
107-
108-
func (m *mocknclient) ListImages(
109-
page, limit *int,
110-
filter, orderby, select_ *string,
111-
args ...map[string]interface{},
112-
) (*vmmv4.ListImagesApiResponse, error) {
113-
return m.listImagesFunc(page, limit, filter, orderby, select_)
114-
}
115-
116-
func (m *mocknclient) GetClusterById(id *string) (*clustermgmtv4.GetClusterApiResponse, error) {
117-
return m.getClusterByIdFunc(id)
118-
}
119-
120-
func (m *mocknclient) ListClusters(
121-
page, limit *int,
122-
filter, orderby, apply, select_ *string,
123-
args ...map[string]interface{},
124-
) (*clustermgmtv4.ListClustersApiResponse, error) {
125-
return m.listClustersFunc(page, limit, filter, orderby, apply, select_, args...)
126-
}
127-
128-
func (m *mocknclient) ListStorageContainers(
129-
page, limit *int,
130-
filter, orderby, select_ *string,
131-
args ...map[string]interface{},
132-
) (*clustermgmtv4.ListStorageContainersApiResponse, error) {
133-
return m.listStorageContainersFunc(page, limit, filter, orderby, select_, args...)
134-
}
135-
136-
func (m *mocknclient) GetSubnetById(id *string) (*netv4.GetSubnetApiResponse, error) {
137-
return m.GetSubnetByIdFunc(id)
138-
}
139-
140-
func (m *mocknclient) ListSubnets(
141-
page_ *int,
142-
limit_ *int,
143-
filter_ *string,
144-
orderby_ *string,
145-
expand_ *string,
146-
select_ *string,
147-
args ...map[string]interface{},
148-
) (*netv4.ListSubnetsApiResponse, error) {
149-
return m.ListSubnetsFunc(page_, limit_, filter_, orderby_, expand_, select_, args...)
39+
func TestCallWithContext(t *testing.T) {
40+
t.Parallel()
41+
testSuccessValue := "success"
42+
testError := errors.New("test error")
43+
44+
tests := []struct {
45+
name string
46+
ctx func() (context.Context, context.CancelFunc)
47+
f func() (string, error)
48+
wantVal string
49+
wantErr error
50+
cancelAfter time.Duration
51+
}{
52+
{
53+
name: "should return value on success",
54+
ctx: func() (context.Context, context.CancelFunc) {
55+
return context.Background(), func() {}
56+
},
57+
f: func() (string, error) {
58+
return testSuccessValue, nil
59+
},
60+
wantVal: testSuccessValue,
61+
wantErr: nil,
62+
},
63+
{
64+
name: "should return error when function fails",
65+
ctx: func() (context.Context, context.CancelFunc) {
66+
return context.Background(), func() {}
67+
},
68+
f: func() (string, error) {
69+
return "", testError
70+
},
71+
wantErr: testError,
72+
},
73+
{
74+
name: "should return context error when context is cancelled during execution",
75+
ctx: func() (context.Context, context.CancelFunc) {
76+
return context.WithCancel(context.Background())
77+
},
78+
f: func() (string, error) {
79+
time.Sleep(100 * time.Millisecond)
80+
return testSuccessValue, nil
81+
},
82+
wantErr: context.Canceled,
83+
cancelAfter: 10 * time.Millisecond,
84+
},
85+
{
86+
name: "should return context error when context is already cancelled",
87+
ctx: func() (context.Context, context.CancelFunc) {
88+
ctx, cancel := context.WithCancel(context.Background())
89+
cancel()
90+
return ctx, func() {}
91+
},
92+
f: func() (string, error) {
93+
t.Log("this function should not have its result returned")
94+
return testSuccessValue, nil
95+
},
96+
wantErr: context.Canceled,
97+
},
98+
}
99+
100+
for _, tt := range tests {
101+
t.Run(tt.name, func(t *testing.T) {
102+
t.Parallel()
103+
ctx, cancel := tt.ctx()
104+
defer cancel()
105+
106+
if tt.cancelAfter > 0 {
107+
go func() {
108+
time.Sleep(tt.cancelAfter)
109+
cancel()
110+
}()
111+
}
112+
113+
gotVal, gotErr := callWithContext(ctx, tt.f)
114+
115+
if !errors.Is(gotErr, tt.wantErr) {
116+
t.Errorf("callWithContext() error = %v, wantErr %v", gotErr, tt.wantErr)
117+
}
118+
119+
if gotVal != tt.wantVal {
120+
t.Errorf("callWithContext() gotVal = %s, wantVal %s", gotVal, tt.wantVal)
121+
}
122+
})
123+
}
150124
}

‎pkg/webhook/preflight/nutanix/credentials_test.go

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,19 @@ import (
1717
"sigs.k8s.io/controller-runtime/pkg/client/fake"
1818

1919
prismgoclient "github.com/nutanix-cloud-native/prism-go-client"
20+
prismv3 "github.com/nutanix-cloud-native/prism-go-client/v3"
2021

2122
carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1"
2223
)
2324

2425
func TestNewCredentialsCheck_Success(t *testing.T) {
2526
cd := validCheckDependencies()
2627
nclientFactory := func(_ prismgoclient.Credentials) (client, error) {
27-
return &mocknclient{}, nil
28+
return &clientWrapper{
29+
GetCurrentLoggedInUserFunc: func(ctx context.Context) (*prismv3.UserIntentResponse, error) {
30+
return &prismv3.UserIntentResponse{}, nil
31+
},
32+
}, nil
2833
}
2934
check := newCredentialsCheck(context.Background(), nclientFactory, cd)
3035
result := check.Run(context.Background())
@@ -37,7 +42,7 @@ func TestNewCredentialsCheck_NoNutanixConfig(t *testing.T) {
3742
cd := validCheckDependencies()
3843
cd.nutanixClusterConfigSpec = nil
3944
nclientFactory := func(_ prismgoclient.Credentials) (client, error) {
40-
return &mocknclient{}, nil
45+
return &clientWrapper{}, nil
4146
}
4247
check := newCredentialsCheck(context.Background(), nclientFactory, cd)
4348
result := check.Run(context.Background())
@@ -50,7 +55,7 @@ func TestNewCredentialsCheck_MissingNutanixField(t *testing.T) {
5055
cd := validCheckDependencies()
5156
cd.nutanixClusterConfigSpec.Nutanix = nil
5257
nclientFactory := func(_ prismgoclient.Credentials) (client, error) {
53-
return &mocknclient{}, nil
58+
return &clientWrapper{}, nil
5459
}
5560
check := newCredentialsCheck(context.Background(), nclientFactory, cd)
5661
result := check.Run(context.Background())
@@ -68,7 +73,7 @@ func TestNewCredentialsCheck_InvalidURL(t *testing.T) {
6873
cd := validCheckDependencies()
6974
cd.nutanixClusterConfigSpec.Nutanix.PrismCentralEndpoint.URL = "not-a-url"
7075
nclientFactory := func(_ prismgoclient.Credentials) (client, error) {
71-
return &mocknclient{}, nil
76+
return &clientWrapper{}, nil
7277
}
7378
check := newCredentialsCheck(context.Background(), nclientFactory, cd)
7479
result := check.Run(context.Background())
@@ -85,7 +90,7 @@ func TestNewCredentialsCheck_SecretNotFound(t *testing.T) {
8590
cd := validCheckDependencies()
8691
cd.kclient = fake.NewClientBuilder().Build() // no secret
8792
nclientFactory := func(_ prismgoclient.Credentials) (client, error) {
88-
return &mocknclient{}, nil
93+
return &clientWrapper{}, nil
8994
}
9095
check := newCredentialsCheck(context.Background(), nclientFactory, cd)
9196
result := check.Run(context.Background())
@@ -123,7 +128,7 @@ func TestNewCredentialsCheck_SecretGetError(t *testing.T) {
123128
},
124129
}
125130
nclientFactory := func(_ prismgoclient.Credentials) (client, error) {
126-
return &mocknclient{}, nil
131+
return &clientWrapper{}, nil
127132
}
128133
check := newCredentialsCheck(context.Background(), nclientFactory, cd)
129134
result := check.Run(context.Background())
@@ -147,7 +152,7 @@ func TestNewCredentialsCheck_SecretEmpty(t *testing.T) {
147152
cd := validCheckDependencies()
148153
cd.kclient = fake.NewClientBuilder().WithObjects(secret).Build()
149154
nclientFactory := func(_ prismgoclient.Credentials) (client, error) {
150-
return &mocknclient{}, nil
155+
return &clientWrapper{}, nil
151156
}
152157
check := newCredentialsCheck(context.Background(), nclientFactory, cd)
153158
result := check.Run(context.Background())
@@ -188,7 +193,7 @@ func TestNewCredentialsCheck_InvalidCredentialsFormat(t *testing.T) {
188193
cd := validCheckDependencies()
189194
cd.kclient = fake.NewClientBuilder().WithObjects(secret).Build()
190195
nclientFactory := func(_ prismgoclient.Credentials) (client, error) {
191-
return &mocknclient{}, nil
196+
return &clientWrapper{}, nil
192197
}
193198
check := newCredentialsCheck(context.Background(), nclientFactory, cd)
194199
result := check.Run(context.Background())
@@ -217,7 +222,11 @@ func TestNewCredentialsCheck_FailedToCreateClient(t *testing.T) {
217222
func TestNewCredentialsCheck_FailedToGetCurrentLoggedInUser(t *testing.T) {
218223
// Simulate a failure in getting the current logged-in user
219224
nclientFactory := func(_ prismgoclient.Credentials) (client, error) {
220-
return &mocknclient{err: assert.AnError}, nil
225+
return &clientWrapper{
226+
GetCurrentLoggedInUserFunc: func(ctx context.Context) (*prismv3.UserIntentResponse, error) {
227+
return nil, assert.AnError
228+
},
229+
}, nil
221230
}
222231
cd := validCheckDependencies()
223232
check := newCredentialsCheck(context.Background(), nclientFactory, cd)
@@ -230,7 +239,11 @@ func TestNewCredentialsCheck_FailedToGetCurrentLoggedInUser(t *testing.T) {
230239

231240
func TestNewCredentialsCheck_GetCurrentLoggedInUserInvalidCredentials(t *testing.T) {
232241
nclientFactory := func(_ prismgoclient.Credentials) (client, error) {
233-
return &mocknclient{err: fmt.Errorf("invalid Nutanix credentials")}, nil
242+
return &clientWrapper{
243+
GetCurrentLoggedInUserFunc: func(ctx context.Context) (*prismv3.UserIntentResponse, error) {
244+
return nil, fmt.Errorf("invalid Nutanix credentials")
245+
},
246+
}, nil
234247
}
235248
cd := validCheckDependencies()
236249
check := newCredentialsCheck(context.Background(), nclientFactory, cd)

‎pkg/webhook/preflight/nutanix/failuredomain.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func (fdc *failureDomainCheck) Run(ctx context.Context) preflight.CheckResult {
112112
// Validate the failure domain configuration
113113
// Validate spec.prismElementCluster configuration
114114
peIdentifier := fdObj.Spec.PrismElementCluster
115-
peClusters, err := getClusters(fdc.nclient, &peIdentifier)
115+
peClusters, err := getClusters(ctx, fdc.nclient, &peIdentifier)
116116
if err != nil {
117117
result.Allowed = false
118118
result.InternalError = true
@@ -143,7 +143,7 @@ func (fdc *failureDomainCheck) Run(ctx context.Context) preflight.CheckResult {
143143

144144
// Validate spec.subnets configuration
145145
for _, id := range fdObj.Spec.Subnets {
146-
subnets, err := getSubnets(fdc.nclient, &id)
146+
subnets, err := getSubnets(ctx, fdc.nclient, &id)
147147
if err != nil {
148148
result.Allowed = false
149149
result.InternalError = true
@@ -186,10 +186,17 @@ func (fdc *failureDomainCheck) Run(ctx context.Context) preflight.CheckResult {
186186
}
187187

188188
// getSubnets returns the subnets found in PC with the input identifier.
189-
func getSubnets(client client, subnetId *capxv1.NutanixResourceIdentifier) ([]netv4.Subnet, error) {
189+
func getSubnets(
190+
ctx context.Context,
191+
client client,
192+
subnetId *capxv1.NutanixResourceIdentifier,
193+
) (
194+
[]netv4.Subnet,
195+
error,
196+
) {
190197
switch {
191198
case subnetId.IsUUID():
192-
resp, err := client.GetSubnetById(subnetId.UUID)
199+
resp, err := client.GetSubnetById(ctx, subnetId.UUID)
193200
if err != nil {
194201
return nil, err
195202
}
@@ -204,7 +211,7 @@ func getSubnets(client client, subnetId *capxv1.NutanixResourceIdentifier) ([]ne
204211
return []netv4.Subnet{subnet}, nil
205212
case subnetId.IsName():
206213
filter_ := fmt.Sprintf("name eq '%s'", *subnetId.Name)
207-
resp, err := client.ListSubnets(nil, nil, &filter_, nil, nil, nil)
214+
resp, err := client.ListSubnets(ctx, nil, nil, &filter_, nil, nil, nil)
208215
if err != nil {
209216
return nil, err
210217
}

0 commit comments

Comments
 (0)