diff --git a/cmd/osctl/cmd/version.go b/cmd/osctl/cmd/version.go index b50d43bdf..4774fe8fd 100644 --- a/cmd/osctl/cmd/version.go +++ b/cmd/osctl/cmd/version.go @@ -51,7 +51,10 @@ var versionCmd = &cobra.Command{ reply, err := c.Version(globalCtx, grpc.Peer(&remotePeer)) if err != nil { - helpers.Fatalf("error getting version: %s", err) + if reply == nil { + helpers.Fatalf("error getting version: %s", err) + } + helpers.Warning("%s", err) } defaultNode := addrFromPeer(&remotePeer) diff --git a/cmd/osctl/pkg/client/client.go b/cmd/osctl/pkg/client/client.go index af50093f0..325ee7c64 100644 --- a/cmd/osctl/pkg/client/client.go +++ b/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 } diff --git a/cmd/osctl/pkg/client/client_test.go b/cmd/osctl/pkg/client/client_test.go deleted file mode 100644 index 125f4a58b..000000000 --- a/cmd/osctl/pkg/client/client_test.go +++ /dev/null @@ -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 -} diff --git a/cmd/osctl/pkg/client/reply.go b/cmd/osctl/pkg/client/reply.go new file mode 100644 index 000000000..55e37984c --- /dev/null +++ b/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() +} diff --git a/cmd/osctl/pkg/client/reply_test.go b/cmd/osctl/pkg/client/reply_test.go new file mode 100644 index 000000000..ce0ef4ce0 --- /dev/null +++ b/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) +} diff --git a/cmd/osctl/pkg/helpers/cli.go b/cmd/osctl/pkg/helpers/cli.go index 040a9d06d..33948f006 100644 --- a/cmd/osctl/pkg/helpers/cli.go +++ b/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...) +} diff --git a/internal/integration/api/version.go b/internal/integration/api/version.go index dc35a5bc5..c07682f40 100644 --- a/internal/integration/api/version.go +++ b/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)