Fix the results reconciler's manifest reference

Earlier reconciler's manifest value was being changed every cycle. This fixes is so that, we have same manifest every reconciler. We take value from reconciler at the start of reconcile cycle and transform it.
tektoncd · Feb 21, 2024 · ffe254d · ffe254d
1 parent ed4443c
commit ffe254d
Show file tree

Hide file tree

Showing 7 changed files with 21 additions and 24 deletions.
diff --git a/pkg/reconciler/common/transformers.go b/pkg/reconciler/common/transformers.go
@@ -488,7 +488,6 @@ func replaceNamespaceInDBAddress(envs []corev1.EnvVar, targetNamespace string) [
 		if slices.Contains(req, e.Name) {
 			envs[i].Value = strings.ReplaceAll(e.Value, "tekton-pipelines", targetNamespace)
 		}
-
 	}
 	return envs
 }

diff --git a/pkg/reconciler/kubernetes/tektonresult/controller.go b/pkg/reconciler/kubernetes/tektonresult/controller.go
@@ -71,7 +71,7 @@ func NewExtendedController(generator common.ExtensionGenerator) injection.Contro
 			kubeClientSet:     kubeclient.Get(ctx),
 			operatorClientSet: operatorclient.Get(ctx),
 			extension:         generator(ctx),
-			manifest:          manifest,
+			manifest:          &manifest,
 			pipelineInformer:  tektonPipelineInformer.Get(ctx),
 			operatorVersion:   operatorVer,
 			resultsVersion:    resultsVer,

diff --git a/pkg/reconciler/kubernetes/tektonresult/filter.go b/pkg/reconciler/kubernetes/tektonresult/filter.go
@@ -26,10 +26,11 @@ const (
 	servicePostgresDB = "tekton-results-postgres-service"
 )
 
-func (r *Reconciler) filterExternalDB(tr *v1alpha1.TektonResult) {
+func filterExternalDB(tr *v1alpha1.TektonResult, manifest mf.Manifest) *mf.Manifest {
 	if tr.Spec.IsExternalDB {
-		r.manifest = r.manifest.Filter(mf.Not(mf.All(mf.ByKind("StatefulSet"), mf.ByName(statefulSetDB))))
-		r.manifest = r.manifest.Filter(mf.Not(mf.All(mf.ByKind("ConfigMap"), mf.ByName(configPostgresDB))))
-		r.manifest = r.manifest.Filter(mf.Not(mf.All(mf.ByKind("Service"), mf.ByName(servicePostgresDB))))
+		manifest = manifest.Filter(mf.Not(mf.All(mf.ByKind("StatefulSet"), mf.ByName(statefulSetDB))))
+		manifest = manifest.Filter(mf.Not(mf.All(mf.ByKind("ConfigMap"), mf.ByName(configPostgresDB))))
+		manifest = manifest.Filter(mf.Not(mf.All(mf.ByKind("Service"), mf.ByName(servicePostgresDB))))
 	}
+	return &manifest
 }
diff --git a/pkg/reconciler/kubernetes/tektonresult/filter_test.go b/pkg/reconciler/kubernetes/tektonresult/filter_test.go
@@ -32,17 +32,13 @@ func Test_filterExternalDB(t *testing.T) {
 	num := len(manifest.Resources())
 	assert.Equal(t, num, 4)
 	assert.Equal(t, manifest.Resources()[0].GetName(), statefulSetDB)
-	r := &Reconciler{
-		manifest: manifest,
-	}
-	r.filterExternalDB(&v1alpha1.TektonResult{
+	manifest = *filterExternalDB(&v1alpha1.TektonResult{
 		Spec: v1alpha1.TektonResultSpec{
 			ResultsAPIProperties: v1alpha1.ResultsAPIProperties{
 				IsExternalDB: true,
 			},
 		},
-	})
-	manifest = r.manifest
+	}, manifest)
 	num = len(manifest.Resources())
 	assert.Equal(t, num, 1)
 	assert.Equal(t, manifest.Resources()[0].GetName(), statefulSetDB+"-external")

diff --git a/pkg/reconciler/kubernetes/tektonresult/installerset.go b/pkg/reconciler/kubernetes/tektonresult/installerset.go
@@ -26,10 +26,9 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func (r *Reconciler) createInstallerSet(ctx context.Context, tr *v1alpha1.TektonResult) (*v1alpha1.TektonInstallerSet, error) {
+func (r *Reconciler) createInstallerSet(ctx context.Context, tr *v1alpha1.TektonResult, manifest mf.Manifest) (*v1alpha1.TektonInstallerSet, error) {
 
-	r.filterExternalDB(tr)
-	if err := r.transform(ctx, &r.manifest, tr); err != nil {
+	if err := r.transform(ctx, &manifest, tr); err != nil {
 		tr.Status.MarkNotReady("transformation failed: " + err.Error())
 		return nil, err
 	}
@@ -44,7 +43,7 @@ func (r *Reconciler) createInstallerSet(ctx context.Context, tr *v1alpha1.Tekton
 	}
 
 	// create installer set
-	tis := r.makeInstallerSet(tr, r.manifest, specHash)
+	tis := r.makeInstallerSet(tr, manifest, specHash)
 	createdIs, err := r.operatorClientSet.OperatorV1alpha1().TektonInstallerSets().
 		Create(ctx, tis, metav1.CreateOptions{})
 	if err != nil {

diff --git a/pkg/reconciler/kubernetes/tektonresult/tektonresult.go b/pkg/reconciler/kubernetes/tektonresult/tektonresult.go
@@ -53,7 +53,7 @@ type Reconciler struct {
 	// manifests are immutable, and any created during reconcile are
 	// expected to be appended to this one, obviating the passing of
 	// client & logger
-	manifest mf.Manifest
+	manifest *mf.Manifest
 	// Platform-specific behavior to affect the transform
 	extension common.Extension
 
@@ -112,6 +112,8 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tr *v1alpha1.TektonResul
 
 	logger.Infow("Reconciling TektonResults", "status", tr.Status)
 
+	manifest := *r.manifest
+
 	if tr.GetName() != v1alpha1.ResultResourceName {
 		msg := fmt.Sprintf("Resource ignored, Expected Name: %s, Got Name: %s",
 			v1alpha1.ResultResourceName,
@@ -172,8 +174,9 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tr *v1alpha1.TektonResul
 	if err != nil {
 		return err
 	}
+
 	if existingInstallerSet == "" {
-		createdIs, err := r.createInstallerSet(ctx, tr)
+		createdIs, err := r.createInstallerSet(ctx, tr, manifest)
 		if err != nil {
 			return err
 		}
@@ -186,7 +189,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tr *v1alpha1.TektonResul
 		Get(ctx, existingInstallerSet, metav1.GetOptions{})
 	if err != nil {
 		if apierrors.IsNotFound(err) {
-			createdIs, err := r.createInstallerSet(ctx, tr)
+			createdIs, err := r.createInstallerSet(ctx, tr, manifest)
 			if err != nil {
 				return err
 			}
@@ -242,9 +245,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tr *v1alpha1.TektonResul
 
 		if lastAppliedHash != expectedSpecHash {
 
-			r.filterExternalDB(tr)
-
-			if err := r.transform(ctx, &r.manifest, tr); err != nil {
+			if err := r.transform(ctx, &manifest, tr); err != nil {
 				logger.Error("manifest transformation failed:  ", err)
 				return err
 			}
@@ -255,7 +256,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tr *v1alpha1.TektonResul
 			installedTIS.SetAnnotations(current)
 
 			// Update the manifests
-			installedTIS.Spec.Manifests = r.manifest.Resources()
+			installedTIS.Spec.Manifests = manifest.Resources()
 
 			if _, err = r.operatorClientSet.OperatorV1alpha1().TektonInstallerSets().
 				Update(ctx, installedTIS, metav1.UpdateOptions{}); err != nil {

diff --git a/pkg/reconciler/kubernetes/tektonresult/transform.go b/pkg/reconciler/kubernetes/tektonresult/transform.go
@@ -68,6 +68,7 @@ func (r *Reconciler) transform(ctx context.Context, manifest *mf.Manifest, comp
 	resultImgs := common.ToLowerCaseKeys(common.ImagesFromEnv(common.ResultsImagePrefix))
 
 	targetNs := comp.GetSpec().GetTargetNamespace()
+	manifest = filterExternalDB(instance, *manifest)
 	extra := []mf.Transformer{
 		common.InjectOperandNameLabelOverwriteExisting(v1alpha1.OperandTektoncdResults),
 		common.ApplyProxySettings,
@@ -98,7 +99,7 @@ func (r *Reconciler) transform(ctx context.Context, manifest *mf.Manifest, comp
 
 func enablePVCLogging(p v1alpha1.ResultsAPIProperties) mf.Transformer {
 	return func(u *unstructured.Unstructured) error {
-		if p.LoggingPVCName == "" || p.LogsPath == "" || u.GetKind() != "Deployment" || u.GetName() != deploymentAPI {
+		if !p.LogsAPI || p.LoggingPVCName == "" || p.LogsPath == "" || u.GetKind() != "Deployment" || u.GetName() != deploymentAPI {
 			return nil
 		}