From 9f9cc927e0aec9be7bc4ad32480d312ade3d6fd5 Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Fri, 25 Nov 2022 01:14:57 +0100 Subject: [PATCH 1/4] Check cluster health before trying to run tests --- cmd/benchmark.go | 4 ++++ cmd/testrunner.go | 4 ++++ internal/elasticsearch/client.go | 34 ++++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/cmd/benchmark.go b/cmd/benchmark.go index 7b1d651dd9..37644146ff 100644 --- a/cmd/benchmark.go +++ b/cmd/benchmark.go @@ -151,6 +151,10 @@ func benchTypeCommandActionFactory(runner benchrunner.BenchRunner) cobraext.Comm if err != nil { return errors.Wrap(err, "can't create Elasticsearch client") } + err = elasticsearch.CheckHealth(cmd.Context(), esClient.API) + if err != nil { + return err + } var results []*benchrunner.Result for _, folder := range benchFolders { diff --git a/cmd/testrunner.go b/cmd/testrunner.go index 3f03ed64c0..acee8259d1 100644 --- a/cmd/testrunner.go +++ b/cmd/testrunner.go @@ -211,6 +211,10 @@ func testTypeCommandActionFactory(runner testrunner.TestRunner) cobraext.Command if err != nil { return errors.Wrap(err, "can't create Elasticsearch client") } + err = elasticsearch.CheckHealth(cmd.Context(), esClient.API) + if err != nil { + return err + } var results []testrunner.TestResult for _, folder := range testFolders { diff --git a/internal/elasticsearch/client.go b/internal/elasticsearch/client.go index 7531d31cf9..fa624a16c9 100644 --- a/internal/elasticsearch/client.go +++ b/internal/elasticsearch/client.go @@ -5,8 +5,11 @@ package elasticsearch import ( + "context" "crypto/tls" + "encoding/json" "fmt" + "io" "net/http" "os" @@ -28,6 +31,9 @@ type IngestSimulateRequest = esapi.IngestSimulateRequest // IngestGetPipelineRequest configures the Ingest Get Pipeline API request. type IngestGetPipelineRequest = esapi.IngestGetPipelineRequest +// ClusterStateRequest configures the Cluster State API request. +type ClusterStateRequest = esapi.ClusterStateRequest + // clientOptions are used to configure a client. type clientOptions struct { address string @@ -110,3 +116,31 @@ func Client(customOptions ...ClientOption) (*elasticsearch.Client, error) { } return client, nil } + +// CheckHealth checks the health of a cluster. +func CheckHealth(ctx context.Context, client *API) error { + resp, err := client.Cluster.Health(client.Cluster.Health.WithContext(ctx)) + if err != nil { + return errors.Wrap(err, "error checking cluster health") + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return errors.Wrap(err, "error reading cluster health response") + } + + var clusterHealth struct { + Status string `json:"status"` + } + err = json.Unmarshal(body, &clusterHealth) + if err != nil { + return errors.Wrap(err, "error decoding cluster health response") + } + + if status := clusterHealth.Status; status != "green" && status != "yellow" { + return errors.Errorf("cluster in unhealthy state: %q", status) + } + + return nil +} From 751984e356eb212c20bb98f746926e1881ea177d Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Fri, 25 Nov 2022 12:34:44 +0100 Subject: [PATCH 2/4] Get red status cause --- cmd/benchmark.go | 2 +- cmd/testrunner.go | 2 +- internal/dump/installedobjects_test.go | 4 +- internal/elasticsearch/client.go | 86 ++++++++++++++++++- internal/elasticsearch/client_test.go | 42 ++++++++- internal/elasticsearch/test/httptest.go | 5 +- .../_cluster-health.json | 1 + .../elasticsearch-8-5-healthy/root.json | 17 ++++ .../_cluster-health.json | 1 + .../_internal-_health.json | 1 + .../root.json | 17 ++++ 11 files changed, 166 insertions(+), 12 deletions(-) create mode 100644 internal/elasticsearch/testdata/elasticsearch-8-5-healthy/_cluster-health.json create mode 100644 internal/elasticsearch/testdata/elasticsearch-8-5-healthy/root.json create mode 100644 internal/elasticsearch/testdata/elasticsearch-8-5-red-out-of-disk/_cluster-health.json create mode 100644 internal/elasticsearch/testdata/elasticsearch-8-5-red-out-of-disk/_internal-_health.json create mode 100644 internal/elasticsearch/testdata/elasticsearch-8-5-red-out-of-disk/root.json diff --git a/cmd/benchmark.go b/cmd/benchmark.go index 37644146ff..9aabae39bd 100644 --- a/cmd/benchmark.go +++ b/cmd/benchmark.go @@ -151,7 +151,7 @@ func benchTypeCommandActionFactory(runner benchrunner.BenchRunner) cobraext.Comm if err != nil { return errors.Wrap(err, "can't create Elasticsearch client") } - err = elasticsearch.CheckHealth(cmd.Context(), esClient.API) + err = elasticsearch.CheckHealth(cmd.Context(), esClient) if err != nil { return err } diff --git a/cmd/testrunner.go b/cmd/testrunner.go index acee8259d1..1dd5d02806 100644 --- a/cmd/testrunner.go +++ b/cmd/testrunner.go @@ -211,7 +211,7 @@ func testTypeCommandActionFactory(runner testrunner.TestRunner) cobraext.Command if err != nil { return errors.Wrap(err, "can't create Elasticsearch client") } - err = elasticsearch.CheckHealth(cmd.Context(), esClient.API) + err = elasticsearch.CheckHealth(cmd.Context(), esClient) if err != nil { return err } diff --git a/internal/dump/installedobjects_test.go b/internal/dump/installedobjects_test.go index 3139fe8620..6b9c90bfa5 100644 --- a/internal/dump/installedobjects_test.go +++ b/internal/dump/installedobjects_test.go @@ -80,7 +80,7 @@ func (s *installedObjectsDumpSuite) TestDumpAll() { client := estest.ElasticsearchClient(s.T(), s.RecordDir) outputDir := s.T().TempDir() - dumper := NewInstalledObjectsDumper(client, s.PackageName) + dumper := NewInstalledObjectsDumper(client.API, s.PackageName) n, err := dumper.DumpAll(context.Background(), outputDir) s.Require().NoError(err) @@ -95,7 +95,7 @@ func (s *installedObjectsDumpSuite) TestDumpAll() { func (s *installedObjectsDumpSuite) TestDumpSome() { client := estest.ElasticsearchClient(s.T(), s.RecordDir) - dumper := NewInstalledObjectsDumper(client, s.PackageName) + dumper := NewInstalledObjectsDumper(client.API, s.PackageName) // In a map so order of execution is randomized. dumpers := map[string]func(ctx context.Context, outputDir string) (int, error){ diff --git a/internal/elasticsearch/client.go b/internal/elasticsearch/client.go index fa624a16c9..a9d79c27a0 100644 --- a/internal/elasticsearch/client.go +++ b/internal/elasticsearch/client.go @@ -12,6 +12,7 @@ import ( "io" "net/http" "os" + "strings" "github.com/pkg/errors" @@ -118,7 +119,7 @@ func Client(customOptions ...ClientOption) (*elasticsearch.Client, error) { } // CheckHealth checks the health of a cluster. -func CheckHealth(ctx context.Context, client *API) error { +func CheckHealth(ctx context.Context, client *elasticsearch.Client) error { resp, err := client.Cluster.Health(client.Cluster.Health.WithContext(ctx)) if err != nil { return errors.Wrap(err, "error checking cluster health") @@ -139,8 +140,89 @@ func CheckHealth(ctx context.Context, client *API) error { } if status := clusterHealth.Status; status != "green" && status != "yellow" { - return errors.Errorf("cluster in unhealthy state: %q", status) + if status != "red" { + return errors.Errorf("cluster in unhealthy state: %q", status) + } + cause, err := redHealthCause(ctx, client) + if err != nil { + return errors.Wrapf(err, "cluster in unhealthy state, failed to identify cause") + } + return errors.Errorf("cluster in unhealthy state: %s", cause) } return nil } + +// redHealthCause tries to identify the cause of a cluster in red state. This could be +// also used as a replacement of CheckHealth, but keeping them separated because it uses +// internal undocumented APIs that might change. +func redHealthCause(ctx context.Context, client *elasticsearch.Client) (string, error) { + req, err := http.NewRequest(http.MethodGet, "/_internal/_health", nil) + if err != nil { + return "", errors.Wrap(err, "error creating internal health request") + } + resp, err := client.Transport.Perform(req) + if err != nil { + return "", errors.Wrap(err, "error performing internal health request") + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return "", errors.Wrap(err, "error reading internal health response") + } + + var internalHealth struct { + Status string `json:"status"` + Indicators map[string]struct { + Status string `json:"status"` + Impacts []struct { + Severity int `json:"severity"` + } `json:"impacts"` + Diagnosis []struct { + Cause string `json:"cause"` + } `json:"diagnosis"` + } `json:"indicators"` + } + err = json.Unmarshal(body, &internalHealth) + if err != nil { + return "", errors.Wrap(err, "error decoding internal health response") + } + if internalHealth.Status != "red" { + return "", errors.New("cluster state is not red?") + } + + // Only diagnostics with the highest severity impacts are returned. + var highestSeverity int + var causes []string + for _, indicator := range internalHealth.Indicators { + if indicator.Status != "red" { + continue + } + + var severity int + for _, impact := range indicator.Impacts { + if impact.Severity > severity { + severity = impact.Severity + } + } + + switch { + case severity < highestSeverity: + continue + case severity == highestSeverity: + break + case severity > highestSeverity: + highestSeverity = severity + causes = nil + } + + for _, diagnosis := range indicator.Diagnosis { + causes = append(causes, diagnosis.Cause) + } + } + if len(causes) == 0 { + return "", errors.New("no causes found") + } + return strings.Join(causes, ", "), nil +} diff --git a/internal/elasticsearch/client_test.go b/internal/elasticsearch/client_test.go index 65a00bec8c..4cc65bcf5b 100644 --- a/internal/elasticsearch/client_test.go +++ b/internal/elasticsearch/client_test.go @@ -2,10 +2,11 @@ // or more contributor license agreements. Licensed under the Elastic License; // you may not use this file except in compliance with the Elastic License. -package elasticsearch +package elasticsearch_test import ( "bytes" + "context" "crypto/x509" "encoding/pem" "net/http" @@ -16,6 +17,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/elastic/elastic-package/internal/elasticsearch" + "github.com/elastic/elastic-package/internal/elasticsearch/test" ) func TestClientWithTLS(t *testing.T) { @@ -26,7 +30,7 @@ func TestClientWithTLS(t *testing.T) { caCertFile := writeCACertFile(t, server.Certificate()) t.Run("no TLS config, should fail", func(t *testing.T) { - client, err := Client(OptionWithAddress(server.URL)) + client, err := elasticsearch.Client(elasticsearch.OptionWithAddress(server.URL)) require.NoError(t, err) _, err = client.Ping() @@ -34,7 +38,7 @@ func TestClientWithTLS(t *testing.T) { }) t.Run("with CA", func(t *testing.T) { - client, err := Client(OptionWithAddress(server.URL), OptionWithCertificateAuthority(caCertFile)) + client, err := elasticsearch.Client(elasticsearch.OptionWithAddress(server.URL), elasticsearch.OptionWithCertificateAuthority(caCertFile)) require.NoError(t, err) _, err = client.Ping() @@ -42,7 +46,7 @@ func TestClientWithTLS(t *testing.T) { }) t.Run("skip TLS verify", func(t *testing.T) { - client, err := Client(OptionWithAddress(server.URL), OptionWithSkipTLSVerify()) + client, err := elasticsearch.Client(elasticsearch.OptionWithAddress(server.URL), elasticsearch.OptionWithSkipTLSVerify()) require.NoError(t, err) _, err = client.Ping() @@ -50,6 +54,36 @@ func TestClientWithTLS(t *testing.T) { }) } +func TestClusterHealth(t *testing.T) { + cases := []struct { + RecordDir string + Expected string + }{ + { + RecordDir: "./testdata/elasticsearch-8-5-healthy", + }, + { + RecordDir: "./testdata/elasticsearch-8-5-red-out-of-disk", + Expected: "cluster in unhealthy state: 33 indices reside on nodes that have run or are likely to run out of disk space, this can temporarily disable writing on these indices.", + }, + } + + for _, c := range cases { + t.Run(c.RecordDir, func(t *testing.T) { + client := test.ElasticsearchClient(t, c.RecordDir) + + err := elasticsearch.CheckHealth(context.Background(), client) + if c.Expected != "" { + if assert.Error(t, err) { + assert.Equal(t, c.Expected, err.Error()) + } + } else { + assert.NoError(t, err) + } + }) + } +} + func writeCACertFile(t *testing.T, cert *x509.Certificate) string { var d bytes.Buffer err := pem.Encode(&d, &pem.Block{ diff --git a/internal/elasticsearch/test/httptest.go b/internal/elasticsearch/test/httptest.go index 7a80cf7516..cd22dbd590 100644 --- a/internal/elasticsearch/test/httptest.go +++ b/internal/elasticsearch/test/httptest.go @@ -13,6 +13,7 @@ import ( "strings" "testing" + esclient "github.com/elastic/go-elasticsearch/v7" "github.com/stretchr/testify/require" "github.com/elastic/elastic-package/internal/elasticsearch" @@ -22,7 +23,7 @@ import ( // responses. If responses are not found, it forwards the query to the server started by // elastic-package stack, and records the response. // Responses are recorded in the directory indicated by serverDataDir. -func ElasticsearchClient(t *testing.T, serverDataDir string) *elasticsearch.API { +func ElasticsearchClient(t *testing.T, serverDataDir string) *esclient.Client { server := testElasticsearchServer(t, serverDataDir) t.Cleanup(func() { server.Close() }) @@ -31,7 +32,7 @@ func ElasticsearchClient(t *testing.T, serverDataDir string) *elasticsearch.API ) require.NoError(t, err) - return client.API + return client } func testElasticsearchServer(t *testing.T, mockServerDir string) *httptest.Server { diff --git a/internal/elasticsearch/testdata/elasticsearch-8-5-healthy/_cluster-health.json b/internal/elasticsearch/testdata/elasticsearch-8-5-healthy/_cluster-health.json new file mode 100644 index 0000000000..26ab1ed050 --- /dev/null +++ b/internal/elasticsearch/testdata/elasticsearch-8-5-healthy/_cluster-health.json @@ -0,0 +1 @@ +{"cluster_name":"elasticsearch","status":"yellow","timed_out":false,"number_of_nodes":1,"number_of_data_nodes":1,"active_primary_shards":33,"active_shards":33,"relocating_shards":0,"initializing_shards":0,"unassigned_shards":19,"delayed_unassigned_shards":0,"number_of_pending_tasks":0,"number_of_in_flight_fetch":0,"task_max_waiting_in_queue_millis":0,"active_shards_percent_as_number":63.46153846153846} \ No newline at end of file diff --git a/internal/elasticsearch/testdata/elasticsearch-8-5-healthy/root.json b/internal/elasticsearch/testdata/elasticsearch-8-5-healthy/root.json new file mode 100644 index 0000000000..b5284be82c --- /dev/null +++ b/internal/elasticsearch/testdata/elasticsearch-8-5-healthy/root.json @@ -0,0 +1,17 @@ +{ + "name" : "6dcb6ee762ec", + "cluster_name" : "elasticsearch", + "cluster_uuid" : "YhxaHz-aRrKl_rtySRVBoQ", + "version" : { + "number" : "8.5.0-SNAPSHOT", + "build_flavor" : "default", + "build_type" : "docker", + "build_hash" : "77b936e44234defdde3c7ded0d1ad9ae5e288f77", + "build_date" : "2022-10-29T04:11:27.132517622Z", + "build_snapshot" : true, + "lucene_version" : "9.4.1", + "minimum_wire_compatibility_version" : "7.17.0", + "minimum_index_compatibility_version" : "7.0.0" + }, + "tagline" : "You Know, for Search" +} diff --git a/internal/elasticsearch/testdata/elasticsearch-8-5-red-out-of-disk/_cluster-health.json b/internal/elasticsearch/testdata/elasticsearch-8-5-red-out-of-disk/_cluster-health.json new file mode 100644 index 0000000000..8def409a5a --- /dev/null +++ b/internal/elasticsearch/testdata/elasticsearch-8-5-red-out-of-disk/_cluster-health.json @@ -0,0 +1 @@ +{"cluster_name":"elasticsearch","status":"red","timed_out":false,"number_of_nodes":1,"number_of_data_nodes":1,"active_primary_shards":33,"active_shards":33,"relocating_shards":0,"initializing_shards":0,"unassigned_shards":20,"delayed_unassigned_shards":0,"number_of_pending_tasks":0,"number_of_in_flight_fetch":0,"task_max_waiting_in_queue_millis":0,"active_shards_percent_as_number":62.264150943396224} \ No newline at end of file diff --git a/internal/elasticsearch/testdata/elasticsearch-8-5-red-out-of-disk/_internal-_health.json b/internal/elasticsearch/testdata/elasticsearch-8-5-red-out-of-disk/_internal-_health.json new file mode 100644 index 0000000000..d140d20c44 --- /dev/null +++ b/internal/elasticsearch/testdata/elasticsearch-8-5-red-out-of-disk/_internal-_health.json @@ -0,0 +1 @@ +{"status":"red","cluster_name":"elasticsearch","indicators":{"master_is_stable":{"status":"green","symptom":"The cluster has a stable master node","details":{"current_master":{"node_id":"PWBH3euxQn2wZwg0OgeCzQ","name":"008309953ac4"},"recent_masters":[{"node_id":"PWBH3euxQn2wZwg0OgeCzQ","name":"008309953ac4"}]}},"repository_integrity":{"status":"green","symptom":"No snapshot repositories configured."},"shards_availability":{"status":"red","symptom":"This cluster has 1 unavailable primary shard, 19 unavailable replica shards.","details":{"creating_primaries":0,"unassigned_replicas":19,"restarting_primaries":0,"restarting_replicas":0,"initializing_primaries":0,"started_replicas":0,"initializing_replicas":0,"unassigned_primaries":1,"started_primaries":33},"impacts":[{"id":"elasticsearch:health:shards_availability:impact:primary_unassigned","severity":1,"description":"Cannot add data to 1 index [.fleet-actions-7]. Searches might return incomplete results.","impact_areas":["ingest","search"]},{"id":"elasticsearch:health:shards_availability:impact:replica_unassigned","severity":2,"description":"Searches might be slower than usual. Fewer redundant copies of the data exist on 19 indices [.ds-logs-elastic_agent-default-2022.11.25-000001, .ds-logs-elastic_agent.filebeat-default-2022.11.25-000001, .ds-logs-elastic_agent.fleet_server-default-2022.11.25-000001, .ds-logs-elastic_agent.metricbeat-default-2022.11.25-000001, .ds-metrics-elastic_agent.elastic_agent-default-2022.11.25-000001, .ds-metrics-elastic_agent.filebeat-default-2022.11.25-000001, .ds-metrics-elastic_agent.fleet_server-default-2022.11.25-000001, .ds-metrics-elastic_agent.metricbeat-default-2022.11.25-000001, .ds-metrics-system.cpu-default-2022.11.25-000001, .ds-metrics-system.diskio-default-2022.11.25-000001, ...].","impact_areas":["search"]}],"diagnosis":[{"id":"elasticsearch:health:shards_availability:diagnosis:increase_tier_capacity_for_allocations:tier:data_hot","cause":"Elasticsearch isn't allowed to allocate some shards from these indices to any of the nodes in the desired data tier because there are not enough nodes in the [data_hot] tier to allocate each shard copy on a different node.","action":"Increase the number of nodes in this tier or decrease the number of replica shards in the affected indices.","affected_resources":[".ds-logs-elastic_agent-default-2022.11.25-000001",".ds-logs-elastic_agent.filebeat-default-2022.11.25-000001",".ds-logs-elastic_agent.fleet_server-default-2022.11.25-000001",".ds-logs-elastic_agent.metricbeat-default-2022.11.25-000001",".ds-metrics-elastic_agent.elastic_agent-default-2022.11.25-000001",".ds-metrics-elastic_agent.filebeat-default-2022.11.25-000001",".ds-metrics-elastic_agent.fleet_server-default-2022.11.25-000001",".ds-metrics-elastic_agent.metricbeat-default-2022.11.25-000001",".ds-metrics-system.cpu-default-2022.11.25-000001",".ds-metrics-system.diskio-default-2022.11.25-000001",".ds-metrics-system.filesystem-default-2022.11.25-000001",".ds-metrics-system.fsstat-default-2022.11.25-000001",".ds-metrics-system.load-default-2022.11.25-000001",".ds-metrics-system.memory-default-2022.11.25-000001",".ds-metrics-system.network-default-2022.11.25-000001",".ds-metrics-system.process-default-2022.11.25-000001",".ds-metrics-system.process.summary-default-2022.11.25-000001",".ds-metrics-system.socket_summary-default-2022.11.25-000001",".ds-metrics-system.uptime-default-2022.11.25-000001"],"help_url":"http://ela.st/tier-capacity"},{"id":"elasticsearch:health:shards_availability:diagnosis:explain_allocations","cause":"Elasticsearch isn't allowed to allocate some shards from these indices to any of the nodes in the cluster.","action":"Diagnose the issue by calling the allocation explain API for an index [GET _cluster/allocation/explain]. Choose a node to which you expect a shard to be allocated, find this node in the node-by-node explanation, and address the reasons which prevent Elasticsearch from allocating the shard.","affected_resources":[".fleet-actions-7"],"help_url":"http://ela.st/diagnose-shards"}]},"disk":{"status":"red","symptom":"33 indices are not allowed to be updated. 1 node is out of disk or running low on disk space.","details":{"indices_with_readonly_block":33,"nodes_with_enough_disk_space":0,"nodes_with_unknown_disk_status":0,"nodes_over_high_watermark":0,"nodes_over_flood_stage_watermark":1},"impacts":[{"id":"elasticsearch:health:disk:impact:ingest_capability_unavailable","severity":1,"description":"Cannot insert or update documents in the affected indices [.kibana_security_session_1, .security-7, .kibana_8.5.0_001, .kibana_task_manager_8.5.0_001, .apm-agent-configuration, .apm-custom-link, .ds-.logs-deprecation.elasticsearch-default-2022.11.25-000001, .ds-ilm-history-5-2022.11.25-000001, .ds-logs-elastic_agent-default-2022.11.25-000001, .ds-logs-elastic_agent.filebeat-default-2022.11.25-000001, ...].","impact_areas":["ingest"]},{"id":"elasticsearch:health:disk:impact:cluster_stability_at_risk","severity":1,"description":"Cluster stability might be impaired.","impact_areas":["deployment_management"]},{"id":"elasticsearch:health:disk:impact:cluster_functionality_unavailable","severity":3,"description":"The [ingest, ml, remote_cluster_client, transform] functionality might be impaired.","impact_areas":["deployment_management"]}],"diagnosis":[{"id":"elasticsearch:health:disk:diagnosis:add_disk_capacity_data_nodes","cause":"33 indices reside on nodes that have run or are likely to run out of disk space, this can temporarily disable writing on these indices.","action":"Enable autoscaling (if applicable), add disk capacity or free up disk space to resolve this. If you have already taken action please wait for the rebalancing to complete.","affected_resources":["PWBH3euxQn2wZwg0OgeCzQ"],"help_url":"https://ela.st/fix-data-disk"}]},"ilm":{"status":"green","symptom":"Index Lifecycle Management is running","details":{"policies":25,"ilm_status":"RUNNING"}},"slm":{"status":"green","symptom":"No Snapshot Lifecycle Management policies configured","details":{"slm_status":"RUNNING","policies":0}}}} \ No newline at end of file diff --git a/internal/elasticsearch/testdata/elasticsearch-8-5-red-out-of-disk/root.json b/internal/elasticsearch/testdata/elasticsearch-8-5-red-out-of-disk/root.json new file mode 100644 index 0000000000..b3e9625dbd --- /dev/null +++ b/internal/elasticsearch/testdata/elasticsearch-8-5-red-out-of-disk/root.json @@ -0,0 +1,17 @@ +{ + "name" : "008309953ac4", + "cluster_name" : "elasticsearch", + "cluster_uuid" : "vYXuo7eQR-ikBlJfH3kQaQ", + "version" : { + "number" : "8.5.0-SNAPSHOT", + "build_flavor" : "default", + "build_type" : "docker", + "build_hash" : "77b936e44234defdde3c7ded0d1ad9ae5e288f77", + "build_date" : "2022-10-29T04:11:27.132517622Z", + "build_snapshot" : true, + "lucene_version" : "9.4.1", + "minimum_wire_compatibility_version" : "7.17.0", + "minimum_index_compatibility_version" : "7.0.0" + }, + "tagline" : "You Know, for Search" +} From 30ce079595d292b0d23405372b3b0b2e961068a4 Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Fri, 25 Nov 2022 18:35:51 +0100 Subject: [PATCH 3/4] Make CheckHealth a client method --- cmd/benchmark.go | 4 ++-- cmd/dump.go | 2 +- cmd/testrunner.go | 4 ++-- internal/dump/installedobjects_test.go | 6 +++--- internal/elasticsearch/client.go | 21 +++++++++++++-------- internal/elasticsearch/client_test.go | 10 +++++----- internal/elasticsearch/test/httptest.go | 9 ++++----- 7 files changed, 30 insertions(+), 26 deletions(-) diff --git a/cmd/benchmark.go b/cmd/benchmark.go index 9aabae39bd..85a4daec28 100644 --- a/cmd/benchmark.go +++ b/cmd/benchmark.go @@ -147,11 +147,11 @@ func benchTypeCommandActionFactory(runner benchrunner.BenchRunner) cobraext.Comm return fmt.Errorf("no %s benchmarks found", benchType) } - esClient, err := elasticsearch.Client() + esClient, err := elasticsearch.NewClient() if err != nil { return errors.Wrap(err, "can't create Elasticsearch client") } - err = elasticsearch.CheckHealth(cmd.Context(), esClient) + err = esClient.CheckHealth(cmd.Context()) if err != nil { return err } diff --git a/cmd/dump.go b/cmd/dump.go index 000fb9bd6d..9b67410887 100644 --- a/cmd/dump.go +++ b/cmd/dump.go @@ -80,7 +80,7 @@ func dumpInstalledObjectsCmdAction(cmd *cobra.Command, args []string) error { if tlsSkipVerify { clientOptions = append(clientOptions, elasticsearch.OptionWithSkipTLSVerify()) } - client, err := elasticsearch.Client(clientOptions...) + client, err := elasticsearch.NewClient(clientOptions...) if err != nil { return errors.Wrap(err, "failed to initialize Elasticsearch client") } diff --git a/cmd/testrunner.go b/cmd/testrunner.go index 1dd5d02806..2be5235f82 100644 --- a/cmd/testrunner.go +++ b/cmd/testrunner.go @@ -207,11 +207,11 @@ func testTypeCommandActionFactory(runner testrunner.TestRunner) cobraext.Command variantFlag, _ := cmd.Flags().GetString(cobraext.VariantFlagName) - esClient, err := elasticsearch.Client() + esClient, err := elasticsearch.NewClient() if err != nil { return errors.Wrap(err, "can't create Elasticsearch client") } - err = elasticsearch.CheckHealth(cmd.Context(), esClient) + err = esClient.CheckHealth(cmd.Context()) if err != nil { return err } diff --git a/internal/dump/installedobjects_test.go b/internal/dump/installedobjects_test.go index 6b9c90bfa5..d83d5a0df4 100644 --- a/internal/dump/installedobjects_test.go +++ b/internal/dump/installedobjects_test.go @@ -64,7 +64,7 @@ type installedObjectsDumpSuite struct { func (s *installedObjectsDumpSuite) SetupTest() { _, err := os.Stat(s.DumpDir) if errors.Is(err, os.ErrNotExist) { - client, err := elasticsearch.Client() + client, err := elasticsearch.NewClient() s.Require().NoError(err) dumper := NewInstalledObjectsDumper(client.API, s.PackageName) @@ -77,7 +77,7 @@ func (s *installedObjectsDumpSuite) SetupTest() { } func (s *installedObjectsDumpSuite) TestDumpAll() { - client := estest.ElasticsearchClient(s.T(), s.RecordDir) + client := estest.NewClient(s.T(), s.RecordDir) outputDir := s.T().TempDir() dumper := NewInstalledObjectsDumper(client.API, s.PackageName) @@ -94,7 +94,7 @@ func (s *installedObjectsDumpSuite) TestDumpAll() { } func (s *installedObjectsDumpSuite) TestDumpSome() { - client := estest.ElasticsearchClient(s.T(), s.RecordDir) + client := estest.NewClient(s.T(), s.RecordDir) dumper := NewInstalledObjectsDumper(client.API, s.PackageName) // In a map so order of execution is randomized. diff --git a/internal/elasticsearch/client.go b/internal/elasticsearch/client.go index a9d79c27a0..f19cd37488 100644 --- a/internal/elasticsearch/client.go +++ b/internal/elasticsearch/client.go @@ -81,8 +81,13 @@ func OptionWithSkipTLSVerify() ClientOption { } } -// Client method creates new instance of the Elasticsearch client. -func Client(customOptions ...ClientOption) (*elasticsearch.Client, error) { +// Client is a wrapper over an Elasticsearch Client. +type Client struct { + *elasticsearch.Client +} + +// NewClient method creates new instance of the Elasticsearch client. +func NewClient(customOptions ...ClientOption) (*Client, error) { options := defaultOptionsFromEnv() for _, option := range customOptions { option(&options) @@ -115,11 +120,11 @@ func Client(customOptions ...ClientOption) (*elasticsearch.Client, error) { if err != nil { return nil, errors.Wrap(err, "can't create instance") } - return client, nil + return &Client{Client: client}, nil } -// CheckHealth checks the health of a cluster. -func CheckHealth(ctx context.Context, client *elasticsearch.Client) error { +// CheckHealth checks the health of the cluster. +func (client *Client) CheckHealth(ctx context.Context) error { resp, err := client.Cluster.Health(client.Cluster.Health.WithContext(ctx)) if err != nil { return errors.Wrap(err, "error checking cluster health") @@ -143,7 +148,7 @@ func CheckHealth(ctx context.Context, client *elasticsearch.Client) error { if status != "red" { return errors.Errorf("cluster in unhealthy state: %q", status) } - cause, err := redHealthCause(ctx, client) + cause, err := client.redHealthCause(ctx) if err != nil { return errors.Wrapf(err, "cluster in unhealthy state, failed to identify cause") } @@ -156,8 +161,8 @@ func CheckHealth(ctx context.Context, client *elasticsearch.Client) error { // redHealthCause tries to identify the cause of a cluster in red state. This could be // also used as a replacement of CheckHealth, but keeping them separated because it uses // internal undocumented APIs that might change. -func redHealthCause(ctx context.Context, client *elasticsearch.Client) (string, error) { - req, err := http.NewRequest(http.MethodGet, "/_internal/_health", nil) +func (client *Client) redHealthCause(ctx context.Context) (string, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "/_internal/_health", nil) if err != nil { return "", errors.Wrap(err, "error creating internal health request") } diff --git a/internal/elasticsearch/client_test.go b/internal/elasticsearch/client_test.go index 4cc65bcf5b..51e218a6d0 100644 --- a/internal/elasticsearch/client_test.go +++ b/internal/elasticsearch/client_test.go @@ -30,7 +30,7 @@ func TestClientWithTLS(t *testing.T) { caCertFile := writeCACertFile(t, server.Certificate()) t.Run("no TLS config, should fail", func(t *testing.T) { - client, err := elasticsearch.Client(elasticsearch.OptionWithAddress(server.URL)) + client, err := elasticsearch.NewClient(elasticsearch.OptionWithAddress(server.URL)) require.NoError(t, err) _, err = client.Ping() @@ -38,7 +38,7 @@ func TestClientWithTLS(t *testing.T) { }) t.Run("with CA", func(t *testing.T) { - client, err := elasticsearch.Client(elasticsearch.OptionWithAddress(server.URL), elasticsearch.OptionWithCertificateAuthority(caCertFile)) + client, err := elasticsearch.NewClient(elasticsearch.OptionWithAddress(server.URL), elasticsearch.OptionWithCertificateAuthority(caCertFile)) require.NoError(t, err) _, err = client.Ping() @@ -46,7 +46,7 @@ func TestClientWithTLS(t *testing.T) { }) t.Run("skip TLS verify", func(t *testing.T) { - client, err := elasticsearch.Client(elasticsearch.OptionWithAddress(server.URL), elasticsearch.OptionWithSkipTLSVerify()) + client, err := elasticsearch.NewClient(elasticsearch.OptionWithAddress(server.URL), elasticsearch.OptionWithSkipTLSVerify()) require.NoError(t, err) _, err = client.Ping() @@ -70,9 +70,9 @@ func TestClusterHealth(t *testing.T) { for _, c := range cases { t.Run(c.RecordDir, func(t *testing.T) { - client := test.ElasticsearchClient(t, c.RecordDir) + client := test.NewClient(t, c.RecordDir) - err := elasticsearch.CheckHealth(context.Background(), client) + err := client.CheckHealth(context.Background()) if c.Expected != "" { if assert.Error(t, err) { assert.Equal(t, c.Expected, err.Error()) diff --git a/internal/elasticsearch/test/httptest.go b/internal/elasticsearch/test/httptest.go index cd22dbd590..486e683276 100644 --- a/internal/elasticsearch/test/httptest.go +++ b/internal/elasticsearch/test/httptest.go @@ -13,21 +13,20 @@ import ( "strings" "testing" - esclient "github.com/elastic/go-elasticsearch/v7" "github.com/stretchr/testify/require" "github.com/elastic/elastic-package/internal/elasticsearch" ) -// ElasticsearchClient returns a client for a testing http server that uses prerecorded +// NewClient returns a client for a testing http server that uses prerecorded // responses. If responses are not found, it forwards the query to the server started by // elastic-package stack, and records the response. // Responses are recorded in the directory indicated by serverDataDir. -func ElasticsearchClient(t *testing.T, serverDataDir string) *esclient.Client { +func NewClient(t *testing.T, serverDataDir string) *elasticsearch.Client { server := testElasticsearchServer(t, serverDataDir) t.Cleanup(func() { server.Close() }) - client, err := elasticsearch.Client( + client, err := elasticsearch.NewClient( elasticsearch.OptionWithAddress(server.URL), ) require.NoError(t, err) @@ -57,7 +56,7 @@ func pathForURL(url string) string { } func recordRequest(t *testing.T, r *http.Request, path string) { - client, err := elasticsearch.Client() + client, err := elasticsearch.NewClient() require.NoError(t, err) t.Logf("Recording %s in %s", r.URL.Path, path) From 5bd11760bd1165d70131e030d6b3fa2bf78cf0fc Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Sun, 27 Nov 2022 01:26:26 +0100 Subject: [PATCH 4/4] Linting --- internal/elasticsearch/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/elasticsearch/client.go b/internal/elasticsearch/client.go index f19cd37488..ef8b8ee289 100644 --- a/internal/elasticsearch/client.go +++ b/internal/elasticsearch/client.go @@ -215,11 +215,11 @@ func (client *Client) redHealthCause(ctx context.Context) (string, error) { switch { case severity < highestSeverity: continue - case severity == highestSeverity: - break case severity > highestSeverity: highestSeverity = severity causes = nil + case severity == highestSeverity: + // Continue appending for current severity. } for _, diagnosis := range indicator.Diagnosis {