review comments

Signed-off-by: Pavol Loffay <[email protected]>

cmd/es-index-cleaner/app/index_client.go

@@ -33,7 +33,7 @@ type Index struct {
 	Aliases map[string]bool
 }
 
-// IndicesClient is a client used to manipulate with indices.
+// IndicesClient is a client used to manipulate indices.
 type IndicesClient struct {
 	// Http client.
 	Client *http.Client
@@ -60,22 +60,18 @@ func (i *IndicesClient) GetJaegerIndices(prefix string) ([]Index, error) {
 		return nil, err
 	}
 	i.setAuthorization(r)
-	response, err := i.Client.Do(r)
+	res, err := i.Client.Do(r)
 	if err != nil {
-		return nil, fmt.Errorf("failed to query Jaeger indices: %q", err)
+		return nil, fmt.Errorf("failed to query indices: %w", err)
 	}
 
-	if response.StatusCode != 200 {
-		body, err := ioutil.ReadAll(response.Body)
-		if err != nil {
-			return nil, err
-		}
-		return nil, fmt.Errorf("failed to query Jaeger indices: %s", string(body))
+	if res.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("failed to query indices: %w", handleFailedRequest(res))
 	}
 
-	body, err := ioutil.ReadAll(response.Body)
+	body, err := ioutil.ReadAll(res.Body)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to query indices and read response body: %w", err)
 	}
 
 	type indexInfo struct {
@@ -84,7 +80,7 @@ func (i *IndicesClient) GetJaegerIndices(prefix string) ([]Index, error) {
 	}
 	var indicesInfo map[string]indexInfo
 	if err = json.Unmarshal(body, &indicesInfo); err != nil {
-		return nil, fmt.Errorf("failed to unmarshall response: %q", err)
+		return nil, fmt.Errorf("failed to query indices and unmarshall response body: %q: %w", body, err)
 	}
 
 	var indices []Index
@@ -119,24 +115,28 @@ func (i *IndicesClient) DeleteIndices(indices []Index) error {
 	}
 	i.setAuthorization(r)
 
-	response, err := i.Client.Do(r)
+	res, err := i.Client.Do(r)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to delete indices: %w", err)
 	}
-	if response.StatusCode != 200 {
-		var body string
-		if response.Body != nil {
-			bodyBytes, err := ioutil.ReadAll(response.Body)
-			if err != nil {
-				return fmt.Errorf("deleting indices failed: %q", err)
-			}
-			body = string(bodyBytes)
-		}
-		return fmt.Errorf("deleting indices failed: %s", body)
+	if res.StatusCode != http.StatusOK {
+		return fmt.Errorf("failed to delete indices: %s, %w", concatIndices, handleFailedRequest(res))
 	}
 	return nil
 }
 
+func handleFailedRequest(res *http.Response) error {
+	var body string
+	if res.Body != nil {
+		bodyBytes, err := ioutil.ReadAll(res.Body)
+		if err != nil {
+			return fmt.Errorf("request failed and failed to read response body, status code: %d, %w", res.StatusCode, err)
+		}
+		body = string(bodyBytes)
+	}
+	return fmt.Errorf("request failed, status code: %d, body: %s", res.StatusCode, body)
+}
+
 func (i *IndicesClient) setAuthorization(r *http.Request) {
 	if i.BasicAuth != "" {
 		r.Header.Add("Authorization", fmt.Sprintf("Basic %s", i.BasicAuth))

cmd/es-index-cleaner/app/index_client_test.go

@@ -103,13 +103,13 @@ func TestClientGetIndices(t *testing.T) {
 			name:         "client error",
 			responseCode: http.StatusBadRequest,
 			response:     esErrResponse,
-			errContains:  "failed to query Jaeger indices: ",
+			errContains:  "failed to query indices: request failed, status code: 400",
 		},
 		{
 			name:         "unmarshall error",
 			responseCode: http.StatusOK,
 			response:     "AAA",
-			errContains:  `failed to unmarshall response: "invalid character 'A' looking for beginning of value`,
+			errContains:  `failed to query indices and unmarshall response body: "AAA"`,
 		},
 	}
 	for _, test := range tests {
@@ -156,7 +156,7 @@ func TestClientDeleteIndices(t *testing.T) {
 			name:         "client error",
 			responseCode: http.StatusBadRequest,
 			response:     esErrResponse,
-			errContains:  "deleting indices failed: ",
+			errContains:  "ailed to delete indices: jaeger-span",
 		},
 	}
 	for _, test := range tests {

cmd/es-index-cleaner/app/index_filter_test.go

@@ -31,7 +31,6 @@ func TestIndexFilter_prefix(t *testing.T) {
 
 func testIndexFilter(t *testing.T, prefix string) {
 	time20200807 := time.Date(2020, time.August, 06, 0, 0, 0, 0, time.UTC).AddDate(0, 0, 1)
-	//firstDay := tomorrowMidnight.Add(-time.Hour*24*time.Duration(numOfDays))
 	indices := []Index{
 		{
 			Index:        prefix + "jaeger-span-2020-08-06",
@@ -151,7 +150,7 @@ func testIndexFilter(t *testing.T, prefix string) {
 		expected []Index
 	}{
 		{
-			name: "normal indices, remove older 2 days",
+			name: "normal indices, remove older than 2 days",
 			filter: &IndexFilter{
 				IndexPrefix:          prefix,
 				IndexDateSeparator:   "-",
@@ -188,7 +187,7 @@ func testIndexFilter(t *testing.T, prefix string) {
 			},
 		},
 		{
-			name: "normal indices, remove older 0 days",
+			name: "normal indices, remove older 0 days - it should remove all indices",
 			filter: &IndexFilter{
 				IndexPrefix:          prefix,
 				IndexDateSeparator:   "-",

cmd/es-index-cleaner/main.go

@@ -32,22 +32,22 @@ import (
 )
 
 func main() {
-	logger, _ := zap.NewDevelopment()
+	logger, _ := zap.NewProduction()
 	v := viper.New()
 	cfg := &app.Config{}
 	tlsFlags := tlscfg.ClientFlagsConfig{Prefix: "es"}
 
 	var command = &cobra.Command{
 		Use:   "jaeger-es-index-cleaner NUM_OF_DAYS http://HOSTNAME:PORT",
-		Short: "Jaeger es-index-cleaner removes Jaeger index",
-		Long:  "Jaeger es-index-cleaner removes Jaeger indexes",
+		Short: "Jaeger es-index-cleaner removes Jaeger indices",
+		Long:  "Jaeger es-index-cleaner removes Jaeger indices",
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if len(args) != 2 {
 				return fmt.Errorf("wrong number of arguments")
 			}
 			numOfDays, err := strconv.Atoi(args[0])
 			if err != nil {
-				return fmt.Errorf("could not parse NUM_OF_DAYS argument: %q", err)
+				return fmt.Errorf("could not parse NUM_OF_DAYS argument: %w", err)
 			}
 
 			cfg.InitFromViper(v)