Repository: iter8-tools/iter8
Pull request #1418 (2023-03-21 13:13:32)

assess rewards and report
Part of https://github.com/iter8-tools/iter8/issues/1414

kalantar: > Could we please run `iter8` as nonRoot (and set user id) in the experiment chart? This will be needed in both job and cronjob templates.

The only writable files for non-root users are in `/tmp`. Moved all downloads to `/tmp' in templates.

Patch:
diff --git a/action/report/common.go b/action/report/common.go
index 49cf4770..d0ca8552 100644
--- a/action/report/common.go
+++ b/action/report/common.go
@@ -92,3 +92,46 @@ func (r *Reporter) MetricWithUnits(metricName string) (string, error) {
 	}
 	return str, nil
 }
+
+// GetBestVersions returns list of best versions for each metric
+func (r *Reporter) GetBestVersions(metrics []string, in *base.Insights) []string {
+	results := make([]string, len(metrics))
+	if in.Rewards == nil {
+		return results
+	}
+
+	rewards := *in.Rewards
+	winners := *in.RewardsWinners
+
+	for i, mn := range metrics {
+		j := indexString(rewards.Max, mn)
+		if j >= 0 {
+			if winners.Max[j] == -1 {
+				results[i] = "insufficient data"
+			} else {
+				results[i] = in.TrackVersionStr(winners.Max[j])
+			}
+		} else {
+			j = indexString(rewards.Min, mn)
+			if j >= 0 {
+				if winners.Min[j] == -1 {
+					results[i] = "insufficient data"
+				} else {
+					results[i] = in.TrackVersionStr(winners.Min[j])
+				}
+			} else {
+				results[i] = "n/a"
+			}
+		}
+	}
+	return results
+}
+
+func indexString(keys []string, item string) int {
+	for i, key := range keys {
+		if key == item {
+			return i
+		}
+	}
+	return -1
+}
diff --git a/action/report/htmlreport.tpl b/action/report/htmlreport.tpl
index b8b13d63..aecedee2 100644
--- a/action/report/htmlreport.tpl
+++ b/action/report/htmlreport.tpl
@@ -155,14 +155,19 @@
                 {{- if ge .Result.Insights.NumVersions 2 }}
                 {{- range until .Result.Insights.NumVersions }}
                 <th scope="col" class="text-center">{{ $.Result.Insights.TrackVersionStr . }}</th>
-                {{- end}}              
-                {{- else }} 
+                {{- end}}
+                {{- if .Result.Insights.Rewards }}
+                <th scope="col" class="text-center">Best</th>
+                {{- end }}
+                {{- else }}
                 <th scope="col" class="text-center">Value</th>
                 {{- end }}
               </tr>
             </thead>
             <tbody>
-                {{- range $ind, $mn := .SortedScalarAndSLOMetrics }}
+                {{- $metrics := .SortedScalarAndSLOMetrics }}
+                {{- $bestVersions := .GetBestVersions $metrics .Result.Insights }}
+                {{- range $ind, $mn := $metrics }}
                 <tr scope="row">
                   <td>
                     <a href="javascript:void(0)" data-toggle="tooltip" data-placement="top" title="{{ $.MetricDescriptionHTML $mn }}">
@@ -174,6 +179,11 @@
                   {{ $.ScalarMetricValueStr . $mn }}
                   </td>
                   {{- end }}
+                  {{- if $.Result.Insights.Rewards }}
+                  <td class="text-center">
+                  {{ index $bestVersions $ind }}
+                  </td>
+                  {{- end }}
                 </tr>
                 {{- end}}          
             </tbody>
diff --git a/action/report/textreporter.go b/action/report/textreporter.go
index 87abd697..1a6e9d9a 100644
--- a/action/report/textreporter.go
+++ b/action/report/textreporter.go
@@ -154,6 +154,9 @@ func (tr *TextReporter) printMetricsText(w *tabwriter.Writer) {
 	fmt.Fprint(w, "Metric")
 	if in.NumVersions > 1 {
 		tr.printVersions(w)
+		if in.Rewards != nil {
+			fmt.Fprintf(w, "\t Best")
+		}
 	} else {
 		fmt.Fprintf(w, "\t value")
 	}
@@ -162,12 +165,16 @@ func (tr *TextReporter) printMetricsText(w *tabwriter.Writer) {
 	for i := 0; i < in.NumVersions; i++ {
 		fmt.Fprint(w, "\t -----")
 	}
+	if in.NumVersions > 1 && in.Rewards != nil {
+		fmt.Fprint(w, "\t ----")
+	}
 	fmt.Fprintln(w)
 
 	// keys contain normalized scalar metric names in sorted order
 	keys := tr.SortedScalarAndSLOMetrics()
+	bestVersions := tr.GetBestVersions(keys, in)
 
-	for _, mn := range keys {
+	for i, mn := range keys {
 		mwu, err := tr.MetricWithUnits(mn)
 		if err == nil {
 			// add metric name with units
@@ -176,6 +183,9 @@ func (tr *TextReporter) printMetricsText(w *tabwriter.Writer) {
 			for j := 0; j < in.NumVersions; j++ {
 				fmt.Fprintf(w, "\t %v", tr.ScalarMetricValueStr(j, mn))
 			}
+			if in.NumVersions > 1 && in.Rewards != nil {
+				fmt.Fprintf(w, "\t %s", bestVersions[i])
+			}
 			fmt.Fprintln(w)
 		} else {
 			log.Logger.Error(err)
diff --git a/base/assess.go b/base/assess.go
index ff383705..7bc44bcf 100644
--- a/base/assess.go
+++ b/base/assess.go
@@ -8,6 +8,9 @@ import (
 
 // assessInputs contain the inputs to the assess-app-versions task to be executed.
 type assessInputs struct {
+	// Rewards are the reward metrics
+	Rewards *Rewards `json:"rewards,omitempty" yaml:"rewards,omitempty"`
+
 	// SLOs are the SLO limits
 	SLOs *SLOLimits `json:"SLOs,omitempty" yaml:"SLOs,omitempty"`
 }
@@ -55,6 +58,12 @@ func (t *assessTask) run(exp *Experiment) error {
 		return nil
 	}
 
+	// set rewards (if needed)
+	err = exp.Result.Insights.setRewards(t.With.Rewards)
+	if err != nil {
+		return err
+	}
+
 	// set SLOs (if needed)
 	err = exp.Result.Insights.setSLOs(t.With.SLOs)
 	if err != nil {
@@ -68,14 +77,53 @@ func (t *assessTask) run(exp *Experiment) error {
 	}
 
 	// set SLOsSatisfied
-	exp.Result.Insights.SLOsSatisfied = &SLOResults{
-		Upper: evaluateSLOs(exp, t.With.SLOs.Upper, true),
-		Lower: evaluateSLOs(exp, t.With.SLOs.Lower, false),
+	if t.With.SLOs != nil {
+		exp.Result.Insights.SLOsSatisfied = &SLOResults{
+			Upper: evaluateSLOs(exp, t.With.SLOs.Upper, true),
+			Lower: evaluateSLOs(exp, t.With.SLOs.Lower, false),
+		}
+	}
+
+	// set RewardsWinners
+	if t.With.Rewards != nil {
+		exp.Result.Insights.RewardsWinners = &RewardsWinners{
+			Max: evaluateRewards(exp, t.With.Rewards.Max, true),
+			Min: evaluateRewards(exp, t.With.Rewards.Min, false),
+		}
 	}
 
 	return err
 }
 
+func evaluateRewards(exp *Experiment, rewards []string, max bool) []int {
+	winners := make([]int, len(rewards))
+	for i := 0; i < len(rewards); i++ {
+		for j := 0; j < exp.Result.Insights.NumVersions; j++ {
+			winners[i] = identifyWinner(exp, rewards[i], max)
+		}
+	}
+	return winners
+}
+
+func identifyWinner(e *Experiment, reward string, max bool) int {
+	currentWinner := -1
+	var currentWinningValue *float64
+
+	for j := 0; j < e.Result.Insights.NumVersions; j++ {
+		val := e.Result.Insights.ScalarMetricValue(j, reward)
+		if val == nil {
+			log.Logger.Warnf("unable to find value for version %v and metric %s", j, reward)
+			continue
+		}
+		if currentWinningValue == nil || (max && *val > *currentWinningValue) || (!max && *val < *currentWinningValue) {
+			currentWinningValue = val
+			currentWinner = j
+		}
+	}
+
+	return currentWinner
+}
+
 // evaluate SLOs and output the boolean SLO X version matrix
 func evaluateSLOs(exp *Experiment, slos []SLO, upper bool) [][]bool {
 	slosSatisfied := make([][]bool, len(slos))
diff --git a/base/experiment.go b/base/experiment.go
index 1e2ce193..8e664560 100644
--- a/base/experiment.go
+++ b/base/experiment.go
@@ -107,6 +107,12 @@ type Insights struct {
 
 	// SLOsSatisfied indicator matrices that show if upper and lower SLO limits are satisfied
 	SLOsSatisfied *SLOResults `json:"SLOsSatisfied,omitempty" yaml:"SLOsSatisfied,omitempty"`
+
+	// Rewards involed in this experiment
+	Rewards *Rewards `json:"rewards,omitempty" yaml:"rewards,omitempty"`
+
+	// RewardsWinners indicate the winners
+	RewardsWinners *RewardsWinners `json:"rewardsWinners,omitempty" yaml:"rewardsWinners,omitempty"`
 }
 
 // MetricMeta describes a metric
@@ -128,6 +134,24 @@ type VersionInfo struct {
 	Track string `json:"track" yaml:"track"`
 }
 
+// Rewards specify max and min rewards
+type Rewards struct {
+	// Max is list of reward metrics where the version with the maximum value wins
+	Max []string `json:"max,omitempty" yaml:"max,omitempty"`
+	// Min is list of reward metrics where the version with the minimum value wins
+	Min []string `json:"min,omitempty" yaml:"min,omitempty"`
+}
+
+// RewardsWinners are indices of the best versions for each reward metric
+type RewardsWinners struct {
+	// Max rewards
+	// Max[i] specifies the index of the winner of reward metric Rewards.Max[i]
+	Max []int `json:"max,omitempty" yaml:"max,omitempty"`
+	// Min rewards
+	// Min[i] specifies the index of the winner of reward metric Rewards.Min[i]
+	Min []int `json:"min,omitempty" yaml:"min,omitempty"`
+}
+
 // SLO is a service level objective
 type SLO struct {
 	// Metric is the fully qualified metric name in the backendName/metricName format
@@ -370,6 +394,23 @@ func (in *Insights) updateMetric(m string, mm MetricMeta, i int, val interface{}
 	return nil
 }
 
+// setRewards sets the Rewards field in insights
+// if this function is called multiple times (example, due to looping), then
+// it is intended to be called with the same argument each time
+func (in *Insights) setRewards(rewards *Rewards) error {
+	if in.SLOs != nil {
+		if reflect.DeepEqual(in.Rewards, rewards) {
+			return nil
+		}
+		e := fmt.Errorf("old and new value of rewards conflict")
+		log.Logger.WithStackTrace(fmt.Sprint("old: ", in.Rewards, "new: ", rewards)).Error(e)
+		return e
+	}
+	// LHS will be nil
+	in.Rewards = rewards
+	return nil
+}
+
 // setSLOs sets the SLOs field in insights
 // if this function is called multiple times (example, due to looping), then
 // it is intended to be called with the same argument each time
diff --git a/charts/iter8/Chart.yaml b/charts/iter8/Chart.yaml
index c7a36118..173ce6d1 100644
--- a/charts/iter8/Chart.yaml
+++ b/charts/iter8/Chart.yaml
@@ -1,6 +1,6 @@
 apiVersion: v2
 name: iter8
-version: 0.14.1
+version: 0.14.2
 description: Iter8 experiment chart
 type: application
 home: https://iter8.tools
diff --git a/charts/iter8/templates/_k-cronjob.tpl b/charts/iter8/templates/_k-cronjob.tpl
index 50df88df..40beedb7 100644
--- a/charts/iter8/templates/_k-cronjob.tpl
+++ b/charts/iter8/templates/_k-cronjob.tpl
@@ -30,6 +30,13 @@ spec:
               iter8 k run --namespace {{ .Release.Namespace }} --group {{ .Release.Name }} -l {{ .Values.logLevel }} --reuseResult
             resources:
               {{ toYaml .Values.resources | indent 14 | trim }}
+            securityContext:
+              allowPrivilegeEscalation: false
+              capabilities:
+                drop:
+                - ALL
+              runAsNonRoot: true
+              runAsUser: 1001040000
           restartPolicy: Never
       backoffLimit: 0
 {{- end }}
diff --git a/charts/iter8/templates/_k-job.tpl b/charts/iter8/templates/_k-job.tpl
index e158ba0f..aac373cd 100644
--- a/charts/iter8/templates/_k-job.tpl
+++ b/charts/iter8/templates/_k-job.tpl
@@ -26,6 +26,13 @@ spec:
           iter8 k run --namespace {{ .Release.Namespace }} --group {{ .Release.Name }} -l {{ .Values.logLevel }}
         resources:
           {{ toYaml .Values.resources | indent 10 | trim }}
+        securityContext:
+          allowPrivilegeEscalation: false
+          capabilities:
+            drop:
+            - ALL
+          runAsNonRoot: true
+          runAsUser: 1001040000
       restartPolicy: Never
   backoffLimit: 0
 {{- end }}
diff --git a/charts/iter8/templates/_task-assess.tpl b/charts/iter8/templates/_task-assess.tpl
index 592ae0e3..8b3e9201 100644
--- a/charts/iter8/templates/_task-assess.tpl
+++ b/charts/iter8/templates/_task-assess.tpl
@@ -21,5 +21,20 @@
 {{- end }}
 {{- end }}
 {{- end }}
+{{- if .rewards }}
+    rewards:
+{{- if .rewards.max }}
+      max:
+{{- range $r, $val := .rewards.max }}
+      - {{ $val }}
+{{- end }}
+{{- end }}
+{{- if .rewards.min }}
+      min:
+{{- range $r, $val := .rewards.min }}
+      - {{ $val }}
+{{- end }}
+{{- end }}
+{{- end }}
 {{- end }}
 {{- end }}
\ No newline at end of file
diff --git a/charts/iter8/templates/_task-grpc.tpl b/charts/iter8/templates/_task-grpc.tpl
index df169908..482f03a8 100644
--- a/charts/iter8/templates/_task-grpc.tpl
+++ b/charts/iter8/templates/_task-grpc.tpl
@@ -33,53 +33,53 @@
 {{- if $vals.protoURL }}
 # task: download proto file from URL
 - run: |
-    curl -o ghz.proto {{ $vals.protoURL }}
-{{- $_ := set $vals "proto" "ghz.proto" }}
+    curl -o /tmp/ghz.proto {{ $vals.protoURL }}
+{{- $_ := set $vals "proto" "/tmp/ghz.proto" }}
 {{- end }}
 {{- if $vals.dataURL }}
 # task: download JSON data file from URL
 - run: |
-    curl -o data.json {{ $vals.dataURL }}
-{{- $_ := set $vals "data-file" "data.json" }}
+    curl -o /tmp/data.json {{ $vals.dataURL }}
+{{- $_ := set $vals "data-file" "/tmp/data.json" }}
 {{- end }}
 {{- if $vals.binaryDataURL }}
 # task: download binary data file from URL
 - run: |
-    curl -o data.bin {{ $vals.binaryDataURL }}
-{{- $_ := set $vals "binary-file" "data.bin" }}
+    curl -o /tmp/data.bin {{ $vals.binaryDataURL }}
+{{- $_ := set $vals "binary-file" "/tmp/data.bin" }}
 {{- end }}
 {{- if $vals.metadataURL }}
 # task: download metadata JSON file from URL
 - run: |
-    curl -o metadata.json {{ $vals.metadataURL }}
-{{- $_ := set $vals "metadata-file" "metadata.json" }}
+    curl -o /tmp/metadata.json {{ $vals.metadataURL }}
+{{- $_ := set $vals "metadata-file" "/tmp/metadata.json" }}
 {{- end }}
 {{- /**************************/ -}}
 {{- /* Repeat above for each endpoint */ -}}
 {{- range $endpointID, $endpoint := $vals.endpoints }}
 {{- if $endpoint.protoURL }}
-{{- $protoFile := print $endpointID "_ghz.proto" }}
+{{- $protoFile := print "/tmp" $endpointID "_ghz.proto" }}
 # task: download proto file from URL for endpoint
 - run: |
     curl -o {{ $protoFile }} {{ $endpoint.protoURL }}
 {{- $_ := set $endpoint "proto" $protoFile }}
 {{- end }}
 {{- if $endpoint.dataURL }}
-{{- $dataFile := print $endpointID "_data.json" }}
+{{- $dataFile := print "/tmp" $endpointID "_data.json" }}
 # task: download JSON data file from URL for endpoint
 - run: |
     curl -o {{ $dataFile }} {{ $endpoint.dataURL }}
 {{- $_ := set $endpoint "data-file" $dataFile }}
 {{- end }}
 {{- if $endpoint.binaryDataURL }}
-{{- $binDataFile := print $endpointID "_data.bin" }}
+{{- $binDataFile := print "/tmp" $endpointID "_data.bin" }}
 # task: download binary data file from URL for endpoint
 - run: |
     curl -o {{ $binDataFile }} {{ $endpoint.binaryDataURL }}
 {{- $_ := set $endpoint "binary-file" $binDataFile }}
 {{- end }}
 {{- if $endpoint.metadataURL }}
-{{- $metadataFile := print $endpointID "_metadata.json" }}
+{{- $metadataFile := print "/tmp" $endpointID "_metadata.json" }}
 # task: download metadata JSON file from URL for endpoint
 - run: |
     curl -o {{ $metadataFile }} {{ $endpoint.metadataURL }}
diff --git a/charts/iter8/templates/_task-http.tpl b/charts/iter8/templates/_task-http.tpl
index 14a84b2d..0e45bbea 100644
--- a/charts/iter8/templates/_task-http.tpl
+++ b/charts/iter8/templates/_task-http.tpl
@@ -21,14 +21,14 @@
 {{- if $vals.payloadURL }}
 # task: download payload from payload URL
 - run: |
-    curl -o payload.dat {{ $vals.payloadURL }}
-{{- $_ := set $vals "payloadFile" "payload.dat" }}
+    curl -o /tmp/payload.dat {{ $vals.payloadURL }}
+{{- $_ := set $vals "payloadFile" "/tmp/payload.dat" }}
 {{- end }}
 {{- /**************************/ -}}
 {{- /* Repeat above for each endpoint */ -}}
 {{- range $endpointID, $endpoint := $vals.endpoints }}
 {{- if $endpoint.payloadURL }}
-{{- $payloadFile := print $endpointID "_payload.dat" }}
+{{- $payloadFile := print "/tmp/" $endpointID "_payload.dat" }}
 # task: download payload from payload URL for endpoint
 - run: |
     curl -o {{ $payloadFile }} {{ $endpoint.payloadURL }}


Test patch:
diff --git a/base/assess_test.go b/base/assess_test.go
index daea8551..bed6128e 100644
--- a/base/assess_test.go
+++ b/base/assess_test.go
@@ -35,6 +35,9 @@ func TestRunAssess(t *testing.T) {
 				Limit:  20.0,
 			}},
 		},
+		Rewards: &Rewards{
+			Max: []string{"a/b"},
+		},
 	}
 	err = task.run(exp)
 	assert.NoError(t, err)
diff --git a/testdata/assertinputs/experiment.yaml b/testdata/assertinputs/experiment.yaml
index 5ee8c2db..b0f9c000 100644
--- a/testdata/assertinputs/experiment.yaml
+++ b/testdata/assertinputs/experiment.yaml
@@ -11,6 +11,12 @@ spec:
   # the metrics collected in the above task
   - task: assess
     with:
+      rewards:
+        max:
+        - "http/latency-mean"
+        - "http/latency-p50"
+        min:
+        - "http/error-rate"
       SLOs:
         Upper:
         - metric: "http/error-rate"
@@ -33,6 +39,18 @@ spec:
 result:
   failure: false
   insights:
+    rewards:
+      max:
+      - "http/latency-mean"
+      - "http/latency-p50"
+      min:
+      - "http/error-rate"
+    rewardsWinners:
+      max:
+      - 0
+      - 0
+      min:
+      - 0
     SLOs:
       upper:
       - metric: "http/error-rate"
diff --git a/testdata/charts/iter8/templates/_task-github.tpl b/testdata/charts/iter8/templates/_task-github.tpl
index 8ee103d0..66908d33 100644
--- a/testdata/charts/iter8/templates/_task-github.tpl
+++ b/testdata/charts/iter8/templates/_task-github.tpl
@@ -25,4 +25,4 @@
       Accept: application/vnd.github+json
     payloadTemplateURL: {{ default "https://raw.githubusercontent.com/iter8-tools/iter8/iter8-0.13.7/templates/notify/_payload-github.tpl" .payloadTemplateURL }}
     softFailure: {{ default true .softFailure }}
-{{ end }}
\ No newline at end of file
+{{ end }}
diff --git a/testdata/charts/iter8/templates/_task-slack.tpl b/testdata/charts/iter8/templates/_task-slack.tpl
index 51233a62..4ee2906a 100644
--- a/testdata/charts/iter8/templates/_task-slack.tpl
+++ b/testdata/charts/iter8/templates/_task-slack.tpl
@@ -16,4 +16,4 @@
     method: POST
     payloadTemplateURL: {{ default "https://raw.githubusercontent.com/iter8-tools/iter8/iter8-0.13.7/templates/notify/_payload-slack.tpl" .payloadTemplateURL }}
     softFailure: {{ default true .softFailure }}
-{{ end }} 
\ No newline at end of file
+{{ end }}