Repository: argoproj/argo
Pull request #2476 (2020-03-19 04:01:37)

feat: Logging the Pod Spec in controller log
Checklist:

* [ ] 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.
* [ ] 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"`.  #2435 
* [ ] 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.
* [ ] I've signed the CLA and required builds are green. 


codecov[bot]: # [Codecov](https://codecov.io/gh/argoproj/argo/pull/2476?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@10a0789`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `12.5%`.

[![Impacted file tree graph](https://codecov.io/gh/argoproj/argo/pull/2476/graphs/tree.svg?width=650&token=4c3DPSTq8o&height=150&src=pr)](https://codecov.io/gh/argoproj/argo/pull/2476?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##             master    #2476   +/-   ##
=========================================
  Coverage          ?   11.23%           
=========================================
  Files             ?       73           
  Lines             ?    30860           
  Branches          ?        0           
=========================================
  Hits              ?     3466           
  Misses            ?    26926           
  Partials          ?      468
```


| [Impacted Files](https://codecov.io/gh/argoproj/argo/pull/2476?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [workflow/config/config.go](https://codecov.io/gh/argoproj/argo/pull/2476/diff?src=pr&el=tree#diff-d29ya2Zsb3cvY29uZmlnL2NvbmZpZy5nbw==) | `33.33% <ø> (ø)` | |
| [workflow/controller/operator.go](https://codecov.io/gh/argoproj/argo/pull/2476/diff?src=pr&el=tree#diff-d29ya2Zsb3cvY29udHJvbGxlci9vcGVyYXRvci5nbw==) | `60.61% <12.5%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/argoproj/argo/pull/2476?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/argoproj/argo/pull/2476?src=pr&el=footer). Last update [10a0789...217b17f](https://codecov.io/gh/argoproj/argo/pull/2476?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
index 6747359b0..c9d093b8a 100644
--- a/api/openapi-spec/swagger.json
+++ b/api/openapi-spec/swagger.json
@@ -10,7 +10,6 @@
   ],
   "swagger": "2.0",
   "info": {
-    "description": "Workflow Service API performs CRUD actions against application resources",
     "title": "Argo",
     "version": "latest"
   },
@@ -5875,31 +5874,5 @@
       "description": "HTTP Basic authentication",
       "type": "basic"
     }
-  },
-  "x-stream-definitions": {
-    "io.argoproj.workflow.v1alpha1.LogEntry": {
-      "properties": {
-        "error": {
-          "$ref": "#/definitions/grpc.gateway.runtime.StreamError"
-        },
-        "result": {
-          "$ref": "#/definitions/io.argoproj.workflow.v1alpha1.LogEntry"
-        }
-      },
-      "title": "Stream result of io.argoproj.workflow.v1alpha1.LogEntry",
-      "type": "object"
-    },
-    "io.argoproj.workflow.v1alpha1.WorkflowWatchEvent": {
-      "properties": {
-        "error": {
-          "$ref": "#/definitions/grpc.gateway.runtime.StreamError"
-        },
-        "result": {
-          "$ref": "#/definitions/io.argoproj.workflow.v1alpha1.WorkflowWatchEvent"
-        }
-      },
-      "title": "Stream result of io.argoproj.workflow.v1alpha1.WorkflowWatchEvent",
-      "type": "object"
-    }
   }
 }
diff --git a/workflow/config/config.go b/workflow/config/config.go
index c8689a280..3e8a742e8 100644
--- a/workflow/config/config.go
+++ b/workflow/config/config.go
@@ -72,6 +72,15 @@ type WorkflowControllerConfig struct {
 
 	// Default workflow spec, will be adde to workflow if the parameters are not set in the workflow
 	DefautWorkflowSpec *wfv1.WorkflowSpec `json:"workflowDefaults,omitempty"`
+
+	// PodSpecLogStrategy enable the logging of podspec on controller log.
+	PodSpecLogStrategy PodSpecLogStrategy `json:"podSpecLogStrategy,omitempty"`
+}
+
+// PodSpecLogStrategy contains the configuration for logging the pod spec in controller log for debugging purpose
+type PodSpecLogStrategy struct {
+	FailedPod bool `json:"failedPod,omitempty"`
+	AllPods   bool `json:"allPods,omitempty"`
 }
 
 // More general feature flags.
diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
index 621833f1b..34a0dd77e 100644
--- a/workflow/controller/operator.go
+++ b/workflow/controller/operator.go
@@ -744,6 +744,9 @@ func (woc *wfOperationCtx) podReconciliation() error {
 					}
 				}
 				woc.completedPods[pod.ObjectMeta.Name] = true
+				if woc.shouldPrintPodSpec(node) {
+					printPodSpecLog(pod, woc.wf.Name)
+				}
 			}
 			if node.Successful() {
 				woc.succeededPods[pod.ObjectMeta.Name] = true
@@ -790,6 +793,12 @@ func (woc *wfOperationCtx) podReconciliation() error {
 	return nil
 }
 
+// shouldPrintPodSpec return eligible to print to the pod spec
+func (woc *wfOperationCtx) shouldPrintPodSpec(node wfv1.NodeStatus) bool {
+	return woc.controller.Config.PodSpecLogStrategy.AllPods ||
+		(woc.controller.Config.PodSpecLogStrategy.FailedPod && node.Failed())
+}
+
 //fails any suspended nodes if the workflow deadline has passed
 func (woc *wfOperationCtx) failSuspendedNodesAfterDeadline() error {
 	if woc.workflowDeadline != nil && time.Now().UTC().After(*woc.workflowDeadline) {
@@ -864,6 +873,13 @@ func (woc *wfOperationCtx) getAllWorkflowPods() (*apiv1.PodList, error) {
 	}
 	return podList, nil
 }
+func printPodSpecLog(pod *apiv1.Pod, wfName string) {
+	podSpecByte, err := json.Marshal(pod)
+	if err != nil {
+		log.WithField("workflow", wfName).WithField("nodename", pod.Name).WithField("namespace", pod.Namespace).Warnf("Unable to mashal pod spec. %v", err)
+	}
+	log.WithField("workflow", wfName).WithField("nodename", pod.Name).WithField("namespace", pod.Namespace).Infof("Pod Spec: %s", string(podSpecByte))
+}
 
 // assessNodeStatus compares the current state of a pod with its corresponding node
 // and returns the new node status if something changed


Test patch:
diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go
index 9a1de973c..5f726adbf 100644
--- a/workflow/controller/operator_test.go
+++ b/workflow/controller/operator_test.go
@@ -2426,3 +2426,41 @@ func TestNestedOptionalOutputArtifacts(t *testing.T) {
 
 	assert.Equal(t, wfv1.NodeSucceeded, woc.wf.Status.Phase)
 }
+
+//  TestPodSpecLogForFailedPods tests PodSpec logging configuration
+func TestPodSpecLogForFailedPods(t *testing.T) {
+	controller := newController()
+	assert.NotNil(t, controller)
+	controller.Config.PodSpecLogStrategy.FailedPod = true
+	wf := unmarshalWF(helloWorldWf)
+	wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("")
+	wf, err := wfcset.Create(wf)
+	assert.NoError(t, err)
+	woc := newWorkflowOperationCtx(wf, controller)
+	assert.NotNil(t, woc)
+	woc.operate()
+	woc.operate()
+	for _, node := range woc.wf.Status.Nodes {
+		assert.True(t, woc.shouldPrintPodSpec(node))
+	}
+
+}
+
+//  TestPodSpecLogForAllPods tests  PodSpec logging configuration
+func TestPodSpecLogForAllPods(t *testing.T) {
+	controller := newController()
+	assert.NotNil(t, controller)
+	controller.Config.PodSpecLogStrategy.AllPods = true
+	wf := unmarshalWF(nestedOptionalOutputArtifacts)
+	wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("")
+	wf, err := wfcset.Create(wf)
+	assert.NoError(t, err)
+	woc := newWorkflowOperationCtx(wf, controller)
+	assert.NotNil(t, woc)
+	woc.operate()
+	woc.operate()
+	for _, node := range woc.wf.Status.Nodes {
+		assert.True(t, woc.shouldPrintPodSpec(node))
+	}
+
+}