From f5bba33140f89648ef07098e582b022e1ea1f9f9 Mon Sep 17 00:00:00 2001
From: Peter Amstutz <pamstutz@veritasgenetics.com>
Date: Fri, 2 Aug 2019 15:00:45 -0400
Subject: [PATCH] 15467: Replace KeepServices with SbatchEnvironmentVariables

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

diff --git a/lib/config/deprecated.go b/lib/config/deprecated.go
index 7ae69708eb..3d4c252528 100644
--- a/lib/config/deprecated.go
+++ b/lib/config/deprecated.go
@@ -7,7 +7,6 @@ package config
 import (
 	"fmt"
 	"io/ioutil"
-	"net/url"
 	"os"
 	"strings"
 
@@ -198,18 +197,14 @@ func loadOldClientConfig(cluster *arvados.Cluster, client *arvados.Client) {
 		cluster.SystemRootToken = client.AuthToken
 	}
 	cluster.TLS.Insecure = client.Insecure
-	for _, r := range client.KeepServiceURIs {
-		if cluster.Containers.SLURM.KeepServices == nil {
-			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"] = arvados.Service{InternalURLs: make(map[arvados.URL]arvados.ServiceInstance)}
-		}
-		p, err := url.Parse(r)
-		if err == nil {
-			cluster.Containers.SLURM.KeepServices["00000-bi6l4-000000000000000"].InternalURLs[arvados.URL(*p)] = struct{}{}
+	ks := ""
+	for i, u := range client.KeepServiceURIs {
+		if i > 0 {
+			ks += " "
 		}
+		ks += u
 	}
+	disp.cluster.Containers.SLURM.SbatchEnvironmentVariables = map[string]string{"ARVADOS_KEEP_SERVICES": ks}
 }
 
 // update config using values from an crunch-dispatch-slurm config file.
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index f63996437f..f6b736d587 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -292,10 +292,10 @@ type ContainersConfig struct {
 		LogUpdateSize                ByteSize
 	}
 	SLURM struct {
-		PrioritySpread      int64
-		SbatchArgumentsList []string
-		KeepServices        map[string]Service
-		Managed             struct {
+		PrioritySpread             int64
+		SbatchArgumentsList        []string
+		SbatchEnvironmentVariables map[string]string
+		Managed                    struct {
 			DNSServerConfDir       string
 			DNSServerConfTemplate  string
 			DNSServerReloadCommand string
diff --git a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
index 982141ad81..e4a1e4840b 100644
--- a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
+++ b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
@@ -13,7 +13,6 @@ import (
 	"fmt"
 	"log"
 	"math"
-	"net/url"
 	"os"
 	"regexp"
 	"strings"
@@ -133,18 +132,10 @@ func (disp *Dispatcher) configure(prog string, args []string) error {
 		if disp.Client.Insecure {
 			os.Setenv("ARVADOS_API_HOST_INSECURE", "1")
 		}
-		ks := ""
-		if len(disp.cluster.Containers.SLURM.KeepServices) > 0 {
-			for _, svc := range disp.cluster.Containers.SLURM.KeepServices {
-				for k, _ := range svc.InternalURLs {
-					u := url.URL(k)
-					ks += u.String()
-					ks += " "
-				}
-			}
-		}
-		os.Setenv("ARVADOS_KEEP_SERVICES", strings.TrimSuffix(ks, " "))
 		os.Setenv("ARVADOS_EXTERNAL_CLIENT", "")
+		for k, v := range disp.cluster.Containers.SLURM.SbatchEnvironmentVariables {
+			os.Setenv(k, v)
+		}
 	} 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 63117128eb..ca757af13d 100644
--- a/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
+++ b/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
@@ -420,6 +420,7 @@ BatchSize: 99
 		c.Error(err)
 
 	}
+	os.Setenv("ARVADOS_KEEP_SERVICES", "")
 	err = s.disp.configure("crunch-dispatch-slurm", []string{"-config", tmpfile.Name()})
 	c.Check(err, IsNil)
 
@@ -433,16 +434,8 @@ 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]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{}{},
-			},
-		},
+	c.Check(s.disp.cluster.Containers.SLURM.SbatchEnvironmentVariables, DeepEquals, map[string]string{
+		"ARVADOS_KEEP_SERVICES": "https://example.com/keep1 https://example.com/keep2",
 	})
-	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")
-	}
+	c.Check(os.Getenv("ARVADOS_KEEP_SERVICES"), Equals, "https://example.com/keep1 https://example.com/keep2")
 }
-- 
2.30.2