Skip to content

Commit 0b30b4e

Browse files
matteogastaldellomatteogastaldello
authored
feat: performance improvent, memory optimization and cpu optimization (#146)
* feat: performance improvent, memory optimization and cpu optimization * fix: errors not correctly reported * fix: return missing error * chore: remove unnecessary comment * chore: comment fix * test: update chart inspector version in tests * test: mock chart-inspector * test: fixing test * test: remove unused code
1 parent 9f9d62a commit 0b30b4e

13 files changed

Lines changed: 287 additions & 165 deletions

File tree

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ require (
1010
github.com/gobuffalo/flect v1.0.3
1111
github.com/golang/mock v1.6.0
1212
github.com/krateoplatformops/plumbing v0.7.2
13-
github.com/krateoplatformops/unstructured-runtime v0.2.7
13+
github.com/krateoplatformops/unstructured-runtime v0.2.8
1414
github.com/pkg/errors v0.9.1
1515
github.com/spf13/pflag v1.0.7
1616
github.com/stretchr/testify v1.10.0

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
178178
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
179179
github.com/krateoplatformops/plumbing v0.7.2 h1:4UuWy9747p9ligMtNEiOOQGsuK6d9lczg7R1no8ERsE=
180180
github.com/krateoplatformops/plumbing v0.7.2/go.mod h1:mQ/sm0viyKgfR2ARzHuwCpY0rcyMKqCv8a8SOu52yYQ=
181-
github.com/krateoplatformops/unstructured-runtime v0.2.7 h1:HwvW/0vjLbNmLvYeh4d4EN43UQSMRS2iHacmadZbHs0=
182-
github.com/krateoplatformops/unstructured-runtime v0.2.7/go.mod h1:19uT87wZzRSjrfk3731Xhdt8ww7vnsXhljy4jk0cuWA=
181+
github.com/krateoplatformops/unstructured-runtime v0.2.8 h1:Hwvf/aPMM1mSiO+AzlxmybHt5yEe/58D8MtkMPxbBZA=
182+
github.com/krateoplatformops/unstructured-runtime v0.2.8/go.mod h1:19uT87wZzRSjrfk3731Xhdt8ww7vnsXhljy4jk0cuWA=
183183
github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
184184
github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
185185
github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 h1:SOEGU9fKiNWd/HOJuq6+3iTQz8KNCLtVX6idSoTLdUw=

‎internal/chartinspector/chartinspector.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,10 @@ type ChartInspector struct {
4242
var _ ChartInspectorInterface = &ChartInspector{}
4343

4444
func NewChartInspector(server string) ChartInspector {
45-
return ChartInspector{server: server, httpClient: http.DefaultClient}
45+
httpcli := http.DefaultClient
46+
httpcli.Timeout = 20 * time.Second
47+
48+
return ChartInspector{server: server, httpClient: httpcli}
4649
}
4750

4851
func (c *ChartInspector) WithHTTPClient(httpClient *http.Client) {

‎internal/composition/composition.go‎

Lines changed: 98 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"path/filepath"
99
"time"
1010

11+
"github.com/krateoplatformops/composition-dynamic-controller/internal/chartinspector"
1112
compositionMeta "github.com/krateoplatformops/composition-dynamic-controller/internal/meta"
1213

1314
"github.com/krateoplatformops/composition-dynamic-controller/internal/helmclient"
@@ -59,13 +60,7 @@ const (
5960

6061
var _ controller.ExternalClient = (*handler)(nil)
6162

62-
func NewHandler(cfg *rest.Config, log logging.Logger, pig archive.Getter, event event.APIRecorder, pluralizer pluralizer.PluralizerInterface, rbacgen rbacgen.RBACGenInterface) controller.ExternalClient {
63-
dyn, err := dynamic.NewForConfig(cfg)
64-
if err != nil {
65-
log.Error(err, "Creating dynamic client.")
66-
return nil
67-
}
68-
63+
func NewHandler(cfg *rest.Config, log logging.Logger, pig archive.Getter, event event.APIRecorder, pluralizer pluralizer.PluralizerInterface, chartInspectorUrl string, saName string, saNamespace string) controller.ExternalClient {
6964
val, ok := os.LookupEnv(helmRegistryConfigPathEnvVar)
7065
if ok {
7166
helmRegistryConfigPath = val
@@ -75,35 +70,46 @@ func NewHandler(cfg *rest.Config, log logging.Logger, pig archive.Getter, event
7570

7671
return &handler{
7772
kubeconfig: cfg,
78-
rbacgen: rbacgen,
7973
pluralizer: pluralizer,
8074
logger: log,
81-
dynamicClient: dyn,
8275
packageInfoGetter: pig,
8376
eventRecorder: event,
77+
78+
chartInspectorUrl: chartInspectorUrl,
79+
saName: saName,
80+
saNamespace: saNamespace,
8481
}
8582
}
8683

8784
type handler struct {
8885
kubeconfig *rest.Config
89-
rbacgen rbacgen.RBACGenInterface
9086
logger logging.Logger
9187
pluralizer pluralizer.PluralizerInterface
92-
dynamicClient dynamic.Interface
9388
packageInfoGetter archive.Getter
9489
eventRecorder event.APIRecorder
90+
91+
chartInspectorUrl string
92+
saName string
93+
saNamespace string
9594
}
9695

9796
func (h *handler) Observe(ctx context.Context, mg *unstructured.Unstructured) (controller.ExternalObservation, error) {
97+
mg = mg.DeepCopy()
9898
log := h.logger.WithValues("op", "Observe").
9999
WithValues("apiVersion", mg.GetAPIVersion()).
100100
WithValues("kind", mg.GetKind()).
101101
WithValues("name", mg.GetName()).
102102
WithValues("namespace", mg.GetNamespace())
103103

104+
dyn, err := dynamic.NewForConfig(h.kubeconfig)
105+
if err != nil {
106+
log.Error(err, "Creating dynamic client.")
107+
return controller.ExternalObservation{}, err
108+
}
109+
104110
updateOpts := tools.UpdateOptions{
105111
Pluralizer: h.pluralizer,
106-
DynamicClient: h.dynamicClient,
112+
DynamicClient: dyn,
107113
}
108114

109115
compositionMeta.SetReleaseName(mg, mg.GetName())
@@ -117,7 +123,7 @@ func (h *handler) Observe(ctx context.Context, mg *unstructured.Unstructured) (c
117123
}
118124
// Immediately remove the gracefully paused time annotation if the composition is not gracefully paused.
119125
meta.RemoveAnnotations(mg, compositionMeta.AnnotationKeyReconciliationGracefullyPausedTime)
120-
mg, err := tools.Update(ctx, mg, updateOpts)
126+
mg, err = tools.Update(ctx, mg, updateOpts)
121127
if err != nil {
122128
log.Error(err, "Updating cr with values")
123129
return controller.ExternalObservation{}, fmt.Errorf("updating cr with values: %w", err)
@@ -185,8 +191,11 @@ func (h *handler) Observe(ctx context.Context, mg *unstructured.Unstructured) (c
185191
log.Error(err, "Converting GVK to GVR")
186192
return controller.ExternalObservation{}, fmt.Errorf("converting GVK to GVR: %w", err)
187193
}
194+
195+
chartInspector := chartinspector.NewChartInspector(h.chartInspectorUrl)
196+
rbgen := rbacgen.NewRBACGen(h.saName, h.saNamespace, &chartInspector)
188197
// Get Resources and generate RBAC
189-
generated, err := h.rbacgen.
198+
generated, err := rbgen.
190199
WithBaseName(meta.GetReleaseName(mg)).
191200
Generate(rbacgen.Parameters{
192201
CompositionName: mg.GetName(),
@@ -200,7 +209,7 @@ func (h *handler) Observe(ctx context.Context, mg *unstructured.Unstructured) (c
200209
log.Error(err, "Generating RBAC using chart-inspector")
201210
return controller.ExternalObservation{}, err
202211
}
203-
rbInstaller := rbac.NewRBACInstaller(h.dynamicClient)
212+
rbInstaller := rbac.NewRBACInstaller(dyn)
204213
err = rbInstaller.ApplyRBAC(generated)
205214
if err != nil {
206215
log.Error(err, "Installing RBAC")
@@ -218,7 +227,7 @@ func (h *handler) Observe(ctx context.Context, mg *unstructured.Unstructured) (c
218227

219228
opts := helmchart.UpdateOptions{
220229
CheckResourceOptions: helmchart.CheckResourceOptions{
221-
DynamicClient: h.dynamicClient,
230+
DynamicClient: dyn,
222231
Pluralizer: h.pluralizer,
223232
},
224233
HelmClient: hc,
@@ -269,7 +278,7 @@ func (h *handler) Observe(ctx context.Context, mg *unstructured.Unstructured) (c
269278
return controller.ExternalObservation{}, err
270279
}
271280

272-
mg, err = tools.UpdateStatus(ctx, mg, updateOpts)
281+
_, err = tools.UpdateStatus(ctx, mg, updateOpts)
273282
if err != nil {
274283
log.Error(err, "Updating cr status with values")
275284
return controller.ExternalObservation{}, err
@@ -284,15 +293,22 @@ func (h *handler) Observe(ctx context.Context, mg *unstructured.Unstructured) (c
284293
}
285294

286295
func (h *handler) Create(ctx context.Context, mg *unstructured.Unstructured) error {
296+
mg = mg.DeepCopy()
287297
log := h.logger.WithValues("op", "Create").
288298
WithValues("apiVersion", mg.GetAPIVersion()).
289299
WithValues("kind", mg.GetKind()).
290300
WithValues("name", mg.GetName()).
291301
WithValues("namespace", mg.GetNamespace())
292302

303+
dyn, err := dynamic.NewForConfig(h.kubeconfig)
304+
if err != nil {
305+
log.Error(err, "Creating dynamic client.")
306+
return fmt.Errorf("creating dynamic client: %w", err)
307+
}
308+
293309
updateOpts := tools.UpdateOptions{
294310
Pluralizer: h.pluralizer,
295-
DynamicClient: h.dynamicClient,
311+
DynamicClient: dyn,
296312
}
297313

298314
if _, p := compositionMeta.GetGracefullyPausedTime(mg); p && compositionMeta.IsGracefullyPaused(mg) {
@@ -302,7 +318,7 @@ func (h *handler) Create(ctx context.Context, mg *unstructured.Unstructured) err
302318
}
303319

304320
compositionMeta.SetReleaseName(mg, mg.GetName())
305-
mg, err := tools.Update(ctx, mg, updateOpts)
321+
mg, err = tools.Update(ctx, mg, updateOpts)
306322
if err != nil {
307323
log.Error(err, "Updating cr with values")
308324
return fmt.Errorf("updating cr with values: %w", err)
@@ -323,8 +339,11 @@ func (h *handler) Create(ctx context.Context, mg *unstructured.Unstructured) err
323339
log.Error(err, "Converting GVK to GVR")
324340
return fmt.Errorf("converting GVK to GVR: %w", err)
325341
}
342+
343+
chartInspector := chartinspector.NewChartInspector(h.chartInspectorUrl)
344+
rbgen := rbacgen.NewRBACGen(h.saName, h.saNamespace, &chartInspector)
326345
// Get Resources and generate RBAC
327-
generated, err := h.rbacgen.
346+
generated, err := rbgen.
328347
WithBaseName(meta.GetReleaseName(mg)).
329348
Generate(rbacgen.Parameters{
330349
CompositionName: mg.GetName(),
@@ -338,7 +357,7 @@ func (h *handler) Create(ctx context.Context, mg *unstructured.Unstructured) err
338357
log.Error(err, "Generating RBAC using chart-inspector")
339358
return err
340359
}
341-
rbInstaller := rbac.NewRBACInstaller(h.dynamicClient)
360+
rbInstaller := rbac.NewRBACInstaller(dyn)
342361
err = rbInstaller.ApplyRBAC(generated)
343362
if err != nil {
344363
log.Error(err, "Installing RBAC")
@@ -354,7 +373,7 @@ func (h *handler) Create(ctx context.Context, mg *unstructured.Unstructured) err
354373

355374
opts := helmchart.InstallOptions{
356375
CheckResourceOptions: helmchart.CheckResourceOptions{
357-
DynamicClient: h.dynamicClient,
376+
DynamicClient: dyn,
358377
Pluralizer: h.pluralizer,
359378
},
360379
HelmClient: hc,
@@ -416,15 +435,23 @@ func (h *handler) Create(ctx context.Context, mg *unstructured.Unstructured) err
416435
}
417436

418437
func (h *handler) Update(ctx context.Context, mg *unstructured.Unstructured) error {
438+
mg = mg.DeepCopy()
439+
419440
log := h.logger.WithValues("op", "Update").
420441
WithValues("apiVersion", mg.GetAPIVersion()).
421442
WithValues("kind", mg.GetKind()).
422443
WithValues("name", mg.GetName()).
423444
WithValues("namespace", mg.GetNamespace())
424445

446+
dyn, err := dynamic.NewForConfig(h.kubeconfig)
447+
if err != nil {
448+
log.Error(err, "Creating dynamic client.")
449+
return fmt.Errorf("creating dynamic client: %w", err)
450+
}
451+
425452
updateOpts := tools.UpdateOptions{
426453
Pluralizer: h.pluralizer,
427-
DynamicClient: h.dynamicClient,
454+
DynamicClient: dyn,
428455
}
429456

430457
if _, p := compositionMeta.GetGracefullyPausedTime(mg); p && compositionMeta.IsGracefullyPaused(mg) {
@@ -482,7 +509,7 @@ func (h *handler) Update(ctx context.Context, mg *unstructured.Unstructured) err
482509
}
483510
mg, err = tools.UpdateStatus(ctx, mg, tools.UpdateOptions{
484511
Pluralizer: h.pluralizer,
485-
DynamicClient: h.dynamicClient,
512+
DynamicClient: dyn,
486513
})
487514
if err != nil {
488515
log.Error(err, "Updating cr status with values")
@@ -501,7 +528,7 @@ func (h *handler) Update(ctx context.Context, mg *unstructured.Unstructured) err
501528
}
502529
mg, err = tools.UpdateStatus(ctx, mg, tools.UpdateOptions{
503530
Pluralizer: h.pluralizer,
504-
DynamicClient: h.dynamicClient,
531+
DynamicClient: dyn,
505532
})
506533
if err != nil {
507534
log.Error(err, "Updating cr status with values")
@@ -520,15 +547,23 @@ func (h *handler) Update(ctx context.Context, mg *unstructured.Unstructured) err
520547
}
521548

522549
func (h *handler) Delete(ctx context.Context, mg *unstructured.Unstructured) error {
550+
mg = mg.DeepCopy()
551+
523552
log := h.logger.WithValues("op", "Delete").
524553
WithValues("apiVersion", mg.GetAPIVersion()).
525554
WithValues("kind", mg.GetKind()).
526555
WithValues("name", mg.GetName()).
527556
WithValues("namespace", mg.GetNamespace())
528557

558+
dyn, err := dynamic.NewForConfig(h.kubeconfig)
559+
if err != nil {
560+
log.Error(err, "Creating dynamic client.")
561+
return fmt.Errorf("creating dynamic client: %w", err)
562+
}
563+
529564
updateOpts := tools.UpdateOptions{
530565
Pluralizer: h.pluralizer,
531-
DynamicClient: h.dynamicClient,
566+
DynamicClient: dyn,
532567
}
533568

534569
if _, p := compositionMeta.GetGracefullyPausedTime(mg); p && compositionMeta.IsGracefullyPaused(mg) {
@@ -562,7 +597,7 @@ func (h *handler) Delete(ctx context.Context, mg *unstructured.Unstructured) err
562597
}
563598

564599
// Check if the release exists before uninstalling
565-
rel, err := helmchart.FindAnyRelease(hc, meta.GetReleaseName(mg))
600+
rel, err := helmchart.FindRelease(hc, meta.GetReleaseName(mg))
566601
if err != nil {
567602
return fmt.Errorf("finding helm release: %w", err)
568603
}
@@ -578,7 +613,7 @@ func (h *handler) Delete(ctx context.Context, mg *unstructured.Unstructured) err
578613
return fmt.Errorf("uninstalling helm chart: %w", err)
579614
}
580615

581-
rel, err = helmchart.FindAnyRelease(hc, meta.GetReleaseName(mg))
616+
rel, err = helmchart.FindRelease(hc, meta.GetReleaseName(mg))
582617
if err != nil {
583618
log.Error(err, "Finding helm release")
584619
return fmt.Errorf("finding helm release: %w", err)
@@ -595,8 +630,10 @@ func (h *handler) Delete(ctx context.Context, mg *unstructured.Unstructured) err
595630
log.Error(err, "Converting GVK to GVR")
596631
return fmt.Errorf("converting GVK to GVR: %w", err)
597632
}
633+
chartInspector := chartinspector.NewChartInspector(h.chartInspectorUrl)
634+
rbgen := rbacgen.NewRBACGen(h.saName, h.saNamespace, &chartInspector)
598635
// Get Resources and generate RBAC
599-
generated, err := h.rbacgen.
636+
generated, err := rbgen.
600637
WithBaseName(meta.GetReleaseName(mg)).
601638
Generate(rbacgen.Parameters{
602639
CompositionName: mg.GetName(),
@@ -606,7 +643,11 @@ func (h *handler) Delete(ctx context.Context, mg *unstructured.Unstructured) err
606643
CompositionDefinitionNamespace: pkg.CompositionDefinitionInfo.Namespace,
607644
CompositionDefintionGVR: pkg.CompositionDefinitionInfo.GVR,
608645
})
609-
rbInstaller := rbac.NewRBACInstaller(h.dynamicClient)
646+
if err != nil {
647+
log.Error(err, "Generating RBAC using chart-inspector")
648+
return err
649+
}
650+
rbInstaller := rbac.NewRBACInstaller(dyn)
610651
err = rbInstaller.UninstallRBAC(generated)
611652
if err != nil {
612653
log.Error(err, "Uninstalling RBAC")
@@ -632,6 +673,12 @@ func (h *handler) helmClientForResource(mg *unstructured.Unstructured, registryA
632673
WithValues("name", mg.GetName()).
633674
WithValues("namespace", mg.GetNamespace())
634675

676+
clientSet, err := helmclient.NewCachedClients(h.kubeconfig)
677+
if err != nil {
678+
log.Error(err, "Creating cached helm client set.")
679+
return nil, err
680+
}
681+
635682
opts := &helmclient.Options{
636683
Namespace: mg.GetNamespace(),
637684
RepositoryCache: "/tmp/.helmcache",
@@ -653,10 +700,13 @@ func (h *handler) helmClientForResource(mg *unstructured.Unstructured, registryA
653700
RegistryAuth: (registryAuth),
654701
}
655702

656-
return helmclient.NewClientFromRestConf(&helmclient.RestConfClientOptions{
657-
Options: opts,
658-
RestConfig: h.kubeconfig,
659-
})
703+
return helmclient.NewCachedClientFromRestConf(
704+
&helmclient.RestConfClientOptions{
705+
Options: opts,
706+
RestConfig: h.kubeconfig,
707+
},
708+
&clientSet,
709+
)
660710
}
661711

662712
func (h *handler) helmClientForResourceWithTransportWrapper(mg *unstructured.Unstructured, registryAuth *helmclient.RegistryAuth, transportWrapper func(http.RoundTripper) http.RoundTripper) (helmclient.Client, error) {
@@ -671,10 +721,19 @@ func (h *handler) helmClientForResourceWithTransportWrapper(mg *unstructured.Uns
671721
RegistryAuth: registryAuth,
672722
}
673723

674-
h.kubeconfig.WrapTransport = transportWrapper
724+
clientSet, err := helmclient.NewCachedClients(h.kubeconfig)
725+
if err != nil {
726+
return nil, err
727+
}
675728

676-
return helmclient.NewClientFromRestConf(&helmclient.RestConfClientOptions{
677-
Options: opts,
678-
RestConfig: h.kubeconfig,
679-
})
729+
cfg := rest.CopyConfig(h.kubeconfig)
730+
cfg.WrapTransport = transportWrapper
731+
732+
return helmclient.NewCachedClientFromRestConf(
733+
&helmclient.RestConfClientOptions{
734+
Options: opts,
735+
RestConfig: cfg,
736+
},
737+
&clientSet,
738+
)
680739
}

0 commit comments

Comments
 (0)