Use reverse SSHFS instead of scp 49/182249/2
authorAleksander Mistewicz <a.mistewicz@samsung.com>
Thu, 21 Jun 2018 13:33:24 +0000 (15:33 +0200)
committerAleksander Mistewicz <a.mistewicz@samsung.com>
Thu, 21 Jun 2018 14:17:44 +0000 (16:17 +0200)
It solves the following problems:
 * some files did not fit into tmpfs or root filesystem,
 * some files could not be copied at all due to their contents,
 * paths needed to be translated.

Change-Id: I581ad3ddf22bf2e3e8aec0d2952f34f76ab90a41
Signed-off-by: Aleksander Mistewicz <a.mistewicz@samsung.com>
manager/dryad/device_communication_provider.go
manager/dryad/device_communication_provider_test.go
manager/dryad/dryad.go
manager/dryad/dryad_session_provider.go
manager/dryad/dryad_session_provider_test.go
manager/dryad/mock_session_provider_test.go
manager/dryad_job.go
manager/mock_dryad_test.go

index 4eb8d62..3fca5bf 100644 (file)
@@ -17,8 +17,7 @@
 package dryad
 
 import (
-       "path/filepath"
-       "strings"
+       "fmt"
 )
 
 // prefixPath is a parent directory DUT scripts.
@@ -48,22 +47,10 @@ func (d *deviceCommunicationProvider) Login(credentials Credentials) error {
 // CopyFilesTo function is a part of DeviceCommunicationProvider interface.
 // It uses tmpfs of MuxPi so caller must take into consideration size of all files that are to be copied.
 func (d *deviceCommunicationProvider) CopyFilesTo(src []string, dest string) error {
-       if !strings.HasSuffix(dest, "/") {
-               dest += "/"
-       }
-
        for _, path := range src {
-               fileName := filepath.Base(path)
-               tmpDst := "/tmp/weles_cft_" + fileName
-
-               err := d.sessionProvider.SendFile(path, tmpDst)
-               if err != nil {
-                       return err
-               }
-
-               _, _, err = d.sessionProvider.Exec(prefixPath+"dut_copyto.sh", tmpDst, dest+fileName)
+               _, _, err := d.sessionProvider.Exec(prefixPath+"dut_copyto.sh", path, dest)
                if err != nil {
-                       return err
+                       return fmt.Errorf("failed to copy %s to %s: %v", path, dest, err)
                }
        }
        return nil
@@ -73,17 +60,9 @@ func (d *deviceCommunicationProvider) CopyFilesTo(src []string, dest string) err
 // It uses tmpfs of MuxPi so caller must take into consideration size of all files that are to be copied.
 func (d *deviceCommunicationProvider) CopyFilesFrom(src []string, dest string) error {
        for _, path := range src {
-               fileName := filepath.Base(path)
-               tmpDst := "/tmp/weles_cff_" + fileName
-
-               _, _, err := d.sessionProvider.Exec(prefixPath+"dut_copyfrom.sh", path, tmpDst)
-               if err != nil {
-                       return err
-               }
-
-               err = d.sessionProvider.ReceiveFile(tmpDst, dest+fileName)
+               _, _, err := d.sessionProvider.Exec(prefixPath+"dut_copyfrom.sh", path, dest)
                if err != nil {
-                       return err
+                       return fmt.Errorf("failed to copy %s to %s: %v", path, dest, err)
                }
        }
        return nil
index df3b5aa..046b695 100644 (file)
@@ -59,5 +59,32 @@ var _ = Describe("DeviceCommunicationProvider", func() {
                Expect(stderr).To(BeEmpty())
        })
 
+       It("should transfer files to and from DUT", func() {
+               file1 := "a"
+               file2 := "b"
+               file3 := "c"
+               files := []string{file1, file2, file3}
+               target := "/tmp/dl"
+               gomock.InOrder(
+                       mockSession.EXPECT().Exec("/usr/local/bin/dut_copyto.sh", file1, target),
+                       mockSession.EXPECT().Exec("/usr/local/bin/dut_copyto.sh", file2, target),
+                       mockSession.EXPECT().Exec("/usr/local/bin/dut_copyto.sh", file3, target),
+               )
+
+               By("Sending files to DUT")
+               err := dcp.CopyFilesTo(files, target)
+               Expect(err).ToNot(HaveOccurred())
+
+               gomock.InOrder(
+                       mockSession.EXPECT().Exec("/usr/local/bin/dut_copyfrom.sh", file1, target),
+                       mockSession.EXPECT().Exec("/usr/local/bin/dut_copyfrom.sh", file2, target),
+                       mockSession.EXPECT().Exec("/usr/local/bin/dut_copyfrom.sh", file3, target),
+               )
+
+               By("Receiving files from DUT")
+               err = dcp.CopyFilesFrom(files, target)
+               Expect(err).ToNot(HaveOccurred())
+       })
+
        //TODO: Test error paths.
 })
index 44a0835..ad69f1b 100644 (file)
@@ -43,12 +43,6 @@ type SessionProvider interface {
 
        // Close terminates session to Dryad.
        Close() error
-
-       // SendFile sends file to Dryad.
-       SendFile(src, dst string) error
-
-       // ReceiveFile receives file from Dryad.
-       ReceiveFile(src, dst string) error
 }
 
 // Credentials are used to login to device.
index 068057f..f0fe99a 100644 (file)
@@ -18,10 +18,8 @@ package dryad
 
 import (
        "bytes"
+       "context"
        "fmt"
-       "io"
-       "os"
-       "path/filepath"
        "strings"
        "time"
 
@@ -46,6 +44,7 @@ type sessionProvider struct {
        SessionProvider
        dryad      Dryad
        connection *sshClient
+       sshfs      *reverseSSHFS
 }
 
 func prepareSSHConfig(userName string, key rsa.PrivateKey) *ssh.ClientConfig {
@@ -63,7 +62,14 @@ func prepareSSHConfig(userName string, key rsa.PrivateKey) *ssh.ClientConfig {
 
 func (d *sessionProvider) connect() (err error) {
        d.connection.client, err = ssh.Dial("tcp", d.dryad.Addr.String(), d.connection.config)
-       return
+       if err != nil {
+               return err
+       }
+       session, err := d.connection.client.NewSession()
+       if err != nil {
+               return err
+       }
+       return d.sshfs.open(session)
 }
 
 func (d *sessionProvider) newSession() (*ssh.Session, error) {
@@ -98,7 +104,7 @@ func (d *sessionProvider) executeRemoteCommand(cmd string) ([]byte, []byte, erro
 }
 
 // NewSessionProvider returns new instance of SessionProvider.
-func NewSessionProvider(dryad Dryad) SessionProvider {
+func NewSessionProvider(dryad Dryad, workdir string) SessionProvider {
        cfg := prepareSSHConfig(dryad.Username, dryad.Key)
 
        return &sessionProvider{
@@ -106,12 +112,24 @@ func NewSessionProvider(dryad Dryad) SessionProvider {
                connection: &sshClient{
                        config: cfg,
                },
+               sshfs: newReverseSSHFS(context.Background(), workdir, workdir),
        }
 }
 
 // Exec is a part of SessionProvider interface.
 // cmd parameter is used as is. Quotations should be added by the user as needed.
 func (d *sessionProvider) Exec(cmd ...string) ([]byte, []byte, error) {
+       session, err := d.newSession()
+       if err != nil {
+               return nil, nil, err
+       }
+       defer session.Close()
+
+       err = d.sshfs.check(session)
+       if err != nil {
+               return nil, nil, err
+       }
+
        return d.executeRemoteCommand(strings.Join(cmd, " "))
 }
 
@@ -151,104 +169,9 @@ func (d *sessionProvider) Close() error {
                return nil
        }
 
+       d.sshfs.close()
+       //TODO: log error.
        err := d.connection.client.Close()
        d.connection.client = nil
        return err
 }
-
-// SendFile is a part of SessionProvider interface.
-func (d *sessionProvider) SendFile(src, dst string) error {
-       f, err := os.Open(src)
-       if err != nil {
-               return err
-       }
-       defer f.Close()
-
-       s, err := f.Stat()
-       if err != nil {
-               return err
-       }
-
-       session, err := d.newSession()
-       if err != nil {
-               return err
-       }
-       defer session.Close()
-
-       filename := filepath.Base(dst)
-       directory := filepath.Dir(dst)
-
-       w, err := session.StdinPipe()
-       if err != nil {
-               return err
-       }
-       defer w.Close()
-
-       var stdout, stderr bytes.Buffer
-       session.Stdout = &stdout
-       session.Stderr = &stderr
-
-       // Trigger SCP sink mode
-       err = session.Start("scp -t " + directory)
-       if err != nil {
-               return err
-       }
-
-       _, err = fmt.Fprintln(w, "C0755", s.Size(), filename)
-       if err != nil {
-               return err
-       }
-
-       _, err = io.Copy(w, f)
-       if err != nil {
-               return err
-       }
-
-       _, err = fmt.Fprintln(w, "\x00")
-       if err != nil {
-               return err
-       }
-
-       err = session.Wait()
-
-       // FIXME: unexpected <newline> is reported by scp every time the transfer is finished properly. Needs to be solved.
-       // Bellow we have a very lousy trick. I hope it will be fixed in the future.
-       // I don't know what is the reason or how to fix it. Has to wait a little bit. Or maybe someone else will find the solution.
-       // First candidate sshfs -o slave
-       if strings.Contains(stdout.String(), "unexpected <newline>") {
-               return nil
-       }
-       return err
-}
-
-// ReceiveFile is a part of SessionProvider interface.
-func (d *sessionProvider) ReceiveFile(src, dst string) error {
-       session, err := d.newSession()
-       if err != nil {
-               return err
-       }
-       defer session.Close()
-
-       r, err := session.StdoutPipe()
-       if err != nil {
-               return err
-       }
-
-       file, err := os.Create(dst)
-       if err != nil {
-               return err
-       }
-       defer file.Close()
-
-       err = session.Start("scp " + src + " /dev/stdout")
-       if err != nil {
-               return err
-       }
-
-       _, err = io.Copy(file, r)
-       if err != nil {
-               return err
-       }
-
-       return session.Wait()
-}
index f00d258..e669d7e 100644 (file)
@@ -17,6 +17,8 @@
 package dryad
 
 import (
+       "io/ioutil"
+       "os"
        "strings"
 
        . "github.com/onsi/ginkgo"
@@ -37,17 +39,27 @@ And the gasses it makes!`
 )
 
 var _ = Describe("SessionProvider", func() {
-       var sp SessionProvider
+       var (
+               sp      SessionProvider
+               testDir string
+       )
 
        BeforeEach(func() {
                if !accessInfoGiven {
                        Skip("No valid access info to Dryad")
                }
-               sp = NewSessionProvider(dryadInfo)
+               var err error
+               testDir, err = ioutil.TempDir("", "test")
+               Expect(err).ToNot(HaveOccurred())
+
+               sp = NewSessionProvider(dryadInfo, testDir)
        })
 
        AfterEach(func() {
                sp.Close()
+
+               err := os.RemoveAll(testDir)
+               Expect(err).ToNot(HaveOccurred())
        })
 
        It("should write poem to a file and read from it", func() {
@@ -65,6 +77,20 @@ var _ = Describe("SessionProvider", func() {
                Expect(err).ToNot(HaveOccurred())
        })
 
+       It("should read local file from remote", func() {
+               content := []byte("test file contents")
+               tmpfile, err := ioutil.TempFile(testDir, "testfile")
+               Expect(err).ToNot(HaveOccurred())
+               _, err = tmpfile.Write(content)
+               Expect(err).ToNot(HaveOccurred())
+               tmpfile.Close()
+
+               stdout, stderr, err := sp.Exec("cat", tmpfile.Name())
+               Expect(err).ToNot(HaveOccurred())
+               Expect(stdout).To(Equal(content))
+               Expect(stderr).To(BeEmpty())
+       })
+
        It("should not read poem from nonexistent file", func() {
                stdout, stderr, err := sp.Exec("cat", "/Ihopethispathdoesnotexist/"+flyingCowsPath+".txt")
                Expect(err).To(HaveOccurred())
@@ -83,24 +109,4 @@ var _ = Describe("SessionProvider", func() {
        It("should switch to TS", func() {
                Expect(sp.TS()).ToNot(HaveOccurred())
        })
-
-       It("should transfer file to Dryad", func() {
-               err := sp.SendFile(keyFile, "/tmp/"+keyFile)
-               Expect(err).ToNot(HaveOccurred())
-       })
-
-       It("should not transfer file to Dryad - insufficient permissions", func() {
-               err := sp.SendFile(keyFile, "/root/"+keyFile)
-               Expect(err).To(HaveOccurred())
-       })
-
-       It("should transfer file from Dryad", func() {
-               err := sp.ReceiveFile("/tmp/"+keyFile, "/tmp/dl-"+keyFile)
-               Expect(err).ToNot(HaveOccurred())
-       })
-
-       It("should not transfer nonexistent file from Dryad", func() {
-               err := sp.ReceiveFile("/tmp/"+keyFile+"noway", "/tmp/dl-"+keyFile)
-               Expect(err).To(HaveOccurred())
-       })
 })
index 34337e3..affc348 100644 (file)
@@ -86,30 +86,6 @@ func (mr *MockSessionProviderMockRecorder) PowerTick() *gomock.Call {
        return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PowerTick", reflect.TypeOf((*MockSessionProvider)(nil).PowerTick))
 }
 
-// ReceiveFile mocks base method
-func (m *MockSessionProvider) ReceiveFile(arg0, arg1 string) error {
-       ret := m.ctrl.Call(m, "ReceiveFile", arg0, arg1)
-       ret0, _ := ret[0].(error)
-       return ret0
-}
-
-// ReceiveFile indicates an expected call of ReceiveFile
-func (mr *MockSessionProviderMockRecorder) ReceiveFile(arg0, arg1 interface{}) *gomock.Call {
-       return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReceiveFile", reflect.TypeOf((*MockSessionProvider)(nil).ReceiveFile), arg0, arg1)
-}
-
-// SendFile mocks base method
-func (m *MockSessionProvider) SendFile(arg0, arg1 string) error {
-       ret := m.ctrl.Call(m, "SendFile", arg0, arg1)
-       ret0, _ := ret[0].(error)
-       return ret0
-}
-
-// SendFile indicates an expected call of SendFile
-func (mr *MockSessionProviderMockRecorder) SendFile(arg0, arg1 interface{}) *gomock.Call {
-       return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendFile", reflect.TypeOf((*MockSessionProvider)(nil).SendFile), arg0, arg1)
-}
-
 // TS mocks base method
 func (m *MockSessionProvider) TS() error {
        ret := m.ctrl.Call(m, "TS")
index 21cc5d1..e5ac2c9 100644 (file)
@@ -1,5 +1,5 @@
 /*
- *  Copyright (c) 2017 Samsung Electronics Co., Ltd All Rights Reserved
+ *  Copyright (c) 2017-2018 Samsung Electronics Co., Ltd All Rights Reserved
  *
  *  Licensed under the Apache License, Version 2.0 (the "License");
  *  you may not use this file except in compliance with the License.
@@ -55,7 +55,8 @@ func newDryadJobWithCancel(job JobID, changes chan<- DryadJobStatusChange,
 // newDryadJob creates an instance of dryadJob and starts a goroutine
 // executing phases of given job implemented by provider of DryadJobRunner interface.
 func newDryadJob(job JobID, rusalka Dryad, changes chan<- DryadJobStatusChange) *dryadJob {
-       session := dryad.NewSessionProvider(rusalka)
+       // FIXME: It should use the proper path to the artifactory.
+       session := dryad.NewSessionProvider(rusalka, "")
        device := dryad.NewDeviceCommunicationProvider(session)
 
        ctx, cancel := context.WithCancel(context.Background())
index aa888a2..9fae5b5 100644 (file)
@@ -87,30 +87,6 @@ func (mr *MockSessionProviderMockRecorder) PowerTick() *gomock.Call {
        return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PowerTick", reflect.TypeOf((*MockSessionProvider)(nil).PowerTick))
 }
 
-// ReceiveFile mocks base method
-func (m *MockSessionProvider) ReceiveFile(arg0, arg1 string) error {
-       ret := m.ctrl.Call(m, "ReceiveFile", arg0, arg1)
-       ret0, _ := ret[0].(error)
-       return ret0
-}
-
-// ReceiveFile indicates an expected call of ReceiveFile
-func (mr *MockSessionProviderMockRecorder) ReceiveFile(arg0, arg1 interface{}) *gomock.Call {
-       return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReceiveFile", reflect.TypeOf((*MockSessionProvider)(nil).ReceiveFile), arg0, arg1)
-}
-
-// SendFile mocks base method
-func (m *MockSessionProvider) SendFile(arg0, arg1 string) error {
-       ret := m.ctrl.Call(m, "SendFile", arg0, arg1)
-       ret0, _ := ret[0].(error)
-       return ret0
-}
-
-// SendFile indicates an expected call of SendFile
-func (mr *MockSessionProviderMockRecorder) SendFile(arg0, arg1 interface{}) *gomock.Call {
-       return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendFile", reflect.TypeOf((*MockSessionProvider)(nil).SendFile), arg0, arg1)
-}
-
 // TS mocks base method
 func (m *MockSessionProvider) TS() error {
        ret := m.ctrl.Call(m, "TS")