Skip to content

Commit 97d2d36

Browse files
akafazovakafazov
committed
address pr comments
On-behalf-of: @SAP angel.kafazov@sap.com Signed-off-by: Angel Kafazov <akafazov@cst-bg.net>
1 parent f12077f commit 97d2d36

6 files changed

Lines changed: 48 additions & 48 deletions

File tree

‎Dockerfile‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ COPY internal/ internal/
2323
COPY pkg/ pkg/
2424
COPY manifests/ manifests/
2525

26+
COPY gotemplates/ gotemplates/
27+
2628
# Build
2729
RUN CGO_ENABLED=0 GOOS=linux GOARCH=${TARGETARCH} go build -ldflags '-w -s' -o manager main.go
2830

@@ -34,6 +36,6 @@ ENV USER_UID=1001
3436
ENV GROUP_UID=1001
3537
COPY --from=builder --chown=${USER_UID}:${GROUP_UID} /workspace/manager /operator/manager
3638
COPY --from=builder --chown=${USER_UID}:${GROUP_UID} /workspace/manifests /operator/manifests
37-
COPY --chown=${USER_UID}:${GROUP_UID} gotemplates/ /operator/gotemplates
39+
COPY --from=builder --chown=${USER_UID}:${GROUP_UID} /workspace/gotemplates /operator/gotemplates
3840
USER ${USER_UID}:${GROUP_UID}
3941
ENTRYPOINT ["/operator/manager"]

‎internal/controller/controller_test.go‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,13 @@ import (
3030
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3131
"k8s.io/apimachinery/pkg/runtime"
3232
"k8s.io/apimachinery/pkg/types"
33+
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
3334
"k8s.io/client-go/rest"
3435
"k8s.io/client-go/tools/record"
35-
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
36-
ctrlconfig "sigs.k8s.io/controller-runtime/pkg/config"
3736
"sigs.k8s.io/controller-runtime/pkg/cache"
3837
"sigs.k8s.io/controller-runtime/pkg/client"
3938
"sigs.k8s.io/controller-runtime/pkg/client/fake"
39+
ctrlconfig "sigs.k8s.io/controller-runtime/pkg/config"
4040
"sigs.k8s.io/controller-runtime/pkg/healthz"
4141
"sigs.k8s.io/controller-runtime/pkg/manager"
4242
"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -58,26 +58,26 @@ func newFakeManager(c client.Client, s *runtime.Scheme) *fakeManager {
5858
return &fakeManager{client: c, scheme: s}
5959
}
6060

61-
func (f *fakeManager) GetClient() client.Client { return f.client }
62-
func (f *fakeManager) GetScheme() *runtime.Scheme { return f.scheme }
63-
func (f *fakeManager) GetConfig() *rest.Config { panic("not implemented") }
64-
func (f *fakeManager) GetCache() cache.Cache { panic("not implemented") }
65-
func (f *fakeManager) GetFieldIndexer() client.FieldIndexer { panic("not implemented") }
66-
func (f *fakeManager) GetEventRecorderFor(_ string) record.EventRecorder { panic("not implemented") }
67-
func (f *fakeManager) GetRESTMapper() meta.RESTMapper { panic("not implemented") }
68-
func (f *fakeManager) GetAPIReader() client.Reader { panic("not implemented") }
69-
func (f *fakeManager) GetHTTPClient() *http.Client { panic("not implemented") }
70-
func (f *fakeManager) Add(_ manager.Runnable) error { panic("not implemented") }
71-
func (f *fakeManager) Elected() <-chan struct{} { panic("not implemented") }
61+
func (f *fakeManager) GetClient() client.Client { return f.client }
62+
func (f *fakeManager) GetScheme() *runtime.Scheme { return f.scheme }
63+
func (f *fakeManager) GetConfig() *rest.Config { panic("not implemented") }
64+
func (f *fakeManager) GetCache() cache.Cache { panic("not implemented") }
65+
func (f *fakeManager) GetFieldIndexer() client.FieldIndexer { panic("not implemented") }
66+
func (f *fakeManager) GetEventRecorderFor(_ string) record.EventRecorder { panic("not implemented") }
67+
func (f *fakeManager) GetRESTMapper() meta.RESTMapper { panic("not implemented") }
68+
func (f *fakeManager) GetAPIReader() client.Reader { panic("not implemented") }
69+
func (f *fakeManager) GetHTTPClient() *http.Client { panic("not implemented") }
70+
func (f *fakeManager) Add(_ manager.Runnable) error { panic("not implemented") }
71+
func (f *fakeManager) Elected() <-chan struct{} { panic("not implemented") }
7272
func (f *fakeManager) AddMetricsServerExtraHandler(_ string, _ http.Handler) error {
7373
panic("not implemented")
7474
}
7575
func (f *fakeManager) AddHealthzCheck(_ string, _ healthz.Checker) error { panic("not implemented") }
7676
func (f *fakeManager) AddReadyzCheck(_ string, _ healthz.Checker) error { panic("not implemented") }
77-
func (f *fakeManager) Start(_ context.Context) error { panic("not implemented") }
78-
func (f *fakeManager) GetWebhookServer() webhook.Server { panic("not implemented") }
79-
func (f *fakeManager) GetLogger() logr.Logger { panic("not implemented") }
80-
func (f *fakeManager) GetControllerOptions() ctrlconfig.Controller { panic("not implemented") }
77+
func (f *fakeManager) Start(_ context.Context) error { panic("not implemented") }
78+
func (f *fakeManager) GetWebhookServer() webhook.Server { panic("not implemented") }
79+
func (f *fakeManager) GetLogger() logr.Logger { panic("not implemented") }
80+
func (f *fakeManager) GetControllerOptions() ctrlconfig.Controller { panic("not implemented") }
8181

8282
// newTestLogger creates a logger suitable for unit tests.
8383
func newTestLogger() *logger.Logger {

‎pkg/subroutines/deployment.go‎

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ import (
3535

3636
const DeploymentSubroutineName = "DeploymentSubroutine"
3737

38+
const (
39+
deploymentTechFluxCD = "fluxcd"
40+
deploymentTechArgoCD = "argocd"
41+
)
42+
3843
type DeploymentSubroutine struct {
3944
clientInfra client.Client
4045
clientRuntime client.Client
@@ -209,7 +214,7 @@ func (r *DeploymentSubroutine) Process(ctx context.Context, runtimeObj runtimeob
209214
}
210215
deploymentTech, _ := tmplVars["deploymentTechnology"].(string)
211216
if deploymentTech == "" {
212-
deploymentTech = "fluxcd" // default to fluxcd if not in profile
217+
deploymentTech = deploymentTechFluxCD // default to fluxcd if not in profile
213218
}
214219
deploymentTech = strings.ToLower(deploymentTech)
215220

@@ -220,7 +225,7 @@ func (r *DeploymentSubroutine) Process(ctx context.Context, runtimeObj runtimeob
220225
log.Error().Err(err).Msg("Failed to get cert-manager resource")
221226
return ctrl.Result{}, errors.NewOperatorError(err, false, false)
222227
}
223-
if deploymentTech == "argocd" {
228+
if deploymentTech == deploymentTechArgoCD {
224229
// For ArgoCD Applications, check status.sync.status and status.health.status directly
225230
// ArgoCD Applications may not have conditions initially, so check status fields directly
226231
syncStatus, found, _ := unstructured.NestedString(rel.Object, "status", "sync", "status")
@@ -234,7 +239,7 @@ func (r *DeploymentSubroutine) Process(ctx context.Context, runtimeObj runtimeob
234239
}
235240
}
236241

237-
if deploymentTech == "fluxcd" {
242+
if deploymentTech == deploymentTechFluxCD {
238243
// For FluxCD HelmReleases, check Ready condition
239244
if !matchesConditionWithStatus(rel, "Ready", "True") {
240245
return ctrl.Result{}, errors.NewOperatorError(errors.New("cert-manager Release is not ready"), true, false)
@@ -271,7 +276,7 @@ func (r *DeploymentSubroutine) Process(ctx context.Context, runtimeObj runtimeob
271276
log.Error().Err(err).Msg("Failed to get istio-istiod resource")
272277
return ctrl.Result{}, errors.NewOperatorError(err, false, false)
273278
}
274-
if deploymentTech == "argocd" {
279+
if deploymentTech == deploymentTechArgoCD {
275280
// For ArgoCD Applications, check status.sync.status and status.health.status directly
276281
syncStatus, found, _ := unstructured.NestedString(rel.Object, "status", "sync", "status")
277282
healthStatus, healthFound, _ := unstructured.NestedString(rel.Object, "status", "health", "status")
@@ -284,7 +289,7 @@ func (r *DeploymentSubroutine) Process(ctx context.Context, runtimeObj runtimeob
284289
}
285290
}
286291

287-
if deploymentTech == "fluxcd" {
292+
if deploymentTech == deploymentTechFluxCD {
288293
// For FluxCD HelmReleases, check Ready condition
289294
if !matchesConditionWithStatus(rel, "Ready", "True") {
290295
return ctrl.Result{}, errors.NewOperatorError(errors.New("istio-istiod Release is not ready"), true, false)
@@ -361,7 +366,7 @@ func (r *DeploymentSubroutine) templateVarsFromProfileInfra(ctx context.Context,
361366
}
362367

363368
// Add deploymentTechnology from profile or templateVars (defaults to fluxcd if not specified)
364-
deploymentTech := "fluxcd" // default
369+
deploymentTech := deploymentTechFluxCD // default
365370
if deploymentTechFromProfile, ok := infraProfileMap["deploymentTechnology"].(string); ok && deploymentTechFromProfile != "" {
366371
deploymentTech = deploymentTechFromProfile
367372
}
@@ -370,17 +375,14 @@ func (r *DeploymentSubroutine) templateVarsFromProfileInfra(ctx context.Context,
370375
}
371376
// Normalize to lowercase
372377
deploymentTech = strings.ToLower(deploymentTech)
373-
if deploymentTech != "fluxcd" && deploymentTech != "argocd" {
374-
deploymentTech = "fluxcd" // default to fluxcd if invalid
378+
if deploymentTech != deploymentTechFluxCD && deploymentTech != deploymentTechArgoCD {
379+
deploymentTech = deploymentTechFluxCD // default to fluxcd if invalid
375380
}
376381
infraProfileMap["deploymentTechnology"] = deploymentTech
377382

378383
// Merge infra profile (base) with templateVars (overrides)
379384
// templateVars take precedence over profile values
380-
log, err := logger.New(logger.DefaultConfig())
381-
if err != nil {
382-
return nil, errors.Wrap(err, "Failed to create logger")
383-
}
385+
log := logger.LoadLoggerFromContext(ctx).ChildLogger("subroutine", r.GetName())
384386
tmplVars, err := merge.MergeMaps(infraProfileMap, templateVarsMap, log)
385387
if err != nil {
386388
return nil, errors.Wrap(err, "Failed to merge infra profile with templateVars")
@@ -523,10 +525,7 @@ func (r *DeploymentSubroutine) buildRuntimeTemplateVars(ctx context.Context, ins
523525
// buildComponentsTemplateVars parses components profile using TemplateVars and produces the data
524526
// structure expected by gotemplates/components (root keys: values, releaseNamespace).
525527
func (r *DeploymentSubroutine) buildComponentsTemplateVars(ctx context.Context, inst *v1alpha1.PlatformMesh, templateVars apiextensionsv1.JSON) (map[string]interface{}, error) {
526-
log, err := logger.New(logger.DefaultConfig())
527-
if err != nil {
528-
return nil, errors.Wrap(err, "Failed to create logger")
529-
}
528+
log := logger.LoadLoggerFromContext(ctx).ChildLogger("subroutine", r.GetName())
530529

531530
// Load components profile from ConfigMap
532531
_, componentsProfileYaml, err := r.loadProfileSections(ctx, inst)
@@ -659,7 +658,7 @@ func (r *DeploymentSubroutine) buildComponentsTemplateVars(ctx context.Context,
659658
}
660659

661660
// Add deploymentTechnology from profile or templateVars (defaults to fluxcd if not specified)
662-
deploymentTech := "fluxcd" // default
661+
deploymentTech := deploymentTechFluxCD // default
663662
if deploymentTechFromProfile, ok := values["deploymentTechnology"].(string); ok && deploymentTechFromProfile != "" {
664663
deploymentTech = deploymentTechFromProfile
665664
}
@@ -668,13 +667,13 @@ func (r *DeploymentSubroutine) buildComponentsTemplateVars(ctx context.Context,
668667
}
669668
// Normalize to lowercase
670669
deploymentTech = strings.ToLower(deploymentTech)
671-
if deploymentTech != "fluxcd" && deploymentTech != "argocd" {
672-
deploymentTech = "fluxcd" // default to fluxcd if invalid
670+
if deploymentTech != deploymentTechFluxCD && deploymentTech != deploymentTechArgoCD {
671+
deploymentTech = deploymentTechFluxCD // default to fluxcd if invalid
673672
}
674673
data["deploymentTechnology"] = deploymentTech
675674

676675
// Calculate sync waves for ArgoCD Applications based on dependsOn
677-
if deploymentTech == "argocd" {
676+
if deploymentTech == deploymentTechArgoCD {
678677
if err := calculateSyncWaves(mergedServices); err != nil {
679678
log.Warn().Err(err).Msg("Failed to calculate sync waves, continuing without sync wave annotations")
680679
}
@@ -1036,7 +1035,7 @@ func (r *DeploymentSubroutine) renderAndApplyInfraTemplates(ctx context.Context,
10361035

10371036
deploymentTech, ok := tmplVars["deploymentTechnology"].(string)
10381037
if !ok {
1039-
deploymentTech = "fluxcd"
1038+
deploymentTech = deploymentTechFluxCD
10401039
}
10411040
deploymentTech = strings.ToLower(deploymentTech)
10421041

@@ -1072,7 +1071,7 @@ func (r *DeploymentSubroutine) renderAndApplyComponentsInfraTemplates(ctx contex
10721071

10731072
deploymentTech, ok := tmplVars["deploymentTechnology"].(string)
10741073
if !ok {
1075-
deploymentTech = "fluxcd"
1074+
deploymentTech = deploymentTechFluxCD
10761075
}
10771076
deploymentTech = strings.ToLower(deploymentTech)
10781077

@@ -1295,7 +1294,7 @@ func getDeploymentResource(ctx context.Context, client client.Client, resourceNa
12951294
deploymentTech = strings.ToLower(deploymentTech)
12961295
obj := &unstructured.Unstructured{}
12971296

1298-
if deploymentTech == "argocd" {
1297+
if deploymentTech == deploymentTechArgoCD {
12991298
obj.SetGroupVersionKind(schema.GroupVersionKind{Group: "argoproj.io", Version: "v1alpha1", Kind: "Application"})
13001299
} else {
13011300
// Default to FluxCD

‎pkg/subroutines/deployment_helpers.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,11 +270,11 @@ func (r *DeploymentSubroutine) preserveExistingArgoSourceFields(
270270
// For fluxcd: skips application files.
271271
func deploymentTechFileFilter(deploymentTech string, log *logger.Logger) func(fileName string) bool {
272272
return func(fileName string) bool {
273-
if deploymentTech == "argocd" && (strings.HasPrefix(fileName, "helmrelease") || strings.HasPrefix(fileName, "kustomization")) {
273+
if deploymentTech == deploymentTechArgoCD && (strings.HasPrefix(fileName, "helmrelease") || strings.HasPrefix(fileName, "kustomization")) {
274274
log.Debug().Str("file", fileName).Str("deploymentTechnology", deploymentTech).Msg("Skipping FluxCD template, ArgoCD is enabled")
275275
return true
276276
}
277-
if deploymentTech == "fluxcd" && strings.HasPrefix(fileName, "application") {
277+
if deploymentTech == deploymentTechFluxCD && strings.HasPrefix(fileName, "application") {
278278
log.Debug().Str("file", fileName).Str("deploymentTechnology", deploymentTech).Msg("Skipping ArgoCD template, FluxCD is enabled")
279279
return true
280280
}

‎pkg/subroutines/providersecret.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,4 +337,3 @@ func (r *ProvidersecretSubroutine) HandleInitializerConnection(
337337

338338
return ctrl.Result{}, nil
339339
}
340-

‎test/e2e/kind/helpers.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package e2e
33
import (
44
"bytes"
55
"context"
6+
stderrors "errors"
67
"fmt"
78
"html/template"
89
"os"
@@ -24,14 +25,13 @@ func applyUnstructureds(
2425
path string,
2526
log *logger.Logger,
2627
) error {
27-
var errRet error = nil
28+
var errRet error
2829
for _, obj := range objs {
2930
if obj.Object == nil {
3031
continue
3132
}
32-
err := k8sClient.Patch(ctx, &obj, client.Apply, client.FieldOwner("platform-mesh-operator"))
33-
if err != nil {
34-
errRet = errors.Wrap(errRet, "Failed to apply manifest file: %s (%s/%s)", path, obj.GetKind(), obj.GetName())
33+
if err := k8sClient.Patch(ctx, &obj, client.Apply, client.FieldOwner("platform-mesh-operator")); err != nil {
34+
errRet = stderrors.Join(errRet, fmt.Errorf("failed to apply manifest file: %s (%s/%s): %w", path, obj.GetKind(), obj.GetName(), err))
3535
}
3636
}
3737
return errRet

0 commit comments

Comments
 (0)