diff --git a/es.go b/es.go index ead8b28..b1aeb97 100644 --- a/es.go +++ b/es.go @@ -761,37 +761,53 @@ func (c *Client) SetAllocation(allocation string) (string, error) { return allocationVal.String(), nil } -//Set a new value for a cluster setting +// Set a new value for a cluster setting. Returns existing value and new value as well as error, in that order +// If the setting is not set in Elasticsearch (it's falling back to default configuration) SetClusterSetting's existingValue will be nil. +// If the value provided is nil, SetClusterSetting will remove the setting so that Elasticsearch falls back on default configuration for that setting. // //Use case: You've doubled the number of nodes in your cluster and you want to increase the number of shards the cluster can relocate at one time. Calling `SetClusterSetting("cluster.routing.allocation.cluster_concurrent_rebalance", "100")` will update that value with the cluster. Once data relocation is complete you can decrease the setting by calling `SetClusterSetting("cluster.routing.allocation.cluster_concurrent_rebalance", "20")`. -func (c *Client) SetClusterSetting(setting string, value string) (string, string, error) { - +func (c *Client) SetClusterSetting(setting string, value *string) (*string, *string, error) { + var existingValue *string + var newValue *string settingsBody, err := handleErrWithBytes(c.buildGetRequest(clusterSettingsPath)) if err != nil { - return "", "", err + return existingValue, newValue, err } - existingValues := gjson.GetManyBytes(settingsBody, fmt.Sprintf("transient.%s", setting), fmt.Sprintf("persistent.%s", setting)) + existingResults := gjson.GetManyBytes(settingsBody, fmt.Sprintf("transient.%s", setting), fmt.Sprintf("persistent.%s", setting)) + + var newSettingBody string + + if value == nil { + newSettingBody = fmt.Sprintf(`{"transient" : { "%s" : null}}`, setting) + } else { + newSettingBody = fmt.Sprintf(`{"transient" : { "%s" : "%s"}}`, setting, *value) + } agent := c.buildPutRequest(clusterSettingsPath). Set("Content-Type", "application/json"). - Send(fmt.Sprintf(`{"transient" : { "%s" : "%s"}}`, setting, value)) + Send(newSettingBody) body, err := handleErrWithBytes(agent) if err != nil { - return "", "", err + return existingValue, newValue, err } - newValue := gjson.GetBytes(body, fmt.Sprintf("transient.%s", setting)).String() - - var existingValue string + newResults := gjson.GetBytes(body, fmt.Sprintf("transient.%s", setting)).String() + if newResults != "" { + newValue = &newResults + } - if existingValues[0].String() == "" { - existingValue = existingValues[1].String() + if existingResults[0].String() == "" { + if existingResults[1].String() != "" { + value := existingResults[1].String() + existingValue = &value + } } else { - existingValue = existingValues[0].String() + value := existingResults[0].String() + existingValue = &value } return existingValue, newValue, nil diff --git a/es_test.go b/es_test.go index 2c748a0..dc9d6f9 100644 --- a/es_test.go +++ b/es_test.go @@ -28,6 +28,10 @@ func buildTestServer(t *testing.T, setups []*ServerSetup, tls bool) *httptest.Se matched := false for _, setup := range setups { + // Extra piece of debug incase there's a typo in your test's response, like a rogue space somewhere + if r.Method == setup.Method && r.URL.EscapedPath() == setup.Path && requestBody != setup.Body { + t.Errorf("request body not matching: %s != %s", requestBody, setup.Body) + } if r.Method == setup.Method && r.URL.EscapedPath() == setup.Path && requestBody == setup.Body { matched = true if setup.HTTPStatus == 0 { @@ -77,6 +81,8 @@ func setupTestTLSServers(t *testing.T, setups []*ServerSetup) (string, int, *htt return url.Hostname(), port, ts } +func stringToPointer(v string) *string { return &v } + func TestGetClusterExcludeSettings(t *testing.T) { testSetup := &ServerSetup{ @@ -680,9 +686,9 @@ func TestSetClusterSettings(t *testing.T) { GetResponse string PutResponse string Setting string - SetValue string + SetValue *string HTTPStatus int - OldValue string + OldValue *string }{ { // Tests for behavior with existing transient setting. @@ -691,8 +697,8 @@ func TestSetClusterSettings(t *testing.T) { Body: `{"transient":{"cluster.routing.allocation.exclude._name":"10.0.0.99"}}`, PutResponse: `{"persistent":{},"transient":{"cluster":{"routing":{"allocation":{"exclude":{"_name":"10.0.0.99"}}}}}}`, Setting: "cluster.routing.allocation.exclude._name", - SetValue: "10.0.0.99", - OldValue: "10.0.0.2", + SetValue: stringToPointer("10.0.0.99"), + OldValue: stringToPointer("10.0.0.2"), }, { @@ -702,8 +708,8 @@ func TestSetClusterSettings(t *testing.T) { Body: `{"transient":{"cluster.routing.allocation.exclude._name":"10.0.0.99"}}`, PutResponse: `{"persistent":{},"transient":{"cluster":{"routing":{"allocation":{"exclude":{"_name":"10.0.0.99"}}}}}}`, Setting: "cluster.routing.allocation.exclude._name", - SetValue: "10.0.0.99", - OldValue: "10.0.0.2", + SetValue: stringToPointer("10.0.0.99"), + OldValue: stringToPointer("10.0.0.2"), }, { @@ -713,8 +719,30 @@ func TestSetClusterSettings(t *testing.T) { Body: `{"transient":{"cluster.routing.allocation.exclude._name":"10.0.0.99"}}`, PutResponse: `{"persistent":{},"transient":{"cluster":{"routing":{"allocation":{"exclude":{"_name":"10.0.0.99"}}}}}}`, Setting: "cluster.routing.allocation.exclude._name", - SetValue: "10.0.0.99", - OldValue: "", + SetValue: stringToPointer("10.0.0.99"), + OldValue: nil, + }, + + { + // Tests for behavior when removing setting (null'ing it). + Name: "Removing Existing Persistent Setting", + GetResponse: `{"transient":{},"persistent":{"cluster":{"routing":{"allocation":{"exclude":{"_name":"10.0.0.2"}}}}}}`, + Body: `{"transient":{"cluster.routing.allocation.exclude._name":null}}`, + PutResponse: `{"transient":{},"persistent":{}}`, + Setting: "cluster.routing.allocation.exclude._name", + SetValue: nil, + OldValue: stringToPointer("10.0.0.2"), + }, + + { + // Tests for behavior when removing setting (null'ing it) that is already null. + Name: "Removing Null Persistent Setting", + GetResponse: `{"transient":{},"persistent":{}}`, + Body: `{"transient":{"cluster.routing.allocation.exclude._name":null}}`, + PutResponse: `{"transient":{},"persistent":{}}`, + Setting: "cluster.routing.allocation.exclude._name", + SetValue: nil, + OldValue: nil, }, } @@ -743,14 +771,35 @@ func TestSetClusterSettings(t *testing.T) { st.Errorf("Expected error to be nil, %s", err) } - if oldSetting != x.OldValue { - st.Errorf("Unexpected old value, got %s", oldSetting) + if oldSetting == nil { + if x.OldValue != nil { + st.Fatalf("Unexpected old value: expected old value to be %s, got nil", *x.OldValue) + } + } + + if oldSetting != nil { + if x.OldValue == nil { + st.Fatalf("Unexpected old value: expected old value to be nil, got %v", *oldSetting) + } + if *oldSetting != *x.OldValue { + st.Errorf("Unexpected old value: expected %s, got %s", *x.OldValue, *oldSetting) + } } - if newSetting != "10.0.0.99" { - st.Errorf("Unexpected new value, got %s", newSetting) + if newSetting == nil { + if x.SetValue != nil { + st.Fatalf("Unexpected new value, got nil, expected %s", *x.SetValue) + } } + if newSetting != nil { + if x.SetValue == nil { + st.Errorf("Unexpected new value, got %s, expected nil", *newSetting) + } + if *newSetting != *x.SetValue { + st.Errorf("Unexpected new value, got %v, expected %v", newSetting, x.SetValue) + } + } }) } } @@ -831,7 +880,7 @@ func TestSetClusterSetting_BadRequest(t *testing.T) { defer ts.Close() client := NewClient(host, port) - _, _, err := client.SetClusterSetting("cluster.routing.allocation.enable", "foo") + _, _, err := client.SetClusterSetting("cluster.routing.allocation.enable", stringToPointer("foo")) if err == nil { t.Errorf("Expected error to not be nil, %s", err) diff --git a/go.mod b/go.mod index 1f0a075..f0a35b1 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/pkg/errors v0.8.0 // indirect github.com/smartystreets/goconvey v1.6.4 // indirect github.com/spf13/cobra v0.0.5 + github.com/spf13/pflag v1.0.5 // indirect github.com/spf13/viper v1.3.2 github.com/stretchr/testify v1.4.0 // indirect github.com/tidwall/gjson v1.1.3 diff --git a/go.sum b/go.sum index c5a175d..378d54c 100644 --- a/go.sum +++ b/go.sum @@ -61,6 +61,8 @@ github.com/spf13/jwalterweatherman v1.0.0 h1:XHEdyB+EcvlqZamSM4ZOMGlc93t6AcsBEu9 github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo= github.com/spf13/pflag v1.0.3 h1:zPAT6CGy6wXeQ7NtTnaTerfKOsV6V6F8agHXFiazDkg= github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= +github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= +github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/viper v1.3.2 h1:VUFqw5KcqRf7i70GOzW7N+Q7+gxVBkSSqiXB12+JQ4M= github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/pkg/cli/entrypoint.go b/pkg/cli/entrypoint.go index bb64a07..ea39655 100644 --- a/pkg/cli/entrypoint.go +++ b/pkg/cli/entrypoint.go @@ -5,11 +5,11 @@ import ( "crypto/tls" "crypto/x509" "fmt" - "github.com/github/vulcanizer" "io" "io/ioutil" "os" + "github.com/github/vulcanizer" "github.com/olekukonko/tablewriter" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -113,6 +113,13 @@ func getClient() *vulcanizer.Client { return v } +func printableNil(ptrValue *string) string { + if ptrValue == nil { + return "null" + } + return *ptrValue +} + func renderTable(rows [][]string, header []string) string { var result bytes.Buffer table := tablewriter.NewWriter(&result) diff --git a/pkg/cli/setting.go b/pkg/cli/setting.go index bf2fc1b..c1feea6 100644 --- a/pkg/cli/setting.go +++ b/pkg/cli/setting.go @@ -9,6 +9,8 @@ import ( var settingToUpdate, valueToUpdate string +var removeValue bool + func init() { cmdSettingUpdate.Flags().StringVarP(&settingToUpdate, "setting", "s", "", "Elasticsearch cluster setting to update (required)") @@ -18,12 +20,9 @@ func init() { os.Exit(1) } - cmdSettingUpdate.Flags().StringVarP(&valueToUpdate, "value", "v", "", "Value of the Elasticsearch cluster setting to update (required)") - err = cmdSettingUpdate.MarkFlagRequired("value") - if err != nil { - fmt.Printf("Error binding value configuration flag: %s \n", err) - os.Exit(1) - } + cmdSettingUpdate.Flags().StringVarP(&valueToUpdate, "value", "v", "", "Value of the Elasticsearch cluster setting to update (can't be used with \"--remove\")") + + cmdSettingUpdate.Flags().BoolVar(&removeValue, "remove", false, "Remove provided cluster setting, resetting it to default configuration (can't be used with \"--value|-v\")") cmdSetting.AddCommand(cmdSettingUpdate) rootCmd.AddCommand(cmdSetting) @@ -41,18 +40,36 @@ var cmdSettingUpdate = &cobra.Command{ Long: `This command will update the cluster's settings with the provided value.`, Run: func(cmd *cobra.Command, args []string) { + if cmd.Flags().Changed("value") && cmd.Flags().Changed("remove") { + fmt.Println("Can't set both \"--value|-v\" and \"--remove\" options") + fmt.Print(cmd.UsageString()) + os.Exit(1) + } + if !cmd.Flags().Changed("value") && !cmd.Flags().Changed("remove") { + fmt.Println("Error: requires one of \"--value|-v\" or \"--remove\"") + fmt.Print(cmd.UsageString()) + os.Exit(1) + } v := getClient() - existingValue, newValue, err := v.SetClusterSetting(settingToUpdate, valueToUpdate) + var ptrValueToUpdate *string + + if removeValue { + ptrValueToUpdate = nil + } else { + ptrValueToUpdate = &valueToUpdate + } + + existingValue, newValue, err := v.SetClusterSetting(settingToUpdate, ptrValueToUpdate) if err != nil { - fmt.Printf("Error when trying to update \"%s\" to \"%s\n", settingToUpdate, valueToUpdate) + fmt.Printf("Error when trying to update \"%s\" to \"%s\"\n", settingToUpdate, printableNil(ptrValueToUpdate)) fmt.Printf("Error is: %s\n", err) os.Exit(1) } fmt.Printf("Updated setting %s\n", settingToUpdate) - fmt.Printf("\tOld value: %s\n", existingValue) - fmt.Printf("\tNew value: %s\n", newValue) + fmt.Printf("\tOld value: %s\n", printableNil(existingValue)) + fmt.Printf("\tNew value: %s\n", printableNil(newValue)) }, } diff --git a/script/integration-test b/script/integration-test index 30f6d8b..27337d6 100755 --- a/script/integration-test +++ b/script/integration-test @@ -1,6 +1,18 @@ #!/bin/bash set -e +# Before we start, check if "vm.max_map_count" is correctly set, or the Docker containers will silently fail and lock the test +maxMap="$(sysctl vm.max_map_count | awk -F "= " '{print $2}')" +if [[ $maxMap -lt 262144 ]] +then + # incorrect config, won't work + echo "You need to increase vm.max_map_count to at least 262144 before Elasticsearch will start" + echo "Currently set to $maxMap" + echo "Run \"sudo sysctl -w vm.max_map_count=262144\" and start the test again." + exit 1 +fi + + # Run regular unit tests first ./script/test