17170: Merge branch 'master'

[arvados.git] / lib / crunchrun / crunchrun.go

diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go
index b252e0dce162fca3473dc0a0c64a9f846295e4b9..7d6fb4ed47bef547f4eb3cb1728163c77021bf02 100644 (file)
--- a/lib/crunchrun/crunchrun.go
+++ b/lib/crunchrun/crunchrun.go
@@ -33,7 +33,6 @@ import (
        "git.arvados.org/arvados.git/sdk/go/arvadosclient"
        "git.arvados.org/arvados.git/sdk/go/keepclient"
        "git.arvados.org/arvados.git/sdk/go/manifest"
-       "golang.org/x/crypto/ssh"
        "golang.org/x/net/context"
 
        dockertypes "github.com/docker/docker/api/types"
@@ -180,9 +179,7 @@ type ContainerRunner struct {
 
        containerWatchdogInterval time.Duration
 
-       gatewayAddress    string
-       gatewaySSHConfig  *ssh.ServerConfig
-       gatewayAuthSecret string
+       gateway Gateway
 }
 
 // setupSignals sets up signal handling to gracefully terminate the underlying
@@ -626,7 +623,7 @@ func (runner *ContainerRunner) SetupMounts() (err error) {
                return fmt.Errorf("output path does not correspond to a writable mount point")
        }
 
-       if wantAPI := runner.Container.RuntimeConstraints.API; needCertMount && wantAPI != nil && *wantAPI {
+       if needCertMount && runner.Container.RuntimeConstraints.API {
                for _, certfile := range arvadosclient.CertFiles {
                        _, err := os.Stat(certfile)
                        if err == nil {
@@ -1097,7 +1094,7 @@ func (runner *ContainerRunner) CreateContainer() error {
                },
        }
 
-       if wantAPI := runner.Container.RuntimeConstraints.API; wantAPI != nil && *wantAPI {
+       if runner.Container.RuntimeConstraints.API {
                tok, err := runner.ContainerToken()
                if err != nil {
                        return err
@@ -1274,7 +1271,7 @@ func (runner *ContainerRunner) updateLogs() {
 // CaptureOutput saves data from the container's output directory if
 // needed, and updates the container output accordingly.
 func (runner *ContainerRunner) CaptureOutput() error {
-       if wantAPI := runner.Container.RuntimeConstraints.API; wantAPI != nil && *wantAPI {
+       if runner.Container.RuntimeConstraints.API {
                // Output may have been set directly by the container, so
                // refresh the container record to check.
                err := runner.DispatcherArvClient.Get("containers", runner.Container.UUID,
@@ -1436,15 +1433,20 @@ func (runner *ContainerRunner) saveLogCollection(final bool) (response arvados.C
                // Already finalized.
                return
        }
-       mt, err := runner.LogCollection.MarshalManifest(".")
-       if err != nil {
-               err = fmt.Errorf("error creating log manifest: %v", err)
-               return
-       }
        updates := arvadosclient.Dict{
-               "name":          "logs for " + runner.Container.UUID,
-               "manifest_text": mt,
+               "name": "logs for " + runner.Container.UUID,
        }
+       mt, err1 := runner.LogCollection.MarshalManifest(".")
+       if err1 == nil {
+               // Only send updated manifest text if there was no
+               // error.
+               updates["manifest_text"] = mt
+       }
+
+       // Even if flushing the manifest had an error, we still want
+       // to update the log record, if possible, to push the trash_at
+       // and delete_at times into the future.  Details on bug
+       // #17293.
        if final {
                updates["is_trashed"] = true
        } else {
@@ -1453,16 +1455,20 @@ func (runner *ContainerRunner) saveLogCollection(final bool) (response arvados.C
                updates["delete_at"] = exp
        }
        reqBody := arvadosclient.Dict{"collection": updates}
+       var err2 error
        if runner.logUUID == "" {
                reqBody["ensure_unique_name"] = true
-               err = runner.DispatcherArvClient.Create("collections", reqBody, &response)
+               err2 = runner.DispatcherArvClient.Create("collections", reqBody, &response)
        } else {
-               err = runner.DispatcherArvClient.Update("collections", runner.logUUID, reqBody, &response)
+               err2 = runner.DispatcherArvClient.Update("collections", runner.logUUID, reqBody, &response)
        }
-       if err != nil {
-               return
+       if err2 == nil {
+               runner.logUUID = response.UUID
+       }
+
+       if err1 != nil || err2 != nil {
+               err = fmt.Errorf("error recording logs: %q, %q", err1, err2)
        }
-       runner.logUUID = response.UUID
        return
 }
 
@@ -1474,7 +1480,7 @@ func (runner *ContainerRunner) UpdateContainerRunning() error {
                return ErrCancelled
        }
        return runner.DispatcherArvClient.Update("containers", runner.Container.UUID,
-               arvadosclient.Dict{"container": arvadosclient.Dict{"state": "Running", "gateway_address": runner.gatewayAddress}}, nil)
+               arvadosclient.Dict{"container": arvadosclient.Dict{"state": "Running", "gateway_address": runner.gateway.Address}}, nil)
 }
 
 // ContainerToken returns the api_token the container (and any
@@ -1873,9 +1879,15 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s
                return 1
        }
 
-       cr.gatewayAuthSecret = os.Getenv("GatewayAuthSecret")
+       cr.gateway = Gateway{
+               Address:           os.Getenv("GatewayAddress"),
+               AuthSecret:        os.Getenv("GatewayAuthSecret"),
+               ContainerUUID:     containerID,
+               DockerContainerID: &cr.ContainerID,
+               Log:               cr.CrunchLog,
+       }
        os.Unsetenv("GatewayAuthSecret")
-       err = cr.startGatewayServer()
+       err = cr.gateway.Start()
        if err != nil {
                log.Printf("error starting gateway server: %s", err)
                return 1