From f67de017630b3be0b1346157437036692ab0f47f Mon Sep 17 00:00:00 2001 From: Andrey Dyatlov Date: Fri, 31 Aug 2018 16:58:18 +0200 Subject: [PATCH 1/9] Get rid of the separate import/bun package. --- bun/cmd/check.go | 1 - {import => bun/cmd}/import.go | 2 +- bun/main.go | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) rename {import => bun/cmd}/import.go (95%) diff --git a/bun/cmd/check.go b/bun/cmd/check.go index 0203e0a..3771c53 100644 --- a/bun/cmd/check.go +++ b/bun/cmd/check.go @@ -7,7 +7,6 @@ import ( "sort" "github.com/adyatlov/bun" - _ "github.com/adyatlov/bun/import" "github.com/spf13/cobra" ) diff --git a/import/import.go b/bun/cmd/import.go similarity index 95% rename from import/import.go rename to bun/cmd/import.go index 99ccbc3..5a5fc18 100644 --- a/import/import.go +++ b/bun/cmd/import.go @@ -1,4 +1,4 @@ -package bun +package cmd import ( _ "github.com/adyatlov/bun/check/dcosversion" diff --git a/bun/main.go b/bun/main.go index 3b741c5..cd5d67a 100644 --- a/bun/main.go +++ b/bun/main.go @@ -2,7 +2,6 @@ package main import ( "github.com/adyatlov/bun/bun/cmd" - _ "github.com/adyatlov/bun/import" ) const printProgress = false From 29eb8495035800cb0b11bf172aa8c9f5a6d83624 Mon Sep 17 00:00:00 2001 From: Andrey Dyatlov Date: Thu, 6 Sep 2018 23:24:02 +0200 Subject: [PATCH 2/9] Add AtomicCheck. Simplify Health check using AtomicCheck --- atomic_check.go | 123 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 atomic_check.go diff --git a/atomic_check.go b/atomic_check.go new file mode 100644 index 0000000..9c79a4f --- /dev/null +++ b/atomic_check.go @@ -0,0 +1,123 @@ +package bun + +import ( + "context" + "fmt" + "strings" +) + +// HostCheck checks an individual host. It returns status, details, and error if +// the function cannot perform a check. If the returned error is not nil, then +// the status is ignored. +type HostCheck func(Host) (bool, string, error) + +// HostCheckResult is a reult of a single host check. +type HostCheckResult struct { + IP string + OK bool + Details string + Err error +} + +// AtomicCheck builds Check as a sum of HostChecks. +type AtomicCheck struct { + ForEachMaster HostCheck + ForEachAgent HostCheck + ForEachPublicAgent HostCheck + ProblemMessage string + OKMessage string +} + +// Check is an impementation of the bun.Check function. +func (a *AtomicCheck) Check(ctx context.Context, + b Bundle, + p chan<- Progress) (fact Fact, err error) { + progress := Progress{} + okResults := make([]HostCheckResult, 0) + problemResults := make([]HostCheckResult, 0) + errResults := make([]HostCheckResult, 0) + if a.ProblemMessage == "" { + a.ProblemMessage = "Problems were found." + } + if a.OKMessage == "" { + a.OKMessage = "No problems were found." + } + check := func(hosts map[string]Host, hc HostCheck) error { + for ip, host := range hosts { + progress.Stage = ip + select { + case p <- progress: + default: + } + select { + case <-ctx.Done(): + return ctx.Err() + default: + } + r := HostCheckResult{} + r.IP = host.IP + r.OK, r.Details, r.Err = hc(host) + if r.Err != nil { + errResults = append(errResults, r) + } else if r.OK { + okResults = append(okResults, r) + } else { + problemResults = append(problemResults, r) + } + progress.Step++ + } + return nil + } + queue := make([]func() error, 0, 3) + if a.ForEachMaster != nil { + progress.Count += len(b.Masters) + queue = append(queue, func() error { return check(b.Masters, a.ForEachMaster) }) + } + if a.ForEachAgent != nil { + progress.Count += len(b.Agents) + queue = append(queue, func() error { return check(b.Agents, a.ForEachAgent) }) + } + if a.ForEachPublicAgent != nil { + progress.Count += len(b.PublicAgents) + queue = append(queue, func() error { return check(b.PublicAgents, a.ForEachPublicAgent) }) + } + for _, c := range queue { + if err = c(); err != nil { + return + } + } + short := make([]string, 0) + if len(errResults) > 0 { + short = append(short, "Error(s) occured when performing the check.") + } + if len(problemResults) > 0 { + fact.Status = SProblem + short = append(short, a.ProblemMessage) + } else if len(errResults) == 0 { + fact.Status = SOK + short = append(short, a.OKMessage) + } else { + fact.Status = SUndefined + } + fact.Short = strings.Join(short, " ") + var long strings.Builder + if len(problemResults) > 0 { + long.WriteString("Problems:\n") + for _, res := range problemResults { + long.WriteString(fmt.Sprintf("%v: %v\n", res.IP, res.Details)) + } + } + if len(okResults) > 0 { + long.WriteString("Successfull checks:\n") + for _, res := range okResults { + long.WriteString(fmt.Sprintf("%v: %v\n", res.IP, res.Details)) + } + } + if len(errResults) > 0 { + for _, res := range errResults { + fact.Errors = append(fact.Errors, res.Err.Error()) + } + } + fact.Long = long.String() + return +} From 0df183fbb7d13c81e35eb66ffd8374dca4114ca1 Mon Sep 17 00:00:00 2001 From: Andrey Dyatlov Date: Fri, 7 Sep 2018 03:49:26 +0200 Subject: [PATCH 3/9] Finish migration to the Filesystem interface. --- atomic_check.go | 2 +- bun/cmd/check.go | 11 ++- bundle.go | 123 ++++++++++++++++++++++++++++-- check/dcosversion/dcos_version.go | 2 +- check/health/health.go | 93 ++++++++-------------- file.go | 113 --------------------------- file_system.go | 43 ----------- file_type.go | 48 ++++++++++++ filesystem.go | 78 +++++++++++++++++++ host.go | 22 ------ 10 files changed, 280 insertions(+), 255 deletions(-) delete mode 100644 file.go delete mode 100644 file_system.go create mode 100644 file_type.go create mode 100644 filesystem.go delete mode 100644 host.go diff --git a/atomic_check.go b/atomic_check.go index 9c79a4f..42e60d3 100644 --- a/atomic_check.go +++ b/atomic_check.go @@ -29,7 +29,7 @@ type AtomicCheck struct { } // Check is an impementation of the bun.Check function. -func (a *AtomicCheck) Check(ctx context.Context, +func (a AtomicCheck) Check(ctx context.Context, b Bundle, p chan<- Progress) (fact Fact, err error) { progress := Progress{} diff --git a/bun/cmd/check.go b/bun/cmd/check.go index 3771c53..2fd27c3 100644 --- a/bun/cmd/check.go +++ b/bun/cmd/check.go @@ -13,16 +13,16 @@ import ( var printLong = false func printReport(r bun.Report) { - fmt.Printf("%v: %v - %v\n", r.Status, r.Name, r.Short) + fmt.Printf("[%v] \"%v\" - %v\n", r.Status, r.Name, r.Short) if r.Status == bun.SProblem || printLong { - fmt.Printf("Details:\n%v\n", r.Long) + fmt.Printf("%v\n", r.Long) } if len(r.Errors) > 0 { - fmt.Printf("Errors: \n") + fmt.Println("Errors:") for i, err := range r.Errors { fmt.Printf("%v: %v\n", i+1, err) } - fmt.Printf("Details:\n%v\n", r.Long) + fmt.Printf("%v\n", r.Long) } } @@ -32,8 +32,7 @@ func preRun(cmd *cobra.Command, args []string) { if bundle != nil { return } - ctx := context.WithValue(context.Background(), "fs", bun.OSFS{}) - b, err := bun.NewBundle(ctx, bundlePath) + b, err := bun.NewBundle(context.Background(), bundlePath) if err != nil { fmt.Printf("Error while identifying basic bundle parameters: %v\n", err.Error()) os.Exit(1) diff --git a/bundle.go b/bundle.go index 0543a09..3a94024 100644 --- a/bundle.go +++ b/bundle.go @@ -1,27 +1,73 @@ package bun import ( + "compress/gzip" "context" + "errors" + "io" "log" + "path" "path/filepath" "regexp" + "strconv" + "strings" ) +// HostType represent different types of the hosts. +type HostType string + +const ( + // Master host type + Master HostType = "master" + // Agent host type + Agent = "agent" + // PublicAgent host type + PublicAgent = "public agent" +) + +// Host represents a host in a DC/OS cluster. +type Host struct { + Type HostType + IP string + Path string + fs Filesystem +} + +// OpenFile opens a host-related file. Caller is responsible for closing the file. +func (h Host) OpenFile(fileType string) (io.ReadCloser, error) { + return openFile(h.fs, h.Path, fileType) +} + +// Bundle describes DC/OS diagnostics bundle. type Bundle struct { - Path string - Hosts map[string]Host // IP to Host map + Path string + Hosts map[string]Host // IP to Host map + Masters map[string]Host + Agents map[string]Host + PublicAgents map[string]Host + fs Filesystem } +// NewBundle creates new Bundle func NewBundle(ctx context.Context, path string) (Bundle, error) { - b := Bundle{Hosts: make(map[string]Host)} + b := Bundle{ + Hosts: make(map[string]Host), + Masters: make(map[string]Host), + Agents: make(map[string]Host), + PublicAgents: make(map[string]Host), + } var err error b.Path, err = filepath.Abs(path) if err != nil { log.Printf("Error occurred while detecting absolute path: %v", err) return b, err } - fs := ctx.Value("fs").(FileSystem) - infos, err := fs.ReadDir(b.Path) + if fs, ok := FSFromContext(ctx); ok { + b.fs = fs + } else { + b.fs = OSFS{} + } + infos, err := b.fs.ReadDir(b.Path) if err != nil { return b, err } @@ -36,18 +82,22 @@ func NewBundle(ctx context.Context, path string) (Bundle, error) { continue } host := Host{} + host.IP = groups[1] + host.Path = filepath.Join(b.Path, info.Name()) + host.fs = b.fs switch groups[5] { case "master": host.Type = Master + b.Masters[host.IP] = host case "agent": host.Type = Agent + b.Agents[host.IP] = host case "agent_public": host.Type = PublicAgent + b.PublicAgents[host.IP] = host default: panic("dcosbundle.NewBundle: unknown host type: " + groups[5]) } - host.IP = groups[1] - host.Path = filepath.Join(b.Path, info.Name()) b.Hosts[host.IP] = host } return b, nil @@ -56,5 +106,62 @@ func NewBundle(ctx context.Context, path string) (Bundle, error) { // OpenFile opens a file in a root directory of the bundle. The caller is // responsible for closing the file. func (b Bundle) OpenFile(fileType string) (File, error) { - return OpenFile(b.Path, fileType) + return openFile(b.fs, b.Path, fileType) +} + +// openFile tries to open one of the files of the typeName file type. +// If the file is not found, it tries to open it from a correspondent .gzip archive. +// If the .gzip archive is not found returns an error. +func openFile(fs Filesystem, basePath string, typeName string) (file File, err error) { + fileType, err := GetFileType(typeName) + if err != nil { + return + } + for _, p := range fileType.Paths { + filePath := path.Join(basePath, p) + if file, err = fs.Open(filePath); err == nil { + return + } + if fs.IsNotExist(err) { + var gzfile File + if gzfile, err = fs.Open(filePath + ".gz"); err != nil { + if fs.IsNotExist(err) { + continue + } else { + return + } + } + if file, err = gzip.NewReader(gzfile); err != nil { + return + } + file = struct { + io.Reader + io.Closer + }{io.Reader(file), bulkCloser{file, gzfile}} + return + } + } + return +} + +type bulkCloser []io.Closer + +func (bc bulkCloser) Close() error { + ee := make([]error, 0) + for _, c := range bc { + if err := c.Close(); err != nil { + ee = append(ee, err) + } + } + if len(ee) > 0 { + var b strings.Builder + for i, e := range ee { + b.WriteString(strconv.Itoa(i)) + b.WriteString(": ") + b.WriteString(e.Error()) + b.WriteString("\n") + } + return errors.New(b.String()) + } + return nil } diff --git a/check/dcosversion/dcos_version.go b/check/dcosversion/dcos_version.go index 0cd47ad..db0a0b5 100644 --- a/check/dcosversion/dcos_version.go +++ b/check/dcosversion/dcos_version.go @@ -32,7 +32,6 @@ func checkVersion(ctx context.Context, b bun.Bundle, default: } func() { - step++ // Read version file, err := host.OpenFile("dcos-version") if err != nil { @@ -63,6 +62,7 @@ func checkVersion(ctx context.Context, b bun.Bundle, host.Type, host.IP, verStruct.Version) // Report progress bun.SendProg(p, "Checked version installed on "+host.IP, step, len(b.Hosts)) + step++ }() } if fact.Status == bun.SOK && len(fact.Errors) > 0 { diff --git a/check/health/health.go b/check/health/health.go index 458362b..43fc0a5 100644 --- a/check/health/health.go +++ b/check/health/health.go @@ -1,7 +1,6 @@ package health import ( - "context" "encoding/json" "fmt" "log" @@ -16,7 +15,7 @@ const ( ) func init() { - bun.RegisterCheck(bun.CheckInfo{name, description}, checkHealth) + bun.RegisterCheck(bun.CheckInfo{name, description}, healthCheck.Check) } type Health struct { @@ -33,69 +32,41 @@ type Unit struct { Output string } -func checkHealth(ctx context.Context, b bun.Bundle, - p chan<- bun.Progress) (bun.Fact, error) { - fact := bun.Fact{Status: bun.SOK} - fact.Errors = make([]string, 0) - he := Health{make([]Host, 0, len(b.Hosts))} - step := 0 - for _, host := range b.Hosts { - // Check if canceled - select { - case <-ctx.Done(): - return fact, ctx.Err() - default: +var healthCheck bun.AtomicCheck = bun.AtomicCheck{ + ForEachMaster: check, + ForEachAgent: check, + ForEachPublicAgent: check, +} + +func check(host bun.Host) (ok bool, msg string, err error) { + file, err := host.OpenFile("health") + if err != nil { + return + } + defer func() { + if err := file.Close(); err != nil { + log.Printf("Error when closing file health: %v", err) } - // For each host - func() { - step++ - file, err := host.OpenFile("health") - if err != nil { - fact.Errors = append(fact.Errors, err.Error()) - return - } - defer func() { - if err := file.Close(); err != nil { - log.Printf("Error when closing file health: %v", err) - } - }() - h := Host{host.IP, make([]Unit, 0)} - defer func() { he.Hosts = append(he.Hosts, h) }() - decoder := json.NewDecoder(file) - // For each unit - if err = decoder.Decode(&h); err != nil { - fact.Errors = append(fact.Errors, - fmt.Sprintf("%v: %v", h.IP, err.Error())) - return - } - }() + }() + h := Host{host.IP, make([]Unit, 0)} + decoder := json.NewDecoder(file) + // For each unit + if err = decoder.Decode(&h); err != nil { + return } - var unhealthy strings.Builder - for _, h := range he.Hosts { - for _, u := range h.Units { - if u.Health != 0 { - fact.Status = bun.SProblem - unhealthy.WriteString( - fmt.Sprintf("%v %v: health = %v\n", h.IP, u.Id, u.Health)) - } + unhealthy := make([]string, 0) + for _, u := range h.Units { + if u.Health != 0 { + unhealthy = append(unhealthy, + fmt.Sprintf("%v: health = %v", u.Id, u.Health)) } } - if unhealthy.Len() > 0 { - fact.Long = "The following components are not healthy:\n" + unhealthy.String() + if len(unhealthy) > 0 { + msg = "The following components are not healthy:\n" + strings.Join(unhealthy, "\n") + ok = false } else { - fact.Long = "All the checked components are healthy." - } - if fact.Status != bun.SProblem && len(fact.Errors) > 0 { - fact.Status = bun.SUndefined - } - switch fact.Status { - case bun.SOK: - fact.Short = "All DC/OS systemd units are healthy." - case bun.SProblem: - fact.Short = "Some DC/OS systemd units are not healthy." - case bun.SUndefined: - fact.Short = "Errors occurred when checking systemd units health." + msg = "All the checked components are healthy." + ok = true } - fact.Structured = he - return fact, nil + return } diff --git a/file.go b/file.go deleted file mode 100644 index 21b0988..0000000 --- a/file.go +++ /dev/null @@ -1,113 +0,0 @@ -package bun - -import ( - "compress/gzip" - "errors" - "io" - "os" - "path" - "strconv" - "strings" - "sync" -) - -type ContentType string - -const ( - JSON ContentType = "JSON" - Journal = "journal" - Dmesg = "dmesg" -) - -type FileType struct { - Name string - ContentType ContentType - // If HostTypes is not empty, then Path is relative to the host's directory, - // otherwise, it's relative to the bundle's root directory. - Paths []string - Description string - HostTypes map[HostType]struct{} -} - -var ( - fileTypes = make(map[string]FileType) - fileTypesMu sync.RWMutex -) - -func RegisterFileType(f FileType) { - fileTypesMu.Lock() - defer fileTypesMu.Unlock() - if _, dup := fileTypes[f.Name]; dup { - panic("dcosbundle.RegisterFileType: called twice for driver " + f.Name) - } - fileTypes[f.Name] = f -} - -func GetFileType(typeName string) (FileType, error) { - fileTypesMu.RLock() - defer fileTypesMu.RUnlock() - fileType, ok := fileTypes[typeName] - if !ok { - return fileType, errors.New("No such fileType: " + typeName) - } - return fileType, nil -} - -// OpenFile opens bundle file. Caller is responsible for closing the file. -func OpenFile(basePath string, typeName string) (file File, err error) { - fileType, err := GetFileType(typeName) - if err != nil { - return - } - for _, p := range fileType.Paths { - filePath := path.Join(basePath, p) - file, err = open(filePath) - if err == nil { - break - } - } - return -} - -// Open tries to open a file. If the file is not found, it tries to open it from a -// correspondent .gzip archive. -func open(filePath string) (file File, err error) { - file, err = os.Open(filePath) - if os.IsNotExist(err) { - var gzfile File - if gzfile, err = os.Open(filePath + ".gz"); err != nil { - return - } - if file, err = gzip.NewReader(gzfile); err != nil { - return - } - file = struct { - io.Reader - io.Closer - }{io.Reader(file), - bulkCloser{file, gzfile}} - } - return -} - -type bulkCloser []io.Closer - -func (bc bulkCloser) Close() error { - ee := make([]error, 0) - for _, c := range bc { - if err := c.Close(); err != nil { - ee = append(ee, err) - } - } - if len(ee) > 0 { - var b strings.Builder - for i, e := range ee { - b.WriteString(strconv.Itoa(i)) - b.WriteString(": ") - b.WriteString(e.Error()) - b.WriteString("\n") - } - return errors.New(b.String()) - } - return nil -} diff --git a/file_system.go b/file_system.go deleted file mode 100644 index 231281e..0000000 --- a/file_system.go +++ /dev/null @@ -1,43 +0,0 @@ -package bun - -import ( - "io" - "io/ioutil" - "os" -) - -type FileSystem interface { - // ReadDir reads the directory named by dirname and returns - // a list of directory entries sorted by filename. - // It's mocking the io/ioutil.ReadDir. - ReadDir(string) ([]os.FileInfo, error) - - // Open opens the named file for reading. If successful, methods on - // the returned file can be used for reading. - // It's partially mocking the os.Open function. - Open(string) (File, error) - - // Getwd returns a rooted path name corresponding to the - // current directory. If the current directory can be - // reached via multiple paths (due to symbolic links), - // Getwd may return any one of them. - // It's mocking os.Getwd. - Getwd() (string, error) -} - -type File io.ReadCloser - -// osFS implements FileSystem -type OSFS struct { -} - -func (osfs OSFS) ReadDir(dirname string) ([]os.FileInfo, error) { - return ioutil.ReadDir(dirname) -} -func (osfs OSFS) Open(name string) (File, error) { - file, err := os.Open(name) - return File(file), err -} -func (osfs OSFS) Getwd() (string, error) { - return os.Getwd() -} diff --git a/file_type.go b/file_type.go new file mode 100644 index 0000000..4ce9e6d --- /dev/null +++ b/file_type.go @@ -0,0 +1,48 @@ +package bun + +import ( + "errors" + "sync" +) + +type ContentType string + +const ( + JSON ContentType = "JSON" + Journal = "journal" + Dmesg = "dmesg" +) + +type FileType struct { + Name string + ContentType ContentType + // If HostTypes is not empty, then Path is relative to the host's directory, + // otherwise, it's relative to the bundle's root directory. + Paths []string + Description string + HostTypes map[HostType]struct{} +} + +var ( + fileTypes = make(map[string]FileType) + fileTypesMu sync.RWMutex +) + +func RegisterFileType(f FileType) { + fileTypesMu.Lock() + defer fileTypesMu.Unlock() + if _, dup := fileTypes[f.Name]; dup { + panic("dcosbundle.RegisterFileType: called twice for driver " + f.Name) + } + fileTypes[f.Name] = f +} + +func GetFileType(typeName string) (FileType, error) { + fileTypesMu.RLock() + defer fileTypesMu.RUnlock() + fileType, ok := fileTypes[typeName] + if !ok { + return fileType, errors.New("No such fileType: " + typeName) + } + return fileType, nil +} diff --git a/filesystem.go b/filesystem.go new file mode 100644 index 0000000..73c5dfb --- /dev/null +++ b/filesystem.go @@ -0,0 +1,78 @@ +package bun + +import ( + "context" + "io" + "io/ioutil" + "os" +) + +type key int + +var fsKey key + +// NewContextWithFS returns a new Context that carries Filesystem. +func NewContextWithFS(ctx context.Context, fs Filesystem) context.Context { + return context.WithValue(ctx, fsKey, fs) +} + +// FSFromContext returns the Filesystem value stored in ctx, if any. +func FSFromContext(ctx context.Context) (Filesystem, bool) { + fs, ok := ctx.Value(fsKey).(Filesystem) + return fs, ok +} + +// Filesystem abstracts filesystem, primarly, for writting unit-tests. +type Filesystem interface { + // ReadDir reads the directory named by dirname and returns + // a list of directory entries sorted by filename. + // It's mocking the io/ioutil.ReadDir. + ReadDir(string) ([]os.FileInfo, error) + + // Open opens the named file for reading. If successful, methods on + // the returned file can be used for reading. + // It's partially mocking the os.Open function. + Open(string) (File, error) + + // Getwd returns a rooted path name corresponding to the + // current directory. If the current directory can be + // reached via multiple paths (due to symbolic links), + // Getwd may return any one of them. + // It's mocking os.Getwd. + Getwd() (string, error) + + // IsNotExist returns a boolean indicating whether the error is known to + // report that a file or directory does not exist. + // It's mocking os.IsNotExist + IsNotExist(err error) bool +} + +// File abstraction +type File io.ReadCloser + +// OSFS implements Filesystem interface using filesystem. +type OSFS struct { +} + +// ReadDir implements Filesystem.ReadDir. +func (osfs OSFS) ReadDir(dirname string) ([]os.FileInfo, error) { + return ioutil.ReadDir(dirname) +} + +// Open implements Filesystem.Open. +func (osfs OSFS) Open(name string) (File, error) { + file, err := os.Open(name) + return File(file), err +} + +// Getwd implements Filesystem.Getwd. +func (osfs OSFS) Getwd() (string, error) { + return os.Getwd() +} + +// IsNotExist returns a boolean indicating whether the error is known to +// report that a file or directory does not exist. It is satisfied by +// ErrNotExist as well as some syscall errors. +func (osfs OSFS) IsNotExist(err error) bool { + return os.IsNotExist(err) +} diff --git a/host.go b/host.go deleted file mode 100644 index 3fdd3cd..0000000 --- a/host.go +++ /dev/null @@ -1,22 +0,0 @@ -package bun - -import "io" - -type HostType string - -const ( - Master HostType = "master" - Agent = "agent" - PublicAgent = "public agent" -) - -type Host struct { - Type HostType - IP string - Path string -} - -// OpenFile opens a host-related file. Caller is responsible for closing the file. -func (h Host) OpenFile(fileType string) (io.ReadCloser, error) { - return OpenFile(h.Path, fileType) -} From 20424a6b2f086e5d3a4fa2eff09f20540efef4cb Mon Sep 17 00:00:00 2001 From: Andrey Dyatlov Date: Fri, 7 Sep 2018 16:50:18 +0200 Subject: [PATCH 4/9] Simplify reading from JSON --- bun/cmd/import.go | 4 +- bundle.go | 76 ++++++++++++------- check/health/health.go | 43 ++++------- file/{ => dcosversion}/dcos_version.go | 2 +- file/{ => health}/health.go | 21 ++++- .../actormailboxes}/mesos_actor_mailboxes.go | 2 +- 6 files changed, 87 insertions(+), 61 deletions(-) rename file/{ => dcosversion}/dcos_version.go (95%) rename file/{ => health}/health.go (52%) rename file/{ => mesos/actormailboxes}/mesos_actor_mailboxes.go (95%) diff --git a/bun/cmd/import.go b/bun/cmd/import.go index 5a5fc18..5e1dee7 100644 --- a/bun/cmd/import.go +++ b/bun/cmd/import.go @@ -5,5 +5,7 @@ import ( _ "github.com/adyatlov/bun/check/health" _ "github.com/adyatlov/bun/check/mesos/actormailboxes" _ "github.com/adyatlov/bun/check/nodecount" - _ "github.com/adyatlov/bun/file" + _ "github.com/adyatlov/bun/file/dcosversion" + _ "github.com/adyatlov/bun/file/health" + _ "github.com/adyatlov/bun/file/mesos/actormailboxes" ) diff --git a/bundle.go b/bundle.go index 3a94024..30e85ca 100644 --- a/bundle.go +++ b/bundle.go @@ -3,8 +3,11 @@ package bun import ( "compress/gzip" "context" + "encoding/json" "errors" + "fmt" "io" + "io/ioutil" "log" "path" "path/filepath" @@ -29,23 +32,16 @@ const ( type Host struct { Type HostType IP string - Path string - fs Filesystem -} - -// OpenFile opens a host-related file. Caller is responsible for closing the file. -func (h Host) OpenFile(fileType string) (io.ReadCloser, error) { - return openFile(h.fs, h.Path, fileType) + fileOwner } // Bundle describes DC/OS diagnostics bundle. type Bundle struct { - Path string Hosts map[string]Host // IP to Host map Masters map[string]Host Agents map[string]Host PublicAgents map[string]Host - fs Filesystem + fileOwner } // NewBundle creates new Bundle @@ -103,36 +99,41 @@ func NewBundle(ctx context.Context, path string) (Bundle, error) { return b, nil } -// OpenFile opens a file in a root directory of the bundle. The caller is -// responsible for closing the file. -func (b Bundle) OpenFile(fileType string) (File, error) { - return openFile(b.fs, b.Path, fileType) +type fileOwner struct { + fs Filesystem + Path string } -// openFile tries to open one of the files of the typeName file type. +// OpenFile tries to open one of the files of the typeName file type. // If the file is not found, it tries to open it from a correspondent .gzip archive. -// If the .gzip archive is not found returns an error. -func openFile(fs Filesystem, basePath string, typeName string) (file File, err error) { - fileType, err := GetFileType(typeName) +// If the .gzip archive is not found as well then returns an error. +// Caller is responsible for closing the file. +func (fo fileOwner) OpenFile(typeName string) (file File, err error) { + var fileType FileType + fileType, err = GetFileType(typeName) if err != nil { return } - for _, p := range fileType.Paths { - filePath := path.Join(basePath, p) - if file, err = fs.Open(filePath); err == nil { - return + notFound := make([]string, 0) + for _, localPath := range fileType.Paths { + filePath := path.Join(fo.Path, localPath) + if file, err = fo.fs.Open(filePath); err == nil { + return // found } - if fs.IsNotExist(err) { + if fo.fs.IsNotExist(err) { // not found + notFound = append(notFound, filePath) var gzfile File - if gzfile, err = fs.Open(filePath + ".gz"); err != nil { - if fs.IsNotExist(err) { - continue + filePath += ".gz" + if gzfile, err = fo.fs.Open(filePath); err != nil { + if fo.fs.IsNotExist(err) { + notFound = append(notFound, filePath) + continue // not found } else { - return + return // error } } if file, err = gzip.NewReader(gzfile); err != nil { - return + return //error } file = struct { io.Reader @@ -141,9 +142,30 @@ func openFile(fs Filesystem, basePath string, typeName string) (file File, err e return } } + err = fmt.Errorf("none of the following files are found:\n%v", + strings.Join(notFound, "\n")) return } +// ReadJSON parses the JSON-encoded data from the file and stores the result in +// the value pointed to by v. +func (fo fileOwner) ReadJSON(typeName string, v interface{}) error { + file, err := fo.OpenFile(typeName) + if err != nil { + return err + } + defer func() { + if err := file.Close(); err != nil { + log.Printf("Error when closing file health: %v", err) + } + }() + data, err := ioutil.ReadAll(file) + if err != nil { + return err + } + return json.Unmarshal(data, v) +} + type bulkCloser []io.Closer func (bc bulkCloser) Close() error { diff --git a/check/health/health.go b/check/health/health.go index 43fc0a5..fc11bf3 100644 --- a/check/health/health.go +++ b/check/health/health.go @@ -1,12 +1,11 @@ package health import ( - "encoding/json" "fmt" - "log" "strings" "github.com/adyatlov/bun" + "github.com/adyatlov/bun/file/health" ) const ( @@ -15,21 +14,12 @@ const ( ) func init() { - bun.RegisterCheck(bun.CheckInfo{name, description}, healthCheck.Check) -} - -type Health struct { - Hosts []Host -} -type Host struct { - IP string - Units []Unit -} -type Unit struct { - Id string - Name string - Health int - Output string + bun.RegisterCheck( + bun.CheckInfo{ + Name: name, + Description: description, + }, + healthCheck.Check) } var healthCheck bun.AtomicCheck = bun.AtomicCheck{ @@ -39,26 +29,19 @@ var healthCheck bun.AtomicCheck = bun.AtomicCheck{ } func check(host bun.Host) (ok bool, msg string, err error) { - file, err := host.OpenFile("health") - if err != nil { + h := health.Host{Units: make([]health.Unit, 0)} + if err = host.ReadJSON("health", &h); err != nil { return } - defer func() { - if err := file.Close(); err != nil { - log.Printf("Error when closing file health: %v", err) - } - }() - h := Host{host.IP, make([]Unit, 0)} - decoder := json.NewDecoder(file) - // For each unit - if err = decoder.Decode(&h); err != nil { - return + if h.IP != host.IP { + err = fmt.Errorf("IP specified in the health JSON file %v != host IP %v", + h.IP, host.IP) } unhealthy := make([]string, 0) for _, u := range h.Units { if u.Health != 0 { unhealthy = append(unhealthy, - fmt.Sprintf("%v: health = %v", u.Id, u.Health)) + fmt.Sprintf("%v: health = %v", u.ID, u.Health)) } } if len(unhealthy) > 0 { diff --git a/file/dcos_version.go b/file/dcosversion/dcos_version.go similarity index 95% rename from file/dcos_version.go rename to file/dcosversion/dcos_version.go index a7cfb54..a86ea57 100644 --- a/file/dcos_version.go +++ b/file/dcosversion/dcos_version.go @@ -1,4 +1,4 @@ -package file +package dcosversion import "github.com/adyatlov/bun" diff --git a/file/health.go b/file/health/health.go similarity index 52% rename from file/health.go rename to file/health/health.go index cde5351..a478a0e 100644 --- a/file/health.go +++ b/file/health/health.go @@ -1,4 +1,4 @@ -package file +package health import "github.com/adyatlov/bun" @@ -14,3 +14,22 @@ func init() { } bun.RegisterFileType(f) } + +// Health represents the health JSON file +type Health struct { + Hosts []Host +} + +// Host represents the "host" object in the health JSON file +type Host struct { + IP string `json:"ip"` + Units []Unit +} + +// Unit represents the "unit" object in the health JSON file +type Unit struct { + ID string `json:"id"` + Name string + Health int + Output string +} diff --git a/file/mesos_actor_mailboxes.go b/file/mesos/actormailboxes/mesos_actor_mailboxes.go similarity index 95% rename from file/mesos_actor_mailboxes.go rename to file/mesos/actormailboxes/mesos_actor_mailboxes.go index 2fa9247..eca164f 100644 --- a/file/mesos_actor_mailboxes.go +++ b/file/mesos/actormailboxes/mesos_actor_mailboxes.go @@ -1,4 +1,4 @@ -package file +package actormailboxes import "github.com/adyatlov/bun" From 05807a62aa7dd0bd6e3db00edb1cd14dd222a15e Mon Sep 17 00:00:00 2001 From: Andrey Dyatlov Date: Fri, 7 Sep 2018 19:58:39 +0200 Subject: [PATCH 5/9] Make health check even smaller. --- bundle.go | 1 + check/health/health.go | 9 ++------- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/bundle.go b/bundle.go index 30e85ca..ddeebd8 100644 --- a/bundle.go +++ b/bundle.go @@ -150,6 +150,7 @@ func (fo fileOwner) OpenFile(typeName string) (file File, err error) { // ReadJSON parses the JSON-encoded data from the file and stores the result in // the value pointed to by v. func (fo fileOwner) ReadJSON(typeName string, v interface{}) error { + // TODO: check if fileType is JSON file, err := fo.OpenFile(typeName) if err != nil { return err diff --git a/check/health/health.go b/check/health/health.go index fc11bf3..7813025 100644 --- a/check/health/health.go +++ b/check/health/health.go @@ -8,16 +8,11 @@ import ( "github.com/adyatlov/bun/file/health" ) -const ( - name = "health" - description = "Check if all DC/OS components are healthy" -) - func init() { bun.RegisterCheck( bun.CheckInfo{ - Name: name, - Description: description, + Name: "health", + Description: "Check if all DC/OS components are healthy", }, healthCheck.Check) } From 0dfd3b65b9f5a920ea224b01fcc455411f829bad Mon Sep 17 00:00:00 2001 From: Andrey Dyatlov Date: Mon, 10 Sep 2018 00:25:35 +0200 Subject: [PATCH 6/9] Transform the atomic check concept to the check builder concept. Improve reporting. Transfer all the existing checks to the new framework. --- atomic_check.go | 123 -------------- bun/cmd/check.go | 73 +++++--- bun/cmd/import.go | 6 +- check.go | 129 +++------------ check/dcosversion/dcos_version.go | 99 +++++------ check/health/health.go | 30 ++-- .../actormailboxes/mesos_actor_mailboxes.go | 125 ++++---------- check/nodecount/node_count.go | 59 ++----- check_builder.go | 156 ++++++++++++++++++ check_registry.go | 43 +++++ .../file.go} | 7 +- file/{health/health.go => healthfile/file.go} | 2 +- .../file.go} | 13 +- 13 files changed, 395 insertions(+), 470 deletions(-) delete mode 100644 atomic_check.go create mode 100644 check_builder.go create mode 100644 check_registry.go rename file/{dcosversion/dcos_version.go => dcosversionfile/file.go} (77%) rename file/{health/health.go => healthfile/file.go} (97%) rename file/mesos/{actormailboxes/mesos_actor_mailboxes.go => actormailboxesfile/file.go} (54%) diff --git a/atomic_check.go b/atomic_check.go deleted file mode 100644 index 42e60d3..0000000 --- a/atomic_check.go +++ /dev/null @@ -1,123 +0,0 @@ -package bun - -import ( - "context" - "fmt" - "strings" -) - -// HostCheck checks an individual host. It returns status, details, and error if -// the function cannot perform a check. If the returned error is not nil, then -// the status is ignored. -type HostCheck func(Host) (bool, string, error) - -// HostCheckResult is a reult of a single host check. -type HostCheckResult struct { - IP string - OK bool - Details string - Err error -} - -// AtomicCheck builds Check as a sum of HostChecks. -type AtomicCheck struct { - ForEachMaster HostCheck - ForEachAgent HostCheck - ForEachPublicAgent HostCheck - ProblemMessage string - OKMessage string -} - -// Check is an impementation of the bun.Check function. -func (a AtomicCheck) Check(ctx context.Context, - b Bundle, - p chan<- Progress) (fact Fact, err error) { - progress := Progress{} - okResults := make([]HostCheckResult, 0) - problemResults := make([]HostCheckResult, 0) - errResults := make([]HostCheckResult, 0) - if a.ProblemMessage == "" { - a.ProblemMessage = "Problems were found." - } - if a.OKMessage == "" { - a.OKMessage = "No problems were found." - } - check := func(hosts map[string]Host, hc HostCheck) error { - for ip, host := range hosts { - progress.Stage = ip - select { - case p <- progress: - default: - } - select { - case <-ctx.Done(): - return ctx.Err() - default: - } - r := HostCheckResult{} - r.IP = host.IP - r.OK, r.Details, r.Err = hc(host) - if r.Err != nil { - errResults = append(errResults, r) - } else if r.OK { - okResults = append(okResults, r) - } else { - problemResults = append(problemResults, r) - } - progress.Step++ - } - return nil - } - queue := make([]func() error, 0, 3) - if a.ForEachMaster != nil { - progress.Count += len(b.Masters) - queue = append(queue, func() error { return check(b.Masters, a.ForEachMaster) }) - } - if a.ForEachAgent != nil { - progress.Count += len(b.Agents) - queue = append(queue, func() error { return check(b.Agents, a.ForEachAgent) }) - } - if a.ForEachPublicAgent != nil { - progress.Count += len(b.PublicAgents) - queue = append(queue, func() error { return check(b.PublicAgents, a.ForEachPublicAgent) }) - } - for _, c := range queue { - if err = c(); err != nil { - return - } - } - short := make([]string, 0) - if len(errResults) > 0 { - short = append(short, "Error(s) occured when performing the check.") - } - if len(problemResults) > 0 { - fact.Status = SProblem - short = append(short, a.ProblemMessage) - } else if len(errResults) == 0 { - fact.Status = SOK - short = append(short, a.OKMessage) - } else { - fact.Status = SUndefined - } - fact.Short = strings.Join(short, " ") - var long strings.Builder - if len(problemResults) > 0 { - long.WriteString("Problems:\n") - for _, res := range problemResults { - long.WriteString(fmt.Sprintf("%v: %v\n", res.IP, res.Details)) - } - } - if len(okResults) > 0 { - long.WriteString("Successfull checks:\n") - for _, res := range okResults { - long.WriteString(fmt.Sprintf("%v: %v\n", res.IP, res.Details)) - } - } - if len(errResults) > 0 { - for _, res := range errResults { - fact.Errors = append(fact.Errors, res.Err.Error()) - } - } - fact.Long = long.String() - return -} diff --git a/bun/cmd/check.go b/bun/cmd/check.go index 2fd27c3..3604c68 100644 --- a/bun/cmd/check.go +++ b/bun/cmd/check.go @@ -5,36 +5,68 @@ import ( "fmt" "os" "sort" + "strings" "github.com/adyatlov/bun" "github.com/spf13/cobra" ) var printLong = false +var bundle *bun.Bundle -func printReport(r bun.Report) { - fmt.Printf("[%v] \"%v\" - %v\n", r.Status, r.Name, r.Short) - if r.Status == bun.SProblem || printLong { - fmt.Printf("%v\n", r.Long) - } - if len(r.Errors) > 0 { - fmt.Println("Errors:") - for i, err := range r.Errors { - fmt.Printf("%v: %v\n", i+1, err) +func printReport(c bun.Check) { + printEmptyLine := false + fmt.Printf("[%v] \"%v\" - %v\n", c.Status, c.Name, c.Summary) + if printLong { + if len(c.Problems) > 0 { + fmt.Println("--------") + fmt.Println("Problems") + fmt.Println("--------") + fmt.Println(strings.Join(c.Problems, "\n")) + printEmptyLine = true + } + if len(c.Errors) > 0 { + fmt.Println("------") + fmt.Println("Errors") + fmt.Println("------") + fmt.Println(strings.Join(c.Errors, "\n")) + printEmptyLine = true + } + if len(c.OKs) > 0 { + fmt.Println("---") + fmt.Println("OKs") + fmt.Println("---") + fmt.Println(strings.Join(c.OKs, "\n")) + printEmptyLine = true + } + } else { + if len(c.Problems) > 0 { + fmt.Println("--------") + fmt.Println("Problems") + fmt.Println("--------") + fmt.Println(strings.Join(c.Problems, "\n")) + printEmptyLine = true } - fmt.Printf("%v\n", r.Long) + if len(c.Errors) > 0 { + fmt.Println("------") + fmt.Println("Errors") + fmt.Println("------") + fmt.Println(strings.Join(c.Errors, "\n")) + printEmptyLine = true + } + } + if printEmptyLine { + fmt.Print("\n") } } -var bundle *bun.Bundle - func preRun(cmd *cobra.Command, args []string) { if bundle != nil { return } b, err := bun.NewBundle(context.Background(), bundlePath) if err != nil { - fmt.Printf("Error while identifying basic bundle parameters: %v\n", err.Error()) + fmt.Printf("Cannot find a bundle: %v\n", err.Error()) os.Exit(1) } bundle = &b @@ -51,13 +83,9 @@ func runCheck(cmd *cobra.Command, args []string) { return checks[i].Name < checks[j].Name }) for _, check := range checks { - report, err := bun.RunCheckSimple(check.Name, *bundle) - if err != nil { - fmt.Printf("Error while running check %v: %v", check.Name, err.Error()) - } - printReport(report) + check.Run(*bundle) + printReport(check) } - return } func init() { @@ -76,11 +104,8 @@ Or run all the available checks by not specifying any, i.e.` + " `bun check`.", for _, check := range bun.Checks() { run := func(cmd *cobra.Command, args []string) { - report, err := bun.RunCheckSimple(cmd.Name(), *bundle) - if err != nil { - fmt.Println(err.Error()) - } - printReport(report) + check.Run(*bundle) + printReport(check) return } var cmd = &cobra.Command{ diff --git a/bun/cmd/import.go b/bun/cmd/import.go index 5e1dee7..2ba5513 100644 --- a/bun/cmd/import.go +++ b/bun/cmd/import.go @@ -5,7 +5,7 @@ import ( _ "github.com/adyatlov/bun/check/health" _ "github.com/adyatlov/bun/check/mesos/actormailboxes" _ "github.com/adyatlov/bun/check/nodecount" - _ "github.com/adyatlov/bun/file/dcosversion" - _ "github.com/adyatlov/bun/file/health" - _ "github.com/adyatlov/bun/file/mesos/actormailboxes" + _ "github.com/adyatlov/bun/file/dcosversionfile" + _ "github.com/adyatlov/bun/file/healthfile" + _ "github.com/adyatlov/bun/file/mesos/actormailboxesfile" ) diff --git a/check.go b/check.go index 70fd02b..53dbdfe 100644 --- a/check.go +++ b/check.go @@ -1,118 +1,35 @@ package bun -import ( - "context" - "errors" - "sync" -) - -type Progress struct { - Stage string - Step int - Count int -} - -type Check func(context.Context, Bundle, chan<- Progress) (Fact, error) - -type CheckInfo struct { - Name string - Description string -} - +// Status defines possible check outcomes. type Status string const ( + // SUndefined means that the check wasn't performed successfully. SUndefined Status = "UNDEFINED" - SOK = "OK" - SProblem = "PROBLEM" -) - -// Fact is a piece of knowledge about DC/OS cluster learned from a -// diagnostic bundle. -type Fact struct { - Status Status - Short string - Long string - Errors []string - Structured interface{} -} - -type Report struct { - CheckInfo - Fact -} - -type checkRecord struct { - CheckInfo - Check -} - -var ( - checks = make(map[string]checkRecord) - checksMu sync.RWMutex + // SOK means that the bundle passed the check. + SOK = "OK" + // SProblem means that the bundle failed to pass the check. + SProblem = "PROBLEM" ) -func RegisterCheck(ci CheckInfo, c Check) { - checksMu.Lock() - defer checksMu.Unlock() - if _, dup := checks[ci.Name]; dup { - panic("dcosbundle.RegisterCheck: called twice for driver " + ci.Name) - } - checks[ci.Name] = checkRecord{ci, c} -} - -func Checks() []CheckInfo { - checksMu.RLock() - defer checksMu.RUnlock() - cc := make([]CheckInfo, 0, len(checks)) - for _, cr := range checks { - cc = append(cc, cr.CheckInfo) - } - return cc -} +// cluster analyzing its diagnostics bundle. Check supposed to populate fields +// of the CheckResult. +// Each check should implement this interface. -type NamedProgress struct { - Name string - Progress -} - -func RunCheckSimple(name string, b Bundle) (Report, error) { - return RunCheck(context.Background(), name, b, nil) -} - -func RunCheck(ctx context.Context, name string, b Bundle, - np chan<- NamedProgress) (Report, error) { - checksMu.RLock() - cr, ok := checks[name] - checksMu.RUnlock() - if !ok { - return Report{}, errors.New("No such check: " + name) - } - // Handle progress - p := make(chan Progress, 100) - localCtx, cancel := context.WithCancel(ctx) - defer cancel() - go func() { - for { - select { - case prog := <-p: - np <- NamedProgress{cr.Name, prog} - case <-localCtx.Done(): - return - } - } - }() - // Running check may take some time - f, err := cr.Check(ctx, b, p) - if err != nil { - return Report{cr.CheckInfo, f}, err - } - return Report{cr.CheckInfo, f}, nil +// Check cheks some aspect of the DC/OS cluster analyzing its diagnostics +// bundle. +// Checks can be registered in the check registry with the egisterCheck function. +type Check struct { + Name string + Description string + Status Status + CheckFunc func(*Check, Bundle) + Summary string + Problems []string + Errors []string + OKs []string } -func SendProg(p chan<- Progress, stage string, step int, count int) { - select { - case p <- Progress{stage, step, count}: - default: - } +func (c *Check) Run(b Bundle) { + c.CheckFunc(c, b) } diff --git a/check/dcosversion/dcos_version.go b/check/dcosversion/dcos_version.go index db0a0b5..15a89a5 100644 --- a/check/dcosversion/dcos_version.go +++ b/check/dcosversion/dcos_version.go @@ -1,72 +1,57 @@ package dcosversion import ( - "context" - "encoding/json" "fmt" - "log" "github.com/adyatlov/bun" -) - -const ( - name = "dcos-version" - description = "Verify that all hosts in the cluster have the same DC/OS version installed" - errDiffVer = "Versions are different" + "github.com/adyatlov/bun/file/dcosversionfile" ) func init() { - bun.RegisterCheck(bun.CheckInfo{name, description}, checkVersion) + builder := bun.CheckBuilder{ + Name: "dcos-version", + Description: "Verify that all hosts in the cluster have the " + + "same DC/OS version installed", + ForEachMaster: check, + ForEachAgent: check, + ForEachPublicAgent: check, + Interpret: interpret, + } + builder.BuildAndRegister() } -func checkVersion(ctx context.Context, b bun.Bundle, - p chan<- bun.Progress) (bun.Fact, error) { - fact := bun.Fact{Status: bun.SOK} - fact.Errors = make([]string, 0) - step := 0 - for _, host := range b.Hosts { - // Check if canceled - select { - case <-ctx.Done(): - return fact, ctx.Err() - default: +func check(host bun.Host) (ok bool, details interface{}, err error) { + v := dcosversionfile.Version{} + if err = host.ReadJSON("dcos-version", &v); err != nil { + return + } + details = v.Version + ok = true + return +} + +func interpret(c *bun.Check, b bun.CheckBuilder) { + version := "" + // Compare versions + details := []string{} + ok := true + for _, r := range b.OKs { + v := r.Details.(string) + if version == "" { + version = v + continue + } + if v != version { + ok = false } - func() { - // Read version - file, err := host.OpenFile("dcos-version") - if err != nil { - fact.Errors = append(fact.Errors, err.Error()) - return - } - defer func() { - if err := file.Close(); err != nil { - log.Printf("Error when closing file dcos-version: %v", err) - } - }() - decoder := json.NewDecoder(file) - verStruct := &struct{ Version string }{} - if err = decoder.Decode(verStruct); err != nil { - fact.Errors = append(fact.Errors, err.Error()) - return - } - // Compare version - if fact.Status == bun.SOK { - if fact.Short == "" { - fact.Short = verStruct.Version - } else if fact.Short != verStruct.Version { - fact.Status = bun.SProblem - fact.Short = errDiffVer - } - } - fact.Long += fmt.Sprintf("%v %v has DC/OS version %v\n", - host.Type, host.IP, verStruct.Version) - // Report progress - bun.SendProg(p, "Checked version installed on "+host.IP, step, len(b.Hosts)) - step++ - }() + details = append(details, fmt.Sprintf("%v %v has DC/OS version %v", + r.Host.Type, r.Host.IP, v)) } - if fact.Status == bun.SOK && len(fact.Errors) > 0 { - fact.Status = bun.SUndefined + // No need to interpret problems, as we didn't create it in the host check. + if ok { + c.Summary = fmt.Sprintf("All versions are the same: %v.", version) + } else { + c.Problems = details + c.Summary = "Versions are different." } - return fact, nil } diff --git a/check/health/health.go b/check/health/health.go index 7813025..ae48e2b 100644 --- a/check/health/health.go +++ b/check/health/health.go @@ -5,26 +5,22 @@ import ( "strings" "github.com/adyatlov/bun" - "github.com/adyatlov/bun/file/health" + "github.com/adyatlov/bun/file/healthfile" ) func init() { - bun.RegisterCheck( - bun.CheckInfo{ - Name: "health", - Description: "Check if all DC/OS components are healthy", - }, - healthCheck.Check) -} - -var healthCheck bun.AtomicCheck = bun.AtomicCheck{ - ForEachMaster: check, - ForEachAgent: check, - ForEachPublicAgent: check, + builder := bun.CheckBuilder{ + Name: "health", + Description: "Check if all DC/OS components are healthy", + ForEachMaster: check, + ForEachAgent: check, + ForEachPublicAgent: check, + } + builder.BuildAndRegister() } -func check(host bun.Host) (ok bool, msg string, err error) { - h := health.Host{Units: make([]health.Unit, 0)} +func check(host bun.Host) (ok bool, details interface{}, err error) { + h := healthfile.Host{Units: make([]healthfile.Unit, 0)} if err = host.ReadJSON("health", &h); err != nil { return } @@ -40,10 +36,10 @@ func check(host bun.Host) (ok bool, msg string, err error) { } } if len(unhealthy) > 0 { - msg = "The following components are not healthy:\n" + strings.Join(unhealthy, "\n") + details = "The following components are not healthy:\n" + strings.Join(unhealthy, "\n") ok = false } else { - msg = "All the checked components are healthy." + details = "" ok = true } return diff --git a/check/mesos/actormailboxes/mesos_actor_mailboxes.go b/check/mesos/actormailboxes/mesos_actor_mailboxes.go index dbdb980..f54da48 100644 --- a/check/mesos/actormailboxes/mesos_actor_mailboxes.go +++ b/check/mesos/actormailboxes/mesos_actor_mailboxes.go @@ -1,112 +1,51 @@ package actormailboxes import ( - "context" - "encoding/json" "fmt" - "io/ioutil" - "log" "strings" "github.com/adyatlov/bun" + "github.com/adyatlov/bun/file/mesos/actormailboxesfile" ) -const ( - name = "mesos-actor-mailboxes" - description = "Check if actor mailboxes in the Mesos process have a reasonable amount of messages" - max_events = 30 // the number of events in an actor's mailbox after which the actor is considered backlogged -) +// number of events in an actor's mailbox after which the actor is +// considered backlogged +const maxEvents = 30 func init() { - bun.RegisterCheck(bun.CheckInfo{name, description}, checkMesosActorMailboxes) + builder := bun.CheckBuilder{ + Name: "mesos-actor-mailboxes", + Description: "Check if actor mailboxes in the Mesos process " + + "have a reasonable amount of messages", + OKSummary: "All Mesos actors are fine.", + ProblemSummary: "Some Mesos actors are backlogged.", + ForEachMaster: check, + ForEachAgent: check, + ForEachPublicAgent: check, + } + builder.BuildAndRegister() } // Truncated JSON schema of __processes__. -type MesosActor struct { - Id string - Events []MailboxEvent -} -type MailboxEvent struct { -} - -func checkMesosActorMailboxes(ctx context.Context, b bun.Bundle, - p chan<- bun.Progress) (bun.Fact, error) { - - // Prepare the output struct. - fact := bun.Fact{Status: bun.SOK} - fact.Errors = make([]string, 0) - - // Used to report progress, which is not quite necessary for this - // lightweight check. - step := 0 - - // Accumulator for error strings. - var unhealthy strings.Builder - - // Iterate over all hosts in the bundle - for _, host := range b.Hosts { - // Check if canceled. - select { - case <-ctx.Done(): - return fact, ctx.Err() - default: - } - - // For each host. - func() { - step++ - - file, err := host.OpenFile("processes") - if err != nil { - fact.Errors = append(fact.Errors, err.Error()) - return - } - defer func() { - if err := file.Close(); err != nil { - log.Printf("Error when closing file __processes__: %v", err) - } - }() - - bytes, err := ioutil.ReadAll(file) - if err != nil { - fact.Errors = append(fact.Errors, err.Error()) - return - } - - var o []MesosActor - err = json.Unmarshal(bytes, &o) - if err != nil { - fact.Errors = append(fact.Errors, err.Error()) - return - } - // Walk through all actors and check the number of events in the mailbox. - for _, a := range o { - if len(a.Events) > max_events { - fact.Status = bun.SProblem - unhealthy.WriteString( - fmt.Sprintf("(Mesos) %v@%v: mailbox size = %v (> %v)\n", a.Id, host.IP, len(a.Events), max_events)) - } - } - }() +func check(host bun.Host) (ok bool, details interface{}, err error) { + details = "" + actors := []actormailboxesfile.MesosActor{} + if err = host.ReadJSON("processes", &actors); err != nil { + return } - - if unhealthy.Len() > 0 { - fact.Long = "The following Mesos actor mailboxes are too big:\n" + unhealthy.String() - } else { - fact.Long = "All checked Mesos actor mailboxes are fine." - } - if fact.Status != bun.SProblem && len(fact.Errors) > 0 { - fact.Status = bun.SUndefined + var u = []string{} + for _, a := range actors { + if len(a.Events) > maxEvents { + u = append(u, fmt.Sprintf("(Mesos) %v@%v: mailbox size = %v (> %v)", + a.Id, host.IP, len(a.Events), maxEvents)) + } } - switch fact.Status { - case bun.SOK: - fact.Short = "All Mesos actors are fine." - case bun.SProblem: - fact.Short = "Some Mesos actors are backlogged." - case bun.SUndefined: - fact.Short = "Errors occurred when checking Mesos actor mailboxes." + if len(u) > 0 { + details = "The following Mesos actor mailboxes are too big:\n" + + strings.Join(u, "\n") + return } - - return fact, nil + ok = true + return } diff --git a/check/nodecount/node_count.go b/check/nodecount/node_count.go index b4000f5..d57201c 100644 --- a/check/nodecount/node_count.go +++ b/check/nodecount/node_count.go @@ -1,57 +1,30 @@ package nodecount import ( - "context" "fmt" "github.com/adyatlov/bun" ) -const ( - name = "node-count" - description = "Count nodes of each type, checks if the amount of master nodes is odd" -) - func init() { - bun.RegisterCheck(bun.CheckInfo{name, description}, countNodes) + check := bun.Check{ + Name: "node-count", + Description: "Count nodes of each type, checks if the amount of " + + "master nodes is odd", + CheckFunc: checkFunc, + } + bun.RegisterCheck(check) } -func countNodes(ctx context.Context, b bun.Bundle, - p chan<- bun.Progress) (bun.Fact, error) { - fact := bun.Fact{} - nMasters := 0 - nAgents := 0 - nPublic := 0 - step := 0 - for _, h := range b.Hosts { - select { - case <-ctx.Done(): - return fact, ctx.Err() - default: - } - step++ - bun.SendProg(p, "Detecting type of "+h.IP, step, len(b.Hosts)) - switch h.Type { - case bun.Master: - nMasters++ - case bun.Agent: - nAgents++ - case bun.PublicAgent: - nPublic++ - } - fact.Long += fmt.Sprintf("%v: %v\n", h.IP, h.Type) - } - fact.Long += fmt.Sprintf("total: %v", len(b.Hosts)) - if nMasters%2 != 0 { - fact.Status = bun.SOK - fact.Short = fmt.Sprintf( - "Masters: %v, Agents: %v, Public Agents: %v, Total: %v", - nMasters, nAgents, nPublic, len(b.Hosts)) +func checkFunc(c *bun.Check, b bun.Bundle) { + stats := fmt.Sprintf( + "Masters: %v, Agents: %v, Public Agents: %v, Total: %v", + len(b.Masters), len(b.Agents), len(b.PublicAgents), len(b.Hosts)) + if len(b.Masters)%2 != 0 { + c.Status = bun.SOK + c.Summary = stats } else { - fact.Status = bun.SProblem - fact.Short = fmt.Sprintf( - "Number of masters is not valid: %v, Agents: %v, Public Agents: %v, Total: %v", - nMasters, nAgents, nPublic, len(b.Hosts)) + c.Status = bun.SProblem + c.Summary = fmt.Sprintf("Number of masters is not valid. %v", stats) } - return fact, nil } diff --git a/check_builder.go b/check_builder.go new file mode 100644 index 0000000..a707837 --- /dev/null +++ b/check_builder.go @@ -0,0 +1,156 @@ +package bun + +import ( + "errors" + "fmt" +) + +const errName = "bun.CheckBuilder.Build: Check Name should be specified" +const errForEach = "bun.CheckBuilder.Build: At least one of the ForEach functions" + + "shoul be specified" + +// MsgErr is a standard message used in the check summary when errors +// occures during the check. +const MsgErr = "Error(s) occurred while performing the check." + +// CheckHost checks an individual host. It returns status, details, and error if +// the function cannot perform a check. If the returned error is not nil, then +// the status is ignored. +type CheckHost func(Host) (bool, interface{}, error) + +// Interpret check reults. +type Interpret func(*Check, CheckBuilder) + +// Result hols results of the CheckHost function. +type Result struct { + Host Host + OK bool + Details interface{} + Err error +} + +// CheckBuilder helps to create checks. +type CheckBuilder struct { + Name string + Description string + ForEachMaster CheckHost + ForEachAgent CheckHost + ForEachPublicAgent CheckHost + ProblemSummary string + OKSummary string + Interpret Interpret + Problems []Result + OKs []Result +} + +// Build returns a Check +func (b *CheckBuilder) Build() (Check, error) { + check := Check{} + if b.Name == "" { + return check, errors.New(errName) + } + if b.ForEachMaster == nil && b.ForEachAgent == nil && + b.ForEachPublicAgent == nil { + return check, errors.New(errForEach) + } + if b.ProblemSummary == "" { + b.ProblemSummary = "Problems were found." + } + if b.OKSummary == "" { + b.OKSummary = "No problems were found." + } + if b.Interpret == nil { + b.Interpret = interpret + } + b.Problems = []Result{} + b.OKs = []Result{} + check = Check{ + Name: b.Name, + Description: b.Description, + CheckFunc: b.check, + Problems: []string{}, + Errors: []string{}, + OKs: []string{}, + } + return check, nil +} + +// BuildAndRegister calls CheckBuilder.Build and register the resulted check. +// This function panics when error occures during the build. +func (b *CheckBuilder) BuildAndRegister() { + check, err := b.Build() + if err != nil { + panic(fmt.Sprintf("Fatal error occurred while building the \"%v\"check: %v", + b.Name, err.Error())) + } + RegisterCheck(check) +} + +func formatMsg(h Host, msg string) string { + return fmt.Sprintf("%v %v: %v", h.Type, h.IP, msg) +} + +func (b *CheckBuilder) checkHosts(c *Check, h map[string]Host, ch CheckHost) { + for _, host := range h { + r := Result{} + r.Host = host + r.OK, r.Details, r.Err = ch(host) + if r.Err != nil { + c.Errors = append(c.Errors, formatMsg(r.Host, r.Err.Error())) + } else if r.OK { + b.OKs = append(b.OKs, r) + } else { + b.Problems = append(b.Problems, r) + } + } +} + +// Default inplementation of the Interpret function. +// It assumes that the CheckHost returns Result Details as a string. +func interpret(c *Check, b CheckBuilder) { + for _, r := range b.Problems { + d := r.Details.(string) + if d != "" { + c.Problems = append(c.Problems, formatMsg(r.Host, d)) + } + } + for _, r := range b.OKs { + d := r.Details.(string) + if d != "" { + c.OKs = append(c.OKs, formatMsg(r.Host, d)) + } + } +} + +// Check.CheckFunc +func (b *CheckBuilder) check(c *Check, bundle Bundle) { + if b.ForEachMaster != nil { + b.checkHosts(c, bundle.Masters, b.ForEachMaster) + } + if b.ForEachAgent != nil { + b.checkHosts(c, bundle.Agents, b.ForEachAgent) + } + if b.ForEachPublicAgent != nil { + b.checkHosts(c, bundle.PublicAgents, b.ForEachPublicAgent) + } + b.Interpret(c, *b) + if len(c.Problems) > 0 { + c.Status = SProblem + if c.Summary == "" { + c.Summary = b.ProblemSummary + } + if len(c.Errors) > 0 { + c.Summary += " " + MsgErr + } + } else if len(c.Errors) == 0 { + c.Status = SOK + if c.Summary == "" { + c.Summary = b.OKSummary + } + } else { + c.Status = SUndefined + if c.Summary == "" { + c.Summary = MsgErr + } + } +} diff --git a/check_registry.go b/check_registry.go new file mode 100644 index 0000000..2160027 --- /dev/null +++ b/check_registry.go @@ -0,0 +1,43 @@ +package bun + +import ( + "fmt" + "sync" +) + +var ( + checkRegistry = make(map[string]Check) + checkRegistryMu sync.RWMutex +) + +// RegisterCheck registers a new check to make it discoverable for consumers. +func RegisterCheck(c Check) { + checkRegistryMu.Lock() + defer checkRegistryMu.Unlock() + if _, exists := checkRegistry[c.Name]; exists { + panic("bun.RegisterCheck: called twice for check " + c.Name) + } + checkRegistry[c.Name] = c +} + +// Checks Returns all registered checks. +func Checks() []Check { + checkRegistryMu.RLock() + defer checkRegistryMu.RUnlock() + checks := make([]Check, 0, len(checkRegistry)) + for _, c := range checkRegistry { + checks = append(checks, c) + } + return checks +} + +// GetCheck returns check by name. +func GetCheck(name string) (Check, error) { + checkRegistryMu.RLock() + check, ok := checkRegistry[name] + checkRegistryMu.RUnlock() + if !ok { + return check, fmt.Errorf("No such check: %v", name) + } + return check, nil +} diff --git a/file/dcosversion/dcos_version.go b/file/dcosversionfile/file.go similarity index 77% rename from file/dcosversion/dcos_version.go rename to file/dcosversionfile/file.go index a86ea57..011e9d4 100644 --- a/file/dcosversion/dcos_version.go +++ b/file/dcosversionfile/file.go @@ -1,4 +1,4 @@ -package dcosversion +package dcosversionfile import "github.com/adyatlov/bun" @@ -14,3 +14,8 @@ func init() { } bun.RegisterFileType(f) } + +// Version represents the dcos-version JSON file +type Version struct { + Version string +} diff --git a/file/health/health.go b/file/healthfile/file.go similarity index 97% rename from file/health/health.go rename to file/healthfile/file.go index a478a0e..879ac03 100644 --- a/file/health/health.go +++ b/file/healthfile/file.go @@ -1,4 +1,4 @@ -package health +package healthfile import "github.com/adyatlov/bun" diff --git a/file/mesos/actormailboxes/mesos_actor_mailboxes.go b/file/mesos/actormailboxesfile/file.go similarity index 54% rename from file/mesos/actormailboxes/mesos_actor_mailboxes.go rename to file/mesos/actormailboxesfile/file.go index eca164f..67c623c 100644 --- a/file/mesos/actormailboxes/mesos_actor_mailboxes.go +++ b/file/mesos/actormailboxesfile/file.go @@ -1,4 +1,4 @@ -package actormailboxes +package actormailboxesfile import "github.com/adyatlov/bun" @@ -6,7 +6,10 @@ func init() { f := bun.FileType{ Name: "processes", ContentType: bun.JSON, - Paths: []string{"5050-__processes__.json", "5051-__processes__.json", "5050:__processes__.json", "5051:__processes__.json"}, + Paths: []string{"5050-__processes__.json", + "5051-__processes__.json", + "5050:__processes__.json", + "5051:__processes__.json"}, Description: "contains mailbox contents for all actors in the Mesos process on the host.", HostTypes: map[bun.HostType]struct{}{ bun.Master: {}, bun.Agent: {}, bun.PublicAgent: {}, @@ -14,3 +17,9 @@ func init() { } bun.RegisterFileType(f) } + +// MesosActor represents the structure of the __processess__ file. +type MesosActor struct { + Id string + Events []struct{} +} From aabb05fecd4122c9720ebd10dc1216c2c0b7dbb1 Mon Sep 17 00:00:00 2001 From: Andrey Dyatlov Date: Mon, 10 Sep 2018 00:38:30 +0200 Subject: [PATCH 7/9] Simplify tests which use the default Interpret function: make the Interpret function accept nil results. --- check/health/health.go | 4 ++-- check/mesos/actormailboxes/mesos_actor_mailboxes.go | 1 - check_builder.go | 10 ++++------ 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/check/health/health.go b/check/health/health.go index ae48e2b..b664793 100644 --- a/check/health/health.go +++ b/check/health/health.go @@ -36,10 +36,10 @@ func check(host bun.Host) (ok bool, details interface{}, err error) { } } if len(unhealthy) > 0 { - details = "The following components are not healthy:\n" + strings.Join(unhealthy, "\n") + details = fmt.Sprintf("The following components are not healthy:\n %v", + strings.Join(unhealthy, "\n")) ok = false } else { - details = "" ok = true } return diff --git a/check/mesos/actormailboxes/mesos_actor_mailboxes.go b/check/mesos/actormailboxes/mesos_actor_mailboxes.go index f54da48..b60f7e2 100644 --- a/check/mesos/actormailboxes/mesos_actor_mailboxes.go +++ b/check/mesos/actormailboxes/mesos_actor_mailboxes.go @@ -29,7 +29,6 @@ func init() { // Truncated JSON schema of __processes__. func check(host bun.Host) (ok bool, details interface{}, err error) { - details = "" actors := []actormailboxesfile.MesosActor{} if err = host.ReadJSON("processes", &actors); err != nil { return diff --git a/check_builder.go b/check_builder.go index a707837..aedb249 100644 --- a/check_builder.go +++ b/check_builder.go @@ -109,15 +109,13 @@ func (b *CheckBuilder) checkHosts(c *Check, h map[string]Host, ch CheckHost) { // It assumes that the CheckHost returns Result Details as a string. func interpret(c *Check, b CheckBuilder) { for _, r := range b.Problems { - d := r.Details.(string) - if d != "" { - c.Problems = append(c.Problems, formatMsg(r.Host, d)) + if r.Details != nil { + c.Problems = append(c.Problems, formatMsg(r.Host, r.Details.(string))) } } for _, r := range b.OKs { - d := r.Details.(string) - if d != "" { - c.OKs = append(c.OKs, formatMsg(r.Host, d)) + if r.Details != nil { + c.OKs = append(c.OKs, formatMsg(r.Host, r.Details.(string))) } } } From 61e710e488cfacb894e4d7abf8bcca7b61f8def8 Mon Sep 17 00:00:00 2001 From: Andrey Dyatlov Date: Mon, 10 Sep 2018 01:10:05 +0200 Subject: [PATCH 8/9] Simplify usage: get rid of the check CLI command. Fix report layout of the health check. --- README.md | 29 ++++++---- bun/cmd/check.go | 123 ---------------------------------------- bun/cmd/print_report.go | 54 ++++++++++++++++++ bun/cmd/root.go | 65 ++++++++++++++++++++- check/health/health.go | 2 +- 5 files changed, 134 insertions(+), 139 deletions(-) delete mode 100644 bun/cmd/check.go create mode 100644 bun/cmd/print_report.go diff --git a/README.md b/README.md index 387bdee..c0138de 100644 --- a/README.md +++ b/README.md @@ -16,24 +16,29 @@ $ go get github.com/adyatlov/bun/bun $ mkdir bundle $ unzip bundle.zip -d bundle $ cd bundle -bundle$ bun check -PROBLEM: dcos-version - Versions are different -Details: -master 172.20.0.22 has DC/OS version 1.11.0 +bundle$ bun +[PROBLEM] "dcos-version" - Versions are different. +-------- +Problems +-------- +master 172.20.0.23 has DC/OS version 1.11.0 master 172.20.0.24 has DC/OS version 1.11.0 +agent 172.20.0.21 has DC/OS version 1.10.1 agent 172.20.0.25 has DC/OS version 1.11.0 -public agent 172.20.0.26 has DC/OS version 1.11.0 agent 172.20.0.27 has DC/OS version 1.11.0 -agent 172.20.0.29 has DC/OS version 1.11.0 -agent 172.20.0.21 has DC/OS version 1.10.1 -master 172.20.0.23 has DC/OS version 1.11.0 agent 172.20.0.28 has DC/OS version 1.11.0 +agent 172.20.0.29 has DC/OS version 1.11.0 +public agent 172.20.0.26 has DC/OS version 1.11.0 -PROBLEM: health - Some DC/OS systemd units are not healthy. -Details: -172.20.0.21 dcos-docker-gc.service: health = 1 +[PROBLEM] "health" - Problems were found. +-------- +Problems +-------- +agent 172.20.0.21: The following components are not healthy: +dcos-docker-gc.service: health = 1 -OK: node-count - Masters: 3, Agents: 5, Public Agents: 1, Total: 9 +[OK] "mesos-actor-mailboxes" - All Mesos actors are fine. +[OK] "node-count" - Masters: 3, Agents: 5, Public Agents: 1, Total: 9 ``` ## Feedback diff --git a/bun/cmd/check.go b/bun/cmd/check.go deleted file mode 100644 index 3604c68..0000000 --- a/bun/cmd/check.go +++ /dev/null @@ -1,123 +0,0 @@ -package cmd - -import ( - "context" - "fmt" - "os" - "sort" - "strings" - - "github.com/adyatlov/bun" - "github.com/spf13/cobra" -) - -var printLong = false -var bundle *bun.Bundle - -func printReport(c bun.Check) { - printEmptyLine := false - fmt.Printf("[%v] \"%v\" - %v\n", c.Status, c.Name, c.Summary) - if printLong { - if len(c.Problems) > 0 { - fmt.Println("--------") - fmt.Println("Problems") - fmt.Println("--------") - fmt.Println(strings.Join(c.Problems, "\n")) - printEmptyLine = true - } - if len(c.Errors) > 0 { - fmt.Println("------") - fmt.Println("Errors") - fmt.Println("------") - fmt.Println(strings.Join(c.Errors, "\n")) - printEmptyLine = true - } - if len(c.OKs) > 0 { - fmt.Println("---") - fmt.Println("OKs") - fmt.Println("---") - fmt.Println(strings.Join(c.OKs, "\n")) - printEmptyLine = true - } - } else { - if len(c.Problems) > 0 { - fmt.Println("--------") - fmt.Println("Problems") - fmt.Println("--------") - fmt.Println(strings.Join(c.Problems, "\n")) - printEmptyLine = true - } - if len(c.Errors) > 0 { - fmt.Println("------") - fmt.Println("Errors") - fmt.Println("------") - fmt.Println(strings.Join(c.Errors, "\n")) - printEmptyLine = true - } - } - if printEmptyLine { - fmt.Print("\n") - } -} - -func preRun(cmd *cobra.Command, args []string) { - if bundle != nil { - return - } - b, err := bun.NewBundle(context.Background(), bundlePath) - if err != nil { - fmt.Printf("Cannot find a bundle: %v\n", err.Error()) - os.Exit(1) - } - bundle = &b -} - -func runCheck(cmd *cobra.Command, args []string) { - if err := cobra.OnlyValidArgs(cmd, args); err != nil { - fmt.Println(err.Error()) - fmt.Printf("Run '%v --help' for usage.\n", cmd.CommandPath()) - os.Exit(1) - } - checks := bun.Checks() - sort.Slice(checks, func(i, j int) bool { - return checks[i].Name < checks[j].Name - }) - for _, check := range checks { - check.Run(*bundle) - printReport(check) - } -} - -func init() { - checkCmd := &cobra.Command{ - Use: "check", - Short: "Check DC/OS diagnostics bundle for possible problems", - Long: `Check DC/OS diagnostics bundle for possible problems. - -Specify a subcommand to run a specific check, e.g.` + " `bun check health`." + - ` -Or run all the available checks by not specifying any, i.e.` + " `bun check`.", - PreRun: preRun, - Run: runCheck, - } - checkCmd.PersistentFlags().BoolVarP(&printLong, "long", "l", false, "print details") - - for _, check := range bun.Checks() { - run := func(cmd *cobra.Command, args []string) { - check.Run(*bundle) - printReport(check) - return - } - var cmd = &cobra.Command{ - Use: check.Name, - Short: check.Description, - Long: check.Description, - PreRun: preRun, - Run: run, - } - checkCmd.AddCommand(cmd) - checkCmd.ValidArgs = append(checkCmd.ValidArgs, check.Name) - checkCmd.PreRun = preRun - } - rootCmd.AddCommand(checkCmd) -} diff --git a/bun/cmd/print_report.go b/bun/cmd/print_report.go new file mode 100644 index 0000000..4d1dbdc --- /dev/null +++ b/bun/cmd/print_report.go @@ -0,0 +1,54 @@ +package cmd + +import ( + "fmt" + "strings" + + "github.com/adyatlov/bun" +) + +func printReport(c bun.Check) { + printEmptyLine := false + fmt.Printf("[%v] \"%v\" - %v\n", c.Status, c.Name, c.Summary) + if printLong { + if len(c.Problems) > 0 { + fmt.Println("--------") + fmt.Println("Problems") + fmt.Println("--------") + fmt.Println(strings.Join(c.Problems, "\n")) + printEmptyLine = true + } + if len(c.Errors) > 0 { + fmt.Println("------") + fmt.Println("Errors") + fmt.Println("------") + fmt.Println(strings.Join(c.Errors, "\n")) + printEmptyLine = true + } + if len(c.OKs) > 0 { + fmt.Println("---") + fmt.Println("OKs") + fmt.Println("---") + fmt.Println(strings.Join(c.OKs, "\n")) + printEmptyLine = true + } + } else { + if len(c.Problems) > 0 { + fmt.Println("--------") + fmt.Println("Problems") + fmt.Println("--------") + fmt.Println(strings.Join(c.Problems, "\n")) + printEmptyLine = true + } + if len(c.Errors) > 0 { + fmt.Println("------") + fmt.Println("Errors") + fmt.Println("------") + fmt.Println(strings.Join(c.Errors, "\n")) + printEmptyLine = true + } + } + if printEmptyLine { + fmt.Print("\n") + } +} diff --git a/bun/cmd/root.go b/bun/cmd/root.go index 32de2bd..efd30d2 100644 --- a/bun/cmd/root.go +++ b/bun/cmd/root.go @@ -1,20 +1,29 @@ package cmd import ( + "context" "fmt" "os" + "sort" + "github.com/adyatlov/bun" "github.com/spf13/cobra" ) var bundlePath string +var bundle *bun.Bundle +var printLong = false var rootCmd = &cobra.Command{ Use: "bun", Short: "DC/OS diagnostics bundle analysis tool", - Long: "Bun extracts useful facts from hundreds of files in a DC/OS diagnostics bundle" + - " and searches for some common problems of a DC/OS cluster." + + Long: "Bun extracts useful facts from hundreds of files in the DC/OS diagnostics bundle\n" + + "and searches for some common problems of the DC/OS cluster.\n" + + "\nSpecify a subcommand to run a specific check, e.g. `bun health`\n" + + "or run all the available checks by not specifying any, i.e. `bun`.\n" + "\nMore information is available at https://github.com/adyatlov/bun", + PreRun: preRun, + Run: runCheck, } func init() { @@ -23,9 +32,59 @@ func init() { fmt.Printf("Error while detecting a working directory: %v\n", err.Error()) os.Exit(1) } - rootCmd.PersistentFlags().StringVarP(&bundlePath, "path", "p", wd, "path to the bundle directory") + rootCmd.PersistentFlags().StringVarP(&bundlePath, "path", "p", wd, + "path to the bundle directory") + rootCmd.PersistentFlags().BoolVarP(&printLong, "long", "l", false, + "print details") + // Adding registered checks as commands. + for _, check := range bun.Checks() { + run := func(cmd *cobra.Command, args []string) { + check.Run(*bundle) + printReport(check) + return + } + var cmd = &cobra.Command{ + Use: check.Name, + Short: check.Description, + Long: check.Description, + PreRun: preRun, + Run: run, + } + rootCmd.AddCommand(cmd) + rootCmd.ValidArgs = append(rootCmd.ValidArgs, check.Name) + rootCmd.PreRun = preRun + } +} + +func preRun(cmd *cobra.Command, args []string) { + if bundle != nil { + return + } + b, err := bun.NewBundle(context.Background(), bundlePath) + if err != nil { + fmt.Printf("Cannot find a bundle: %v\n", err.Error()) + os.Exit(1) + } + bundle = &b +} + +func runCheck(cmd *cobra.Command, args []string) { + if err := cobra.OnlyValidArgs(cmd, args); err != nil { + fmt.Println(err.Error()) + fmt.Printf("Run '%v --help' for usage.\n", cmd.CommandPath()) + os.Exit(1) + } + checks := bun.Checks() + sort.Slice(checks, func(i, j int) bool { + return checks[i].Name < checks[j].Name + }) + for _, check := range checks { + check.Run(*bundle) + printReport(check) + } } +// Execute starts Bun. func Execute() { if err := rootCmd.Execute(); err != nil { fmt.Println(err) diff --git a/check/health/health.go b/check/health/health.go index b664793..76dc7e6 100644 --- a/check/health/health.go +++ b/check/health/health.go @@ -36,7 +36,7 @@ func check(host bun.Host) (ok bool, details interface{}, err error) { } } if len(unhealthy) > 0 { - details = fmt.Sprintf("The following components are not healthy:\n %v", + details = fmt.Sprintf("The following components are not healthy:\n%v", strings.Join(unhealthy, "\n")) ok = false } else { From e3999a02496a1d036716b0c683948ae9ef839695 Mon Sep 17 00:00:00 2001 From: Andrey Dyatlov Date: Mon, 10 Sep 2018 01:19:38 +0200 Subject: [PATCH 9/9] Improve report layout --- README.md | 16 ++++++++-------- bun/cmd/print_report.go | 18 +++++++++--------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index c0138de..41e373a 100644 --- a/README.md +++ b/README.md @@ -18,22 +18,22 @@ $ unzip bundle.zip -d bundle $ cd bundle bundle$ bun [PROBLEM] "dcos-version" - Versions are different. --------- -Problems --------- +--------------- +Problem details +--------------- master 172.20.0.23 has DC/OS version 1.11.0 master 172.20.0.24 has DC/OS version 1.11.0 -agent 172.20.0.21 has DC/OS version 1.10.1 -agent 172.20.0.25 has DC/OS version 1.11.0 agent 172.20.0.27 has DC/OS version 1.11.0 agent 172.20.0.28 has DC/OS version 1.11.0 agent 172.20.0.29 has DC/OS version 1.11.0 +agent 172.20.0.21 has DC/OS version 1.10.1 +agent 172.20.0.25 has DC/OS version 1.11.0 public agent 172.20.0.26 has DC/OS version 1.11.0 [PROBLEM] "health" - Problems were found. --------- -Problems --------- +--------------- +Problem details +--------------- agent 172.20.0.21: The following components are not healthy: dcos-docker-gc.service: health = 1 diff --git a/bun/cmd/print_report.go b/bun/cmd/print_report.go index 4d1dbdc..73c1934 100644 --- a/bun/cmd/print_report.go +++ b/bun/cmd/print_report.go @@ -12,9 +12,9 @@ func printReport(c bun.Check) { fmt.Printf("[%v] \"%v\" - %v\n", c.Status, c.Name, c.Summary) if printLong { if len(c.Problems) > 0 { - fmt.Println("--------") - fmt.Println("Problems") - fmt.Println("--------") + fmt.Println("---------------") + fmt.Println("Problem details") + fmt.Println("---------------") fmt.Println(strings.Join(c.Problems, "\n")) printEmptyLine = true } @@ -26,17 +26,17 @@ func printReport(c bun.Check) { printEmptyLine = true } if len(c.OKs) > 0 { - fmt.Println("---") - fmt.Println("OKs") - fmt.Println("---") + fmt.Println("-------") + fmt.Println("Details") + fmt.Println("-------") fmt.Println(strings.Join(c.OKs, "\n")) printEmptyLine = true } } else { if len(c.Problems) > 0 { - fmt.Println("--------") - fmt.Println("Problems") - fmt.Println("--------") + fmt.Println("---------------") + fmt.Println("Problem details") + fmt.Println("---------------") fmt.Println(strings.Join(c.Problems, "\n")) printEmptyLine = true }