Delete istio CRDs when uninstalling istio. (#1000)

* Delete istio CRDs when uninstalling istio.

This is an attemp to fix the flaky tc of the affiliatedcertification
test suite that has been failing too often lately.

Istio CRDs must be removed when uninstalling istio. Otherwise, this
error might appear when re-installing it:

"If istio is not uninstalled properly, the next installation retry fails
because of this error:
Error: INSTALLATION FAILED: Unable to continue with install:
CustomResourceDefinition "wasmplugins.extensions.istio.io" in namespace
"" exists and cannot be imported into the current release: invalid
ownership metadata; annotation validation error: key
"meta.helm.sh/release-namespace" must equal
"affiliatedcert-tests-hrzzgmvrkr": current value is
"affiliatedcert-tests-maahivhcff"

See istio/istio#43204

Also:
- DeferCleanup func installed before the step that installs istio.
- cmd.Run() changed to cmd.CombinedOutput() so cmd output can be shown
  in the gomega error assertion message. If this fix doesn't work, at
  least we'll get more info from the commands that install/uninstall
  istio.
- Added some error return wrappings and minor code refactors.
- Avoided abrupt exit in some scenarios to allow the normal recovery to
  continue.
- Avoid the need of certsuite repo env var if running in container mode.

* Fix typo.

* Avoid returning err when deleting non existing CRDs.

tests/affiliatedcertification/helper/helper.go

@@ -10,7 +10,10 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/redhat-best-practices-for-k8s/certsuite-qe/tests/globalhelper"
+	"github.com/redhat-best-practices-for-k8s/certsuite-qe/tests/utils/crd"
 	utils "github.com/redhat-best-practices-for-k8s/certsuite-qe/tests/utils/operator"
+
+	"github.com/golang/glog"
 )
 
 // AddLabelToInstalledCSV adds given label to existing csv object.
@@ -120,6 +123,29 @@ func DeployOperatorSubscription(subscriptionName, operatorPackage, channel, name
 	return nil
 }
 
+func DeleteIstioCRDs() error {
+	crdList, err := crd.GetAllCustomResourceDefinitions()
+	if err != nil {
+		return fmt.Errorf("failed to list cluster CRDs: %w", err)
+	}
+
+	for i := range crdList {
+		crdName := crdList[i].Name
+		if !strings.Contains(crdName, "istio.io") {
+			continue
+		}
+
+		glog.Infof("Deleting istio CRD %s", crdName)
+
+		err = crd.DeleteCustomResourceDefinition(crdName)
+		if err != nil {
+			return fmt.Errorf("failed to delete istio CRD %s: %w", crdName, err)
+		}
+	}
+
+	return nil
+}
+
 func updateCsv(namespace string, csv *v1alpha1.ClusterServiceVersion) error {
 	_, err := globalhelper.GetAPIClient().ClusterServiceVersions(namespace).Update(
 		context.TODO(), csv, metav1.UpdateOptions{},

tests/affiliatedcertification/tests/affillated_certification_helm_chart.go

@@ -5,6 +5,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	tshelper "github.com/redhat-best-practices-for-k8s/certsuite-qe/tests/affiliatedcertification/helper"
 	tsparams "github.com/redhat-best-practices-for-k8s/certsuite-qe/tests/affiliatedcertification/parameters"
 	"github.com/redhat-best-practices-for-k8s/certsuite-qe/tests/globalhelper"
 	"github.com/redhat-best-practices-for-k8s/certsuite-qe/tests/globalparameters"
@@ -113,17 +114,17 @@ var _ = Describe("Affiliated-certification helm chart certification,", Serial, f
 		By("Check if helm is installed")
 		cmd := exec.Command("/bin/bash", "-c",
 			"helm version")
-		err := cmd.Run()
+		out, err := cmd.CombinedOutput()
 		if err != nil {
-			Skip("helm does not exist please install it to run the test.")
+			Skip("helm does not exist please install it to run the test. Output: " + string(out))
 		}
 
 		By("Check that helm version is v3")
 		cmd = exec.Command("/bin/bash", "-c",
 			"helm version --short | grep v3")
-		err = cmd.Run()
+		out, err = cmd.CombinedOutput()
 		if err != nil {
-			Fail("Helm version is not v3")
+			Fail("Helm version is not v3. Output: " + string(out))
 		}
 
 		By("Delete istio-system namespace")
@@ -138,22 +139,15 @@ var _ = Describe("Affiliated-certification helm chart certification,", Serial, f
 		cmd = exec.Command("/bin/bash", "-c",
 			"helm repo add istio https://istio-release.storage.googleapis.com/charts --force-update "+
 				"&& helm repo update")
-		err = cmd.Run()
-		Expect(err).ToNot(HaveOccurred(), "Error adding istio charts repo")
-
-		By("Install istio-base helm chart")
-		cmd = exec.Command("/bin/bash", "-c",
-			"helm install istio-base istio/base --set defaultRevision=default -n "+randomNamespace+
-				" --set hub=gcr.io/istio-release")
-		err = cmd.Run()
-		Expect(err).ToNot(HaveOccurred(), "Error installing istio-base helm chart")
+		out, err = cmd.CombinedOutput()
+		Expect(err).ToNot(HaveOccurred(), "Error adding istio charts repo. Output: %s", string(out))
 
 		DeferCleanup(func() {
 			By("Remove istio-base helm chart")
 			cmd = exec.Command("/bin/bash", "-c", // uninstall the chart
 				"helm uninstall istio-base --ignore-not-found -n "+randomNamespace)
-			err = cmd.Run()
-			Expect(err).ToNot(HaveOccurred(), "Error uninstalling helm chart")
+			out, err := cmd.CombinedOutput()
+			Expect(err).ToNot(HaveOccurred(), "Error uninstalling helm chart. Output: %s", string(out))
 
 			By("Delete istio-system namespace")
 			err = globalhelper.DeleteNamespaceAndWait("istio-system", tsparams.Timeout)
@@ -162,8 +156,21 @@ var _ = Describe("Affiliated-certification helm chart certification,", Serial, f
 			By("Remove validating webhook configuration")
 			err = globalhelper.DeleteValidatingWebhookConfiguration("istiod-default-validator")
 			Expect(err).ToNot(HaveOccurred(), "Error deleting validating webhook configuration")
+
+			// BugFix: see https://github.com/istio/istio/issues/43204 , which suggests removing CRDs:
+			// https://istio.io/latest/docs/setup/install/helm/#optional-deleting-crds-installed-by-istio
+			By("Delete istio CRDs")
+			err = tshelper.DeleteIstioCRDs()
+			Expect(err).ToNot(HaveOccurred(), "Error deleting istio CRDs.")
 		})
 
+		By("Install istio-base helm chart")
+		cmd = exec.Command("/bin/bash", "-c",
+			"helm install istio-base istio/base --set defaultRevision=default -n "+randomNamespace+
+				" --set hub=gcr.io/istio-release")
+		out, err = cmd.CombinedOutput()
+		Expect(err).ToNot(HaveOccurred(), "Error installing istio-base helm chart. Output: %s", string(out))
+
 		By("Start test")
 		err = globalhelper.LaunchTests(
 			tsparams.TestHelmChartCertified,

tests/globalhelper/reporthelper.go

@@ -203,6 +203,6 @@ func CopyClaimFileToTcFolder(tcName, formattedTcName, reportDir string) {
 
 	err = CopyFiles(srcClaim, dstClaim)
 	if err != nil {
-		glog.Fatalf("failed to copy %s to %s", srcClaim, dstClaim)
+		glog.Errorf("failed to copy %s to %s", srcClaim, dstClaim)
 	}
 }

tests/globalhelper/runhelper.go

@@ -1,6 +1,7 @@
 package globalhelper
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -120,8 +121,9 @@ func launchTestsViaImage(testCaseName string, tcNameForReport string, reportDir
 		return fmt.Errorf("failed to set env var CERTSUITE_LOG_LEVEL: %w", err)
 	}
 
+	var outfile *os.File
 	if debugCertsuite {
-		outfile := GetConfiguration().CreateLogFile(getTestSuiteName(testCaseName), tcNameForReport)
+		outfile = GetConfiguration().CreateLogFile(getTestSuiteName(testCaseName), tcNameForReport)
 
 		defer outfile.Close()
 
@@ -136,13 +138,18 @@ func launchTestsViaImage(testCaseName string, tcNameForReport string, reportDir
 
 	err = cmd.Run()
 	if err != nil {
-		err = fmt.Errorf("failed to run tc: %s, err: %w, cmd: %s",
+		errStr := fmt.Sprintf("failed to run tc: %s, err: %v, cmd: %s",
 			testCaseName, err, cmd.String())
+		if debugCertsuite {
+			errStr += ", outFile=" + outfile.Name()
+		}
+
+		return errors.New(errStr)
 	}
 
 	CopyClaimFileToTcFolder(testCaseName, tcNameForReport, reportDir)
 
-	return err
+	return nil
 }
 
 // LaunchTests stats tests based on given parameters.

tests/utils/config/config.go

@@ -65,38 +65,37 @@ func NewConfig() (*Config, error) {
 	}
 
 	err = readFile(&conf, confFile)
-
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to read config file: %w", err)
 	}
 
 	conf.General.ReportDirAbsPath = filepath.Join(baseDir, conf.General.ReportDirAbsPath)
 
 	err = readEnv(&conf)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to read env vars: %w", err)
 	}
 
 	err = conf.deployCertsuiteConfigDir(confFile)
-
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to deploy certsuite config dir: %w", err)
 	}
 
 	err = conf.deployCertsuiteReportDir(confFile)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to deploy certsuite report dir: %w", err)
 	}
 
 	err = conf.makeDockerConfig()
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to create docker config: %w", err)
 	}
 
-	conf.General.CertsuiteRepoPath, err = conf.defineCertsuiteRepoPath()
-
-	if err != nil {
-		glog.Fatal(err)
+	if conf.General.UseBinary == "true" {
+		conf.General.CertsuiteRepoPath, err = conf.defineCertsuiteRepoPath()
+		if err != nil {
+			glog.Fatalf("Failed to define certsuite repo path: %w", err)
+		}
 	}
 
 	return &conf, nil
@@ -170,7 +169,7 @@ func readFile(cfg *Config, cfgFile string) error {
 
 	err = decoder.Decode(&cfg)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to decode config file: %w", err)
 	}
 
 	return nil
@@ -216,44 +215,49 @@ func checkFileExists(filePath, name string) (string, error) {
 
 func deployCertsuiteDir(confFileName string, dirName string, yamlTag string, envVar string) error {
 	_, err := os.Stat(dirName)
-
-	if os.IsNotExist(err) {
-		glog.V(4).Info(fmt.Sprintf("%s directory is not present. Creating directory", dirName))
-
-		return os.MkdirAll(dirName, 0777)
+	if err == nil {
+		// Directory exists: no need to recreate.
+		return nil
 	}
 
-	if err != nil {
+	if !os.IsNotExist(err) {
 		return fmt.Errorf(
 			"error in verifying the %s directory. Check if either %s is present in %s or "+
 				"%s env var is set", dirName, yamlTag, envVar, confFileName)
 	}
 
-	return err
+	glog.V(4).Info(fmt.Sprintf("%s directory is not present. Creating directory", dirName))
+
+	err = os.MkdirAll(dirName, 0777)
+	if err != nil {
+		return fmt.Errorf("failed to create dir %v: %w", dirName, err)
+	}
+
+	return nil
 }
 
 func (c *Config) makeDockerConfig() error {
 	var configFile *os.File
 
 	err := os.MkdirAll(c.General.DockerConfigDir, 0777)
-
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to create dir %v: %w", c.General.DockerConfigDir, err)
 	}
 
 	err = os.Chdir(c.General.DockerConfigDir)
-
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to change dir to %v: %w", c.General.DockerConfigDir, err)
 	}
 
 	configFile, err = os.Create("config")
-
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to create docker config file: %w", err)
 	}
 
 	_, err = configFile.Write([]byte("{ \"auths\": {} }"))
+	if err != nil {
+		return fmt.Errorf("failed to write docker config content: %w", err)
+	}
 
-	return err
+	return nil
 }

tests/utils/crd/crd.go

@@ -131,3 +131,21 @@ func CreateCustomResourceScale(name, namespace, operatorLabels string, operatorL
 
 	return "success", nil
 }
+
+func GetAllCustomResourceDefinitions() ([]apiextv1.CustomResourceDefinition, error) {
+	crdList, err := globalhelper.GetAPIClient().CustomResourceDefinitions().List(context.TODO(), metav1.ListOptions{})
+	if err != nil {
+		return nil, fmt.Errorf("failed to list CRDs: %w", err)
+	}
+
+	return crdList.Items, nil
+}
+
+func DeleteCustomResourceDefinition(name string) error {
+	err := globalhelper.GetAPIClient().CustomResourceDefinitions().Delete(context.TODO(), name, metav1.DeleteOptions{})
+	if err != nil && !k8serrors.IsNotFound(err) {
+		return err
+	}
+
+	return nil
+}