Repository: argoproj/argo (go)

Issue:
Add `Conditions` to workflow status
# Summary

Add `Conditions` to workflow status.

# Motivation

From the [kubernetes api conventions](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties):

> Some resources in the v1 API contain fields called phase, and associated message, reason, and other status fields. The pattern of using phase is deprecated. Newer API types should use conditions instead.

This would facilitate using e.g. `kubectl wait` to wait for workflows:

`kubectl wait --for=condition=complete workflow/myworkflow`

The `argo wait` command UI is very nice, not sure if it's possible to get any similar experience with `kubectl wait`. But where `kubectl wait` is handy, is if you are trying to wait on a data drive list of resources, some of which may be workflows, some jobs, or any other type of resource for certain conditions, which our team is looking to do.

# Proposal

Add a `Conditions` field to workflow status, which includes at least a `completed` or similar condition, and whatever other conditions may make sense. Deprecate the `phase` etc. fields in status which are superceded by `Conditions`.

---
<!-- Issue Author: Don't delete this message to ecourage other users to support your issue! -->
**Message from the maintainers**:

If you wish to see this enhancement implemented please add a 👍 reaction to this issue! We often sort issues this way to know what to prioritize.


Pull request description:
fix(controller): Updates to add condition to workflow status. Fixes #2421
This PR intends to fix the requested features in #2421. 

The following changes have been made:

- Added condition to workflow status
- Set the condition in the controller. 

Would we like to set more conditions? Would we also like to

```
Deprecate the phase etc. fields in status which are superceded by Conditions
``` 
as suggested in the issue? 

Happy for feedback! 
Checklist:

* [x] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
* [x] The title of the PR is (a) [conventional](https://www.conventionalcommits.org/en/v1.0.0/), (b) states what changed, and (c) suffixes the related issues number. E.g. `"fix(controller): Updates such and such. Fixes #1234"`.  
* [x] I have written unit and/or e2e tests for my change. PRs without these are unlike to be merged.
* [ ] Optional. I've added My organization is added to the USERS.md.
* [x] I've signed the CLA and required builds are green. 

Patch:
diff --git a/examples/artifact-path-placeholders.yaml b/examples/artifact-path-placeholders.yaml
index 3371b5e89..3f8d4da84 100644
--- a/examples/artifact-path-placeholders.yaml
+++ b/examples/artifact-path-placeholders.yaml
@@ -37,4 +37,4 @@ spec:
         path: /outputs/text/data
     container:
       image: busybox
-      command: [sh, -c, 'head -n {{inputs.parameters.lines-count}} <"{{inputs.artifacts.text.path}}" | tee "{{outputs.artifacts.text.path}}" | wc -l > "{{outputs.parameters.actual-lines-count.path}}"']
+      command: [sh, -c, 'mkdir -p "$(dirname "{{outputs.artifacts.text.path}}")" "$(dirname "{{outputs.parameters.actual-lines-count.path}}")" ; head -n {{inputs.parameters.lines-count}} < "{{inputs.artifacts.text.path}}" | tee "{{outputs.artifacts.text.path}}" | wc -l > "{{outputs.parameters.actual-lines-count.path}}"']
diff --git a/pkg/apis/workflow/v1alpha1/workflow_types.go b/pkg/apis/workflow/v1alpha1/workflow_types.go
index d8d42c5c4..b11b14cfd 100644
--- a/pkg/apis/workflow/v1alpha1/workflow_types.go
+++ b/pkg/apis/workflow/v1alpha1/workflow_types.go
@@ -873,6 +873,15 @@ type WorkflowStatus struct {
 
 	// Outputs captures output values and artifact locations produced by the workflow via global outputs
 	Outputs *Outputs `json:"outputs,omitempty" protobuf:"bytes,8,opt,name=outputs"`
+
+	// Condition for k8s conditions https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties
+	Conditions []Condition `json:"condition,omitempty" protobuf:"bytes,11,opt,name=condition"`
+}
+
+type Condition struct {
+	Status metav1.ConditionStatus `json:"status,omitempty" protobuf:"varint,1,opt,name=status"`
+
+	Type string `json:"type,omitempty" protobuf:"varint,2,opt,name=type"`
 }
 
 func (ws *WorkflowStatus) IsOffloadNodeStatus() bool {
diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
index 74d052e67..e2116c430 100644
--- a/workflow/controller/operator.go
+++ b/workflow/controller/operator.go
@@ -1457,6 +1457,11 @@ func (woc *wfOperationCtx) markWorkflowPhase(phase wfv1.NodePhase, markCompleted
 				woc.wf.ObjectMeta.Labels = make(map[string]string)
 			}
 			woc.wf.ObjectMeta.Labels[common.LabelKeyCompleted] = "true"
+			conditions := []wfv1.Condition{wfv1.Condition{
+				Status: metav1.ConditionStatus("True"),
+				Type:   "completed"}}
+			woc.wf.Status.Conditions = conditions
+
 			err := woc.deletePDBResource()
 			if err != nil {
 				woc.wf.Status.Phase = wfv1.NodeError


Test patch:
diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go
index 861d19cdd..852ac07cf 100644
--- a/workflow/controller/operator_test.go
+++ b/workflow/controller/operator_test.go
@@ -2354,6 +2354,19 @@ func TestPDBCreation(t *testing.T) {
 	assert.Nil(t, pdb)
 }
 
+func TestStatusConditions(t *testing.T) {
+	controller := newController()
+	wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("")
+	wf := unmarshalWF(pdbwf)
+	wf, err := wfcset.Create(wf)
+	assert.NoError(t, err)
+	woc := newWorkflowOperationCtx(wf, controller)
+	//woc.operate()
+	assert.Equal(t, len(woc.wf.Status.Conditions), 0)
+	woc.markWorkflowSuccess()
+	assert.Equal(t, woc.wf.Status.Conditions[0].Status, metav1.ConditionStatus("True"))
+}
+
 var nestedOptionalOutputArtifacts = `
 apiVersion: argoproj.io/v1alpha1
 kind: Workflow
@@ -2410,4 +2423,4 @@ func TestNestedOptionalOutputArtifacts(t *testing.T) {
 	woc.operate()
 
 	assert.Equal(t, wfv1.NodeSucceeded, woc.wf.Status.Phase)
-}
\ No newline at end of file
+}