pkg/repo: Debugging for query logic

Signed-off-by: Stephen Augustus <[email protected]>

Author: justaugustus

cmd/kepctl/main.go

@@ -16,8 +16,14 @@ limitations under the License.
 
 package main
 
-import "k8s.io/enhancements/pkg/kepctl/commands"
+import (
+	"github.com/sirupsen/logrus"
+
+	"k8s.io/enhancements/pkg/kepctl/commands"
+)
 
 func main() {
-	commands.New().Execute()
+	if err := commands.New().Execute(); err != nil {
+		logrus.Fatalf("error during command execution: %v", err)
+	}
 }

pkg/repo/output.go

@@ -72,7 +72,7 @@ var defaultConfig = map[string]printConfig{
 }
 
 func DefaultPrintConfigs(names ...string) []PrintConfig {
-	configs := make([]PrintConfig, 10)
+	configs := make([]PrintConfig, 0, 10)
 	for _, n := range names {
 		// copy to allow it to be tweaked by the caller
 		c := defaultConfig[n]
@@ -90,7 +90,7 @@ func (r *Repo) PrintTable(configs []PrintConfig, proposals []*api.Proposal) {
 
 	table := tablewriter.NewWriter(r.Out)
 
-	headers := make([]string, 10)
+	headers := make([]string, 0, 10)
 	for _, c := range configs {
 		headers = append(headers, c.Title())
 	}

pkg/repo/query.go

@@ -21,6 +21,7 @@ import (
 	"regexp"
 
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 
 	"k8s.io/enhancements/api"
 )
@@ -113,11 +114,16 @@ func (r *Repo) Query(opts *QueryOpts) error {
 		return errors.Wrapf(tokenErr, "setting GitHub token")
 	}
 
-	allKEPs := make([]*api.Proposal, 10)
+	allKEPs := make([]*api.Proposal, 0, 10)
 	// load the KEPs for each listed SIG
 	for _, sig := range opts.Groups {
 		// KEPs in the local filesystem
-		allKEPs = append(allKEPs, r.LoadLocalKEPs(sig)...)
+		localKEPs, err := r.LoadLocalKEPs(sig)
+		if err != nil {
+			return errors.Wrap(err, "loading local KEPs")
+		}
+
+		allKEPs = append(allKEPs, localKEPs...)
 
 		// Open PRs; existing KEPs with open PRs will be shown twice
 		if opts.IncludePRs {
@@ -131,15 +137,23 @@ func (r *Repo) Query(opts *QueryOpts) error {
 		}
 	}
 
+	logrus.Debugf("all KEPs collected: %v", allKEPs)
+
 	// filter the KEPs by criteria
 	allowedStatus := sliceToMap(opts.Status)
 	allowedStage := sliceToMap(opts.Stage)
 	allowedPRR := sliceToMap(opts.PRRApprover)
 	allowedAuthor := sliceToMap(opts.Author)
 	allowedApprover := sliceToMap(opts.Approver)
 
-	keps := make([]*api.Proposal, 10)
+	results := make([]*api.Proposal, 0, 10)
 	for _, k := range allKEPs {
+		if k == nil {
+			return errors.New("one of the KEPs in query was nil")
+		}
+
+		logrus.Debugf("current KEP: %v", k)
+
 		if len(opts.Status) > 0 && !allowedStatus[k.Status] {
 			continue
 		}
@@ -156,7 +170,7 @@ func (r *Repo) Query(opts *QueryOpts) error {
 			continue
 		}
 
-		keps = append(keps, k)
+		results = append(results, k)
 	}
 
 	switch opts.Output {
@@ -171,12 +185,12 @@ func (r *Repo) Query(opts *QueryOpts) error {
 				"Title",
 				"Link",
 			),
-			keps,
+			results,
 		)
 	case "yaml":
-		r.PrintYAML(keps)
+		r.PrintYAML(results)
 	case "json":
-		r.PrintJSON(keps)
+		r.PrintJSON(results)
 	default:
 		// this check happens as a validation step in cobra as well
 		// added it for additional verbosity

pkg/repo/repo.go

@@ -30,6 +30,7 @@ import (
 
 	"github.com/google/go-github/v33/github"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 	"golang.org/x/oauth2"
 	"gopkg.in/yaml.v3"
 
@@ -195,15 +196,30 @@ func (r *Repo) findLocalKEPMeta(sig string) ([]string, error) {
 	err := filepath.Walk(
 		sigPath,
 		func(path string, info os.FileInfo, err error) error {
+			logrus.Debugf("processing filename %s", info.Name())
+
 			if err != nil {
 				return err
 			}
 
+			if ignore(info.Name(), "yaml", "md") {
+				return nil
+			}
+
+			// true if the file is a symlink
+			if info.Mode()&os.ModeSymlink != 0 {
+				// Assume symlink from old KEP location to new. The new location
+				// will be processed separately, so no need to process it here.
+				logrus.Debugf("%s is a symlink", info.Name())
+				return nil
+			}
+
 			if !info.Mode().IsRegular() {
 				return nil
 			}
 
 			if info.Name() == ProposalMetadataFilename {
+				logrus.Debugf("adding %s as KEP metadata", info.Name())
 				keps = append(keps, path)
 				return filepath.SkipDir
 			}
@@ -216,6 +232,7 @@ func (r *Repo) findLocalKEPMeta(sig string) ([]string, error) {
 				return nil
 			}
 
+			logrus.Debugf("adding %s as KEP metadata", info.Name())
 			keps = append(keps, path)
 			return nil
 		},
@@ -224,33 +241,49 @@ func (r *Repo) findLocalKEPMeta(sig string) ([]string, error) {
 	return keps, err
 }
 
-func (r *Repo) LoadLocalKEPs(sig string) []*api.Proposal {
+func (r *Repo) LoadLocalKEPs(sig string) ([]*api.Proposal, error) {
 	// KEPs in the local filesystem
 	files, err := r.findLocalKEPMeta(sig)
 	if err != nil {
-		fmt.Fprintf(r.Err, "error searching for local KEPs from %s: %s\n", sig, err)
+		return nil, errors.Wrapf(
+			err,
+			"searching for local KEPs from %s",
+			sig,
+		)
 	}
 
+	logrus.Debugf("loading the following local KEPs: %v", files)
+
 	var allKEPs []*api.Proposal
 	for _, k := range files {
 		if filepath.Ext(k) == ".yaml" {
 			kep, err := r.loadKEPFromYaml(k)
 			if err != nil {
-				fmt.Fprintf(r.Err, "error reading KEP %s: %s\n", k, err)
+				return nil, errors.Wrapf(
+					err,
+					"reading KEP %s from yaml",
+					k,
+				)
 			} else {
 				allKEPs = append(allKEPs, kep)
 			}
 		} else {
 			kep, err := r.loadKEPFromOldStyle(k)
 			if err != nil {
-				fmt.Fprintf(r.Err, "error reading KEP %s: %s\n", k, err)
+				return nil, errors.Wrapf(
+					err,
+					"reading KEP %s from markdown",
+					k,
+				)
 			} else {
 				allKEPs = append(allKEPs, kep)
 			}
 		}
 	}
 
-	return allKEPs
+	logrus.Debugf("returning %d local KEPs", len(allKEPs))
+
+	return allKEPs, nil
 }
 
 func (r *Repo) loadKEPPullRequests(sig string) ([]*api.Proposal, error) {

pkg/repo/repo_test.go

@@ -84,7 +84,9 @@ func TestFindLocalKEPs(t *testing.T) {
 	require.Nil(t, repoErr)
 
 	for i, tc := range testcases {
-		k := r.LoadLocalKEPs(tc.sig)
+		k, err := r.LoadLocalKEPs(tc.sig)
+		require.Nil(t, err)
+
 		if len(k) != len(tc.keps) {
 			t.Errorf("Test case %d: expected %d but got %d", i, len(tc.keps), len(k))
 			continue

pkg/repo/validate.go

@@ -44,34 +44,41 @@ func (r *Repo) Validate() (
 	err = filepath.Walk(
 		kepDir,
 		func(path string, info os.FileInfo, err error) error {
+			logrus.Debugf("processing filename %s", info.Name())
+
 			if err != nil {
 				return err
 			}
 
 			if info.IsDir() {
+				if info.Name() == PRRApprovalPathStub {
+					return filepath.SkipDir
+				}
+
 				return nil
 			}
 
-			dir := filepath.Dir(path)
 			// true if the file is a symlink
 			if info.Mode()&os.ModeSymlink != 0 {
 				// Assume symlink from old KEP location to new. The new location
 				// will be processed separately, so no need to process it here.
 				return nil
 			}
 
+			dir := filepath.Dir(path)
+
 			metadataFilename := ProposalMetadataFilename
 			metadataFilepath := filepath.Join(dir, metadataFilename)
 			if _, err := os.Stat(metadataFilepath); err == nil {
-				// There is kep metadata file in this directory, only that one should be processed.
+				// There is KEP metadata file in this directory, only that one should be processed.
 				if info.Name() == metadataFilename {
 					files = append(files, metadataFilepath)
 				}
 
 				return nil
 			}
 
-			if ignore(info.Name()) {
+			if ignore(info.Name(), "yaml", "md") {
 				return nil
 			}