From 590c491796ece16caa4251dd443412f323492fea Mon Sep 17 00:00:00 2001
From: Peter Amstutz <pamstutz@veritasgenetics.com>
Date: Fri, 2 Aug 2019 10:46:11 -0400
Subject: [PATCH] 15467: Added tests for KeepServices

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@veritasgenetics.com>
---
 lib/config/deprecated.go                      |  6 +++--
 lib/config/generated_config.go                |  3 +++
 .../crunch-dispatch-slurm.go                  |  8 +++---
 .../crunch-dispatch-slurm_test.go             | 25 +++++++++++--------
 4 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/lib/config/deprecated.go b/lib/config/deprecated.go
index ce9b2e4d01..7ae69708eb 100644
--- a/lib/config/deprecated.go
+++ b/lib/config/deprecated.go
@@ -203,10 +203,12 @@ func loadOldClientConfig(cluster *arvados.Cluster, client *arvados.Client) {
 			cluster.Containers.SLURM.KeepServices = make(map[string]arvados.Service)
 		}
 		if cluster.Containers.SLURM.KeepServices["00000-bi6l4-000000000000000"].InternalURLs == nil {
-			cluster.Containers.SLURM.KeepServices["00000-bi6l4-000000000000000"].InternalURLs = map[arvados.URL]arvados.ServiceInstance{}
+			cluster.Containers.SLURM.KeepServices["00000-bi6l4-000000000000000"] = arvados.Service{InternalURLs: make(map[arvados.URL]arvados.ServiceInstance)}
 		}
 		p, err := url.Parse(r)
-		cluster.Containers.SLURM.KeepServices["00000-bi6l4-000000000000000"].InternalURLs[arvados.URL(p)] = struct{}{}
+		if err == nil {
+			cluster.Containers.SLURM.KeepServices["00000-bi6l4-000000000000000"].InternalURLs[arvados.URL(*p)] = struct{}{}
+		}
 	}
 }
 
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 35edb05bcd..4e7790603a 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -531,6 +531,9 @@ Clusters:
       SLURM:
         PrioritySpread: 0
         SbatchArgumentsList: []
+        KeepServices:
+          SAMPLE:
+            InternalURLs: {}
         Managed:
           # Path to dns server configuration directory
           # (e.g. /etc/unbound.d/conf.d). If false, do not write any config
diff --git a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
index 300e923640..982141ad81 100644
--- a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
+++ b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
@@ -13,6 +13,7 @@ import (
 	"fmt"
 	"log"
 	"math"
+	"net/url"
 	"os"
 	"regexp"
 	"strings"
@@ -133,15 +134,16 @@ func (disp *Dispatcher) configure(prog string, args []string) error {
 			os.Setenv("ARVADOS_API_HOST_INSECURE", "1")
 		}
 		ks := ""
-		if length(disp.cluster.Containers.SLURM.KeepServices) > 0 {
+		if len(disp.cluster.Containers.SLURM.KeepServices) > 0 {
 			for _, svc := range disp.cluster.Containers.SLURM.KeepServices {
 				for k, _ := range svc.InternalURLs {
-					ks += k
+					u := url.URL(k)
+					ks += u.String()
 					ks += " "
 				}
 			}
 		}
-		os.Setenv("ARVADOS_KEEP_SERVICES", ks)
+		os.Setenv("ARVADOS_KEEP_SERVICES", strings.TrimSuffix(ks, " "))
 		os.Setenv("ARVADOS_EXTERNAL_CLIENT", "")
 	} else {
 		disp.logger.Warnf("Client credentials missing from config, so falling back on environment variables (deprecated).")
diff --git a/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go b/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
index 4a0213d477..63117128eb 100644
--- a/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
+++ b/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
@@ -11,7 +11,6 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	"log"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -396,8 +395,9 @@ func (s *StubbedSuite) TestLoadLegacyConfig(c *C) {
 Client:
   APIHost: example.com
   AuthToken: abcdefg
-  KeepServices:
-    - https://example.com/keep
+  KeepServiceURIs:
+    - https://example.com/keep1
+    - https://example.com/keep2
 SbatchArguments: ["--foo", "bar"]
 PollPeriod: 12s
 PrioritySpread: 42
@@ -408,16 +408,16 @@ BatchSize: 99
 `)
 	tmpfile, err := ioutil.TempFile("", "example")
 	if err != nil {
-		log.Fatal(err)
+		c.Error(err)
 	}
 
 	defer os.Remove(tmpfile.Name()) // clean up
 
 	if _, err := tmpfile.Write(content); err != nil {
-		log.Fatal(err)
+		c.Error(err)
 	}
 	if err := tmpfile.Close(); err != nil {
-		log.Fatal(err)
+		c.Error(err)
 
 	}
 	err = s.disp.configure("crunch-dispatch-slurm", []string{"-config", tmpfile.Name()})
@@ -433,11 +433,16 @@ BatchSize: 99
 	c.Check(s.disp.cluster.Containers.ReserveExtraRAM, Equals, arvados.ByteSize(12345))
 	c.Check(s.disp.cluster.Containers.MinRetryPeriod, Equals, arvados.Duration(13*time.Second))
 	c.Check(s.disp.cluster.API.MaxItemsPerResponse, Equals, 99)
-	c.Check(s.disp.cluster.Containers.SLURM.KeepServices, DeepEquals, map[string]Service{
-		"00000-bi6l4-000000000000000": Service{
-			InternalURLs: map[string]struct{}{
-				"https://example.com/keep": struct{}{},
+	c.Check(s.disp.cluster.Containers.SLURM.KeepServices, DeepEquals, map[string]arvados.Service{
+		"00000-bi6l4-000000000000000": arvados.Service{
+			InternalURLs: map[arvados.URL]arvados.ServiceInstance{
+				arvados.URL{Scheme: "https", Path: "/keep1", Host: "example.com"}: struct{}{},
+				arvados.URL{Scheme: "https", Path: "/keep2", Host: "example.com"}: struct{}{},
 			},
 		},
 	})
+	ks := os.Getenv("ARVADOS_KEEP_SERVICES")
+	if ks != "https://example.com/keep1 https://example.com/keep2" && ks != "https://example.com/keep2 https://example.com/keep1" {
+		c.Assert(ks, Equals, "https://example.com/keep1 https://example.com/keep2")
+	}
 }
-- 
2.30.2