Allow more than one skip functions per check. (redhat-best-practices-…

…for-k8s#1645)

The check.WithSkipFn(func) has been replaced by a variadic func version
to allow more than one skip functions to be called:
check.WithSkipFn(skipFunc1, skipFunc2...)

By default, the check will be skipped if any skip function returns true
when they're called (which happens before running the CheckFn).

If we need to skip the check in case all the skip functions return true,
the check.WithSkipModeAll() needs to be used.

Examples:
1. Skip check if no deployments *or* no statefulsets were found
(testhelper.SkipIfEmptyAny()):
check := checksdb.NewCheck(id, labels).
  WithSkipCheckFn(testhelper.GetNoDeploymentsUnderTestSkipFn(&env), testhelper.GetNoStatefulSetsUnderTestSkipFn(&env)).
  WithCheckFn(func() error {
    ..
  })

2. We want to skip the check if no deployments *and* no statefulsets
were found (testhelper.SkipIfEmptyAll()):
check := checksdb.NewCheck(id, labels).
  WithSkipCheckFn(testhelper.GetNoDeploymentsUnderTestSkipFn(&env), testhelper.GetNoStatefulSetsUnderTestSkipFn(&env)).
  WithSkipCheckModeAll().
  WithCheckFn(func() error {
    ..
  })

The skip functions are appended to a slice in the same order as they're
passed to the check.WithSkipCheckFn() calls, and it can be called more
than once. E.g.: in the previous examples, this call:
  ...
  WithSkipCheckFn(testhelper.GetNoDeploymentsUnderTestSkipFn(&env), testhelper.GetNoStatefulSetsUnderTestSkipFn(&env)).
  ...
is equal to:
  ...
  WithSkipCheckFn(testhelper.GetNoDeploymentsUnderTestSkipFn(&env)).
  WithSkipCheckFn(testhelper.GetNoStatefulSetsUnderTestSkipFn(&env)).
  ...

Extra changes:
- Added helper skip func creators in the testhelper package for no CRDs,
  no statefulSets and no deployments found.
- Adapted the observability ts.

cnf-certification-test/observability/suite.go

@@ -27,7 +27,6 @@ import (
 	"github.com/test-network-function/cnf-certification-test/cnf-certification-test/identifiers"
 	pdbv1 "github.com/test-network-function/cnf-certification-test/cnf-certification-test/observability/pdb"
 
-	// "github.com/test-network-function/cnf-certification-test/cnf-certification-test/results"
 	"github.com/test-network-function/cnf-certification-test/internal/clientsholder"
 	"github.com/test-network-function/cnf-certification-test/pkg/checksdb"
 	"github.com/test-network-function/cnf-certification-test/pkg/provider"
@@ -44,32 +43,6 @@ var (
 		env = provider.GetTestEnvironment()
 		return nil
 	}
-
-	skipIfNoContainersFn = func() (bool, string) {
-		if len(env.Containers) == 0 {
-			logrus.Warnf("No containers to check...")
-			return true, "There are no containers to check. Please check under test labels."
-		}
-
-		return false, ""
-	}
-
-	skipIfNoCrdsFn = func() (bool, string) {
-		if len(env.Crds) == 0 {
-			logrus.Warn("No CRDs to check.")
-			return true, "There are no CRDs to check."
-		}
-
-		return false, ""
-	}
-
-	skipIfNoDeploymentsNorStatefulSets = func() (bool, string) {
-		if len(env.Deployments) == 0 && len(env.StatefulSets) == 0 {
-			logrus.Warn("No deployments nor statefulsets to check.")
-			return true, "There are no deployments nor statefulsets to check."
-		}
-		return false, ""
-	}
 )
 
 func init() {
@@ -80,7 +53,7 @@ func init() {
 
 	testID, tags := identifiers.GetGinkgoTestIDAndLabels(identifiers.TestLoggingIdentifier)
 	check := checksdb.NewCheck(testID, tags).
-		WithSkipCheckFn(skipIfNoContainersFn).
+		WithSkipCheckFn(testhelper.GetNoContainersUnderTestSkipFn(&env)).
 		WithCheckFn(func(c *checksdb.Check) error {
 			testContainersLogging(c, &env)
 			return nil
@@ -90,7 +63,7 @@ func init() {
 
 	testID, tags = identifiers.GetGinkgoTestIDAndLabels(identifiers.TestCrdsStatusSubresourceIdentifier)
 	check = checksdb.NewCheck(testID, tags).
-		WithSkipCheckFn(skipIfNoCrdsFn).
+		WithSkipCheckFn(testhelper.GetNoCrdsUnderTestSkipFn(&env)).
 		WithCheckFn(func(c *checksdb.Check) error {
 			testCrds(c, &env)
 			return nil
@@ -100,7 +73,7 @@ func init() {
 
 	testID, tags = identifiers.GetGinkgoTestIDAndLabels(identifiers.TestTerminationMessagePolicyIdentifier)
 	check = checksdb.NewCheck(testID, tags).
-		WithSkipCheckFn(skipIfNoContainersFn).
+		WithSkipCheckFn(testhelper.GetNoContainersUnderTestSkipFn(&env)).
 		WithCheckFn(func(c *checksdb.Check) error {
 			testTerminationMessagePolicy(c, &env)
 			return nil
@@ -110,7 +83,8 @@ func init() {
 
 	testID, tags = identifiers.GetGinkgoTestIDAndLabels(identifiers.TestPodDisruptionBudgetIdentifier)
 	check = checksdb.NewCheck(testID, tags).
-		WithSkipCheckFn(skipIfNoDeploymentsNorStatefulSets).
+		WithSkipCheckFn(testhelper.GetNoDeploymentsUnderTestSkipFn(&env), testhelper.GetNoStatefulSetsUnderTestSkipFn(&env)).
+		WithSkipModeAll().
 		WithCheckFn(func(c *checksdb.Check) error {
 			testPodDisruptionBudgets(c, &env)
 			return nil

pkg/checksdb/check.go

@@ -17,6 +17,13 @@ const (
 	CheckResultAborted = "aborted"
 )
 
+type skipMode int
+
+const (
+	SkipModeAny skipMode = iota
+	SkipModeAll
+)
+
 type CheckResult string
 
 func (cr CheckResult) String() string {
@@ -31,7 +38,8 @@ type Check struct {
 	BeforeCheckFn, AfterCheckFn func(check *Check) error
 	CheckFn                     func(check *Check) error
 
-	SkipCheckFn func() (skip bool, reason string)
+	SkipCheckFns []func() (skip bool, reason string)
+	SkipMode     skipMode
 
 	Result         CheckResult
 	CapturedOutput string
@@ -77,12 +85,34 @@ func (check *Check) WithAfterCheckFn(afterCheckFn func(check *Check) error) *Che
 	return check
 }
 
-func (check *Check) WithSkipCheckFn(skipCheckFn func() (skip bool, reason string)) *Check {
+func (check *Check) WithSkipCheckFn(skipCheckFn ...func() (skip bool, reason string)) *Check {
+	if check.Error != nil {
+		return check
+	}
+
+	check.SkipCheckFns = append(check.SkipCheckFns, skipCheckFn...)
+
+	return check
+}
+
+// This modifier is provided for the sake of completeness, but it's not necessary to use it,
+// as the SkipModeAny is the default skip mode.
+func (check *Check) WithSkipModeAny() *Check {
+	if check.Error != nil {
+		return check
+	}
+
+	check.SkipMode = SkipModeAny
+
+	return check
+}
+
+func (check *Check) WithSkipModeAll() *Check {
 	if check.Error != nil {
 		return check
 	}
 
-	check.SkipCheckFn = skipCheckFn
+	check.SkipMode = SkipModeAll
 
 	return check
 }

pkg/checksdb/checksgroup.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"runtime/debug"
+	"strings"
 
 	"github.com/sirupsen/logrus"
 )
@@ -195,13 +196,56 @@ func runAfterEachFn(group *ChecksGroup, check *Check, remainingChecks []*Check)
 	return nil
 }
 
-func shouldSkipCheck(check *Check) (skip bool, reason string) {
-	if check.SkipCheckFn == nil {
-		return false, ""
+func shouldSkipCheck(check *Check) (skip bool, reasons []string) {
+	if len(check.SkipCheckFns) == 0 {
+		return false, []string{}
 	}
 
-	logrus.Tracef("Running check %s skipCheck function.", check.ID)
-	return check.SkipCheckFn()
+	logrus.Tracef("Running check %s skipCheck functions (%d).", check.ID, len(check.SkipCheckFns))
+
+	// Short-circuit
+	if len(check.SkipCheckFns) == 0 {
+		return false, []string{}
+	}
+
+	// Save the skipFn index in case it panics so it can be used in the log trace.
+	currentSkipFnIndex := 0
+
+	defer func() {
+		if r := recover(); r != nil {
+			stackTrace := fmt.Sprint(r) + "\n" + string(debug.Stack())
+			logrus.Errorf("Skip check function (idx=%d) panic'ed: %s", currentSkipFnIndex, stackTrace)
+			skip = true
+			reasons = []string{fmt.Sprintf("skipCheckFn (idx=%d) panic:\n%s", currentSkipFnIndex, stackTrace)}
+		}
+	}()
+
+	// Call all the skip functions first.
+	for _, skipFn := range check.SkipCheckFns {
+		if skip, reason := skipFn(); skip {
+			reasons = append(reasons, reason)
+		}
+		currentSkipFnIndex++
+	}
+
+	// If none of the skipFn returned true, exit now.
+	if len(reasons) == 0 {
+		return false, []string{}
+	}
+
+	// Now we need to check the skipMode for this check.
+	switch check.SkipMode {
+	case SkipModeAny:
+		return true, reasons
+	case SkipModeAll:
+		// Only skip if all the skipFn returned true.
+		if len(reasons) == len(check.SkipCheckFns) {
+			return true, reasons
+		}
+		return false, []string{}
+	}
+
+	return false, []string{}
 }
 
 func runCheck(check *Check, group *ChecksGroup, remainingChecks []*Check) (err error) {
@@ -298,9 +342,9 @@ func (group *ChecksGroup) RunChecks(labelsExpr string, stopChan <-chan bool) (er
 
 		if !beforeEachFailed {
 			// Should we skip this check?
-			skip, reason := shouldSkipCheck(check)
+			skip, reasons := shouldSkipCheck(check)
 			if skip {
-				skipCheck(check, reason)
+				skipCheck(check, strings.Join(reasons, ", "))
 			} else {
 				err := runCheck(check, group, remainingChecks)
 				if err != nil {

pkg/testhelper/testhelper.go

@@ -329,7 +329,7 @@ func ResultToString(result int) (str string) {
 func GetNoContainersUnderTestSkipFn(env *provider.TestEnvironment) func() (bool, string) {
 	return func() (bool, string) {
 		if len(env.Containers) == 0 {
-			return true, "There are no containers to check. Please check under test labels."
+			return true, "no containers to check found"
 		}
 
 		return false, ""
@@ -339,7 +339,37 @@ func GetNoContainersUnderTestSkipFn(env *provider.TestEnvironment) func() (bool,
 func GetNoPodsUnderTestSkipFn(env *provider.TestEnvironment) func() (bool, string) {
 	return func() (bool, string) {
 		if len(env.Pods) == 0 {
-			return true, "There are no pods to check. Please check under test labels."
+			return true, "no pods to check found"
+		}
+
+		return false, ""
+	}
+}
+
+func GetNoDeploymentsUnderTestSkipFn(env *provider.TestEnvironment) func() (bool, string) {
+	return func() (bool, string) {
+		if len(env.Deployments) == 0 {
+			return true, "no deployments to check found"
+		}
+
+		return false, ""
+	}
+}
+
+func GetNoStatefulSetsUnderTestSkipFn(env *provider.TestEnvironment) func() (bool, string) {
+	return func() (bool, string) {
+		if len(env.StatefulSets) == 0 {
+			return true, "no statefulSets to check found"
+		}
+
+		return false, ""
+	}
+}
+
+func GetNoCrdsUnderTestSkipFn(env *provider.TestEnvironment) func() (bool, string) {
+	return func() (bool, string) {
+		if len(env.Crds) == 0 {
+			return true, "no CRDs to check found"
 		}
 
 		return false, ""