fix: extract errors from API response

This PR only touches `Version` method, but I will expand it to other
methods in the next PR.

When proxying to many upstreams, errors are wrapped as responses as we
can't return error and response from grpc call. Reflect-based function
was introduced to filter out responses which contain errors as
multierror. Reflection was used, as each response is a different Go
type, and we can't write a generic function for it.

osctl was updated to support having both resp & err not nil. One failed
response shouldn't result in error.

Re-enabled integration test for multiple targets and version
consistency, need e2e validation.

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>

cmd/osctl/cmd/version.go

@@ -51,8 +51,11 @@ var versionCmd = &cobra.Command{
 
 			reply, err := c.Version(globalCtx, grpc.Peer(&remotePeer))
 			if err != nil {
+				if reply == nil {
 					helpers.Fatalf("error getting version: %s", err)
 				}
+				helpers.Warning("%s", err)
+			}
 
 			defaultNode := addrFromPeer(&remotePeer)
 

cmd/osctl/pkg/client/client.go

@@ -286,6 +286,10 @@ func (c *Client) Version(ctx context.Context, callOptions ...grpc.CallOption) (r
 		callOptions...,
 	)
 
+	var filtered interface{}
+	filtered, err = FilterReply(reply, err)
+	reply, _ = filtered.(*machineapi.VersionReply) //nolint: errcheck
+
 	return
 }
 

cmd/osctl/pkg/client/client_test.go

@@ -1,14 +0,0 @@
-// This Source Code Form is subject to the terms of the Mozilla Public
-// License, v. 2.0. If a copy of the MPL was not distributed with this
-// file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-package client_test
-
-import "testing"
-
-func TestEmpty(t *testing.T) {
-	// added for accurate coverage estimation
-	//
-	// please remove it once any unit-test is added
-	// for this package
-}

cmd/osctl/pkg/client/reply.go

@@ -0,0 +1,127 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+package client
+
+import (
+	"fmt"
+	"reflect"
+
+	"github.com/hashicorp/go-multierror"
+)
+
+// NodeError is RPC error from some node
+type NodeError struct {
+	Node string
+	Err  string
+}
+
+func (ne *NodeError) Error() string {
+	return fmt.Sprintf("%s: %s", ne.Node, ne.Err)
+}
+
+// FilterReply removes error responses from reply and builds multierror.
+//
+//nolint: gocyclo
+func FilterReply(reply interface{}, err error) (interface{}, error) {
+	if reply == nil {
+		return nil, err
+	}
+
+	replyStructPtr := reflect.ValueOf(reply)
+	if replyStructPtr.Kind() != reflect.Ptr {
+		panic("reply should be pointer to struct")
+	}
+
+	replyStruct := replyStructPtr.Elem()
+	if replyStruct.Kind() != reflect.Struct {
+		panic("reply should be struct")
+	}
+
+	responseField := replyStruct.FieldByName("Response")
+	if !responseField.IsValid() {
+		panic("Response field missing")
+	}
+
+	if responseField.Kind() != reflect.Slice {
+		panic("Response field should be a slice")
+	}
+
+	var multiErr *multierror.Error
+
+	for i := 0; i < responseField.Len(); {
+		responsePtr := responseField.Index(i)
+		if responsePtr.Kind() != reflect.Ptr {
+			panic("response slice should container pointers")
+		}
+
+		response := responsePtr.Elem()
+		if response.Kind() != reflect.Struct {
+			panic("response slice should container pointers to structs")
+		}
+
+		metadataField := response.FieldByName("Metadata")
+		if !metadataField.IsValid() {
+			panic("response metadata field missing")
+		}
+
+		if metadataField.Kind() != reflect.Ptr {
+			panic("response metadata field should be a pointer")
+		}
+
+		if metadataField.IsNil() {
+			// missing metadata, skip the field
+			i++
+			continue
+		}
+
+		metadata := metadataField.Elem()
+		if metadata.Kind() != reflect.Struct {
+			panic("response metadata should be struct")
+		}
+
+		errorField := metadata.FieldByName("Error")
+		if !errorField.IsValid() {
+			panic("metadata.Error field missing")
+		}
+
+		if errorField.Kind() != reflect.String {
+			panic("metadata.Error should be string")
+		}
+
+		if errorField.IsZero() {
+			// no error, leave it as is
+			i++
+			continue
+		}
+
+		hostnameField := metadata.FieldByName("Hostname")
+		if !hostnameField.IsValid() {
+			panic("metadata.Hostname field missing")
+		}
+
+		if hostnameField.Kind() != reflect.String {
+			panic("metadata.Hostname should be string")
+		}
+
+		// extract error
+		nodeError := &NodeError{
+			Node: hostnameField.String(),
+			Err:  errorField.String(),
+		}
+
+		multiErr = multierror.Append(multiErr, nodeError)
+
+		// remove ith response
+		reflect.Copy(responseField.Slice(i, responseField.Len()), responseField.Slice(i+1, responseField.Len()))
+		responseField.SetLen(responseField.Len() - 1)
+	}
+
+	// if all the responses were error responses...
+	if multiErr != nil && responseField.Len() == 0 {
+		reply = nil
+	}
+
+	return reply, multiErr.ErrorOrNil()
+}

cmd/osctl/pkg/client/reply_test.go

@@ -0,0 +1,91 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+package client_test
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/talos-systems/talos/api/common"
+	"github.com/talos-systems/talos/cmd/osctl/pkg/client"
+)
+
+func TestFilterResponse(t *testing.T) {
+	reply := &common.DataReply{
+		Response: []*common.DataResponse{
+			{
+				Metadata: &common.ResponseMetadata{
+					Hostname: "host1",
+				},
+				Bytes: []byte("abc"),
+			},
+			{
+				Metadata: &common.ResponseMetadata{
+					Hostname: "host2",
+					Error:    "something wrong",
+				},
+			},
+			{
+				Bytes: []byte("def"),
+			},
+			{
+				Metadata: &common.ResponseMetadata{
+					Hostname: "host4",
+					Error:    "even more wrong",
+				},
+			},
+		},
+	}
+
+	filtered, err := client.FilterReply(reply, nil)
+	assert.EqualError(t, err, "2 errors occurred:\n\t* host2: something wrong\n\t* host4: even more wrong\n\n")
+	assert.Equal(t, filtered,
+		&common.DataReply{
+			Response: []*common.DataResponse{
+				{
+					Metadata: &common.ResponseMetadata{
+						Hostname: "host1",
+					},
+					Bytes: []byte("abc"),
+				},
+				{
+					Bytes: []byte("def"),
+				},
+			},
+		})
+}
+
+func TestFilterResponseNil(t *testing.T) {
+	e := errors.New("wrong")
+
+	filtered, err := client.FilterReply(nil, e)
+	assert.Nil(t, filtered)
+	assert.Equal(t, e, err)
+}
+
+func TestFilterResponseOnlyErrors(t *testing.T) {
+	reply := &common.DataReply{
+		Response: []*common.DataResponse{
+			{
+				Metadata: &common.ResponseMetadata{
+					Hostname: "host2",
+					Error:    "something wrong",
+				},
+			},
+			{
+				Metadata: &common.ResponseMetadata{
+					Hostname: "host4",
+					Error:    "even more wrong",
+				},
+			},
+		},
+	}
+
+	filtered, err := client.FilterReply(reply, nil)
+	assert.EqualError(t, err, "2 errors occurred:\n\t* host2: something wrong\n\t* host4: even more wrong\n\n")
+	assert.Nil(t, filtered)
+}

cmd/osctl/pkg/helpers/cli.go

@@ -19,3 +19,12 @@ func Fatalf(message string, args ...interface{}) {
 	fmt.Fprintf(os.Stderr, message, args...)
 	os.Exit(1)
 }
+
+// Warning prints formatted message to stderr
+func Warning(message string, args ...interface{}) {
+	if !strings.HasSuffix(message, "\n") {
+		message += "\n"
+	}
+
+	fmt.Fprintf(os.Stderr, "WARNING: "+message, args...)
+}

internal/integration/api/version.go

@@ -33,8 +33,6 @@ func (suite *VersionSuite) TestExpectedVersionMaster() {
 
 // TestSameVersionCluster verifies that all the nodes are on the same version
 func (suite *VersionSuite) TestSameVersionCluster() {
-	suite.T().Skip("disabled test as it frequently fails on e2e (less responses than expected)")
-
 	nodes := suite.DiscoverNodes()
 	suite.Require().NotEmpty(nodes)