Skip to content

Commit d68711a

Browse files
g7rg7r
authored
Do less Kubernetes API requests (#2699)
* Do less Kubernetes API requests The first reason why provider does so many requests is because its caches aren't goroutine-safe. For example, Terraform invokes provider concurrently and every individual goroutine starts its own getOAPIv2Foundry() invocation. Every getOAPIv2Foundry() starts its own Kubernetes API request and these requests consume Kubernetes client Burst leading eventually to a stall. The second reason is that CRDs should also be cached because production Kubernetes clusters may have lots and lots of CRDs and getting them all is not cheap. Furthermore, getting them over and over consumes Kubernetes client Burst leading to a major stall.
1 parent 37a1dc2 commit d68711a

File tree

5 files changed

+161
-144
lines changed

5 files changed

+161
-144
lines changed

‎.changelog/2699.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
Fixed goroutine-safety in the CRD and metadata cache, resulting in far fewer provider metadata requests.
3+
```

‎manifest/provider/cache.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Copyright (c) HashiCorp, Inc.
2+
// SPDX-License-Identifier: MPL-2.0
3+
4+
package provider
5+
6+
import "sync"
7+
8+
type cache[T any] struct {
9+
once sync.Once
10+
value T
11+
err error
12+
}
13+
14+
func (c *cache[T]) Get(f func() (T, error)) (T, error) {
15+
c.once.Do(func() {
16+
c.value, c.err = f()
17+
})
18+
return c.value, c.err
19+
}

‎manifest/provider/clients.go‎

Lines changed: 66 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -31,98 +31,71 @@ const (
3131

3232
// getDynamicClient returns a configured unstructured (dynamic) client instance
3333
func (ps *RawProviderServer) getDynamicClient() (dynamic.Interface, error) {
34-
if ps.dynamicClient != nil {
35-
return ps.dynamicClient, nil
36-
}
3734
if ps.clientConfig == nil {
3835
return nil, fmt.Errorf("cannot create dynamic client: no client config")
3936
}
40-
dynClient, err := dynamic.NewForConfig(ps.clientConfig)
41-
if err != nil {
42-
return nil, err
43-
}
44-
ps.dynamicClient = dynClient
45-
return dynClient, nil
37+
38+
return ps.dynamicClient.Get(func() (dynamic.Interface, error) {
39+
return dynamic.NewForConfig(ps.clientConfig)
40+
})
4641
}
4742

4843
// getDiscoveryClient returns a configured discovery client instance.
4944
func (ps *RawProviderServer) getDiscoveryClient() (discovery.DiscoveryInterface, error) {
50-
if ps.discoveryClient != nil {
51-
return ps.discoveryClient, nil
52-
}
5345
if ps.clientConfig == nil {
5446
return nil, fmt.Errorf("cannot create discovery client: no client config")
5547
}
56-
discoClient, err := discovery.NewDiscoveryClientForConfig(ps.clientConfig)
57-
if err != nil {
58-
return nil, err
59-
}
60-
ps.discoveryClient = discoClient
61-
return discoClient, nil
48+
49+
return ps.discoveryClient.Get(func() (discovery.DiscoveryInterface, error) {
50+
return discovery.NewDiscoveryClientForConfig(ps.clientConfig)
51+
})
6252
}
6353

6454
// getRestMapper returns a RESTMapper client instance
6555
func (ps *RawProviderServer) getRestMapper() (meta.RESTMapper, error) {
66-
if ps.restMapper != nil {
67-
return ps.restMapper, nil
68-
}
69-
dc, err := ps.getDiscoveryClient()
70-
if err != nil {
71-
return nil, err
72-
}
73-
74-
// agr, err := restmapper.GetAPIGroupResources(dc)
75-
// if err != nil {
76-
// return nil, err
77-
// }
78-
// mapper := restmapper.NewDeferredDiscoveryRESTMapper(agr)
56+
return ps.restMapper.Get(func() (meta.RESTMapper, error) {
57+
dc, err := ps.getDiscoveryClient()
58+
if err != nil {
59+
return nil, err
60+
}
7961

80-
cache := memory.NewMemCacheClient(dc)
81-
ps.restMapper = restmapper.NewDeferredDiscoveryRESTMapper(cache)
82-
return ps.restMapper, nil
62+
cacheClient := memory.NewMemCacheClient(dc)
63+
return restmapper.NewDeferredDiscoveryRESTMapper(cacheClient), nil
64+
})
8365
}
8466

8567
// getRestClient returns a raw REST client instance
8668
func (ps *RawProviderServer) getRestClient() (rest.Interface, error) {
87-
if ps.restClient != nil {
88-
return ps.restClient, nil
89-
}
9069
if ps.clientConfig == nil {
9170
return nil, fmt.Errorf("cannot create REST client: no client config")
9271
}
93-
restClient, err := rest.UnversionedRESTClientFor(ps.clientConfig)
94-
if err != nil {
95-
return nil, err
96-
}
97-
ps.restClient = restClient
98-
return restClient, nil
72+
73+
return ps.restClient.Get(func() (rest.Interface, error) {
74+
return rest.UnversionedRESTClientFor(ps.clientConfig)
75+
})
9976
}
10077

10178
// getOAPIv2Foundry returns an interface to request tftype types from an OpenAPIv2 spec
10279
func (ps *RawProviderServer) getOAPIv2Foundry() (openapi.Foundry, error) {
103-
if ps.OAPIFoundry != nil {
104-
return ps.OAPIFoundry, nil
105-
}
106-
107-
rc, err := ps.getRestClient()
108-
if err != nil {
109-
return nil, fmt.Errorf("failed get OpenAPI spec: %s", err)
110-
}
111-
112-
rq := rc.Verb("GET").Timeout(30*time.Second).AbsPath("openapi", "v2")
113-
rs, err := rq.DoRaw(context.TODO())
114-
if err != nil {
115-
return nil, fmt.Errorf("failed get OpenAPI spec: %s", err)
116-
}
80+
return ps.OAPIFoundry.Get(func() (openapi.Foundry, error) {
81+
rc, err := ps.getRestClient()
82+
if err != nil {
83+
return nil, fmt.Errorf("failed get OpenAPI spec: %s", err)
84+
}
11785

118-
oapif, err := openapi.NewFoundryFromSpecV2(rs)
119-
if err != nil {
120-
return nil, fmt.Errorf("failed construct OpenAPI foundry: %s", err)
121-
}
86+
rq := rc.Verb("GET").Timeout(30*time.Second).AbsPath("openapi", "v2")
87+
rs, err := rq.DoRaw(context.TODO())
88+
if err != nil {
89+
return nil, fmt.Errorf("failed get OpenAPI spec: %s", err)
90+
}
12291

123-
ps.OAPIFoundry = oapif
92+
oapif, err := openapi.NewFoundryFromSpecV2(rs)
93+
if err != nil {
94+
return nil, fmt.Errorf("failed construct OpenAPI foundry: %s", err)
95+
}
12496

125-
return oapif, nil
97+
return oapif, nil
98+
})
12699
}
127100

128101
func loggingTransport(rt http.RoundTripper) http.RoundTripper {
@@ -145,34 +118,38 @@ func (t *loggingRountTripper) RoundTrip(req *http.Request) (*http.Response, erro
145118
return t.lt.RoundTrip(req)
146119
}
147120

148-
func (ps *RawProviderServer) checkValidCredentials(ctx context.Context) (diags []*tfprotov5.Diagnostic) {
149-
rc, err := ps.getRestClient()
150-
if err != nil {
151-
diags = append(diags, &tfprotov5.Diagnostic{
152-
Severity: tfprotov5.DiagnosticSeverityError,
153-
Summary: "Failed to construct REST client",
154-
Detail: err.Error(),
155-
})
156-
return
157-
}
158-
vpath := []string{"/apis"}
159-
rs := rc.Get().AbsPath(vpath...).Do(ctx)
160-
if rs.Error() != nil {
161-
switch {
162-
case apierrors.IsUnauthorized(rs.Error()):
121+
func (ps *RawProviderServer) checkValidCredentials(ctx context.Context) []*tfprotov5.Diagnostic {
122+
diagnostics, _ := ps.checkValidCredentialsResult.Get(func() (diags []*tfprotov5.Diagnostic, err error) {
123+
rc, err := ps.getRestClient()
124+
if err != nil {
163125
diags = append(diags, &tfprotov5.Diagnostic{
164126
Severity: tfprotov5.DiagnosticSeverityError,
165-
Summary: "Invalid credentials",
166-
Detail: fmt.Sprintf("The credentials configured in the provider block are not accepted by the API server. Error: %s\n\nSet TF_LOG=debug and look for '[InvalidClientConfiguration]' in the log to see actual configuration.", rs.Error().Error()),
167-
})
168-
default:
169-
diags = append(diags, &tfprotov5.Diagnostic{
170-
Severity: tfprotov5.DiagnosticSeverityError,
171-
Summary: "Invalid configuration for API client",
172-
Detail: rs.Error().Error(),
127+
Summary: "Failed to construct REST client",
128+
Detail: err.Error(),
173129
})
130+
return
174131
}
175-
ps.logger.Debug("[InvalidClientConfiguration]", "Config", dump(ps.clientConfig))
176-
}
177-
return
132+
vpath := []string{"/apis"}
133+
rs := rc.Get().AbsPath(vpath...).Do(ctx)
134+
if rs.Error() != nil {
135+
switch {
136+
case apierrors.IsUnauthorized(rs.Error()):
137+
diags = append(diags, &tfprotov5.Diagnostic{
138+
Severity: tfprotov5.DiagnosticSeverityError,
139+
Summary: "Invalid credentials",
140+
Detail: fmt.Sprintf("The credentials configured in the provider block are not accepted by the API server. Error: %s\n\nSet TF_LOG=debug and look for '[InvalidClientConfiguration]' in the log to see actual configuration.", rs.Error().Error()),
141+
})
142+
default:
143+
diags = append(diags, &tfprotov5.Diagnostic{
144+
Severity: tfprotov5.DiagnosticSeverityError,
145+
Summary: "Invalid configuration for API client",
146+
Detail: rs.Error().Error(),
147+
})
148+
}
149+
ps.logger.Debug("[InvalidClientConfiguration]", "Config", dump(ps.clientConfig))
150+
}
151+
return
152+
})
153+
154+
return diagnostics
178155
}

‎manifest/provider/resource.go‎

Lines changed: 62 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -202,69 +202,84 @@ func RemoveServerSideFields(in map[string]interface{}) map[string]interface{} {
202202
}
203203

204204
func (ps *RawProviderServer) lookUpGVKinCRDs(ctx context.Context, gvk schema.GroupVersionKind) (interface{}, error) {
205-
c, err := ps.getDynamicClient()
206-
if err != nil {
207-
return nil, err
208-
}
209-
m, err := ps.getRestMapper()
205+
// check CRD versions
206+
crds, err := ps.fetchCRDs(ctx)
210207
if err != nil {
211208
return nil, err
212209
}
213210

214-
crd := schema.GroupKind{Group: "apiextensions.k8s.io", Kind: "CustomResourceDefinition"}
215-
crms, err := m.RESTMappings(crd)
216-
if err != nil {
217-
return nil, fmt.Errorf("could not extract resource version mappings for apiextensions.k8s.io.CustomResourceDefinition: %s", err)
218-
}
219-
// check CRD versions
220-
for _, crm := range crms {
221-
crdRes, err := c.Resource(crm.Resource).List(ctx, v1.ListOptions{})
222-
if err != nil {
223-
return nil, err
211+
for _, r := range crds {
212+
spec := r.Object["spec"].(map[string]interface{})
213+
if spec == nil {
214+
continue
224215
}
225-
226-
for _, r := range crdRes.Items {
227-
spec := r.Object["spec"].(map[string]interface{})
228-
if spec == nil {
229-
continue
230-
}
231-
grp := spec["group"].(string)
232-
if grp != gvk.Group {
233-
continue
234-
}
235-
names := spec["names"]
236-
if names == nil {
216+
grp := spec["group"].(string)
217+
if grp != gvk.Group {
218+
continue
219+
}
220+
names := spec["names"]
221+
if names == nil {
222+
continue
223+
}
224+
kind := names.(map[string]interface{})["kind"]
225+
if kind != gvk.Kind {
226+
continue
227+
}
228+
ver := spec["versions"]
229+
if ver == nil {
230+
ver = spec["version"]
231+
if ver == nil {
237232
continue
238233
}
239-
kind := names.(map[string]interface{})["kind"]
240-
if kind != gvk.Kind {
234+
}
235+
for _, rv := range ver.([]interface{}) {
236+
if rv == nil {
241237
continue
242238
}
243-
ver := spec["versions"]
244-
if ver == nil {
245-
ver = spec["version"]
246-
if ver == nil {
247-
continue
248-
}
249-
}
250-
for _, rv := range ver.([]interface{}) {
251-
if rv == nil {
252-
continue
253-
}
254-
v := rv.(map[string]interface{})
255-
if v["name"] == gvk.Version {
256-
s, ok := v["schema"].(map[string]interface{})
257-
if !ok {
258-
return nil, nil // non-structural CRD
259-
}
260-
return s["openAPIV3Schema"], nil
239+
v := rv.(map[string]interface{})
240+
if v["name"] == gvk.Version {
241+
s, ok := v["schema"].(map[string]interface{})
242+
if !ok {
243+
return nil, nil // non-structural CRD
261244
}
245+
return s["openAPIV3Schema"], nil
262246
}
263247
}
264248
}
265249
return nil, nil
266250
}
267251

252+
func (ps *RawProviderServer) fetchCRDs(ctx context.Context) ([]unstructured.Unstructured, error) {
253+
return ps.crds.Get(func() ([]unstructured.Unstructured, error) {
254+
c, err := ps.getDynamicClient()
255+
if err != nil {
256+
return nil, err
257+
}
258+
m, err := ps.getRestMapper()
259+
if err != nil {
260+
return nil, err
261+
}
262+
263+
crd := schema.GroupKind{Group: "apiextensions.k8s.io", Kind: "CustomResourceDefinition"}
264+
crms, err := m.RESTMappings(crd)
265+
if err != nil {
266+
return nil, fmt.Errorf("could not extract resource version mappings for apiextensions.k8s.io.CustomResourceDefinition: %s", err)
267+
}
268+
269+
var crds []unstructured.Unstructured
270+
for _, crm := range crms {
271+
crdRes, err := c.Resource(crm.Resource).List(ctx, v1.ListOptions{})
272+
if err != nil {
273+
return nil, err
274+
}
275+
276+
crds = append(crds, crdRes.Items...)
277+
}
278+
279+
return crds, nil
280+
})
281+
}
282+
268283
// privateStateSchema describes the structure of the private state payload that
269284
// Terraform can store along with the "regular" resource state state.
270285
var privateStateSchema tftypes.Object = tftypes.Object{AttributeTypes: map[string]tftypes.Type{

‎manifest/provider/server.go‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"google.golang.org/grpc/status"
1414
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install"
1515
"k8s.io/apimachinery/pkg/api/meta"
16+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1617
"k8s.io/client-go/discovery"
1718
"k8s.io/client-go/dynamic"
1819
"k8s.io/client-go/kubernetes/scheme"
@@ -34,14 +35,16 @@ type RawProviderServer struct {
3435
// Since the provider is essentially a gRPC server, the execution flow is dictated by the order of the client (Terraform) request calls.
3536
// Thus it needs a way to persist state between the gRPC calls. These attributes store values that need to be persisted between gRPC calls,
3637
// such as instances of the Kubernetes clients, configuration options needed at runtime.
37-
logger hclog.Logger
38-
clientConfig *rest.Config
39-
clientConfigUnknown bool
40-
dynamicClient dynamic.Interface
41-
discoveryClient discovery.DiscoveryInterface
42-
restMapper meta.RESTMapper
43-
restClient rest.Interface
44-
OAPIFoundry openapi.Foundry
38+
logger hclog.Logger
39+
clientConfig *rest.Config
40+
clientConfigUnknown bool
41+
dynamicClient cache[dynamic.Interface]
42+
discoveryClient cache[discovery.DiscoveryInterface]
43+
restMapper cache[meta.RESTMapper]
44+
restClient cache[rest.Interface]
45+
OAPIFoundry cache[openapi.Foundry]
46+
crds cache[[]unstructured.Unstructured]
47+
checkValidCredentialsResult cache[[]*tfprotov5.Diagnostic]
4548

4649
hostTFVersion string
4750
}

0 commit comments

Comments
 (0)