Merge pull request #188 from acmenezes/refactor_decouple

Refactor decouple

Author: exe-prow-github-app[bot]

cmd/check.go

@@ -7,7 +7,6 @@ import (
 	"os"
 
 	"github.com/opdev/opcap/internal/capability"
-	"github.com/opdev/opcap/internal/logger"
 	"github.com/opdev/opcap/internal/operator"
 
 	pkgserverv1 "github.com/operator-framework/operator-lifecycle-manager/pkg/package-server/apis/operators/v1"
@@ -45,7 +44,7 @@ Flags:
 			return types.Error{Msg: "Unable to create OpCap client."}
 		}
 		var packageManifestList pkgserverv1.PackageManifestList
-		err = psc.ListPackageManifests(context.TODO(), &packageManifestList, checkflags)
+		err = psc.ListPackageManifests(context.TODO(), &packageManifestList, checkflags.CatalogSource, checkflags.Packages)
 		if err != nil {
 			return types.Error{Msg: "Unable to list PackageManifests.\n" + err.Error()}
 		}
@@ -64,17 +63,27 @@ Flags:
 		return nil
 	},
 	Run: func(cmd *cobra.Command, args []string) {
-		// Build auditor by catalog
-		auditor, err := capability.BuildAuditorByCatalog(checkflags)
-		if err != nil {
-			logger.Sugar.Fatal("Unable to build auditor")
+		capAuditor := &capability.CapAuditor{
+			AuditPlan:              checkflags.AuditPlan,
+			CatalogSource:          checkflags.CatalogSource,
+			CatalogSourceNamespace: checkflags.CatalogSourceNamespace,
+			Packages:               checkflags.Packages,
 		}
+
 		// run all dynamically built audits in the auditor workqueue
-		auditor.RunAudits()
+		capAuditor.RunAudits()
 	},
 }
 
-var checkflags operator.OperatorCheckOptions
+type CheckCommandFlags struct {
+	AuditPlan              []string `json:"auditPlan"`
+	CatalogSource          string   `json:"catalogsource"`
+	CatalogSourceNamespace string   `json:"catalogsourcenamespace"`
+	ListPackages           bool     `json:"listPackages"`
+	Packages               []string `json:"packages"`
+}
+
+var checkflags CheckCommandFlags
 
 func init() {
 
@@ -87,8 +96,7 @@ func init() {
 		"specifies the catalogsource to test against")
 	flags.StringVar(&checkflags.CatalogSourceNamespace, "catalogsourcenamespace", "openshift-marketplace",
 		"specifies the namespace where the catalogsource exists")
-	flags.StringSliceVar(&checkflags.AuditPlan, "auditplan", defaultAuditPlan, "audit plan is the ordered list of operator test functions to be called during a capability audit.")
+	flags.StringSliceVar(&checkflags.AuditPlan, "audit-plan", defaultAuditPlan, "audit plan is the ordered list of operator test functions to be called during a capability audit.")
 	flags.BoolVar(&checkflags.ListPackages, "list-packages", false, "list packages in the catalog")
-	flags.StringSliceVar(&checkflags.FilterPackages, "filter-packages", []string{}, "a list of package(s) which limits audits and/or other flag(s) output")
-	flags.BoolVar(&checkflags.AllInstallModes, "all-installmodes", false, "when set, all install modes supported by an operator will be tested")
+	flags.StringSliceVar(&checkflags.Packages, "packages", []string{}, "a list of package(s) which limits audits and/or other flag(s) output")
 }

internal/capability/audit.go

@@ -10,78 +10,66 @@ import (
 	operatorv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
 )
 
-// Audit defines all the methods used to run a full audit Plan against a single operator
-// All new capability tests should be added to this interface and used with a CapAudit
-// instance and as part of an auditPlan
-type Audit interface {
-	OperatorInstall() error
-	OperandInstall() error
-	OperandCleanUp() error
-	OperatorCleanUp() error
-	Report() error
-}
-
 // CapAudit is an implementation of the Audit interface
-type CapAudit struct {
+type capAudit struct {
 
 	// client has access to all operator methods
-	Client operator.Client
+	client operator.Client
 
 	// OpenShift Cluster Version under test
-	OcpVersion string
+	ocpVersion string
 
 	// namespace is the ns where the operator will be installed
-	Namespace string
+	namespace string
 
 	// operatorGroupData contains information to create operator groups
-	OperatorGroupData operator.OperatorGroupData
+	operatorGroupData operator.OperatorGroupData
 
 	// subscription holds the data to install an operator via OLM
-	Subscription operator.SubscriptionData
+	subscription operator.SubscriptionData
 
 	// Cluster CSV for current operator under test
-	Csv operatorv1alpha1.ClusterServiceVersion
+	csv operatorv1alpha1.ClusterServiceVersion
 
 	// How much time to wait for a CSV before timeout
-	CsvWaitTime time.Duration
+	csvWaitTime time.Duration
 
 	// If the given CSV timed out on install
-	CsvTimeout bool
+	csvTimeout bool
 
 	// auditPlan is a list of functions to be run in sequence in a given audit
 	// all of them must be an implemented method of CapAudit and must be part
 	// of the Audit interface
-	AuditPlan []string
+	auditPlan []string
 
 	// CustomResources stores CR manifests to deploy operands
-	CustomResources []map[string]interface{}
+	customResources []map[string]interface{}
 
 	// Operands stores a list of unstructured custom resources that were created at the API level
 	// This data is used for further analysis on statuses, conditions and other patterns
-	Operands []unstructured.Unstructured
+	operands []unstructured.Unstructured
 }
 
-func newCapAudit(c operator.Client, subscription operator.SubscriptionData, auditPlan []string) (CapAudit, error) {
+func newCapAudit(c operator.Client, subscription operator.SubscriptionData, auditPlan []string) (capAudit, error) {
 
-	// TODO: optimize performance by re-using the namespace for the same operator
-	ns := strings.Join([]string{"opcap", strings.ReplaceAll(subscription.Package, ".", "-"), strings.ToLower(string(subscription.InstallModeType))}, "-")
+	ns := strings.Join([]string{"opcap", strings.ReplaceAll(subscription.Package, ".", "-")}, "-")
 	operatorGroupName := strings.Join([]string{subscription.Name, subscription.Channel, "group"}, "-")
 
 	ocpVersion, err := c.GetOpenShiftVersion()
 	if err != nil {
 		logger.Debugw("Couldn't get OpenShift version for testing", "Err:", err)
-		return CapAudit{}, err
+		return capAudit{}, err
 	}
 
-	return CapAudit{
-		Client:            c,
-		OcpVersion:        ocpVersion,
-		Namespace:         ns,
-		OperatorGroupData: newOperatorGroupData(operatorGroupName, getTargetNamespaces(subscription, ns)),
-		Subscription:      subscription,
-		CsvWaitTime:       time.Minute,
-		CsvTimeout:        false,
-		AuditPlan:         auditPlan,
+	return capAudit{
+		client:            c,
+		ocpVersion:        ocpVersion,
+		namespace:         ns,
+		operatorGroupData: newOperatorGroupData(operatorGroupName, getTargetNamespaces(subscription, ns)),
+		subscription:      subscription,
+		csvWaitTime:       time.Minute,
+		csvTimeout:        false,
+		auditPlan:         auditPlan,
 	}, nil
 }
 

internal/capability/auditor.go

@@ -6,34 +6,26 @@ import (
 	"github.com/opdev/opcap/internal/operator"
 )
 
-// Auditor interface represents the object running capability audits against operators
-// It has methods to create a workqueue with all the package and audit requirements for a
-// particular audit run
-type Auditor interface {
-	BuildWorkQueueByCatalog(opts operator.OperatorCheckOptions) error
-	RunAudits() error
-}
-
 // capAuditor implements Auditor
-type capAuditor struct {
+type CapAuditor struct {
 
-	// Workqueue holds capAudits in a buffered channel in order to execute them
-	WorkQueue chan CapAudit
-}
+	// AuditPlan holds the tests that should be run during an audit
+	AuditPlan []string
 
-// BuildAuditorByCatalog creates a new Auditor with workqueue based on a selected catalog
-func BuildAuditorByCatalog(options operator.OperatorCheckOptions) (capAuditor, error) {
+	// CatalogSource may be built-in OLM or custom
+	CatalogSource string
+	// CatalogSourceNamespace will be openshift-marketplace or custom
+	CatalogSourceNamespace string
 
-	var auditor capAuditor
-	err := auditor.BuildWorkQueueByCatalog(options)
-	if err != nil {
-		logger.Fatalf("Unable to build workqueue err := %s", err.Error())
-	}
-	return auditor, nil
+	// Packages is a subset of packages to be tested from a catalogSource
+	Packages []string
+
+	// WorkQueue holds capAudits in a buffered channel in order to execute them
+	WorkQueue chan capAudit
 }
 
 // BuildWorkQueueByCatalog fills in the auditor workqueue with all package information found in a specific catalog
-func (capAuditor *capAuditor) BuildWorkQueueByCatalog(options operator.OperatorCheckOptions) error {
+func (capAuditor *CapAuditor) buildWorkQueueByCatalog() error {
 
 	c, err := operator.NewOpCapClient()
 	if err != nil {
@@ -43,20 +35,20 @@ func (capAuditor *capAuditor) BuildWorkQueueByCatalog(options operator.OperatorC
 	}
 
 	// Getting subscription data form the package manifests available in the selected catalog
-	subscriptions, err := c.GetSubscriptionData(options)
+	subscriptions, err := c.GetSubscriptionData(capAuditor.CatalogSource, capAuditor.CatalogSourceNamespace, capAuditor.Packages)
 	if err != nil {
-		logger.Errorf("Error while getting bundles from CatalogSource %s: %w", options.CatalogSource, err)
+		logger.Errorf("Error while getting bundles from CatalogSource %s: %w", capAuditor.CatalogSource, err)
 		return err
 	}
 
 	// build workqueue as buffered channel based subscriptionData list size
-	capAuditor.WorkQueue = make(chan CapAudit, len(subscriptions))
+	capAuditor.WorkQueue = make(chan capAudit, len(subscriptions))
 	defer close(capAuditor.WorkQueue)
 
 	// add capAudits to the workqueue
 	for _, subscription := range subscriptions {
 
-		capAudit, err := newCapAudit(c, subscription, options.AuditPlan)
+		capAudit, err := newCapAudit(c, subscription, capAuditor.AuditPlan)
 		if err != nil {
 			logger.Debugf("Couldn't build capAudit for subscription %s", "Err:", err)
 			return err
@@ -70,14 +62,19 @@ func (capAuditor *capAuditor) BuildWorkQueueByCatalog(options operator.OperatorC
 }
 
 // RunAudits executes all selected functions in order for a given audit at a time
-func (capAuditor *capAuditor) RunAudits() error {
+func (capAuditor *CapAuditor) RunAudits() error {
+	err := capAuditor.buildWorkQueueByCatalog()
+	if err != nil {
+		logger.Debugf("Unable to build workqueue err := %s", err.Error())
+		return err
+	}
 
 	// read workqueue for audits
 	for audit := range capAuditor.WorkQueue {
 
 		// read a particular audit's auditPlan for functions
 		// to be executed against operator
-		for _, function := range audit.AuditPlan {
+		for _, function := range audit.auditPlan {
 
 			// run function/method by name
 			m := reflect.ValueOf(&audit).MethodByName(function)

internal/capability/operand_cleanup.go

@@ -2,7 +2,6 @@ package capability
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/opdev/opcap/internal/operator"
 
@@ -14,11 +13,12 @@ import (
 )
 
 // OperandCleanup removes the operand from the OCP cluster in the ca.namespace
-func (ca *CapAudit) OperandCleanUp() error {
-	logger.Debugw("cleaningUp operand for operator", "package", ca.Subscription.Package, "channel", ca.Subscription.Channel, "installmode", ca.Subscription.InstallModeType)
+func (ca *capAudit) OperandCleanUp() error {
+	logger.Debugw("cleaningUp operand for operator", "package", ca.subscription.Package, "channel", ca.subscription.Channel, "installmode",
+		ca.subscription.InstallModeType)
 
-	if len(ca.CustomResources) > 0 {
-		for _, cr := range ca.CustomResources {
+	if len(ca.customResources) > 0 {
+		for _, cr := range ca.customResources {
 			obj := &unstructured.Unstructured{Object: cr}
 
 			// using dynamic client to create Unstructured objests in k8s
@@ -28,7 +28,7 @@ func (ca *CapAudit) OperandCleanUp() error {
 			}
 
 			var crdList apiextensionsv1.CustomResourceDefinitionList
-			err = ca.Client.ListCRDs(context.TODO(), &crdList)
+			err = ca.client.ListCRDs(context.TODO(), &crdList)
 			if err != nil {
 				return err
 			}
@@ -52,12 +52,12 @@ func (ca *CapAudit) OperandCleanUp() error {
 			name := obj.Object["metadata"].(map[string]interface{})["name"].(string)
 
 			// check if CR exists, only then cleanup the operand
-			crInstance, _ := client.Resource(gvr).Namespace(ca.Namespace).Get(context.TODO(), name, v1.GetOptions{})
+			crInstance, _ := client.Resource(gvr).Namespace(ca.namespace).Get(context.TODO(), name, v1.GetOptions{})
 			if crInstance != nil {
 				// delete the resource using the dynamic client
-				err = client.Resource(gvr).Namespace(ca.Namespace).Delete(context.TODO(), name, v1.DeleteOptions{})
+				err = client.Resource(gvr).Namespace(ca.namespace).Delete(context.TODO(), name, v1.DeleteOptions{})
 				if err != nil {
-					fmt.Printf("failed operandCleanUp: %s package: %s error: %s\n", Resource, ca.Subscription.Package, err.Error())
+					logger.Debugf("failed operandCleanUp: %s package: %s error: %s\n", Resource, ca.subscription.Package, err.Error())
 					return err
 				}
 			}