diff --git a/pkg/util/codeinspector/codeinspector.go b/pkg/util/codeinspector/codeinspector.go index ebd87ed52a3..7ecf0c1f106 100644 --- a/pkg/util/codeinspector/codeinspector.go +++ b/pkg/util/codeinspector/codeinspector.go @@ -24,11 +24,27 @@ import ( "io/ioutil" "strings" "unicode" + + go2idlparser "k8s.io/kubernetes/cmd/libs/go2idl/parser" + "k8s.io/kubernetes/cmd/libs/go2idl/types" ) // GetPublicFunctions lists all public functions (not methods) from a golang source file. -func GetPublicFunctions(filePath string) ([]string, error) { - var functionNames []string +func GetPublicFunctions(pkg, filePath string) ([]*types.Type, error) { + builder := go2idlparser.New() + data, err := ioutil.ReadFile(filePath) + if err != nil { + return nil, err + } + if err := builder.AddFile(pkg, filePath, data); err != nil { + return nil, err + } + universe, err := builder.FindTypes() + if err != nil { + return nil, err + } + + var functions []*types.Type // Create the AST by parsing src. fset := token.NewFileSet() // positions are relative to fset @@ -45,13 +61,13 @@ func GetPublicFunctions(filePath string) ([]string, error) { s = x.Name.Name // It's a function (not method), and is public, record it. if x.Recv == nil && isPublic(s) { - functionNames = append(functionNames, s) + functions = append(functions, universe[pkg].Function(x.Name.Name)) } } return true }) - return functionNames, nil + return functions, nil } // isPublic checks if a given string is a public function name. diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go index f3f0d78b6b6..af858b9671e 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go @@ -19,10 +19,13 @@ package predicates import ( "fmt" "os/exec" + "path/filepath" "reflect" "strings" "testing" + "k8s.io/kubernetes/cmd/libs/go2idl/parser" + "k8s.io/kubernetes/cmd/libs/go2idl/types" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/resource" "k8s.io/kubernetes/pkg/util/codeinspector" @@ -1584,8 +1587,26 @@ func TestEBSVolumeCountConflicts(t *testing.T) { } } +func getPredicateSignature() (*types.Signature, error) { + filePath := "./../types.go" + pkgName := filepath.Dir(filePath) + builder := parser.New() + if err := builder.AddDir(pkgName); err != nil { + return nil, err + } + universe, err := builder.FindTypes() + if err != nil { + return nil, err + } + result, ok := universe[pkgName].Types["FitPredicate"] + if !ok { + return nil, fmt.Errorf("FitPredicate type not defined") + } + return result.Signature, nil +} + func TestPredicatesRegistered(t *testing.T) { - var functionNames []string + var functions []*types.Type // Files and directories which predicates may be referenced targetFiles := []string{ @@ -1603,27 +1624,42 @@ func TestPredicatesRegistered(t *testing.T) { // Get all public predicates in files. for _, filePath := range files { - functions, err := codeinspector.GetPublicFunctions(filePath) + fileFunctions, err := codeinspector.GetPublicFunctions("k8s.io/kubernetes/plugin/pkg/scheduler/algorithm/predicates", filePath) if err == nil { - functionNames = append(functionNames, functions...) + functions = append(functions, fileFunctions...) } else { t.Errorf("unexpected error when parsing %s", filePath) } } + predSignature, err := getPredicateSignature() + if err != nil { + t.Fatalf("Couldn't get predicates signature") + } + // Check if all public predicates are referenced in target files. - for _, functionName := range functionNames { - args := []string{"-rl", functionName} + for _, function := range functions { + // Ignore functions that doesn't match FitPredicate signature. + signature := function.Underlying.Signature + if len(predSignature.Parameters) != len(signature.Parameters) { + continue + } + if len(predSignature.Results) != len(signature.Results) { + continue + } + // TODO: Check exact types of parameters and results. + + args := []string{"-rl", function.Name.Name} 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) + t.Errorf("unexpected error when checking %s", function.Name) case "exit status 1": t.Errorf("predicate %s is implemented as public but seems not registered or used in any other place", - functionName) + function.Name) } } } diff --git a/plugin/pkg/scheduler/algorithm/priorities/priorities_test.go b/plugin/pkg/scheduler/algorithm/priorities/priorities_test.go index db96feffd41..20dddbedb6b 100644 --- a/plugin/pkg/scheduler/algorithm/priorities/priorities_test.go +++ b/plugin/pkg/scheduler/algorithm/priorities/priorities_test.go @@ -23,6 +23,7 @@ import ( "strconv" "testing" + "k8s.io/kubernetes/cmd/libs/go2idl/types" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/resource" "k8s.io/kubernetes/pkg/apis/extensions" @@ -887,7 +888,7 @@ func makeImageNode(node string, status api.NodeStatus) api.Node { } func TestPrioritiesRegistered(t *testing.T) { - var functionNames []string + var functions []*types.Type // Files and directories which priorities may be referenced targetFiles := []string{ @@ -904,27 +905,27 @@ func TestPrioritiesRegistered(t *testing.T) { // Get all public priorities in files. for _, filePath := range files { - functions, err := codeinspector.GetPublicFunctions(filePath) + fileFunctions, err := codeinspector.GetPublicFunctions("k8s.io/kubernetes/plugin/pkg/scheduler/algorithm/priorities", filePath) if err == nil { - functionNames = append(functionNames, functions...) + functions = append(functions, fileFunctions...) } else { t.Errorf("unexpected error when parsing %s", filePath) } } // Check if all public priorities are referenced in target files. - for _, functionName := range functionNames { - args := []string{"-rl", functionName} + for _, function := range functions { + args := []string{"-rl", function.Name.Name} 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) + t.Errorf("unexpected error when checking %s", function.Name) case "exit status 1": t.Errorf("priority %s is implemented as public but seems not registered or used in any other place", - functionName) + function.Name) } } }