feat: Ability to set step analysis/experiment arg value from field pa…

…th like .status.desiredVersion
mumoshu · Jan 19, 2022 · 9189a03 · 9189a03
1 parent 6b71cb3
commit 9189a03
Show file tree

Hide file tree

Showing 7 changed files with 106 additions and 10 deletions.
diff --git a/api/v1alpha1/cell.go b/api/v1alpha1/cell.go
@@ -113,11 +113,12 @@ type CellUpdateStrategyBlueGreen struct {
 
 // CellStatus defines the observed state of ClusterSet
 type CellStatus struct {
-	Clusters     ClusterSetStatusClusters `json:"clusters"`
-	LastSyncTime metav1.Time              `json:"lastSyncTime"`
-	Phase        string                   `json:"phase"`
-	Reason       string                   `json:"reason"`
-	Message      string                   `json:"message"`
+	DesiredVersion string                   `json:"desiredVersion"`
+	Clusters       ClusterSetStatusClusters `json:"clusters"`
+	LastSyncTime   metav1.Time              `json:"lastSyncTime"`
+	Phase          string                   `json:"phase"`
+	Reason         string                   `json:"reason"`
+	Message        string                   `json:"message"`
 }
 
 // +kubebuilder:object:root=true

diff --git a/charts/okra/crds/okra.mumo.co_cells.yaml b/charts/okra/crds/okra.mumo.co_cells.yaml
@@ -140,7 +140,7 @@ spec:
                       analysis:
                         description: Analysis runs a separate analysisRun while all
                           the steps execute. This is intended to be a continuous validation
-                          of the new ReplicaSet
+                          of the new set of clusters
                         properties:
                           args:
                             description: Args the arguments that will be added to
@@ -516,6 +516,8 @@ spec:
                       type: string
                     type: array
                 type: object
+              desiredVersion:
+                type: string
               lastSyncTime:
                 format: date-time
                 type: string
@@ -527,6 +529,7 @@ spec:
                 type: string
             required:
             - clusters
+            - desiredVersion
             - lastSyncTime
             - message
             - phase

diff --git a/config/crd/bases/okra.mumo.co_cells.yaml b/config/crd/bases/okra.mumo.co_cells.yaml
@@ -140,7 +140,7 @@ spec:
                       analysis:
                         description: Analysis runs a separate analysisRun while all
                           the steps execute. This is intended to be a continuous validation
-                          of the new ReplicaSet
+                          of the new set of clusters
                         properties:
                           args:
                             description: Args the arguments that will be added to
@@ -516,6 +516,8 @@ spec:
                       type: string
                     type: array
                 type: object
+              desiredVersion:
+                type: string
               lastSyncTime:
                 format: date-time
                 type: string
@@ -527,6 +529,7 @@ spec:
                 type: string
             required:
             - clusters
+            - desiredVersion
             - lastSyncTime
             - message
             - phase

diff --git a/pkg/cell/analysis.go b/pkg/cell/analysis.go
@@ -113,6 +113,12 @@ func (s cellComponentReconciler) reconcileAnalysisRun(ctx context.Context, compo
 
 				if a.Value != "" {
 					arg.Value = &a.Value
+				} else if a.ValueFrom != nil && arg.ValueFrom.FieldRef != nil {
+					v, err := extractValueFromCell(&cell, a.ValueFrom.FieldRef.FieldPath)
+					if err != nil {
+						return ComponentFailed, err
+					}
+					arg.Value = &v
 				}
 
 				argsMap[a.Name] = arg
@@ -227,10 +233,23 @@ func (s cellComponentReconciler) reconcileExperiment(ctx context.Context, compon
 		for _, a := range exTemplate.Analyses {
 			var args []rolloutsv1alpha1.Argument
 			for _, arg := range a.Args {
+				var (
+					value *string
+				)
+
+				if arg.Value != "" {
+					value = &arg.Value
+				} else if arg.ValueFrom != nil && arg.ValueFrom.FieldRef != nil {
+					v, err := extractValueFromCell(&cell, arg.ValueFrom.FieldRef.FieldPath)
+					if err != nil {
+						return ComponentFailed, err
+					}
+					value = &v
+				}
+
 				args = append(args, rolloutsv1alpha1.Argument{
-					Name: arg.Name,
-					// TODO
-					Value: &arg.Value,
+					Name:  arg.Name,
+					Value: value,
 				})
 			}
 			analyses = append(analyses, rolloutsv1alpha1.ExperimentAnalysisTemplateRef{

diff --git a/pkg/cell/cell.go b/pkg/cell/cell.go
@@ -333,6 +333,11 @@ func Sync(config SyncInput) error {
 		return nil
 	}
 
+	// Now, we need to update cell.status
+	// so that values in it can be used from within field paths
+	// contained in experiment and analysis step args.
+	cell.Status.DesiredVersion = desiredVer.String()
+
 	canary := cell.Spec.UpdateStrategy.Canary
 	canarySteps := canary.Steps
 

diff --git a/pkg/cell/value_from.go b/pkg/cell/value_from.go
@@ -0,0 +1,59 @@
+package cell
+
+import (
+	"encoding/json"
+	"fmt"
+	"regexp"
+	"strconv"
+
+	okrav1alpha1 "github.com/mumoshu/okra/api/v1alpha1"
+)
+
+// extractValueFromCell fetches the value at the field path out of the cell.
+//
+// The implementation of this function is highly insipired by how Argo Rollouts
+// transforms a field path to extract a value out of a rollout object.
+//
+// See the below for how Argo CD treats ValueFrom/FieldPath
+// https://github.com/argoproj/argo-rollouts/blob/4739bcd2d9b4910805fd9529d5afd385dbe51bbd/utils/analysis/factory.go#L32-L43
+//
+// See the below for the original implementation of extractValueFromRollout
+// https://github.com/argoproj/argo-rollouts/blob/4739bcd2d9b4910805fd9529d5afd385dbe51bbd/utils/analysis/factory.go#L231
+func extractValueFromCell(r *okrav1alpha1.Cell, path string) (string, error) {
+	j, _ := json.Marshal(r)
+	m := interface{}(nil)
+	json.Unmarshal(j, &m)
+	sections := regexp.MustCompile("[\\.\\[\\]]+").Split(path, -1)
+	for _, section := range sections {
+		if section == "" {
+			continue // if path ends with a separator char, Split returns an empty last section
+		}
+
+		if asArray, ok := m.([]interface{}); ok {
+			if i, err := strconv.Atoi(section); err != nil {
+				return "", fmt.Errorf("invalid index '%s'", section)
+			} else if i >= len(asArray) {
+				return "", fmt.Errorf("index %d out of range", i)
+			} else {
+				m = asArray[i]
+			}
+		} else if asMap, ok := m.(map[string]interface{}); ok {
+			m = asMap[section]
+		} else {
+			return "", fmt.Errorf("invalid path %s in cell", path)
+		}
+	}
+
+	if m == nil {
+		return "", fmt.Errorf("invalid path %s in cell", path)
+	}
+
+	var isArray, isMap bool
+	_, isArray = m.([]interface{})
+	_, isMap = m.(map[string]interface{})
+	if isArray || isMap {
+		return "", fmt.Errorf("path %s in cell must terminate in a primitive value", path)
+	}
+
+	return fmt.Sprintf("%v", m), nil
+}
diff --git a/pkg/controllers/cell.go b/pkg/controllers/cell.go
@@ -107,6 +107,12 @@ func (r *CellReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
 	}
 
+	if err := r.Client.Status().Update(ctx, &cellResource); err != nil {
+		log.Error(err, "Updating Cell Status")
+
+		return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
+	}
+
 	r.Recorder.Event(&cellResource, corev1.EventTypeNormal, "SyncFinished", fmt.Sprintf("Sync finished on '%s'", cellResource.Name))
 
 	return ctrl.Result{}, nil