From 2a439b9667d3d9ba522aaf727981cc05313e3543 Mon Sep 17 00:00:00 2001 From: Andreas Neumann Date: Tue, 2 Jun 2020 13:14:05 +0200 Subject: [PATCH] Reworked collectors and runner Signed-off-by: Andreas Neumann --- pkg/kudoctl/cmd/diagnostics/collectors.go | 60 ++-- .../cmd/diagnostics/processing_context.go | 8 +- pkg/kudoctl/cmd/diagnostics/runner.go | 29 +- pkg/kudoctl/cmd/diagnostics/runner_helper.go | 273 +++++++++--------- 4 files changed, 202 insertions(+), 168 deletions(-) diff --git a/pkg/kudoctl/cmd/diagnostics/collectors.go b/pkg/kudoctl/cmd/diagnostics/collectors.go index 3d7a2efaa..4d66806e4 100644 --- a/pkg/kudoctl/cmd/diagnostics/collectors.go +++ b/pkg/kudoctl/cmd/diagnostics/collectors.go @@ -9,6 +9,8 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" + + "github.com/kudobuilder/kudo/pkg/kudoctl/clog" ) // Ensure collector is implemented @@ -21,41 +23,44 @@ type resourceCollector struct { parentDir func() string // parent dir to attach the printer's output failOnError bool // define whether the collector should return the error callback func(runtime.Object) // will be called with the retrieved resource after collection to update shared context - printer *nonFailingPrinter printMode printMode } // collect - load a resource and send either the resource or collection error to printer // return error if failOnError field is set to true // if failOnError is true, finding no object(s) is treated as an error -func (c *resourceCollector) collect() error { +func (c *resourceCollector) collect(printer *nonFailingPrinter) error { + clog.V(4).Printf("Collect Resource %s in parent dir %s", c.name, c.parentDir()) obj, err := c._collect(c.failOnError) if err != nil { - c.printer.printError(err, c.parentDir(), c.name) + printer.printError(err, c.parentDir(), c.name) if c.failOnError { return err } } if obj != nil { - c.printer.printObject(obj, c.parentDir(), c.printMode) + printer.printObject(obj, c.parentDir(), c.printMode) } return nil } +func emptyResult(obj runtime.Object) bool { + return obj == nil || reflect.ValueOf(obj).IsNil() || (meta.IsListType(obj) && meta.LenList(obj) == 0) +} + func (c *resourceCollector) _collect(failOnError bool) (runtime.Object, error) { obj, err := c.loadResourceFn() - switch { - case err != nil: + if err != nil { return nil, fmt.Errorf("failed to retrieve object(s) of kind %s: %v", c.name, err) - case obj == nil || reflect.ValueOf(obj).IsNil() || (meta.IsListType(obj) && meta.LenList(obj) == 0): + } + if emptyResult(obj) { obj = nil if failOnError { return nil, fmt.Errorf("no object(s) of kind %s retrieved", c.name) } - default: - if c.callback != nil { - c.callback(obj) - } + } + if c.callback != nil { + c.callback(obj) } return obj, nil } @@ -67,24 +72,24 @@ var _ collector = &resourceCollectorGroup{} // each other's side-effects on the shared context type resourceCollectorGroup struct { collectors []resourceCollector - parentDir func() string } // collect - collect resource and run callback for each collector, print all afterwards // collection failures are treated as fatal regardless of the collectors failOnError flag setting -func (g resourceCollectorGroup) collect() error { +func (g resourceCollectorGroup) collect(printer *nonFailingPrinter) error { + clog.V(0).Printf("Collect ResourceGroup for %d collectors", len(g.collectors)) objs := make([]runtime.Object, len(g.collectors)) modes := make([]printMode, len(g.collectors)) for i, c := range g.collectors { obj, err := c._collect(true) if err != nil { - c.printer.printError(err, g.parentDir(), c.name) + printer.printError(err, c.parentDir(), c.name) return err } objs[i], modes[i] = obj, c.printMode } for i, c := range g.collectors { - c.printer.printObject(objs[i], c.parentDir(), modes[i]) + printer.printObject(objs[i], c.parentDir(), modes[i]) } return nil } @@ -94,22 +99,35 @@ var _ collector = &logsCollector{} type logsCollector struct { loadLogFn func(string, string) (io.ReadCloser, error) - pods []v1.Pod + pods func() []v1.Pod parentDir func() string - printer *nonFailingPrinter } -func (c *logsCollector) collect() error { - for _, pod := range c.pods { +func (c *logsCollector) collect(printer *nonFailingPrinter) error { + clog.V(0).Printf("Collect Logs for %d pods", len(c.pods())) + for _, pod := range c.pods() { for _, container := range pod.Spec.Containers { log, err := c.loadLogFn(pod.Name, container.Name) if err != nil { - c.printer.printError(err, filepath.Join(c.parentDir(), fmt.Sprintf("pod_%s", pod.Name)), fmt.Sprintf("%s.log", container.Name)) + printer.printError(err, filepath.Join(c.parentDir(), fmt.Sprintf("pod_%s", pod.Name)), fmt.Sprintf("%s.log", container.Name)) } else { - c.printer.printLog(log, filepath.Join(c.parentDir(), fmt.Sprintf("pod_%s", pod.Name)), container.Name) + printer.printLog(log, filepath.Join(c.parentDir(), fmt.Sprintf("pod_%s", pod.Name)), container.Name) _ = log.Close() } } } return nil } + +var _ collector = &objCollector{} + +type objCollector struct { + obj interface{} + parentDir stringGetter + name string +} + +func (c *objCollector) collect(printer *nonFailingPrinter) error { + printer.printYaml(c.obj, c.parentDir(), c.name) + return nil +} diff --git a/pkg/kudoctl/cmd/diagnostics/processing_context.go b/pkg/kudoctl/cmd/diagnostics/processing_context.go index 4fabd0603..ab8fe146f 100644 --- a/pkg/kudoctl/cmd/diagnostics/processing_context.go +++ b/pkg/kudoctl/cmd/diagnostics/processing_context.go @@ -3,10 +3,10 @@ package diagnostics import ( "fmt" - "github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1" - v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" + + "github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1" ) // processingContext - shared data for the resource collectors @@ -51,3 +51,7 @@ func (ctx *processingContext) operatorVersionName() string { func (ctx *processingContext) operatorName() string { return ctx.opName } + +func (ctx *processingContext) podList() []v1.Pod { + return ctx.pods +} diff --git a/pkg/kudoctl/cmd/diagnostics/runner.go b/pkg/kudoctl/cmd/diagnostics/runner.go index 71c9d4b1d..1e15b38c8 100644 --- a/pkg/kudoctl/cmd/diagnostics/runner.go +++ b/pkg/kudoctl/cmd/diagnostics/runner.go @@ -3,22 +3,31 @@ package diagnostics // collector - generic interface for diagnostic data collection // implementors are expected to return only fatal errors and handle non-fatal ones themselves type collector interface { - collect() error + collect(printer *nonFailingPrinter) error } // runner - sequential runner for Collectors reducing error checking boilerplate code type runner struct { - fatalErr error + collectors []collector } -func (r *runner) run(c collector) *runner { - if r.fatalErr == nil { - r.fatalErr = c.collect() - } - return r +func (r *runner) addCollector(c collector) { + r.collectors = append(r.collectors, c) +} + +func (r *runner) addObjDump(v interface{}, dir stringGetter, name string) { + r.addCollector(&objCollector{ + obj: v, + parentDir: dir, + name: name, + }) } -func (r *runner) dumpToYaml(v interface{}, dir stringGetter, name string, p *nonFailingPrinter) *runner { - p.printYaml(v, dir(), name) - return r +func (r *runner) run(printer *nonFailingPrinter) error { + for _, c := range r.collectors { + if err := c.collect(printer); err != nil { + return err + } + } + return nil } diff --git a/pkg/kudoctl/cmd/diagnostics/runner_helper.go b/pkg/kudoctl/cmd/diagnostics/runner_helper.go index 4f876b5a1..55c08fe77 100644 --- a/pkg/kudoctl/cmd/diagnostics/runner_helper.go +++ b/pkg/kudoctl/cmd/diagnostics/runner_helper.go @@ -15,110 +15,15 @@ func diagForInstance(instance string, options *Options, c *kudo.Client, info ver ctx := &processingContext{root: DiagDir, instanceName: instance} - instanceDiagRunner := runForInstance(ir, ctx, p). - dumpToYaml(info, ctx.rootDirectory, "version", p). - dumpToYaml(s, ctx.rootDirectory, "settings", p) + runner := runnerForInstance(ir, ctx) + runner.addObjDump(info, ctx.rootDirectory, "version") + runner.addObjDump(s, ctx.rootDirectory, "settings") - return instanceDiagRunner.fatalErr -} + if err := runner.run(p); err != nil { + return err + } -func runForInstance(ir *resourceFuncsConfig, ctx *processingContext, p *nonFailingPrinter) *runner { - - instanceDiagRunner := &runner{} - instanceDiagRunner. - run(resourceCollectorGroup{[]resourceCollector{ - { - loadResourceFn: ir.instance, - name: "instance", - parentDir: ctx.operatorDirectory, - failOnError: true, - callback: ctx.setOperatorVersionNameFromInstance, - printer: p, - printMode: ObjectWithDir}, - { - loadResourceFn: ir.operatorVersion(ctx.operatorVersionName), - name: "operatorversion", - parentDir: ctx.operatorDirectory, - failOnError: true, - callback: ctx.setOperatorNameFromOperatorVersion, - printer: p, - printMode: ObjectWithDir}, - { - loadResourceFn: ir.operator(ctx.operatorName), - name: "operator", - parentDir: ctx.rootDirectory, - failOnError: true, - printer: p, - printMode: ObjectWithDir}, - }, ctx.rootDirectory}). - run(&resourceCollector{ - loadResourceFn: ir.pods, - name: "pod", - parentDir: ctx.instanceDirectory, - callback: ctx.setPods, - printer: p, - printMode: ObjectListWithDirs}). - run(&resourceCollector{ - loadResourceFn: ir.services, - name: "service", - parentDir: ctx.instanceDirectory, - printer: p, - printMode: RuntimeObject}). - run(&resourceCollector{ - loadResourceFn: ir.deployments, - name: "deployment", - parentDir: ctx.instanceDirectory, - printer: p, - printMode: RuntimeObject}). - run(&resourceCollector{ - loadResourceFn: ir.replicaSets, - name: "replicaset", - parentDir: ctx.instanceDirectory, - printer: p, - printMode: RuntimeObject}). - run(&resourceCollector{ - loadResourceFn: ir.statefulSets, - name: "statefulset", - parentDir: ctx.instanceDirectory, - printer: p, - printMode: RuntimeObject}). - run(&resourceCollector{ - loadResourceFn: ir.serviceAccounts, - name: "serviceaccount", - parentDir: ctx.instanceDirectory, - printer: p, - printMode: RuntimeObject}). - run(&resourceCollector{ - loadResourceFn: ir.clusterRoleBindings, - name: "clusterrolebinding", - parentDir: ctx.instanceDirectory, - printer: p, - printMode: RuntimeObject}). - run(&resourceCollector{ - loadResourceFn: ir.roleBindings, - name: "rolebinding", - parentDir: ctx.instanceDirectory, - printer: p, - printMode: RuntimeObject}). - run(&resourceCollector{ - loadResourceFn: ir.clusterRoles, - name: "clusterrole", - parentDir: ctx.instanceDirectory, - printer: p, - printMode: RuntimeObject}). - run(&resourceCollector{ - loadResourceFn: ir.roles, - name: "role", - parentDir: ctx.instanceDirectory, - printer: p, - printMode: RuntimeObject}). - run(&logsCollector{ - loadLogFn: ir.log, - pods: ctx.pods, - parentDir: ctx.instanceDirectory, - printer: p, - }) - return instanceDiagRunner + return nil } func diagForKudoManager(options *Options, c *kudo.Client, p *nonFailingPrinter) error { @@ -127,38 +32,136 @@ func diagForKudoManager(options *Options, c *kudo.Client, p *nonFailingPrinter) return err } ctx := &processingContext{root: KudoDir} - kudoDiagRunner := &runner{} - kudoDiagRunner. - run(&resourceCollector{ - loadResourceFn: kr.pods, - name: "pod", - parentDir: ctx.rootDirectory, - callback: ctx.setPods, - printer: p, - printMode: ObjectListWithDirs}). - run(&resourceCollector{ - loadResourceFn: kr.services, - name: "service", - parentDir: ctx.rootDirectory, - printer: p, - printMode: RuntimeObject}). - run(&resourceCollector{ - loadResourceFn: kr.statefulSets, - name: "statefulset", - parentDir: ctx.rootDirectory, - printer: p, - printMode: RuntimeObject}). - run(&resourceCollector{ - loadResourceFn: kr.serviceAccounts, - name: "serviceaccount", + + runner := runnerForKudoManager(kr, ctx) + + if err := runner.run(p); err != nil { + return err + } + + return nil +} + +func runnerForInstance(ir *resourceFuncsConfig, ctx *processingContext) *runner { + r := &runner{} + + instance := resourceCollectorGroup{[]resourceCollector{ + { + loadResourceFn: ir.instance, + name: "instance", + parentDir: ctx.operatorDirectory, + failOnError: true, + callback: ctx.setOperatorVersionNameFromInstance, + printMode: ObjectWithDir}, + { + loadResourceFn: ir.operatorVersion(ctx.operatorVersionName), + name: "operatorversion", + parentDir: ctx.operatorDirectory, + failOnError: true, + callback: ctx.setOperatorNameFromOperatorVersion, + printMode: ObjectWithDir}, + { + loadResourceFn: ir.operator(ctx.operatorName), + name: "operator", parentDir: ctx.rootDirectory, - printer: p, - printMode: RuntimeObject}). - run(&logsCollector{ - loadLogFn: kr.log, - pods: ctx.pods, - parentDir: ctx.rootDirectory, - printer: p, - }) - return kudoDiagRunner.fatalErr + failOnError: true, + printMode: ObjectWithDir}, + }} + r.addCollector(instance) + + r.addCollector(&resourceCollector{ + loadResourceFn: ir.pods, + name: "pod", + parentDir: ctx.instanceDirectory, + callback: ctx.setPods, + printMode: ObjectListWithDirs}) + r.addCollector(&resourceCollector{ + loadResourceFn: ir.services, + name: "service", + parentDir: ctx.instanceDirectory, + printMode: RuntimeObject}) + r.addCollector(&resourceCollector{ + loadResourceFn: ir.deployments, + name: "deployment", + parentDir: ctx.instanceDirectory, + printMode: RuntimeObject}) + r.addCollector(&resourceCollector{ + loadResourceFn: ir.statefulSets, + name: "statefulset", + parentDir: ctx.instanceDirectory, + printMode: RuntimeObject}) + r.addCollector(&resourceCollector{ + loadResourceFn: ir.replicaSets, + name: "replicaset", + parentDir: ctx.instanceDirectory, + printMode: RuntimeObject}) + r.addCollector(&resourceCollector{ + loadResourceFn: ir.statefulSets, + name: "statefulset", + parentDir: ctx.instanceDirectory, + printMode: RuntimeObject}) + r.addCollector(&resourceCollector{ + loadResourceFn: ir.serviceAccounts, + name: "serviceaccount", + parentDir: ctx.instanceDirectory, + printMode: RuntimeObject}) + r.addCollector(&resourceCollector{ + loadResourceFn: ir.clusterRoleBindings, + name: "clusterrolebinding", + parentDir: ctx.instanceDirectory, + printMode: RuntimeObject}) + r.addCollector(&resourceCollector{ + loadResourceFn: ir.roleBindings, + name: "rolebinding", + parentDir: ctx.instanceDirectory, + printMode: RuntimeObject}) + r.addCollector(&resourceCollector{ + loadResourceFn: ir.clusterRoles, + name: "clusterrole", + parentDir: ctx.instanceDirectory, + printMode: RuntimeObject}) + r.addCollector(&resourceCollector{ + loadResourceFn: ir.roles, + name: "role", + parentDir: ctx.instanceDirectory, + printMode: RuntimeObject}) + r.addCollector(&logsCollector{ + loadLogFn: ir.log, + pods: ctx.podList, + parentDir: ctx.instanceDirectory, + }) + + return r +} + +func runnerForKudoManager(kr *resourceFuncsConfig, ctx *processingContext) *runner { + r := &runner{} + + r.addCollector(&resourceCollector{ + loadResourceFn: kr.pods, + name: "pod", + parentDir: ctx.rootDirectory, + callback: ctx.setPods, + printMode: ObjectListWithDirs}) + r.addCollector(&resourceCollector{ + loadResourceFn: kr.services, + name: "service", + parentDir: ctx.rootDirectory, + printMode: RuntimeObject}) + r.addCollector(&resourceCollector{ + loadResourceFn: kr.statefulSets, + name: "statefulset", + parentDir: ctx.rootDirectory, + printMode: RuntimeObject}) + r.addCollector(&resourceCollector{ + loadResourceFn: kr.serviceAccounts, + name: "serviceaccount", + parentDir: ctx.rootDirectory, + printMode: RuntimeObject}) + r.addCollector(&logsCollector{ + loadLogFn: kr.log, + pods: ctx.podList, + parentDir: ctx.rootDirectory}) + + return r }