From 9e817ea0a119c91d187cb97c179a21117d210056 Mon Sep 17 00:00:00 2001 From: Paolo Calao Date: Wed, 10 Nov 2021 17:27:11 +0100 Subject: [PATCH 01/14] Ota Upload on multiple devices --- cli/ota/upload.go | 30 ++++++++++----- command/ota/upload.go | 76 ++++++++++++++++++++++++++++++++++---- command/ota/upload_test.go | 36 ++++++++++++++++++ 3 files changed, 126 insertions(+), 16 deletions(-) create mode 100644 command/ota/upload_test.go diff --git a/cli/ota/upload.go b/cli/ota/upload.go index 03e131ce..85f0eb0a 100644 --- a/cli/ota/upload.go +++ b/cli/ota/upload.go @@ -28,9 +28,11 @@ import ( ) var uploadFlags struct { - deviceID string - file string - deferred bool + deviceIDs []string + tags map[string]string + file string + deferred bool + fqbn string } func initUploadCommand() *cobra.Command { @@ -41,23 +43,33 @@ func initUploadCommand() *cobra.Command { Run: runUploadCommand, } - uploadCommand.Flags().StringVarP(&uploadFlags.deviceID, "device-id", "d", "", "Device ID") + uploadCommand.Flags().StringSliceVarP(&uploadFlags.deviceIDs, "device-ids", "d", []string{}, + "Comma-separated list of device IDs to update") + uploadCommand.Flags().StringToStringVar(&uploadFlags.tags, "tags", nil, + "Comma-separated list of tags with format =.\n"+ + "Perform and OTA upload on all devices that match the provided tags.\n"+ + "Mutually exclusive with `--device-id`.", + ) uploadCommand.Flags().StringVarP(&uploadFlags.file, "file", "", "", "Binary file (.bin) to be uploaded") uploadCommand.Flags().BoolVar(&uploadFlags.deferred, "deferred", false, "Perform a deferred OTA. It can take up to 1 week.") + uploadCommand.Flags().StringVarP(&uploadFlags.fqbn, "fqbn", "b", "", "FQBN of the devices to update") - uploadCommand.MarkFlagRequired("device-id") uploadCommand.MarkFlagRequired("file") + uploadCommand.MarkFlagRequired("fqbn") return uploadCommand } func runUploadCommand(cmd *cobra.Command, args []string) { - logrus.Infof("Uploading binary %s to device %s", uploadFlags.file, uploadFlags.deviceID) + logrus.Infof("Uploading binary %s", uploadFlags.file) params := &ota.UploadParams{ - DeviceID: uploadFlags.deviceID, - File: uploadFlags.file, - Deferred: uploadFlags.deferred, + DeviceIDs: uploadFlags.deviceIDs, + Tags: uploadFlags.tags, + File: uploadFlags.file, + Deferred: uploadFlags.deferred, + FQBN: uploadFlags.fqbn, } + err := ota.Upload(params) if err != nil { feedback.Errorf("Error during ota upload: %v", err) diff --git a/command/ota/upload.go b/command/ota/upload.go index f10f2ba9..1017324f 100644 --- a/command/ota/upload.go +++ b/command/ota/upload.go @@ -18,10 +18,12 @@ package ota import ( + "errors" "fmt" "io/ioutil" "os" "path/filepath" + "strings" "github.com/arduino/arduino-cloud-cli/internal/config" "github.com/arduino/arduino-cloud-cli/internal/iot" @@ -32,19 +34,29 @@ const ( otaExpirationMins = 10 // deferred ota can take up to 1 week (equal to 10080 minutes) otaDeferredExpirationMins = 10080 + + numConcurrentUploads = 10 ) // UploadParams contains the parameters needed to // perform an OTA upload. type UploadParams struct { - DeviceID string - File string - Deferred bool + DeviceIDs []string + Tags map[string]string + File string + Deferred bool + FQBN string } // Upload command is used to upload a firmware OTA, // on a device of Arduino IoT Cloud. func Upload(params *UploadParams) error { + if params.DeviceIDs == nil && params.Tags == nil { + return errors.New("provide either DeviceID or Tags") + } else if params.DeviceIDs != nil && params.Tags != nil { + return errors.New("cannot use both DeviceID and Tags. only one of them should be not nil") + } + conf, err := config.Retrieve() if err != nil { return err @@ -54,10 +66,14 @@ func Upload(params *UploadParams) error { return err } - dev, err := iotClient.DeviceShow(params.DeviceID) + d, err := idsGivenTags(iotClient, params.Tags) if err != nil { return err } + devs := append(params.DeviceIDs, d...) + if len(devs) == 0 { + return errors.New("no device found") + } otaDir, err := ioutil.TempDir("", "") if err != nil { @@ -66,7 +82,7 @@ func Upload(params *UploadParams) error { otaFile := filepath.Join(otaDir, "temp.ota") defer os.RemoveAll(otaDir) - err = Generate(params.File, otaFile, dev.Fqbn) + err = Generate(params.File, otaFile, params.FQBN) if err != nil { return fmt.Errorf("%s: %w", "cannot generate .ota file", err) } @@ -81,10 +97,56 @@ func Upload(params *UploadParams) error { expiration = otaDeferredExpirationMins } - err = iotClient.DeviceOTA(params.DeviceID, file, expiration) + return run(iotClient, devs, file, expiration) +} + +func idsGivenTags(iotClient iot.Client, tags map[string]string) ([]string, error) { + if tags == nil { + return nil, nil + } + devs, err := iotClient.DeviceList(tags) if err != nil { - return err + return nil, fmt.Errorf("%s: %w", "cannot retrieve devices from cloud", err) + } + devices := make([]string, 0, len(devs)) + for _, d := range devs { + devices = append(devices, d.Id) + } + return devices, nil +} + +func run(iotClient iot.Client, ids []string, file *os.File, expiration int) error { + idsToProcess := make(chan string, 2000) + idsFailed := make(chan string, 2000) + for _, id := range ids { + idsToProcess <- id + } + close(idsToProcess) + + for i := 0; i < numConcurrentUploads; i++ { + go func() { + for id := range idsToProcess { + err := iotClient.DeviceOTA(id, file, expiration) + fail := "" + if err != nil { + fail = id + } + idsFailed <- fail + } + }() + } + + failMsg := "" + for range ids { + i := <-idsFailed + if i != "" { + failMsg = strings.Join([]string{i, failMsg}, ",") + } } + if failMsg != "" { + failMsg = strings.TrimRight(failMsg, ",") + return fmt.Errorf("failed to update these devices: %s", failMsg) + } return nil } diff --git a/command/ota/upload_test.go b/command/ota/upload_test.go new file mode 100644 index 00000000..823f7ca8 --- /dev/null +++ b/command/ota/upload_test.go @@ -0,0 +1,36 @@ +package ota + +import ( + "errors" + "fmt" + "os" + "strings" + "testing" + "time" + + "github.com/arduino/arduino-cloud-cli/internal/iot/mocks" + "github.com/stretchr/testify/mock" +) + +func TestRun(t *testing.T) { + mockClient := &mocks.Client{} + mockDeviceOTA := func(id string, file *os.File, expireMins int) error { + time.Sleep(3 * time.Second) + if strings.Split(id, "-")[0] == "fail" { + return errors.New("err") + } + return nil + } + mockClient.On("DeviceOTA", mock.Anything, mock.Anything, mock.Anything).Return(mockDeviceOTA, nil) + + err := run(mockClient, []string{"dont-fail", "fail-1", "dont-fail", "fail-2"}, nil, 0) + if err == nil { + t.Error("should return error") + } + fmt.Println(err.Error()) + failed := strings.Split(err.Error(), ",") + if len(failed) != 2 { + fmt.Println(len(failed), failed) + t.Error("two updates should have failed") + } +} From 0f5c408533c2cde62ffaa4d9a44a578cb8dc161d Mon Sep 17 00:00:00 2001 From: Paolo Calao Date: Thu, 11 Nov 2021 11:43:20 +0100 Subject: [PATCH 02/14] Improve readability --- command/ota/upload.go | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/command/ota/upload.go b/command/ota/upload.go index 1017324f..d48574ab 100644 --- a/command/ota/upload.go +++ b/command/ota/upload.go @@ -116,37 +116,42 @@ func idsGivenTags(iotClient iot.Client, tags map[string]string) ([]string, error } func run(iotClient iot.Client, ids []string, file *os.File, expiration int) error { - idsToProcess := make(chan string, 2000) - idsFailed := make(chan string, 2000) + targets := make(chan string, len(ids)) + type result struct { + id string + err error + } + results := make(chan result, len(ids)) + for _, id := range ids { - idsToProcess <- id + targets <- id } - close(idsToProcess) + close(targets) for i := 0; i < numConcurrentUploads; i++ { go func() { - for id := range idsToProcess { + for id := range targets { err := iotClient.DeviceOTA(id, file, expiration) - fail := "" - if err != nil { - fail = id - } - idsFailed <- fail + results <- result{id: id, err: err} } }() } - failMsg := "" + var fails []string + var details []string for range ids { - i := <-idsFailed - if i != "" { - failMsg = strings.Join([]string{i, failMsg}, ",") + r := <-results + if r.err != nil { + fails = append(fails, r.id) + details = append(details, fmt.Sprintf("%s: %s", r.id, r.err.Error())) } } - if failMsg != "" { - failMsg = strings.TrimRight(failMsg, ",") - return fmt.Errorf("failed to update these devices: %s", failMsg) + if len(fails) > 0 { + f := strings.Join(fails, ",") + f = strings.TrimRight(f, ",") + d := strings.Join(details, "\n") + return fmt.Errorf("failed to update these devices: %s\nreasons:\n%s", f, d) } return nil } From e5c6d9e628cfdd5079faaa343dbdc121f6fd0727 Mon Sep 17 00:00:00 2001 From: Paolo Calao Date: Fri, 12 Nov 2021 16:56:43 +0100 Subject: [PATCH 03/14] Validate devices given the fqbn --- command/ota/upload.go | 33 ++++++++++++++++++++++++++++++++- command/ota/upload_test.go | 23 +++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/command/ota/upload.go b/command/ota/upload.go index d48574ab..b0b42704 100644 --- a/command/ota/upload.go +++ b/command/ota/upload.go @@ -115,7 +115,38 @@ func idsGivenTags(iotClient iot.Client, tags map[string]string) ([]string, error return devices, nil } -func run(iotClient iot.Client, ids []string, file *os.File, expiration int) error { +func validateDevices(iotClient iot.Client, ids []string, fqbn string) (valid, invalid, details []string, err error) { + devs, err := iotClient.DeviceList(nil) + if err != nil { + return nil, nil, nil, fmt.Errorf("%s: %w", "cannot retrieve devices from cloud", err) + } + + for _, id := range ids { + var found *iotclient.ArduinoDevicev2 + for _, d := range devs { + if d.Id == id { + found = &d + break + } + } + // Device not found on the cloud + if found == nil { + invalid = append(invalid, id) + details = append(details, fmt.Sprintf("%s : not found", id)) + continue + } + // Device FQBN doesn't match the passed one + if found.Fqbn != fqbn { + invalid = append(invalid, id) + details = append(details, fmt.Sprintf("%s : has FQBN `%s` instead of `%s`", found.Id, found.Fqbn, fqbn)) + continue + } + valid = append(valid, id) + } + return valid, invalid, details, nil +} + +func run(iotClient iot.Client, ids []string, file *os.File, expiration int) (updated, failed, errors []string) { targets := make(chan string, len(ids)) type result struct { id string diff --git a/command/ota/upload_test.go b/command/ota/upload_test.go index 823f7ca8..2995be47 100644 --- a/command/ota/upload_test.go +++ b/command/ota/upload_test.go @@ -33,4 +33,27 @@ func TestRun(t *testing.T) { fmt.Println(len(failed), failed) t.Error("two updates should have failed") } + if len(good) != 3 { + t.Error("two updates should have succeded") + } +} + +func TestValidateDevices(t *testing.T) { + mockClient := &mocks.Client{} + mockDeviceList := func(tags map[string]string) []iotclient.ArduinoDevicev2 { + return []iotclient.ArduinoDevicev2{ + {Id: "xxxx", Fqbn: "samd"}, + {Id: "yyyy", Fqbn: "samd"}, + {Id: "zzzz", Fqbn: "avr"}, + } + } + mockClient.On("DeviceList", mock.Anything).Return(mockDeviceList, nil) + + ids := []string{ + "xxxx", + "aaaa", + "zzzz", + } + v, i, d, err := validateDevices(mockClient, ids, "samd") + fmt.Println("valid: ", v, "inv: ", i, "det: ", d, "err: ", err) } From 475f6a758ea29b7a7eddbbbe2b34e4da69c73d6c Mon Sep 17 00:00:00 2001 From: Paolo Calao Date: Fri, 12 Nov 2021 17:01:26 +0100 Subject: [PATCH 04/14] Print feedbacks about successful, fail and invalid ota reqs --- cli/ota/upload.go | 21 +++++++++- command/ota/upload.go | 86 ++++++++++++++++++++++---------------- command/ota/upload_test.go | 8 ++-- 3 files changed, 73 insertions(+), 42 deletions(-) diff --git a/cli/ota/upload.go b/cli/ota/upload.go index 85f0eb0a..c554cfdb 100644 --- a/cli/ota/upload.go +++ b/cli/ota/upload.go @@ -18,7 +18,9 @@ package ota import ( + "fmt" "os" + "strings" "github.com/arduino/arduino-cli/cli/errorcodes" "github.com/arduino/arduino-cli/cli/feedback" @@ -70,11 +72,28 @@ func runUploadCommand(cmd *cobra.Command, args []string) { FQBN: uploadFlags.fqbn, } - err := ota.Upload(params) + resp, err := ota.Upload(params) if err != nil { feedback.Errorf("Error during ota upload: %v", err) os.Exit(errorcodes.ErrGeneric) } + devs := strings.Join(resp.Updated, ",") + devs = strings.TrimRight(devs, ",") + success := fmt.Sprintf("Successfully sent OTA request to: %s", devs) + + devs = strings.Join(resp.Invalid, ",") + devs = strings.TrimRight(devs, ",") + invalid := fmt.Sprintf("Cannot send OTA request to: %s", devs) + + devs = strings.Join(resp.Failed, ",") + devs = strings.TrimRight(devs, ",") + fail := fmt.Sprintf("Failed to send OTA request to: %s", devs) + + det := strings.Join(resp.Errors, "\n") + det = strings.TrimRight(det, ",") + details := fmt.Sprintf("\nDetails:\n%s", det) + + feedback.Printf(success, invalid, fail, details) logrus.Info("Upload successfully started") } diff --git a/command/ota/upload.go b/command/ota/upload.go index b0b42704..297158c4 100644 --- a/command/ota/upload.go +++ b/command/ota/upload.go @@ -23,10 +23,10 @@ import ( "io/ioutil" "os" "path/filepath" - "strings" "github.com/arduino/arduino-cloud-cli/internal/config" "github.com/arduino/arduino-cloud-cli/internal/iot" + iotclient "github.com/arduino/iot-client-go" ) const ( @@ -48,48 +48,61 @@ type UploadParams struct { FQBN string } +// UploadResp contains the results of the ota upload +type UploadResp struct { + Updated []string // Ids of devices updated + Invalid []string // Ids of device not valid (mismatched fqbn) + Failed []string // Ids of device failed + Errors []string // Contains detailed errors for each failure +} + // Upload command is used to upload a firmware OTA, // on a device of Arduino IoT Cloud. -func Upload(params *UploadParams) error { +func Upload(params *UploadParams) (*UploadResp, error) { if params.DeviceIDs == nil && params.Tags == nil { - return errors.New("provide either DeviceID or Tags") + return nil, errors.New("provide either DeviceIDs or Tags") } else if params.DeviceIDs != nil && params.Tags != nil { - return errors.New("cannot use both DeviceID and Tags. only one of them should be not nil") + return nil, errors.New("cannot use both DeviceIDs and Tags. only one of them should be not nil") } - conf, err := config.Retrieve() + // Generate .ota file + otaDir, err := ioutil.TempDir("", "") if err != nil { - return err + return nil, fmt.Errorf("%s: %w", "cannot create temporary folder", err) } - iotClient, err := iot.NewClient(conf.Client, conf.Secret) + otaFile := filepath.Join(otaDir, "temp.ota") + defer os.RemoveAll(otaDir) + + err = Generate(params.File, otaFile, params.FQBN) if err != nil { - return err + return nil, fmt.Errorf("%s: %w", "cannot generate .ota file", err) } - d, err := idsGivenTags(iotClient, params.Tags) + file, err := os.Open(otaFile) if err != nil { - return err - } - devs := append(params.DeviceIDs, d...) - if len(devs) == 0 { - return errors.New("no device found") + return nil, fmt.Errorf("%s: %w", "cannot open ota file", err) } - otaDir, err := ioutil.TempDir("", "") + conf, err := config.Retrieve() if err != nil { - return fmt.Errorf("%s: %w", "cannot create temporary folder", err) + return nil, err } - otaFile := filepath.Join(otaDir, "temp.ota") - defer os.RemoveAll(otaDir) - - err = Generate(params.File, otaFile, params.FQBN) + iotClient, err := iot.NewClient(conf.Client, conf.Secret) if err != nil { - return fmt.Errorf("%s: %w", "cannot generate .ota file", err) + return nil, err } - file, err := os.Open(otaFile) + d, err := idsGivenTags(iotClient, params.Tags) + if err != nil { + return nil, err + } + d = append(params.DeviceIDs, d...) + valid, invalid, details, err := validateDevices(iotClient, d, params.FQBN) if err != nil { - return fmt.Errorf("%s: %w", "cannot open ota file", err) + return nil, fmt.Errorf("failed to validate devices: %w", err) + } + if len(valid) == 0 { + return &UploadResp{Invalid: invalid}, nil } expiration := otaExpirationMins @@ -97,7 +110,15 @@ func Upload(params *UploadParams) error { expiration = otaDeferredExpirationMins } - return run(iotClient, devs, file, expiration) + good, fail, ers := run(iotClient, valid, file, expiration) + if err != nil { + return nil, err + } + + // Merge the failure details with the details of invalid devices + ers = append(details, ers...) + + return &UploadResp{Updated: good, Invalid: invalid, Failed: fail, Errors: ers}, nil } func idsGivenTags(iotClient iot.Client, tags map[string]string) ([]string, error) { @@ -168,21 +189,14 @@ func run(iotClient iot.Client, ids []string, file *os.File, expiration int) (upd }() } - var fails []string - var details []string for range ids { r := <-results if r.err != nil { - fails = append(fails, r.id) - details = append(details, fmt.Sprintf("%s: %s", r.id, r.err.Error())) + failed = append(failed, r.id) + errors = append(errors, fmt.Sprintf("%s: %s", r.id, r.err.Error())) + } else { + updated = append(updated, r.id) } } - - if len(fails) > 0 { - f := strings.Join(fails, ",") - f = strings.TrimRight(f, ",") - d := strings.Join(details, "\n") - return fmt.Errorf("failed to update these devices: %s\nreasons:\n%s", f, d) - } - return nil + return } diff --git a/command/ota/upload_test.go b/command/ota/upload_test.go index 2995be47..018bf51c 100644 --- a/command/ota/upload_test.go +++ b/command/ota/upload_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/arduino/arduino-cloud-cli/internal/iot/mocks" + iotclient "github.com/arduino/iot-client-go" "github.com/stretchr/testify/mock" ) @@ -23,14 +24,11 @@ func TestRun(t *testing.T) { } mockClient.On("DeviceOTA", mock.Anything, mock.Anything, mock.Anything).Return(mockDeviceOTA, nil) - err := run(mockClient, []string{"dont-fail", "fail-1", "dont-fail", "fail-2"}, nil, 0) + good, fail, err := run(mockClient, []string{"dont-fail", "fail-1", "dont-fail", "fail-2", "dont-fail"}, nil, 0) if err == nil { t.Error("should return error") } - fmt.Println(err.Error()) - failed := strings.Split(err.Error(), ",") - if len(failed) != 2 { - fmt.Println(len(failed), failed) + if len(fail) != 2 { t.Error("two updates should have failed") } if len(good) != 3 { From 8dba78f7d260ba561434b2cd6da18a49bb23d10b Mon Sep 17 00:00:00 2001 From: Paolo Calao Date: Fri, 12 Nov 2021 17:42:33 +0100 Subject: [PATCH 05/14] Improve output --- cli/ota/upload.go | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/cli/ota/upload.go b/cli/ota/upload.go index c554cfdb..72ae7655 100644 --- a/cli/ota/upload.go +++ b/cli/ota/upload.go @@ -18,7 +18,6 @@ package ota import ( - "fmt" "os" "strings" @@ -78,22 +77,18 @@ func runUploadCommand(cmd *cobra.Command, args []string) { os.Exit(errorcodes.ErrGeneric) } - devs := strings.Join(resp.Updated, ",") - devs = strings.TrimRight(devs, ",") - success := fmt.Sprintf("Successfully sent OTA request to: %s", devs) + success := strings.Join(resp.Updated, ",") + success = strings.TrimRight(success, ",") + feedback.Printf("\nSuccessfully sent OTA request to: %s", success) - devs = strings.Join(resp.Invalid, ",") - devs = strings.TrimRight(devs, ",") - invalid := fmt.Sprintf("Cannot send OTA request to: %s", devs) + invalid := strings.Join(resp.Invalid, ",") + invalid = strings.TrimRight(invalid, ",") + feedback.Printf("Cannot send OTA request to: %s", invalid) - devs = strings.Join(resp.Failed, ",") - devs = strings.TrimRight(devs, ",") - fail := fmt.Sprintf("Failed to send OTA request to: %s", devs) + fail := strings.Join(resp.Failed, ",") + fail = strings.TrimRight(fail, ",") + feedback.Printf("Failed to send OTA request to: %s", fail) det := strings.Join(resp.Errors, "\n") - det = strings.TrimRight(det, ",") - details := fmt.Sprintf("\nDetails:\n%s", det) - - feedback.Printf(success, invalid, fail, details) - logrus.Info("Upload successfully started") + feedback.Printf("\nDetails:\n%s", det) } From fc8fa40720330ad6cf5231f4490dd959c9a2c772 Mon Sep 17 00:00:00 2001 From: Paolo Calao Date: Fri, 12 Nov 2021 20:20:13 +0100 Subject: [PATCH 06/14] Open otaFile for each worker --- command/ota/upload.go | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/command/ota/upload.go b/command/ota/upload.go index 297158c4..d19de6c7 100644 --- a/command/ota/upload.go +++ b/command/ota/upload.go @@ -78,11 +78,6 @@ func Upload(params *UploadParams) (*UploadResp, error) { return nil, fmt.Errorf("%s: %w", "cannot generate .ota file", err) } - file, err := os.Open(otaFile) - if err != nil { - return nil, fmt.Errorf("%s: %w", "cannot open ota file", err) - } - conf, err := config.Retrieve() if err != nil { return nil, err @@ -110,7 +105,7 @@ func Upload(params *UploadParams) (*UploadResp, error) { expiration = otaDeferredExpirationMins } - good, fail, ers := run(iotClient, valid, file, expiration) + good, fail, ers := run(iotClient, valid, otaFile, expiration) if err != nil { return nil, err } @@ -167,8 +162,13 @@ func validateDevices(iotClient iot.Client, ids []string, fqbn string) (valid, in return valid, invalid, details, nil } -func run(iotClient iot.Client, ids []string, file *os.File, expiration int) (updated, failed, errors []string) { - targets := make(chan string, len(ids)) +func run(iotClient iot.Client, ids []string, otaFile string, expiration int) (updated, failed, errors []string) { + type job struct { + id string + file *os.File + } + jobs := make(chan job, len(ids)) + type result struct { id string err error @@ -176,15 +176,20 @@ func run(iotClient iot.Client, ids []string, file *os.File, expiration int) (upd results := make(chan result, len(ids)) for _, id := range ids { - targets <- id + file, err := os.Open(otaFile) + if err != nil { + failed = append(failed, id) + errors = append(errors, fmt.Sprintf("%s: cannot open ota file", id)) + } + jobs <- job{id: id, file: file} } - close(targets) + close(jobs) for i := 0; i < numConcurrentUploads; i++ { go func() { - for id := range targets { - err := iotClient.DeviceOTA(id, file, expiration) - results <- result{id: id, err: err} + for job := range jobs { + err := iotClient.DeviceOTA(job.id, job.file, expiration) + results <- result{id: job.id, err: err} } }() } From 798a597e077ebd327517ff67c1a311d60aab47f1 Mon Sep 17 00:00:00 2001 From: Paolo Calao Date: Mon, 15 Nov 2021 12:55:52 +0100 Subject: [PATCH 07/14] Introduce new ota mass-upload command --- cli/ota/massupload.go | 94 ++++++++ cli/ota/ota.go | 1 + cli/ota/upload.go | 48 +---- command/ota/massupload.go | 202 ++++++++++++++++++ .../{upload_test.go => massupload_test.go} | 0 command/ota/upload.go | 163 ++------------ 6 files changed, 331 insertions(+), 177 deletions(-) create mode 100644 cli/ota/massupload.go create mode 100644 command/ota/massupload.go rename command/ota/{upload_test.go => massupload_test.go} (100%) diff --git a/cli/ota/massupload.go b/cli/ota/massupload.go new file mode 100644 index 00000000..7bccd78c --- /dev/null +++ b/cli/ota/massupload.go @@ -0,0 +1,94 @@ +// This file is part of arduino-cloud-cli. +// +// Copyright (C) 2021 ARDUINO SA (http://www.arduino.cc/) +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published +// by the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package ota + +import ( + "os" + "strings" + + "github.com/arduino/arduino-cli/cli/errorcodes" + "github.com/arduino/arduino-cli/cli/feedback" + "github.com/arduino/arduino-cloud-cli/command/ota" + "github.com/sirupsen/logrus" + "github.com/spf13/cobra" +) + +var massUploadFlags struct { + deviceIDs []string + tags map[string]string + file string + deferred bool + fqbn string +} + +func initMassUploadCommand() *cobra.Command { + massUploadCommand := &cobra.Command{ + Use: "mass-upload", + Short: "Mass OTA upload", + Long: "Mass OTA upload on devices of Arduino IoT Cloud", + Run: runMassUploadCommand, + } + + massUploadCommand.Flags().StringSliceVarP(&massUploadFlags.deviceIDs, "device-ids", "d", nil, + "Comma-separated list of device IDs to update") + massUploadCommand.Flags().StringToStringVar(&massUploadFlags.tags, "tags", nil, + "Comma-separated list of tags with format =.\n"+ + "Perform and OTA upload on all devices that match the provided tags.\n"+ + "Mutually exclusive with `--device-id`.", + ) + massUploadCommand.Flags().StringVarP(&massUploadFlags.file, "file", "", "", "Binary file (.bin) to be uploaded") + massUploadCommand.Flags().BoolVar(&massUploadFlags.deferred, "deferred", false, "Perform a deferred OTA. It can take up to 1 week.") + massUploadCommand.Flags().StringVarP(&massUploadFlags.fqbn, "fqbn", "b", "", "FQBN of the devices to update") + + massUploadCommand.MarkFlagRequired("file") + massUploadCommand.MarkFlagRequired("fqbn") + return massUploadCommand +} + +func runMassUploadCommand(cmd *cobra.Command, args []string) { + logrus.Infof("Uploading binary %s", massUploadFlags.file) + + params := &ota.MassUploadParams{ + DeviceIDs: massUploadFlags.deviceIDs, + Tags: massUploadFlags.tags, + File: massUploadFlags.file, + Deferred: massUploadFlags.deferred, + FQBN: massUploadFlags.fqbn, + } + + resp, err := ota.MassUpload(params) + if err != nil { + feedback.Errorf("Error during ota upload: %v", err) + os.Exit(errorcodes.ErrGeneric) + } + + success := strings.Join(resp.Updated, ",") + success = strings.TrimRight(success, ",") + feedback.Printf("\nSuccessfully sent OTA request to: %s", success) + + invalid := strings.Join(resp.Invalid, ",") + invalid = strings.TrimRight(invalid, ",") + feedback.Printf("Cannot send OTA request to: %s", invalid) + + fail := strings.Join(resp.Failed, ",") + fail = strings.TrimRight(fail, ",") + feedback.Printf("Failed to send OTA request to: %s", fail) + + det := strings.Join(resp.Errors, "\n") + feedback.Printf("\nDetails:\n%s", det) +} diff --git a/cli/ota/ota.go b/cli/ota/ota.go index 0a34125c..757c8272 100644 --- a/cli/ota/ota.go +++ b/cli/ota/ota.go @@ -29,6 +29,7 @@ func NewCommand() *cobra.Command { } otaCommand.AddCommand(initUploadCommand()) + otaCommand.AddCommand(initMassUploadCommand()) return otaCommand } diff --git a/cli/ota/upload.go b/cli/ota/upload.go index 72ae7655..03e131ce 100644 --- a/cli/ota/upload.go +++ b/cli/ota/upload.go @@ -19,7 +19,6 @@ package ota import ( "os" - "strings" "github.com/arduino/arduino-cli/cli/errorcodes" "github.com/arduino/arduino-cli/cli/feedback" @@ -29,11 +28,9 @@ import ( ) var uploadFlags struct { - deviceIDs []string - tags map[string]string - file string - deferred bool - fqbn string + deviceID string + file string + deferred bool } func initUploadCommand() *cobra.Command { @@ -44,51 +41,28 @@ func initUploadCommand() *cobra.Command { Run: runUploadCommand, } - uploadCommand.Flags().StringSliceVarP(&uploadFlags.deviceIDs, "device-ids", "d", []string{}, - "Comma-separated list of device IDs to update") - uploadCommand.Flags().StringToStringVar(&uploadFlags.tags, "tags", nil, - "Comma-separated list of tags with format =.\n"+ - "Perform and OTA upload on all devices that match the provided tags.\n"+ - "Mutually exclusive with `--device-id`.", - ) + uploadCommand.Flags().StringVarP(&uploadFlags.deviceID, "device-id", "d", "", "Device ID") uploadCommand.Flags().StringVarP(&uploadFlags.file, "file", "", "", "Binary file (.bin) to be uploaded") uploadCommand.Flags().BoolVar(&uploadFlags.deferred, "deferred", false, "Perform a deferred OTA. It can take up to 1 week.") - uploadCommand.Flags().StringVarP(&uploadFlags.fqbn, "fqbn", "b", "", "FQBN of the devices to update") + uploadCommand.MarkFlagRequired("device-id") uploadCommand.MarkFlagRequired("file") - uploadCommand.MarkFlagRequired("fqbn") return uploadCommand } func runUploadCommand(cmd *cobra.Command, args []string) { - logrus.Infof("Uploading binary %s", uploadFlags.file) + logrus.Infof("Uploading binary %s to device %s", uploadFlags.file, uploadFlags.deviceID) params := &ota.UploadParams{ - DeviceIDs: uploadFlags.deviceIDs, - Tags: uploadFlags.tags, - File: uploadFlags.file, - Deferred: uploadFlags.deferred, - FQBN: uploadFlags.fqbn, + DeviceID: uploadFlags.deviceID, + File: uploadFlags.file, + Deferred: uploadFlags.deferred, } - - resp, err := ota.Upload(params) + err := ota.Upload(params) if err != nil { feedback.Errorf("Error during ota upload: %v", err) os.Exit(errorcodes.ErrGeneric) } - success := strings.Join(resp.Updated, ",") - success = strings.TrimRight(success, ",") - feedback.Printf("\nSuccessfully sent OTA request to: %s", success) - - invalid := strings.Join(resp.Invalid, ",") - invalid = strings.TrimRight(invalid, ",") - feedback.Printf("Cannot send OTA request to: %s", invalid) - - fail := strings.Join(resp.Failed, ",") - fail = strings.TrimRight(fail, ",") - feedback.Printf("Failed to send OTA request to: %s", fail) - - det := strings.Join(resp.Errors, "\n") - feedback.Printf("\nDetails:\n%s", det) + logrus.Info("Upload successfully started") } diff --git a/command/ota/massupload.go b/command/ota/massupload.go new file mode 100644 index 00000000..61e87dfe --- /dev/null +++ b/command/ota/massupload.go @@ -0,0 +1,202 @@ +// This file is part of arduino-cloud-cli. +// +// Copyright (C) 2021 ARDUINO SA (http://www.arduino.cc/) +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published +// by the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package ota + +import ( + "errors" + "fmt" + "io/ioutil" + "os" + "path/filepath" + + "github.com/arduino/arduino-cloud-cli/internal/config" + "github.com/arduino/arduino-cloud-cli/internal/iot" + iotclient "github.com/arduino/iot-client-go" +) + +const ( + numConcurrentUploads = 10 +) + +// MassUploadParams contains the parameters needed to +// perform a Mass OTA upload. +type MassUploadParams struct { + DeviceIDs []string + Tags map[string]string + File string + Deferred bool + FQBN string +} + +// MassUploadResp contains the results of the mass ota upload +type MassUploadResp struct { + Updated []string // Ids of devices updated + Invalid []string // Ids of device not valid (mismatched fqbn) + Failed []string // Ids of device failed + Errors []string // Contains detailed errors for each failure +} + +// MassUpload command is used to mass upload a firmware OTA, +// on devices of Arduino IoT Cloud. +func MassUpload(params *MassUploadParams) (*MassUploadResp, error) { + if params.DeviceIDs == nil && params.Tags == nil { + return nil, errors.New("provide either DeviceIDs or Tags") + } else if params.DeviceIDs != nil && params.Tags != nil { + return nil, errors.New("cannot use both DeviceIDs and Tags. only one of them should be not nil") + } + + // Generate .ota file + otaDir, err := ioutil.TempDir("", "") + if err != nil { + return nil, fmt.Errorf("%s: %w", "cannot create temporary folder", err) + } + otaFile := filepath.Join(otaDir, "temp.ota") + defer os.RemoveAll(otaDir) + + err = Generate(params.File, otaFile, params.FQBN) + if err != nil { + return nil, fmt.Errorf("%s: %w", "cannot generate .ota file", err) + } + + conf, err := config.Retrieve() + if err != nil { + return nil, err + } + iotClient, err := iot.NewClient(conf.Client, conf.Secret) + if err != nil { + return nil, err + } + + d, err := idsGivenTags(iotClient, params.Tags) + if err != nil { + return nil, err + } + d = append(params.DeviceIDs, d...) + valid, invalid, details, err := validateDevices(iotClient, d, params.FQBN) + if err != nil { + return nil, fmt.Errorf("failed to validate devices: %w", err) + } + if len(valid) == 0 { + return &MassUploadResp{Invalid: invalid}, nil + } + + expiration := otaExpirationMins + if params.Deferred { + expiration = otaDeferredExpirationMins + } + + good, fail, ers := run(iotClient, valid, otaFile, expiration) + if err != nil { + return nil, err + } + + // Merge the failure details with the details of invalid devices + ers = append(details, ers...) + + return &MassUploadResp{Updated: good, Invalid: invalid, Failed: fail, Errors: ers}, nil +} + +func idsGivenTags(iotClient iot.Client, tags map[string]string) ([]string, error) { + if tags == nil { + return nil, nil + } + devs, err := iotClient.DeviceList(tags) + if err != nil { + return nil, fmt.Errorf("%s: %w", "cannot retrieve devices from cloud", err) + } + devices := make([]string, 0, len(devs)) + for _, d := range devs { + devices = append(devices, d.Id) + } + return devices, nil +} + +func validateDevices(iotClient iot.Client, ids []string, fqbn string) (valid, invalid, details []string, err error) { + devs, err := iotClient.DeviceList(nil) + if err != nil { + return nil, nil, nil, fmt.Errorf("%s: %w", "cannot retrieve devices from cloud", err) + } + + for _, id := range ids { + var found *iotclient.ArduinoDevicev2 + for _, d := range devs { + if d.Id == id { + found = &d + break + } + } + // Device not found on the cloud + if found == nil { + invalid = append(invalid, id) + details = append(details, fmt.Sprintf("%s : not found", id)) + continue + } + // Device FQBN doesn't match the passed one + if found.Fqbn != fqbn { + invalid = append(invalid, id) + details = append(details, fmt.Sprintf("%s : has FQBN `%s` instead of `%s`", found.Id, found.Fqbn, fqbn)) + continue + } + valid = append(valid, id) + } + return valid, invalid, details, nil +} + +func run(iotClient iot.Client, ids []string, otaFile string, expiration int) (updated, failed, errors []string) { + type job struct { + id string + file *os.File + } + jobs := make(chan job, len(ids)) + + type result struct { + id string + err error + } + results := make(chan result, len(ids)) + + for _, id := range ids { + file, err := os.Open(otaFile) + if err != nil { + failed = append(failed, id) + errors = append(errors, fmt.Sprintf("%s: cannot open ota file", id)) + } + jobs <- job{id: id, file: file} + } + close(jobs) + + for i := 0; i < numConcurrentUploads; i++ { + go func() { + for job := range jobs { + err := iotClient.DeviceOTA(job.id, job.file, expiration) + results <- result{id: job.id, err: err} + } + }() + } + + for range ids { + r := <-results + if r.err != nil { + failed = append(failed, r.id) + errors = append(errors, fmt.Sprintf("%s: %s", r.id, r.err.Error())) + } else { + updated = append(updated, r.id) + } + } + return +} diff --git a/command/ota/upload_test.go b/command/ota/massupload_test.go similarity index 100% rename from command/ota/upload_test.go rename to command/ota/massupload_test.go diff --git a/command/ota/upload.go b/command/ota/upload.go index d19de6c7..f10f2ba9 100644 --- a/command/ota/upload.go +++ b/command/ota/upload.go @@ -18,7 +18,6 @@ package ota import ( - "errors" "fmt" "io/ioutil" "os" @@ -26,7 +25,6 @@ import ( "github.com/arduino/arduino-cloud-cli/internal/config" "github.com/arduino/arduino-cloud-cli/internal/iot" - iotclient "github.com/arduino/iot-client-go" ) const ( @@ -34,174 +32,59 @@ const ( otaExpirationMins = 10 // deferred ota can take up to 1 week (equal to 10080 minutes) otaDeferredExpirationMins = 10080 - - numConcurrentUploads = 10 ) // UploadParams contains the parameters needed to // perform an OTA upload. type UploadParams struct { - DeviceIDs []string - Tags map[string]string - File string - Deferred bool - FQBN string -} - -// UploadResp contains the results of the ota upload -type UploadResp struct { - Updated []string // Ids of devices updated - Invalid []string // Ids of device not valid (mismatched fqbn) - Failed []string // Ids of device failed - Errors []string // Contains detailed errors for each failure + DeviceID string + File string + Deferred bool } // Upload command is used to upload a firmware OTA, // on a device of Arduino IoT Cloud. -func Upload(params *UploadParams) (*UploadResp, error) { - if params.DeviceIDs == nil && params.Tags == nil { - return nil, errors.New("provide either DeviceIDs or Tags") - } else if params.DeviceIDs != nil && params.Tags != nil { - return nil, errors.New("cannot use both DeviceIDs and Tags. only one of them should be not nil") - } - - // Generate .ota file - otaDir, err := ioutil.TempDir("", "") - if err != nil { - return nil, fmt.Errorf("%s: %w", "cannot create temporary folder", err) - } - otaFile := filepath.Join(otaDir, "temp.ota") - defer os.RemoveAll(otaDir) - - err = Generate(params.File, otaFile, params.FQBN) - if err != nil { - return nil, fmt.Errorf("%s: %w", "cannot generate .ota file", err) - } - +func Upload(params *UploadParams) error { conf, err := config.Retrieve() if err != nil { - return nil, err + return err } iotClient, err := iot.NewClient(conf.Client, conf.Secret) if err != nil { - return nil, err + return err } - d, err := idsGivenTags(iotClient, params.Tags) + dev, err := iotClient.DeviceShow(params.DeviceID) if err != nil { - return nil, err - } - d = append(params.DeviceIDs, d...) - valid, invalid, details, err := validateDevices(iotClient, d, params.FQBN) - if err != nil { - return nil, fmt.Errorf("failed to validate devices: %w", err) - } - if len(valid) == 0 { - return &UploadResp{Invalid: invalid}, nil - } - - expiration := otaExpirationMins - if params.Deferred { - expiration = otaDeferredExpirationMins + return err } - good, fail, ers := run(iotClient, valid, otaFile, expiration) + otaDir, err := ioutil.TempDir("", "") if err != nil { - return nil, err + return fmt.Errorf("%s: %w", "cannot create temporary folder", err) } + otaFile := filepath.Join(otaDir, "temp.ota") + defer os.RemoveAll(otaDir) - // Merge the failure details with the details of invalid devices - ers = append(details, ers...) - - return &UploadResp{Updated: good, Invalid: invalid, Failed: fail, Errors: ers}, nil -} - -func idsGivenTags(iotClient iot.Client, tags map[string]string) ([]string, error) { - if tags == nil { - return nil, nil - } - devs, err := iotClient.DeviceList(tags) + err = Generate(params.File, otaFile, dev.Fqbn) if err != nil { - return nil, fmt.Errorf("%s: %w", "cannot retrieve devices from cloud", err) - } - devices := make([]string, 0, len(devs)) - for _, d := range devs { - devices = append(devices, d.Id) + return fmt.Errorf("%s: %w", "cannot generate .ota file", err) } - return devices, nil -} -func validateDevices(iotClient iot.Client, ids []string, fqbn string) (valid, invalid, details []string, err error) { - devs, err := iotClient.DeviceList(nil) + file, err := os.Open(otaFile) if err != nil { - return nil, nil, nil, fmt.Errorf("%s: %w", "cannot retrieve devices from cloud", err) + return fmt.Errorf("%s: %w", "cannot open ota file", err) } - for _, id := range ids { - var found *iotclient.ArduinoDevicev2 - for _, d := range devs { - if d.Id == id { - found = &d - break - } - } - // Device not found on the cloud - if found == nil { - invalid = append(invalid, id) - details = append(details, fmt.Sprintf("%s : not found", id)) - continue - } - // Device FQBN doesn't match the passed one - if found.Fqbn != fqbn { - invalid = append(invalid, id) - details = append(details, fmt.Sprintf("%s : has FQBN `%s` instead of `%s`", found.Id, found.Fqbn, fqbn)) - continue - } - valid = append(valid, id) - } - return valid, invalid, details, nil -} - -func run(iotClient iot.Client, ids []string, otaFile string, expiration int) (updated, failed, errors []string) { - type job struct { - id string - file *os.File - } - jobs := make(chan job, len(ids)) - - type result struct { - id string - err error - } - results := make(chan result, len(ids)) - - for _, id := range ids { - file, err := os.Open(otaFile) - if err != nil { - failed = append(failed, id) - errors = append(errors, fmt.Sprintf("%s: cannot open ota file", id)) - } - jobs <- job{id: id, file: file} + expiration := otaExpirationMins + if params.Deferred { + expiration = otaDeferredExpirationMins } - close(jobs) - for i := 0; i < numConcurrentUploads; i++ { - go func() { - for job := range jobs { - err := iotClient.DeviceOTA(job.id, job.file, expiration) - results <- result{id: job.id, err: err} - } - }() + err = iotClient.DeviceOTA(params.DeviceID, file, expiration) + if err != nil { + return err } - for range ids { - r := <-results - if r.err != nil { - failed = append(failed, r.id) - errors = append(errors, fmt.Sprintf("%s: %s", r.id, r.err.Error())) - } else { - updated = append(updated, r.id) - } - } - return + return nil } From 2d60932742efbf794f57ad9bf47a25091ec57ac4 Mon Sep 17 00:00:00 2001 From: Paolo Calao Date: Mon, 15 Nov 2021 13:35:27 +0100 Subject: [PATCH 08/14] Update go.sum --- go.sum | 6 ------ 1 file changed, 6 deletions(-) diff --git a/go.sum b/go.sum index 97016506..d79361d5 100644 --- a/go.sum +++ b/go.sum @@ -60,8 +60,6 @@ github.com/arduino/go-properties-orderedmap v1.3.0/go.mod h1:DKjD2VXY/NZmlingh4l github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b/go.mod h1:uwGy5PpN4lqW97FiLnbcx+xx8jly5YuPMJWfVwwjJiQ= github.com/arduino/go-win32-utils v0.0.0-20180330194947-ed041402e83b h1:3PjgYG5gVPA7cipp7vIR2lF96KkEJIFBJ+ANnuv6J20= github.com/arduino/go-win32-utils v0.0.0-20180330194947-ed041402e83b/go.mod h1:iIPnclBMYm1g32Q5kXoqng4jLhMStReIP7ZxaoUC2y8= -github.com/arduino/iot-client-go v1.3.4-0.20210930122852-04551f4cb061 h1:uQeaIHzj0tOlqnHaRskSy6UwMgQ6LIOECySpaYBCt5M= -github.com/arduino/iot-client-go v1.3.4-0.20210930122852-04551f4cb061/go.mod h1:gYvpMt7Qw+OSScTLyIlCnpbvy9y96ey/2zhB4w6FoK0= github.com/arduino/iot-client-go v1.3.4-0.20211103115604-d4d372164262 h1:qVq8cdkaRPaLc9DAjY/6rH3ocs6ZvnEJtD26f5++/RU= github.com/arduino/iot-client-go v1.3.4-0.20211103115604-d4d372164262/go.mod h1:gYvpMt7Qw+OSScTLyIlCnpbvy9y96ey/2zhB4w6FoK0= github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o= @@ -499,8 +497,6 @@ golang.org/x/net v0.0.0-20200707034311-ab3426394381/go.mod h1:/O7V0waA8r7cgGh81R golang.org/x/net v0.0.0-20200822124328-c89045814202/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4/go.mod h1:RBQZq4jEuRlivfhVLdyRGr576XBO4/greRjx4P4O3yc= -golang.org/x/net v0.0.0-20210813160813-60bc85c4be6d h1:LO7XpTYMwTqxjLcGWPijK3vRXg1aWdlNOVOHRq45d7c= -golang.org/x/net v0.0.0-20210813160813-60bc85c4be6d/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20211101193420-4a448f8816b3 h1:VrJZAjbekhoRn7n5FBujY31gboH+iB3pdLxn3gE9FjU= golang.org/x/net v0.0.0-20211101193420-4a448f8816b3/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -508,8 +504,6 @@ golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4Iltr golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20191202225959-858c2ad4c8b6/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= -golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f h1:Qmd2pbz05z7z6lm0DrgQVVPuBm92jqujBKMHMOlOQEw= -golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A= golang.org/x/oauth2 v0.0.0-20211028175245-ba495a64dcb5 h1:v79phzBz03tsVCUTbvTBmmC3CUXF5mKYt7DA4ZVldpM= golang.org/x/oauth2 v0.0.0-20211028175245-ba495a64dcb5/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= From 8d4cbd01080729dd1ebfc1558fa4516aa9d0ca2d Mon Sep 17 00:00:00 2001 From: Paolo Calao Date: Mon, 15 Nov 2021 15:47:37 +0100 Subject: [PATCH 09/14] Improve tests --- command/ota/massupload_test.go | 62 +++++++++++++++++++++++++--------- command/ota/testdata/empty.bin | 0 2 files changed, 46 insertions(+), 16 deletions(-) create mode 100644 command/ota/testdata/empty.bin diff --git a/command/ota/massupload_test.go b/command/ota/massupload_test.go index 018bf51c..880d2695 100644 --- a/command/ota/massupload_test.go +++ b/command/ota/massupload_test.go @@ -2,7 +2,6 @@ package ota import ( "errors" - "fmt" "os" "strings" "testing" @@ -13,45 +12,76 @@ import ( "github.com/stretchr/testify/mock" ) +const testFilename = "testdata/empty.bin" + func TestRun(t *testing.T) { + var ( + failID1 = "00000000-b39d-47a2-adf3-d26cdf474707" + failID2 = "00000000-9efd-4670-a478-df76ebdeeb4f" + okID1 = "11111111-4838-4f46-8930-d735c5b76cd1" + okID2 = "11111111-003f-42f9-a80c-85a1de36753b" + okID3 = "11111111-dac4-4a6a-80a4-698062fe2af5" + ) mockClient := &mocks.Client{} mockDeviceOTA := func(id string, file *os.File, expireMins int) error { - time.Sleep(3 * time.Second) - if strings.Split(id, "-")[0] == "fail" { + time.Sleep(100 * time.Millisecond) + if strings.Split(id, "-")[0] == "00000000" { return errors.New("err") } return nil } mockClient.On("DeviceOTA", mock.Anything, mock.Anything, mock.Anything).Return(mockDeviceOTA, nil) - good, fail, err := run(mockClient, []string{"dont-fail", "fail-1", "dont-fail", "fail-2", "dont-fail"}, nil, 0) - if err == nil { - t.Error("should return error") + good, fail, err := run(mockClient, []string{okID1, failID1, okID2, failID2, okID3}, testFilename, 0) + if len(err) != 2 { + t.Errorf("two errors should have been returned, got %d: %v", len(err), err) } if len(fail) != 2 { - t.Error("two updates should have failed") + t.Errorf("two updates should have failed, got %d: %v", len(fail), fail) } if len(good) != 3 { - t.Error("two updates should have succeded") + t.Errorf("two updates should have succeded, got %d: %v", len(good), good) } } func TestValidateDevices(t *testing.T) { + var ( + correctFQBN = "arduino:samd:nano_33_iot" + wrongFQBN = "arduino:samd:mkrwifi1010" + + idCorrect1 = "88d683a4-525e-423d-bad2-66a54d3585df" + idCorrect2 = "84b593fa-86dd-4954-904d-60f657158715" + idNotValid = "e3a3a667-a859-4317-be97-a61fb6f63487" + idNotFound = "deb17b7f-b39d-47a2-adf3-d26cdf474707" + ) + mockClient := &mocks.Client{} mockDeviceList := func(tags map[string]string) []iotclient.ArduinoDevicev2 { return []iotclient.ArduinoDevicev2{ - {Id: "xxxx", Fqbn: "samd"}, - {Id: "yyyy", Fqbn: "samd"}, - {Id: "zzzz", Fqbn: "avr"}, + {Id: idCorrect1, Fqbn: correctFQBN}, + {Id: idCorrect2, Fqbn: correctFQBN}, + {Id: idNotValid, Fqbn: wrongFQBN}, } } mockClient.On("DeviceList", mock.Anything).Return(mockDeviceList, nil) ids := []string{ - "xxxx", - "aaaa", - "zzzz", + idCorrect1, + idNotFound, + idCorrect2, + idNotValid, + } + v, i, d, _ := validateDevices(mockClient, ids, correctFQBN) + + if len(v) != 2 { + t.Errorf("expected 2 valid devices, but found %d: %v", len(v), v) + } + + if len(i) != 2 { + t.Errorf("expected 2 invalid devices, but found %d: %v", len(i), i) + } + + if len(d) != 2 { + t.Errorf("expected 2 error details, but found %d: %v", len(d), d) } - v, i, d, err := validateDevices(mockClient, ids, "samd") - fmt.Println("valid: ", v, "inv: ", i, "det: ", d, "err: ", err) } diff --git a/command/ota/testdata/empty.bin b/command/ota/testdata/empty.bin new file mode 100644 index 00000000..e69de29b From 479290bd7020b529c57078c950cf5800946d0201 Mon Sep 17 00:00:00 2001 From: Paolo Calao Date: Wed, 17 Nov 2021 18:54:24 +0100 Subject: [PATCH 10/14] Update cli/ota/massupload.go Co-authored-by: Giuseppe Lumia --- cli/ota/massupload.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/ota/massupload.go b/cli/ota/massupload.go index 7bccd78c..4157a22f 100644 --- a/cli/ota/massupload.go +++ b/cli/ota/massupload.go @@ -48,7 +48,7 @@ func initMassUploadCommand() *cobra.Command { "Comma-separated list of device IDs to update") massUploadCommand.Flags().StringToStringVar(&massUploadFlags.tags, "tags", nil, "Comma-separated list of tags with format =.\n"+ - "Perform and OTA upload on all devices that match the provided tags.\n"+ + "Perform an OTA upload on all devices that match the provided tags.\n"+ "Mutually exclusive with `--device-id`.", ) massUploadCommand.Flags().StringVarP(&massUploadFlags.file, "file", "", "", "Binary file (.bin) to be uploaded") From a397468cecea51916b70c22f7ed3fa1a551766d4 Mon Sep 17 00:00:00 2001 From: Paolo Calao Date: Wed, 17 Nov 2021 19:03:38 +0100 Subject: [PATCH 11/14] Update command/ota/massupload.go Co-authored-by: Giuseppe Lumia --- command/ota/massupload.go | 1 + 1 file changed, 1 insertion(+) diff --git a/command/ota/massupload.go b/command/ota/massupload.go index 61e87dfe..f588b449 100644 --- a/command/ota/massupload.go +++ b/command/ota/massupload.go @@ -175,6 +175,7 @@ func run(iotClient iot.Client, ids []string, otaFile string, expiration int) (up if err != nil { failed = append(failed, id) errors = append(errors, fmt.Sprintf("%s: cannot open ota file", id)) + continue } jobs <- job{id: id, file: file} } From de24be072f1d16ccb6b02f462778296c6f35f770 Mon Sep 17 00:00:00 2001 From: Paolo Calao Date: Wed, 17 Nov 2021 20:56:33 +0100 Subject: [PATCH 12/14] Update readme --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index c2422bb6..e10a57bf 100644 --- a/README.md +++ b/README.md @@ -156,6 +156,14 @@ The default OTA upload should complete in 10 minutes. Use `--deferred` flag to e `$ arduino-cloud-cli ota upload --device-id --file --deferred` +It is also possible to perform a mass ota upload through a specific command. +The fqbn is mandatory. +To select the devices to update you can either provide a list of device ids or device tags. + +`$ arduino-cloud-cli ota mass-upload --fqbn --device-ids --file ` + +`$ arduino-cloud-cli ota mass-upload --fqbn --device-tags =,= --file ` + ## Dashboard commands Print a list of available dashboards and their widgets by using this command: From 29d4861b7d6fdbb0803fb0f849ef3d0a1c17eeaf Mon Sep 17 00:00:00 2001 From: Paolo Calao Date: Wed, 17 Nov 2021 20:56:47 +0100 Subject: [PATCH 13/14] Fix flags --- cli/ota/massupload.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/ota/massupload.go b/cli/ota/massupload.go index 4157a22f..722c6f20 100644 --- a/cli/ota/massupload.go +++ b/cli/ota/massupload.go @@ -46,10 +46,10 @@ func initMassUploadCommand() *cobra.Command { massUploadCommand.Flags().StringSliceVarP(&massUploadFlags.deviceIDs, "device-ids", "d", nil, "Comma-separated list of device IDs to update") - massUploadCommand.Flags().StringToStringVar(&massUploadFlags.tags, "tags", nil, + massUploadCommand.Flags().StringToStringVar(&massUploadFlags.tags, "device-tags", nil, "Comma-separated list of tags with format =.\n"+ "Perform an OTA upload on all devices that match the provided tags.\n"+ - "Mutually exclusive with `--device-id`.", + "Mutually exclusive with `--device-ids`.", ) massUploadCommand.Flags().StringVarP(&massUploadFlags.file, "file", "", "", "Binary file (.bin) to be uploaded") massUploadCommand.Flags().BoolVar(&massUploadFlags.deferred, "deferred", false, "Perform a deferred OTA. It can take up to 1 week.") From ff4b638e9b5868af1e1d2f7de8a467b6287ca2b2 Mon Sep 17 00:00:00 2001 From: Paolo Calao Date: Wed, 17 Nov 2021 21:04:07 +0100 Subject: [PATCH 14/14] Refactor mass-ota results --- cli/ota/massupload.go | 59 +++++++++++++++++++++++------ command/ota/massupload.go | 69 +++++++++++++--------------------- command/ota/massupload_test.go | 46 ++++++++++++----------- 3 files changed, 100 insertions(+), 74 deletions(-) diff --git a/cli/ota/massupload.go b/cli/ota/massupload.go index 722c6f20..e6dc1fd0 100644 --- a/cli/ota/massupload.go +++ b/cli/ota/massupload.go @@ -19,10 +19,12 @@ package ota import ( "os" + "sort" "strings" "github.com/arduino/arduino-cli/cli/errorcodes" "github.com/arduino/arduino-cli/cli/feedback" + "github.com/arduino/arduino-cli/table" "github.com/arduino/arduino-cloud-cli/command/ota" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -77,18 +79,53 @@ func runMassUploadCommand(cmd *cobra.Command, args []string) { os.Exit(errorcodes.ErrGeneric) } - success := strings.Join(resp.Updated, ",") - success = strings.TrimRight(success, ",") - feedback.Printf("\nSuccessfully sent OTA request to: %s", success) + // Put successful devices ahead + sort.SliceStable(resp, func(i, j int) bool { + return resp[i].Err == nil + }) - invalid := strings.Join(resp.Invalid, ",") - invalid = strings.TrimRight(invalid, ",") - feedback.Printf("Cannot send OTA request to: %s", invalid) + feedback.PrintResult(massUploadResult{resp}) - fail := strings.Join(resp.Failed, ",") - fail = strings.TrimRight(fail, ",") - feedback.Printf("Failed to send OTA request to: %s", fail) + var failed []string + for _, r := range resp { + if r.Err != nil { + failed = append(failed, r.ID) + } + } + if len(failed) == 0 { + return + } + failStr := strings.Join(failed, ",") + feedback.Printf( + "You can try to perform the OTA again on the failed devices using the following command:\n"+ + "$ arduino-cloud-cli ota upload --file %s -d %s", params.File, failStr, + ) +} + +type massUploadResult struct { + res []ota.Result +} - det := strings.Join(resp.Errors, "\n") - feedback.Printf("\nDetails:\n%s", det) +func (r massUploadResult) Data() interface{} { + return r.res +} + +func (r massUploadResult) String() string { + if len(r.res) == 0 { + return "No OTA done." + } + t := table.New() + t.SetHeader("ID", "Result") + for _, r := range r.res { + outcome := "Success" + if r.Err != nil { + outcome = r.Err.Error() + } + + t.AddRow( + r.ID, + outcome, + ) + } + return t.Render() } diff --git a/command/ota/massupload.go b/command/ota/massupload.go index f588b449..85e2d539 100644 --- a/command/ota/massupload.go +++ b/command/ota/massupload.go @@ -43,17 +43,15 @@ type MassUploadParams struct { FQBN string } -// MassUploadResp contains the results of the mass ota upload -type MassUploadResp struct { - Updated []string // Ids of devices updated - Invalid []string // Ids of device not valid (mismatched fqbn) - Failed []string // Ids of device failed - Errors []string // Contains detailed errors for each failure +// Result of an ota upload on a device +type Result struct { + ID string + Err error } // MassUpload command is used to mass upload a firmware OTA, // on devices of Arduino IoT Cloud. -func MassUpload(params *MassUploadParams) (*MassUploadResp, error) { +func MassUpload(params *MassUploadParams) ([]Result, error) { if params.DeviceIDs == nil && params.Tags == nil { return nil, errors.New("provide either DeviceIDs or Tags") } else if params.DeviceIDs != nil && params.Tags != nil { @@ -82,17 +80,18 @@ func MassUpload(params *MassUploadParams) (*MassUploadResp, error) { return nil, err } + // Prepare the list of device-ids to update d, err := idsGivenTags(iotClient, params.Tags) if err != nil { return nil, err } d = append(params.DeviceIDs, d...) - valid, invalid, details, err := validateDevices(iotClient, d, params.FQBN) + valid, invalid, err := validateDevices(iotClient, d, params.FQBN) if err != nil { return nil, fmt.Errorf("failed to validate devices: %w", err) } if len(valid) == 0 { - return &MassUploadResp{Invalid: invalid}, nil + return invalid, nil } expiration := otaExpirationMins @@ -100,15 +99,9 @@ func MassUpload(params *MassUploadParams) (*MassUploadResp, error) { expiration = otaDeferredExpirationMins } - good, fail, ers := run(iotClient, valid, otaFile, expiration) - if err != nil { - return nil, err - } - - // Merge the failure details with the details of invalid devices - ers = append(details, ers...) - - return &MassUploadResp{Updated: good, Invalid: invalid, Failed: fail, Errors: ers}, nil + res := run(iotClient, valid, otaFile, expiration) + res = append(res, invalid...) + return res, nil } func idsGivenTags(iotClient iot.Client, tags map[string]string) ([]string, error) { @@ -126,10 +119,10 @@ func idsGivenTags(iotClient iot.Client, tags map[string]string) ([]string, error return devices, nil } -func validateDevices(iotClient iot.Client, ids []string, fqbn string) (valid, invalid, details []string, err error) { +func validateDevices(iotClient iot.Client, ids []string, fqbn string) (valid []string, invalid []Result, err error) { devs, err := iotClient.DeviceList(nil) if err != nil { - return nil, nil, nil, fmt.Errorf("%s: %w", "cannot retrieve devices from cloud", err) + return nil, nil, fmt.Errorf("%s: %w", "cannot retrieve devices from cloud", err) } for _, id := range ids { @@ -142,39 +135,36 @@ func validateDevices(iotClient iot.Client, ids []string, fqbn string) (valid, in } // Device not found on the cloud if found == nil { - invalid = append(invalid, id) - details = append(details, fmt.Sprintf("%s : not found", id)) + inv := Result{ID: id, Err: fmt.Errorf("not found")} + invalid = append(invalid, inv) continue } // Device FQBN doesn't match the passed one if found.Fqbn != fqbn { - invalid = append(invalid, id) - details = append(details, fmt.Sprintf("%s : has FQBN `%s` instead of `%s`", found.Id, found.Fqbn, fqbn)) + inv := Result{ID: id, Err: fmt.Errorf("has FQBN '%s' instead of '%s'", found.Fqbn, fqbn)} + invalid = append(invalid, inv) continue } valid = append(valid, id) } - return valid, invalid, details, nil + return valid, invalid, nil } -func run(iotClient iot.Client, ids []string, otaFile string, expiration int) (updated, failed, errors []string) { +func run(iotClient iot.Client, ids []string, otaFile string, expiration int) []Result { type job struct { id string file *os.File } jobs := make(chan job, len(ids)) - type result struct { - id string - err error - } - results := make(chan result, len(ids)) + resCh := make(chan Result, len(ids)) + results := make([]Result, 0, len(ids)) for _, id := range ids { file, err := os.Open(otaFile) if err != nil { - failed = append(failed, id) - errors = append(errors, fmt.Sprintf("%s: cannot open ota file", id)) + r := Result{ID: id, Err: fmt.Errorf("cannot open ota file")} + results = append(results, r) continue } jobs <- job{id: id, file: file} @@ -185,19 +175,14 @@ func run(iotClient iot.Client, ids []string, otaFile string, expiration int) (up go func() { for job := range jobs { err := iotClient.DeviceOTA(job.id, job.file, expiration) - results <- result{id: job.id, err: err} + resCh <- Result{ID: job.id, Err: err} } }() } for range ids { - r := <-results - if r.err != nil { - failed = append(failed, r.id) - errors = append(errors, fmt.Sprintf("%s: %s", r.id, r.err.Error())) - } else { - updated = append(updated, r.id) - } + r := <-resCh + results = append(results, r) } - return + return results } diff --git a/command/ota/massupload_test.go b/command/ota/massupload_test.go index 880d2695..4c8b6f73 100644 --- a/command/ota/massupload_test.go +++ b/command/ota/massupload_test.go @@ -5,7 +5,6 @@ import ( "os" "strings" "testing" - "time" "github.com/arduino/arduino-cloud-cli/internal/iot/mocks" iotclient "github.com/arduino/iot-client-go" @@ -16,31 +15,37 @@ const testFilename = "testdata/empty.bin" func TestRun(t *testing.T) { var ( - failID1 = "00000000-b39d-47a2-adf3-d26cdf474707" - failID2 = "00000000-9efd-4670-a478-df76ebdeeb4f" - okID1 = "11111111-4838-4f46-8930-d735c5b76cd1" - okID2 = "11111111-003f-42f9-a80c-85a1de36753b" - okID3 = "11111111-dac4-4a6a-80a4-698062fe2af5" + failPrefix = "00000000" + failID1 = failPrefix + "-b39d-47a2-adf3-d26cdf474707" + failID2 = failPrefix + "-9efd-4670-a478-df76ebdeeb4f" + okPrefix = "11111111" + okID1 = okPrefix + "-4838-4f46-8930-d735c5b76cd1" + okID2 = okPrefix + "-003f-42f9-a80c-85a1de36753b" + okID3 = okPrefix + "-dac4-4a6a-80a4-698062fe2af5" ) mockClient := &mocks.Client{} mockDeviceOTA := func(id string, file *os.File, expireMins int) error { - time.Sleep(100 * time.Millisecond) - if strings.Split(id, "-")[0] == "00000000" { + if strings.Split(id, "-")[0] == failPrefix { return errors.New("err") } return nil } mockClient.On("DeviceOTA", mock.Anything, mock.Anything, mock.Anything).Return(mockDeviceOTA, nil) - good, fail, err := run(mockClient, []string{okID1, failID1, okID2, failID2, okID3}, testFilename, 0) - if len(err) != 2 { - t.Errorf("two errors should have been returned, got %d: %v", len(err), err) + devs := []string{okID1, failID1, okID2, failID2, okID3} + res := run(mockClient, devs, testFilename, 0) + if len(res) != len(devs) { + t.Errorf("expected %d results, got %d", len(devs), len(res)) } - if len(fail) != 2 { - t.Errorf("two updates should have failed, got %d: %v", len(fail), fail) - } - if len(good) != 3 { - t.Errorf("two updates should have succeded, got %d: %v", len(good), good) + + for _, r := range res { + pre := strings.Split(r.ID, "-")[0] + if pre == okPrefix && r.Err != nil { + t.Errorf("device %s expected to succeed, but got error %s", r.ID, r.Err.Error()) + } + if pre == failPrefix && r.Err == nil { + t.Errorf("device %s expected to fail, but got no error", r.ID) + } } } @@ -71,7 +76,10 @@ func TestValidateDevices(t *testing.T) { idCorrect2, idNotValid, } - v, i, d, _ := validateDevices(mockClient, ids, correctFQBN) + v, i, err := validateDevices(mockClient, ids, correctFQBN) + if err != nil { + t.Errorf("unexpected error: %s", err.Error()) + } if len(v) != 2 { t.Errorf("expected 2 valid devices, but found %d: %v", len(v), v) @@ -80,8 +88,4 @@ func TestValidateDevices(t *testing.T) { if len(i) != 2 { t.Errorf("expected 2 invalid devices, but found %d: %v", len(i), i) } - - if len(d) != 2 { - t.Errorf("expected 2 error details, but found %d: %v", len(d), d) - } }