Implement checking in predicates

pkg/util/codeinspector/codeinspector.go

@@ -21,11 +21,13 @@ import (
 	"go/ast"
 	"go/parser"
 	"go/token"
+	"io/ioutil"
+	"strings"
 	"unicode"
 )
 
-// Get all public functions (not methods) from a golang source file.
+// GetPublicFunctions lists all public functions (not methods) from a golang source file.
-func GetPulicFunctions(filePath string) ([]string, error) {
+func GetPublicFunctions(filePath string) ([]string, error) {
 	var functionNames []string
 
 	// Create the AST by parsing src.
@@ -52,9 +54,27 @@ func GetPulicFunctions(filePath string) ([]string, error) {
 	return functionNames, nil
 }
 
-// Check if a given string is a public function name.
+// isPublic checks if a given string is a public function name.
 func isPublic(myString string) bool {
 	a := []rune(myString)
 	a[0] = unicode.ToUpper(a[0])
 	return myString == string(a)
 }
+
+// GetSourceCodeFiles lists golang source code files from directory, excluding sub-directory and tests files.
+func GetSourceCodeFiles(dir string) ([]string, error) {
+	files, err := ioutil.ReadDir(dir)
+	if err != nil {
+		return nil, err
+	}
+
+	var filenames []string
+
+	for _, file := range files {
+		if strings.HasSuffix(file.Name(), ".go") && !strings.HasSuffix(file.Name(), "_test.go") {
+			filenames = append(filenames, file.Name())
+		}
+	}
+
+	return filenames, nil
+}

plugin/pkg/scheduler/algorithm/predicates/predicates.go

@@ -469,9 +469,9 @@ func NewSelectorMatchPredicate(info NodeInfo) algorithm.FitPredicate {
 	return selector.PodSelectorMatches
 }
 
-// NodeMatchesNodeSelectorTerms checks if a node's labels satisfy a list of node selector terms,
+// nodeMatchesNodeSelectorTerms checks if a node's labels satisfy a list of node selector terms,
 // terms are ORed, and an emtpy a list of terms will match nothing.
-func NodeMatchesNodeSelectorTerms(node *api.Node, nodeSelectorTerms []api.NodeSelectorTerm) bool {
+func nodeMatchesNodeSelectorTerms(node *api.Node, nodeSelectorTerms []api.NodeSelectorTerm) bool {
 	for _, req := range nodeSelectorTerms {
 		nodeSelector, err := api.NodeSelectorRequirementsAsSelector(req.MatchExpressions)
 		if err != nil {
@@ -524,14 +524,14 @@ func PodMatchesNodeLabels(pod *api.Pod, node *api.Node) bool {
 		// if nodeAffinity.RequiredDuringSchedulingRequiredDuringExecution != nil {
 		// 	nodeSelectorTerms := nodeAffinity.RequiredDuringSchedulingRequiredDuringExecution.NodeSelectorTerms
 		// 	glog.V(10).Infof("Match for RequiredDuringSchedulingRequiredDuringExecution node selector terms %+v", nodeSelectorTerms)
-		// 	nodeAffinityMatches = NodeMatchesNodeSelectorTerms(node, nodeSelectorTerms)
+		// 	nodeAffinityMatches = nodeMatchesNodeSelectorTerms(node, nodeSelectorTerms)
 		// }
 
 		// Match node selector for requiredDuringSchedulingIgnoredDuringExecution.
 		if nodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution != nil {
 			nodeSelectorTerms := nodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms
 			glog.V(10).Infof("Match for RequiredDuringSchedulingIgnoredDuringExecution node selector terms %+v", nodeSelectorTerms)
-			nodeAffinityMatches = nodeAffinityMatches && NodeMatchesNodeSelectorTerms(node, nodeSelectorTerms)
+			nodeAffinityMatches = nodeAffinityMatches && nodeMatchesNodeSelectorTerms(node, nodeSelectorTerms)
 		}
 
 	}

plugin/pkg/scheduler/algorithm/predicates/predicates_test.go

@@ -18,11 +18,13 @@ package predicates
 
 import (
 	"fmt"
+	"os/exec"
 	"reflect"
 	"testing"
 
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/resource"
+	"k8s.io/kubernetes/pkg/util/codeinspector"
 	"k8s.io/kubernetes/plugin/pkg/scheduler/algorithm"
 	"k8s.io/kubernetes/plugin/pkg/scheduler/schedulercache"
 )
@@ -1408,3 +1410,48 @@ func TestEBSVolumeCountConflicts(t *testing.T) {
 		}
 	}
 }
+
+func TestPredicatesRegistered(t *testing.T) {
+	var functionNames []string
+
+	// Files and directories which predicates may be referenced
+	targetFiles := []string{
+		"./../../algorithmprovider/defaults/defaults.go", // Default algorithm
+		"./../../factory/plugins.go",                     // Registered in init()
+		"./../../../../../pkg/",                          // kubernetes/pkg, often used by kubelet or controller
+	}
+
+	// List all golang source files under ./predicates/, excluding test files and sub-directories.
+	files, err := codeinspector.GetSourceCodeFiles(".")
+
+	if err != nil {
+		t.Errorf("unexpected error: %v when listing files in current directory", err)
+	}
+
+	// Get all public predicates in files.
+	for _, filePath := range files {
+		functions, err := codeinspector.GetPublicFunctions(filePath)
+		if err == nil {
+			functionNames = append(functionNames, functions...)
+		} else {
+			t.Errorf("unexpected error when parsing %s", filePath)
+		}
+	}
+
+	// Check if all public predicates are referenced in target files.
+	for _, functionName := range functionNames {
+		args := []string{"-rl", functionName}
+		args = append(args, targetFiles...)
+
+		err := exec.Command("grep", args...).Run()
+		if err != nil {
+			switch err.Error() {
+			case "exit status 2":
+				t.Errorf("unexpected error when checking %s", functionName)
+			case "exit status 1":
+				t.Errorf("predicate %s is implemented as public but seems not registered or used in any other place",
+					functionName)
+			}
+		}
+	}
+}

plugin/pkg/scheduler/algorithm/priorities/priorities_test.go

@@ -887,13 +887,25 @@ func makeImageNode(node string, status api.NodeStatus) api.Node {
 	}
 }
 
-func TestPriorityRegistered(t *testing.T) {
+func TestPrioritiesRegistered(t *testing.T) {
 	var functionNames []string
 
-	files := []string{"priorities.go", "node_affinity.go", "selector_spreading.go"}
+	// Files and directories which priorities may be referenced
+	targetFiles := []string{
+		"./../../algorithmprovider/defaults/defaults.go", // Default algorithm
+		"./../../factory/plugins.go",                     // Registered in init()
+	}
 
+	// List all golang source files under ./priorities/, excluding test files and sub-directories.
+	files, err := codeinspector.GetSourceCodeFiles(".")
+
+	if err != nil {
+		t.Errorf("unexpected error: %v when listing files in current directory", err)
+	}
+
+	// Get all public priorities in files.
 	for _, filePath := range files {
-		functions, err := codeinspector.GetPulicFunctions(filePath)
+		functions, err := codeinspector.GetPublicFunctions(filePath)
 		if err == nil {
 			functionNames = append(functionNames, functions...)
 		} else {
@@ -901,14 +913,19 @@ func TestPriorityRegistered(t *testing.T) {
 		}
 	}
 
-	for _, funcionName := range functionNames {
+	// Check if all public priorities are referenced in target files.
-		err := exec.Command("grep", "-rl", funcionName, "./../../algorithmprovider/defaults/defaults.go", "./../../factory/plugins.go").Run()
+	for _, functionName := range functionNames {
+		args := []string{"-rl", functionName}
+		args = append(args, targetFiles...)
+
+		err := exec.Command("grep", args...).Run()
 		if err != nil {
 			switch err.Error() {
 			case "exit status 2":
-				t.Errorf("unexpected error when checking %s", funcionName)
+				t.Errorf("unexpected error when checking %s", functionName)
 			case "exit status 1":
-				t.Errorf("priority %s is implemented but seems not registered in any place", funcionName)
+				t.Errorf("priority %s is implemented as public but seems not registered or used in any other place",
+					functionName)
 			}
 		}
 	}