From e846d801a12d653b4cdff492eb4d500b178d63e4 Mon Sep 17 00:00:00 2001 From: Joe Corall Date: Tue, 9 Jun 2026 10:48:42 +0000 Subject: [PATCH] fix: improve healthcheck retries and solr checks --- cmd/healthcheck.go | 175 +++++++++++++++++++++++++--- cmd/healthcheck_test.go | 95 +++++++++++++++ pkg/healthcheck/healthcheck.go | 58 +++++++-- pkg/healthcheck/healthcheck_test.go | 56 +++++++++ 4 files changed, 357 insertions(+), 27 deletions(-) create mode 100644 cmd/healthcheck_test.go create mode 100644 pkg/healthcheck/healthcheck_test.go diff --git a/cmd/healthcheck.go b/cmd/healthcheck.go index 9db818a..741f44b 100644 --- a/cmd/healthcheck.go +++ b/cmd/healthcheck.go @@ -2,9 +2,13 @@ package cmd import ( "fmt" + "io" "strings" "time" + "charm.land/bubbles/v2/spinner" + tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" "github.com/libops/sitectl/pkg/config" "github.com/libops/sitectl/pkg/healthcheck" "github.com/libops/sitectl/pkg/plugin" @@ -24,9 +28,9 @@ runtime checks are also run and merged into the report. All flags not consumed by sitectl itself are forwarded to the plugin's healthcheck handler, allowing plugin-specific flags such as --codebase-rootfs. -By default, healthcheck runs once, prints the current status of each check, and -exits non-zero if any check fails. Use --persist to keep retrying until all -checks pass or --timeout is reached. +By default, healthcheck waits for Docker services that are still starting, then +prints one status report and exits non-zero if any check fails. Use --persist to +keep retrying all failures until every check passes or --timeout is reached. Examples: sitectl healthcheck @@ -43,7 +47,7 @@ Examples: if err != nil { return err } - if hostParams.Persist && hostParams.Interval <= 0 { + if hostParams.Interval <= 0 { return fmt.Errorf("--interval must be greater than zero") } @@ -73,21 +77,16 @@ func init() { } func runHealthcheckReport(cmd *cobra.Command, ctx *config.Context, contextName string, hostParams healthcheckHostParams, healthcheckParams plugin.HealthcheckRunParams, pluginArgs []string) (sitevalidate.Report, error) { - if hostParams.Persist { - return runHealthcheckUntilHealthy(cmd, ctx, contextName, hostParams, healthcheckParams, pluginArgs) - } - - results, err := runHealthcheckOnce(cmd, ctx, contextName, healthcheckParams, pluginArgs) - if err != nil { - return sitevalidate.Report{}, err - } - sitevalidate.SortResults(results) - return sitevalidate.NewReport(ctx, results), nil -} - -func runHealthcheckUntilHealthy(cmd *cobra.Command, ctx *config.Context, contextName string, hostParams healthcheckHostParams, healthcheckParams plugin.HealthcheckRunParams, pluginArgs []string) (sitevalidate.Report, error) { deadline := time.Now().Add(hostParams.Timeout) var last sitevalidate.Report + var progress *healthcheckProgress + defer func() { + if progress != nil { + progress.Stop() + } + }() + + attempt := 1 for { results, err := runHealthcheckOnce(cmd, ctx, contextName, healthcheckParams, pluginArgs) if err != nil { @@ -95,9 +94,15 @@ func runHealthcheckUntilHealthy(cmd *cobra.Command, ctx *config.Context, context } sitevalidate.SortResults(results) last = sitevalidate.NewReport(ctx, results) - if last.Valid || hostParams.Timeout <= 0 || time.Now().Add(hostParams.Interval).After(deadline) { + if last.Valid || !shouldRetryHealthcheck(last, hostParams) || hostParams.Timeout <= 0 || time.Now().Add(hostParams.Interval).After(deadline) { return last, nil } + message := healthcheckRetryMessage(last, hostParams, attempt) + if progress == nil { + progress = startHealthcheckProgress(cmd.ErrOrStderr(), message) + } else { + progress.Update(message) + } timer := time.NewTimer(hostParams.Interval) select { case <-cmd.Context().Done(): @@ -105,6 +110,140 @@ func runHealthcheckUntilHealthy(cmd *cobra.Command, ctx *config.Context, context return sitevalidate.Report{}, cmd.Context().Err() case <-timer.C: } + attempt++ + } +} + +func shouldRetryHealthcheck(report sitevalidate.Report, hostParams healthcheckHostParams) bool { + if report.Valid { + return false + } + if hostParams.Persist { + return true + } + return reportHasStartingComposeService(report) +} + +func reportHasStartingComposeService(report sitevalidate.Report) bool { + for _, result := range report.Results { + if result.Status != sitevalidate.StatusFailed { + continue + } + if !strings.HasPrefix(strings.TrimSpace(result.Name), "service:") { + continue + } + if strings.Contains(result.Detail, "health=starting") { + return true + } + } + return false +} + +func healthcheckRetryMessage(report sitevalidate.Report, hostParams healthcheckHostParams, attempt int) string { + reason := "healthcheck failed" + if !hostParams.Persist { + if services := startingComposeServiceNames(report); len(services) > 0 { + reason = strings.Join(services, ", ") + " starting" + } + } + return fmt.Sprintf("Waiting for healthcheck retry %d: %s; next check in %s", attempt, reason, hostParams.Interval) +} + +func startingComposeServiceNames(report sitevalidate.Report) []string { + services := []string{} + for _, result := range report.Results { + if result.Status != sitevalidate.StatusFailed { + continue + } + name := strings.TrimSpace(result.Name) + if !strings.HasPrefix(name, "service:") || !strings.Contains(result.Detail, "health=starting") { + continue + } + services = append(services, strings.TrimPrefix(name, "service:")) + } + return services +} + +type healthcheckProgressUpdateMsg struct { + message string +} + +type healthcheckProgressDoneMsg struct{} + +type healthcheckSpinnerModel struct { + spin spinner.Model + message string + quitting bool +} + +func newHealthcheckSpinnerModel(message string) healthcheckSpinnerModel { + return healthcheckSpinnerModel{ + spin: spinner.New(spinner.WithSpinner(spinner.Dot), spinner.WithStyle(lipgloss.NewStyle().Foreground(lipgloss.Color("205")))), + message: strings.TrimSpace(message), + } +} + +func (m healthcheckSpinnerModel) Init() tea.Cmd { + return m.spin.Tick +} + +func (m healthcheckSpinnerModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + switch msg := msg.(type) { + case spinner.TickMsg: + var cmd tea.Cmd + m.spin, cmd = m.spin.Update(msg) + return m, cmd + case healthcheckProgressUpdateMsg: + m.message = strings.TrimSpace(msg.message) + return m, nil + case healthcheckProgressDoneMsg: + m.quitting = true + return m, tea.Quit + } + return m, nil +} + +func (m healthcheckSpinnerModel) View() tea.View { + if m.quitting { + return tea.NewView("") + } + return tea.NewView(m.spin.View() + " " + m.message + "\n") +} + +type healthcheckProgress struct { + program *tea.Program + done chan error +} + +func startHealthcheckProgress(out io.Writer, message string) *healthcheckProgress { + progress := &healthcheckProgress{ + program: tea.NewProgram(newHealthcheckSpinnerModel(message), tea.WithInput(nil), tea.WithOutput(out)), + done: make(chan error, 1), + } + go func() { + _, err := progress.program.Run() + progress.done <- err + }() + return progress +} + +func (p *healthcheckProgress) Update(message string) { + if p == nil || p.program == nil { + return + } + p.program.Send(healthcheckProgressUpdateMsg{message: message}) +} + +func (p *healthcheckProgress) Stop() { + if p == nil || p.program == nil { + return + } + p.program.Send(healthcheckProgressDoneMsg{}) + select { + case <-p.done: + case <-time.After(2 * time.Second): + p.program.Kill() + <-p.done } } diff --git a/cmd/healthcheck_test.go b/cmd/healthcheck_test.go new file mode 100644 index 0000000..a81565d --- /dev/null +++ b/cmd/healthcheck_test.go @@ -0,0 +1,95 @@ +package cmd + +import ( + "strings" + "testing" + "time" + + sitevalidate "github.com/libops/sitectl/pkg/validate" +) + +func TestShouldRetryHealthcheckDefaultRetriesStartingServices(t *testing.T) { + report := sitevalidate.Report{ + Valid: false, + Results: []sitevalidate.Result{ + {Name: "service:solr", Status: sitevalidate.StatusFailed, Detail: "drupal-solr-1: health=starting"}, + {Name: "http:drupal", Status: sitevalidate.StatusFailed, Detail: "connection refused"}, + }, + } + + if !shouldRetryHealthcheck(report, healthcheckHostParams{}) { + t.Fatal("expected default healthcheck to retry while a compose service is starting") + } +} + +func TestShouldRetryHealthcheckDefaultDoesNotRetryStableFailures(t *testing.T) { + tests := []struct { + name string + report sitevalidate.Report + }{ + { + name: "unhealthy service", + report: sitevalidate.Report{ + Valid: false, + Results: []sitevalidate.Result{ + {Name: "service:solr", Status: sitevalidate.StatusFailed, Detail: "drupal-solr-1: health=unhealthy"}, + }, + }, + }, + { + name: "plugin check failure", + report: sitevalidate.Report{ + Valid: false, + Results: []sitevalidate.Result{ + {Name: "solr:solr", Status: sitevalidate.StatusFailed, Detail: "curl failed"}, + }, + }, + }, + { + name: "valid report", + report: sitevalidate.Report{ + Valid: true, + Results: []sitevalidate.Result{ + {Name: "service:solr", Status: sitevalidate.StatusOK, Detail: "drupal-solr-1: healthy"}, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if shouldRetryHealthcheck(tt.report, healthcheckHostParams{}) { + t.Fatal("expected default healthcheck not to retry") + } + }) + } +} + +func TestShouldRetryHealthcheckPersistRetriesStableFailures(t *testing.T) { + report := sitevalidate.Report{ + Valid: false, + Results: []sitevalidate.Result{ + {Name: "http:drupal", Status: sitevalidate.StatusFailed, Detail: "connection refused"}, + }, + } + + if !shouldRetryHealthcheck(report, healthcheckHostParams{Persist: true}) { + t.Fatal("expected persistent healthcheck to retry stable failures") + } +} + +func TestHealthcheckRetryMessageNamesStartingServices(t *testing.T) { + report := sitevalidate.Report{ + Valid: false, + Results: []sitevalidate.Result{ + {Name: "service:solr", Status: sitevalidate.StatusFailed, Detail: "drupal-solr-1: health=starting"}, + }, + } + + message := healthcheckRetryMessage(report, healthcheckHostParams{Interval: 3 * time.Second}, 2) + for _, want := range []string{"retry 2", "solr starting", "3s"} { + if !strings.Contains(message, want) { + t.Fatalf("message %q missing %q", message, want) + } + } +} diff --git a/pkg/healthcheck/healthcheck.go b/pkg/healthcheck/healthcheck.go index 0ab93d0..3887d35 100644 --- a/pkg/healthcheck/healthcheck.go +++ b/pkg/healthcheck/healthcheck.go @@ -2,6 +2,7 @@ package healthcheck import ( "context" + "encoding/json" "fmt" "net/http" "net/url" @@ -102,18 +103,40 @@ func (c *DockerChecker) CheckMariaDB(ctx context.Context, service string) siteva }) } -// CheckSolrCore verifies that a Solr core answers its admin ping endpoint from -// inside the Solr container. +// CheckSolrCore verifies that a Solr core is loaded from inside the Solr +// container. func (c *DockerChecker) CheckSolrCore(ctx context.Context, service, core string) sitevalidate.Result { service = firstNonEmpty(service, "solr") core = firstNonEmpty(core, "default") - endpoint := fmt.Sprintf("http://127.0.0.1:8983/solr/%s/admin/ping?wt=json", url.PathEscape(core)) - command := fmt.Sprintf(`if command -v curl >/dev/null 2>&1; then curl -fsS %q; elif command -v wget >/dev/null 2>&1; then wget -q -O- %q; else solr status >/dev/null; fi`, endpoint, endpoint) - result := c.checkExec(ctx, "solr:"+service, service, []string{"sh", "-lc", command}) - if result.Status == sitevalidate.StatusOK && strings.TrimSpace(result.Detail) == "" { - result.Detail = "Solr core " + core + " answered ping" + name := "solr:" + service + if c == nil || c.Client == nil || c.Context == nil { + return failed(name, "docker checker is not initialized") + } + containerName, err := c.Client.GetContainerNameContext(ctx, c.Context, service) + if err != nil { + return failed(name, err.Error()) + } + if strings.TrimSpace(containerName) == "" { + return failed(name, "service "+service+" is not running") + } + + endpoint := fmt.Sprintf("http://127.0.0.1:8983/solr/admin/cores?action=STATUS&core=%s&wt=json", url.QueryEscape(core)) + command := fmt.Sprintf(`if command -v curl >/dev/null 2>&1; then curl -fsS --max-time 10 %q; elif command -v wget >/dev/null 2>&1; then wget -q --timeout=10 -O- %q; else echo "curl or wget is required"; exit 127; fi`, endpoint, endpoint) + execCtx, cancel := context.WithTimeout(ctx, defaultHTTPTimeout) + defer cancel() + output, err := sitectldocker.ExecCapture(execCtx, c.Client, containerName, "", []string{"sh", "-lc", command}) + if err != nil { + detail := strings.TrimSpace(output) + if detail != "" { + detail += ": " + } + return failed(name, detail+err.Error()) + } + detail, err := solrCoreStatusDetail(output, core) + if err != nil { + return failed(name, err.Error()) } - return result + return sitevalidate.Result{Name: name, Status: sitevalidate.StatusOK, Detail: detail} } // CheckHTTPFromContainer verifies that url is reachable from inside service. @@ -332,13 +355,30 @@ func isDefaultPort(scheme, port string) bool { } func trimOutput(output string) string { - output = strings.TrimSpace(output) + output = strings.Join(strings.Fields(output), " ") if len(output) <= 200 { return output } return output[:200] + "..." } +func solrCoreStatusDetail(output, core string) (string, error) { + var payload struct { + InitFailures map[string]json.RawMessage `json:"initFailures"` + Status map[string]json.RawMessage `json:"status"` + } + if err := json.Unmarshal([]byte(output), &payload); err != nil { + return "", fmt.Errorf("parse Solr core status: %w: %s", err, trimOutput(output)) + } + if raw, ok := payload.InitFailures[core]; ok { + return "", fmt.Errorf("solr core %s has init failure: %s", core, trimOutput(string(raw))) + } + if _, ok := payload.Status[core]; !ok { + return "", fmt.Errorf("solr core %s not found", core) + } + return "Solr core " + core + " is loaded", nil +} + func min(a, b int) int { if a < b { return a diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go new file mode 100644 index 0000000..2e316d5 --- /dev/null +++ b/pkg/healthcheck/healthcheck_test.go @@ -0,0 +1,56 @@ +package healthcheck + +import ( + "strings" + "testing" +) + +func TestSolrCoreStatusDetailLoaded(t *testing.T) { + detail, err := solrCoreStatusDetail(`{ + "responseHeader": {"status": 0}, + "initFailures": {}, + "status": { + "default": {"name": "default", "instanceDir": "/opt/solr/server/solr/default"} + } + }`, "default") + if err != nil { + t.Fatalf("solrCoreStatusDetail() error = %v", err) + } + if detail != "Solr core default is loaded" { + t.Fatalf("detail = %q", detail) + } +} + +func TestSolrCoreStatusDetailInitFailure(t *testing.T) { + _, err := solrCoreStatusDetail(`{ + "initFailures": { + "default": { + "msg": "Error loading class 'solr.ICUCollationField'" + } + }, + "status": {} + }`, "default") + if err == nil { + t.Fatal("expected init failure error") + } + if !strings.Contains(err.Error(), "solr core default has init failure") || !strings.Contains(err.Error(), "ICUCollationField") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestSolrCoreStatusDetailMissingCore(t *testing.T) { + _, err := solrCoreStatusDetail(`{"initFailures": {}, "status": {}}`, "default") + if err == nil { + t.Fatal("expected missing core error") + } + if !strings.Contains(err.Error(), "solr core default not found") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestTrimOutputCollapsesWhitespace(t *testing.T) { + got := trimOutput("{\n \"status\":\"OK\"\n}") + if got != `{ "status":"OK" }` { + t.Fatalf("trimOutput() = %q", got) + } +}