From 52a804419b6f7ee9850f7b30c4844917e4d9272b Mon Sep 17 00:00:00 2001
From: Michal Kralik <michal.kralik@percona.com>
Date: Thu, 22 Feb 2024 16:31:37 +0100
Subject: [PATCH] Resolve a couple TODOs

---
 pkg/install/install.go       | 6 ++++--
 pkg/kubernetes/kubernetes.go | 5 ++---
 pkg/uninstall/uninstall.go   | 1 +
 pkg/upgrade/upgrade.go       | 9 +++++----
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/pkg/install/install.go b/pkg/install/install.go
index 9d7afc28..45c3cfcc 100644
--- a/pkg/install/install.go
+++ b/pkg/install/install.go
@@ -141,6 +141,10 @@ func NewInstall(c Config, l *zap.SugaredLogger) (*Install, error) {
 
 // Run runs the operators installation process.
 func (o *Install) Run(ctx context.Context) error {
+	// TODO: we shall probably split this into "install" and "add namespaces"
+	// Otherwise the logic is hard to maintain - we need to make sure not to,
+	// for example, install a different version of operators per namespace, if
+	// we are always installing the "latest" version.
 	if err := o.populateConfig(); err != nil {
 		return err
 	}
@@ -323,7 +327,6 @@ func (o *Install) provisionEverest(ctx context.Context, v *goversion.Version) er
 			return err
 		}
 	} else {
-		// TODO: revisit - we shall probably not restart but offer upgrade.
 		o.l.Info("Restarting Everest")
 		if err := o.kubeClient.RestartEverest(ctx, everestOperatorName, SystemNamespace); err != nil {
 			return err
@@ -333,7 +336,6 @@ func (o *Install) provisionEverest(ctx context.Context, v *goversion.Version) er
 		}
 	}
 
-	// TODO: get from Everest, not cli.
 	o.l.Info("Updating cluster role bindings for everest-admin")
 	if err := o.kubeClient.UpdateClusterRoleBinding(ctx, everestServiceAccountClusterRoleBinding, o.config.NamespacesList()); err != nil {
 		return err
diff --git a/pkg/kubernetes/kubernetes.go b/pkg/kubernetes/kubernetes.go
index d213c825..1e302811 100644
--- a/pkg/kubernetes/kubernetes.go
+++ b/pkg/kubernetes/kubernetes.go
@@ -1016,16 +1016,15 @@ func (k *Kubernetes) getManifestData(ctx context.Context, version *goversion.Ver
 }
 
 // DeleteEverest downloads the manifest file and deletes it from provisioned k8s cluster.
-// TODO: how do we need to change this after upgrade changes.
 func (k *Kubernetes) DeleteEverest(ctx context.Context, namespace string, version *goversion.Version) error {
 	data, err := k.getManifestData(ctx, version)
 	if err != nil {
-		return errors.Join(err, errors.New("failed downloading everest monitoring file"))
+		return errors.Join(err, errors.New("failed downloading Everest manifest file"))
 	}
 
 	err = k.client.DeleteManifestFile(data, namespace)
 	if err != nil {
-		return errors.Join(err, errors.New("failed deleting manifest file"))
+		return errors.Join(err, errors.New("failed deleting Everest based on a manifest file"))
 	}
 	return nil
 }
diff --git a/pkg/uninstall/uninstall.go b/pkg/uninstall/uninstall.go
index 8811e079..6f68a2b9 100644
--- a/pkg/uninstall/uninstall.go
+++ b/pkg/uninstall/uninstall.go
@@ -116,6 +116,7 @@ This will uninstall Everest and all its components from the cluster.`
 	if err != nil {
 		return err
 	}
+	// TODO: How do we ensure we delete all resources based on the correct Everest version?
 	if err := u.kubeClient.DeleteEverest(ctx, install.SystemNamespace, v); err != nil {
 		return err
 	}
diff --git a/pkg/upgrade/upgrade.go b/pkg/upgrade/upgrade.go
index c722288b..dd895aa3 100644
--- a/pkg/upgrade/upgrade.go
+++ b/pkg/upgrade/upgrade.go
@@ -105,9 +105,10 @@ func (u *Upgrade) Run(ctx context.Context) error {
 		return err
 	}
 
-	// TODO: figure out catalog version
-	u.l.Infof("Upgrading Percona Catalog to %s", upgradeEverestTo)
-	if err := u.kubeClient.InstallPerconaCatalog(ctx, upgradeEverestTo); err != nil {
+	// We cannot use the latest version of catalog yet since
+	// at the time of writing, each catalog version supports only one Everest version.
+	u.l.Infof("Upgrading Percona Catalog to %s", recVer.Catalog)
+	if err := u.kubeClient.InstallPerconaCatalog(ctx, recVer.Catalog); err != nil {
 		return err
 	}
 
@@ -276,7 +277,7 @@ func (u *Upgrade) upgradeOLM(ctx context.Context, minimumVersion *goversion.Vers
 	}
 
 	u.l.Info("Upgrading OLM to version %s", minimumVersion)
-	// TODO: actually upgrade OLM operator instead of installation/skip.
+	// TODO: shall we actually upgrade OLM operator instead of installation/skip?
 	if err := u.kubeClient.InstallOLMOperator(ctx, true); err != nil {
 		return errors.Join(err, errors.New("could not upgrade OLM"))
 	}