Repository: argoproj/argo
Pull request #4227 (2020-10-06 21:37:17)

docs: Reinstate swagger.md so docs can build.
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] My organization is added to [USERS.md](https://github.com/argoproj/argo/blob/master/USERS.md).
* [x] I've signed the CLA.
* [ ] ~~I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.~~
* [ ] My builds are green. Try syncing with master if they are not. 

Our doc are not being deployed due to this warning:

```
WARNING -  Documentation file 'rest-api.md' contains a link to 'swagger.md' which is not found in the documentation files. 
```

https://github.com/argoproj/argo/runs/1216143545?check_suite_focus=true


Patch:
diff --git a/cmd/argo/commands/delete.go b/cmd/argo/commands/delete.go
index 441e54d13..524f11175 100644
--- a/cmd/argo/commands/delete.go
+++ b/cmd/argo/commands/delete.go
@@ -56,6 +56,12 @@ func NewDeleteCommand() *cobra.Command {
 				errors.CheckError(err)
 				workflows = append(workflows, listed...)
 			}
+
+			if len(workflows) == 0 {
+				fmt.Printf("No resources found\n")
+				return
+			}
+
 			for _, wf := range workflows {
 				if !dryRun {
 					_, err := serviceClient.DeleteWorkflow(ctx, &workflowpkg.WorkflowDeleteRequest{Name: wf.Name, Namespace: wf.Namespace})
diff --git a/docs/swagger.md b/docs/swagger.md
new file mode 100644
index 000000000..b5209079d
--- /dev/null
+++ b/docs/swagger.md
@@ -0,0 +1,3 @@
+# Argo Server API
+
+[Open the Swagger API docs](https://raw.githubusercontent.com/argoproj/argo/master/api/openapi-spec/swagger.json).
\ No newline at end of file
diff --git a/pkg/apis/workflow/v1alpha1/workflow_types.go b/pkg/apis/workflow/v1alpha1/workflow_types.go
index c9d77fc1e..b8af83835 100644
--- a/pkg/apis/workflow/v1alpha1/workflow_types.go
+++ b/pkg/apis/workflow/v1alpha1/workflow_types.go
@@ -144,6 +144,25 @@ func (w Workflows) Filter(predicate WorkflowPredicate) Workflows {
 	return out
 }
 
+// GetTTLStrategy return TTLStrategy based on Order of precedence:
+//1. Workflow, 2. WorkflowTemplate, 3. Workflowdefault
+func (w *Workflow) GetTTLStrategy(defaultTTLStrategy *TTLStrategy) *TTLStrategy {
+	var ttlStrategy *TTLStrategy
+	// TTLStrategy from Workflow default from Config
+	if defaultTTLStrategy != nil {
+		ttlStrategy = defaultTTLStrategy
+	}
+	// TTLStrategy from WorkflowTemplate
+	if w.Status.StoredWorkflowSpec != nil && w.Status.StoredWorkflowSpec.GetTTLStrategy() != nil {
+		ttlStrategy = w.Status.StoredWorkflowSpec.GetTTLStrategy()
+	}
+	//TTLStrategy from Workflow
+	if w.Spec.GetTTLStrategy() != nil {
+		ttlStrategy = w.Spec.GetTTLStrategy()
+	}
+	return ttlStrategy
+}
+
 var (
 	WorkflowCreatedAfter = func(t time.Time) WorkflowPredicate {
 		return func(wf Workflow) bool {
@@ -356,6 +375,18 @@ func (wfs WorkflowSpec) GetVolumeClaimGC() *VolumeClaimGC {
 	return wfs.VolumeClaimGC
 }
 
+func (wfs WorkflowSpec) GetTTLStrategy() *TTLStrategy {
+	if wfs.TTLSecondsAfterFinished != nil {
+		if wfs.TTLStrategy == nil {
+			ttlstrategy := TTLStrategy{SecondsAfterCompletion: wfs.TTLSecondsAfterFinished}
+			wfs.TTLStrategy = &ttlstrategy
+		} else if wfs.TTLStrategy.SecondsAfterCompletion == nil {
+			wfs.TTLStrategy.SecondsAfterCompletion = wfs.TTLSecondsAfterFinished
+		}
+	}
+	return wfs.TTLStrategy
+}
+
 type ShutdownStrategy string
 
 const (
diff --git a/util/printer/workflow-printer.go b/util/printer/workflow-printer.go
index 67a388832..3ca6ddb5a 100644
--- a/util/printer/workflow-printer.go
+++ b/util/printer/workflow-printer.go
@@ -16,6 +16,11 @@ import (
 )
 
 func PrintWorkflows(workflows wfv1.Workflows, out io.Writer, opts PrintOpts) error {
+	if len(workflows) == 0 {
+		_, _ = fmt.Fprintln(out, "No workflows found")
+		return nil
+	}
+
 	switch opts.Output {
 	case "", "wide":
 		printTable(workflows, out, opts)
diff --git a/workflow/controller/controller.go b/workflow/controller/controller.go
index 21e80fc6b..543ca9596 100644
--- a/workflow/controller/controller.go
+++ b/workflow/controller/controller.go
@@ -148,7 +148,9 @@ func NewWorkflowController(restConfig *rest.Config, kubeclientset kubernetes.Int
 
 // RunTTLController runs the workflow TTL controller
 func (wfc *WorkflowController) runTTLController(ctx context.Context) {
-	ttlCtrl := ttlcontroller.NewController(wfc.wfclientset, wfc.wfInformer)
+	ttlCtrl := ttlcontroller.NewController(wfc.wfclientset, wfc.wfInformer, func() *config.Config {
+		return &wfc.Config
+	})
 	err := ttlCtrl.Run(ctx.Done())
 	if err != nil {
 		panic(err)
diff --git a/workflow/ttlcontroller/ttlcontroller.go b/workflow/ttlcontroller/ttlcontroller.go
index 294b4bcc0..386415956 100644
--- a/workflow/ttlcontroller/ttlcontroller.go
+++ b/workflow/ttlcontroller/ttlcontroller.go
@@ -14,6 +14,7 @@ import (
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/util/workqueue"
 
+	"github.com/argoproj/argo/config"
 	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
 	wfclientset "github.com/argoproj/argo/pkg/client/clientset/versioned"
 	commonutil "github.com/argoproj/argo/util"
@@ -25,22 +26,26 @@ const (
 	workflowTTLResyncPeriod = 20 * time.Minute
 )
 
+type ConfigSupplier func() *config.Config
+
 type Controller struct {
-	wfclientset  wfclientset.Interface
-	wfInformer   cache.SharedIndexInformer
-	workqueue    workqueue.DelayingInterface
-	resyncPeriod time.Duration
-	clock        clock.Clock
+	wfclientset    wfclientset.Interface
+	wfInformer     cache.SharedIndexInformer
+	workqueue      workqueue.DelayingInterface
+	resyncPeriod   time.Duration
+	clock          clock.Clock
+	configSupplier ConfigSupplier
 }
 
 // NewController returns a new workflow ttl controller
-func NewController(wfClientset wfclientset.Interface, wfInformer cache.SharedIndexInformer) *Controller {
+func NewController(wfClientset wfclientset.Interface, wfInformer cache.SharedIndexInformer, configSupplier ConfigSupplier) *Controller {
 	controller := &Controller{
-		wfclientset:  wfClientset,
-		wfInformer:   wfInformer,
-		workqueue:    workqueue.NewDelayingQueue(),
-		resyncPeriod: workflowTTLResyncPeriod,
-		clock:        clock.RealClock{},
+		wfclientset:    wfClientset,
+		wfInformer:     wfInformer,
+		workqueue:      workqueue.NewDelayingQueue(),
+		resyncPeriod:   workflowTTLResyncPeriod,
+		clock:          clock.RealClock{},
+		configSupplier: configSupplier,
 	}
 
 	wfInformer.AddEventHandler(cache.FilteringResourceEventHandler{
@@ -132,7 +137,7 @@ func (c *Controller) enqueueWF(obj interface{}) {
 		return
 	}
 	now := c.clock.Now()
-	remaining, expiration := timeLeft(wf, &now)
+	remaining, expiration := timeLeft(wf, &now, c.getDefaultTTLStrategy())
 	if remaining == nil || *remaining > c.resyncPeriod {
 		return
 	}
@@ -187,9 +192,17 @@ func (c *Controller) deleteWorkflow(key string) error {
 	}
 	return nil
 }
+func (c *Controller) getDefaultTTLStrategy() *wfv1.TTLStrategy {
+	wfDefault := c.configSupplier().WorkflowDefaults
+	if wfDefault != nil {
+		return wfDefault.Spec.GetTTLStrategy()
+	}
+	return nil
+}
 
 func (c *Controller) ttlExpired(wf *wfv1.Workflow) bool {
-	ttlStrategy := getTTLStrategy(wf)
+	ttlStrategy := wf.GetTTLStrategy(c.getDefaultTTLStrategy())
+
 	// We don't care about the Workflows that are going to be deleted, or the ones that don't need clean up.
 	if wf.DeletionTimestamp != nil || ttlStrategy == nil || wf.Status.FinishedAt.IsZero() {
 		return false
@@ -208,8 +221,8 @@ func (c *Controller) ttlExpired(wf *wfv1.Workflow) bool {
 	}
 }
 
-func timeLeft(wf *wfv1.Workflow, since *time.Time) (*time.Duration, *time.Time) {
-	ttlStrategy := getTTLStrategy(wf)
+func timeLeft(wf *wfv1.Workflow, since *time.Time, defaultTTLStrategy *wfv1.TTLStrategy) (*time.Duration, *time.Time) {
+	ttlStrategy := wf.GetTTLStrategy(defaultTTLStrategy)
 	if wf.DeletionTimestamp != nil || ttlStrategy == nil || wf.Status.FinishedAt.IsZero() {
 		return nil, nil
 	}
@@ -235,12 +248,3 @@ func timeLeft(wf *wfv1.Workflow, since *time.Time) (*time.Duration, *time.Time)
 		return nil, nil
 	}
 }
-
-func getTTLStrategy(wf *wfv1.Workflow) *wfv1.TTLStrategy {
-	ttlStrategy := wf.Spec.TTLStrategy
-
-	if ttlStrategy == nil && wf.Status.StoredWorkflowSpec != nil && wf.Status.StoredWorkflowSpec.TTLStrategy != nil {
-		return wf.Status.StoredWorkflowSpec.TTLStrategy
-	}
-	return ttlStrategy
-}
diff --git a/workflow/util/util.go b/workflow/util/util.go
index 462089425..95a1fd74c 100644
--- a/workflow/util/util.go
+++ b/workflow/util/util.go
@@ -120,14 +120,6 @@ func NewWorkflowLister(informer cache.SharedIndexInformer) WorkflowLister {
 func FromUnstructured(un *unstructured.Unstructured) (*wfv1.Workflow, error) {
 	var wf wfv1.Workflow
 	err := runtime.DefaultUnstructuredConverter.FromUnstructured(un.Object, &wf)
-	if wf.Spec.TTLSecondsAfterFinished != nil {
-		if wf.Spec.TTLStrategy == nil {
-			ttlstrategy := wfv1.TTLStrategy{SecondsAfterCompletion: wf.Spec.TTLSecondsAfterFinished}
-			wf.Spec.TTLStrategy = &ttlstrategy
-		} else if wf.Spec.TTLStrategy.SecondsAfterCompletion == nil {
-			wf.Spec.TTLStrategy.SecondsAfterCompletion = wf.Spec.TTLSecondsAfterFinished
-		}
-	}
 	return &wf, err
 }
 


Test patch:
diff --git a/cmd/argo/commands/list_test.go b/cmd/argo/commands/list_test.go
index 936279606..81f027681 100644
--- a/cmd/argo/commands/list_test.go
+++ b/cmd/argo/commands/list_test.go
@@ -17,6 +17,12 @@ import (
 )
 
 func Test_listWorkflows(t *testing.T) {
+	t.Run("Empty", func(t *testing.T) {
+		workflows, err := listEmpty(&metav1.ListOptions{}, listFlags{})
+		if assert.NoError(t, err) {
+			assert.Len(t, workflows, 0)
+		}
+	})
 	t.Run("Nothing", func(t *testing.T) {
 		workflows, err := list(&metav1.ListOptions{}, listFlags{})
 		if assert.NoError(t, err) {
@@ -87,3 +93,10 @@ func list(listOptions *metav1.ListOptions, flags listFlags) (wfv1.Workflows, err
 	workflows, err := listWorkflows(context.Background(), c, flags)
 	return workflows, err
 }
+
+func listEmpty(listOptions *metav1.ListOptions, flags listFlags) (wfv1.Workflows, error) {
+	c := &workflowmocks.WorkflowServiceClient{}
+	c.On("ListWorkflows", mock.Anything, &workflow.WorkflowListRequest{ListOptions: listOptions}).Return(&wfv1.WorkflowList{Items: wfv1.Workflows{}}, nil)
+	workflows, err := listWorkflows(context.Background(), c, flags)
+	return workflows, err
+}
diff --git a/pkg/apis/workflow/v1alpha1/workflow_types_test.go b/pkg/apis/workflow/v1alpha1/workflow_types_test.go
index d192fc5d5..9bcdaeb85 100644
--- a/pkg/apis/workflow/v1alpha1/workflow_types_test.go
+++ b/pkg/apis/workflow/v1alpha1/workflow_types_test.go
@@ -222,3 +222,35 @@ func TestWorkflowSpec_GetVolumeGC(t *testing.T) {
 	assert.NotNil(t, spec.GetVolumeClaimGC())
 	assert.Equal(t, &VolumeClaimGC{Strategy: VolumeClaimGCOnSuccess}, spec.GetVolumeClaimGC())
 }
+
+func TestGetTTLStrategy(t *testing.T) {
+
+	spec := WorkflowSpec{TTLSecondsAfterFinished: pointer.Int32Ptr(10)}
+	ttl := spec.GetTTLStrategy()
+	assert.Equal(t, int32(10), *ttl.SecondsAfterCompletion)
+
+	spec = WorkflowSpec{TTLSecondsAfterFinished: pointer.Int32Ptr(10), TTLStrategy: &TTLStrategy{SecondsAfterCompletion: pointer.Int32Ptr(20)}}
+	ttl = spec.GetTTLStrategy()
+	assert.Equal(t, int32(20), *ttl.SecondsAfterCompletion)
+
+	spec = WorkflowSpec{TTLStrategy: &TTLStrategy{SecondsAfterCompletion: pointer.Int32Ptr(20)}}
+	ttl = spec.GetTTLStrategy()
+	assert.Equal(t, int32(20), *ttl.SecondsAfterCompletion)
+}
+
+func TestWfGetTTLStrategy(t *testing.T) {
+	ttl := TTLStrategy{
+		SecondsAfterCompletion: pointer.Int32Ptr(10),
+	}
+	wf := Workflow{}
+	result := wf.GetTTLStrategy(&ttl)
+	assert.Equal(t, int32(10), *result.SecondsAfterCompletion)
+
+	wf.Status.StoredWorkflowSpec = &WorkflowSpec{TTLStrategy: &TTLStrategy{SecondsAfterCompletion: pointer.Int32Ptr(20)}}
+	result = wf.GetTTLStrategy(&ttl)
+	assert.Equal(t, int32(20), *result.SecondsAfterCompletion)
+
+	wf.Spec.TTLStrategy = &TTLStrategy{SecondsAfterCompletion: pointer.Int32Ptr(30)}
+	result = wf.GetTTLStrategy(&ttl)
+	assert.Equal(t, int32(30), *result.SecondsAfterCompletion)
+}
diff --git a/util/printer/workflow-printer_test.go b/util/printer/workflow-printer_test.go
index ed8dc01d1..afdbc317b 100644
--- a/util/printer/workflow-printer_test.go
+++ b/util/printer/workflow-printer_test.go
@@ -42,6 +42,14 @@ func TestPrintWorkflows(t *testing.T) {
 			},
 		},
 	}
+
+	var emptyWorkflows wfv1.Workflows
+	t.Run("Empty", func(t *testing.T) {
+		var b bytes.Buffer
+		assert.NoError(t, PrintWorkflows(emptyWorkflows, &b, PrintOpts{}))
+		assert.Equal(t, `No workflows found
+`, b.String())
+	})
 	t.Run("Default", func(t *testing.T) {
 		var b bytes.Buffer
 		assert.NoError(t, PrintWorkflows(workflows, &b, PrintOpts{}))
diff --git a/workflow/ttlcontroller/ttlcontroller_test.go b/workflow/ttlcontroller/ttlcontroller_test.go
index 7951c97bc..a07e90f20 100644
--- a/workflow/ttlcontroller/ttlcontroller_test.go
+++ b/workflow/ttlcontroller/ttlcontroller_test.go
@@ -10,7 +10,9 @@ import (
 	"k8s.io/apimachinery/pkg/util/clock"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/util/workqueue"
+	"k8s.io/utils/pointer"
 
+	"github.com/argoproj/argo/config"
 	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
 	fakewfclientset "github.com/argoproj/argo/pkg/client/clientset/versioned/fake"
 	"github.com/argoproj/argo/test"
@@ -338,6 +340,7 @@ status:
     ttlStrategy:
       secondsAfterCompletion: 60
 `
+var testConfig config.Config
 
 func newTTLController() *Controller {
 	clock := clock.NewFakeClock(time.Now())
@@ -349,6 +352,9 @@ func newTTLController() *Controller {
 		resyncPeriod: workflowTTLResyncPeriod,
 		clock:        clock,
 		workqueue:    workqueue.NewDelayingQueue(),
+		configSupplier: func() *config.Config {
+			return &testConfig
+		},
 	}
 }
 
@@ -599,29 +605,62 @@ func TestTTLlExpired(t *testing.T) {
 
 func TestGetTTLStrategy(t *testing.T) {
 	var ten int32 = 10
-	wf := test.LoadWorkflowFromBytes([]byte(succeededWf))
-	wf.Spec.TTLStrategy = &wfv1.TTLStrategy{
-		SecondsAfterCompletion: &ten,
+	var twenty int32 = 20
+	var thirty int32 = 30
+	defaultTTLStrategy := &wfv1.TTLStrategy{
+		SecondsAfterCompletion: &thirty,
 	}
+	t.Run("TTLFromWfDefaults", func(t *testing.T) {
+		wf := test.LoadWorkflowFromBytes([]byte(succeededWf))
+		ttl := wf.GetTTLStrategy(defaultTTLStrategy)
+		assert.NotNil(t, ttl)
+		assert.Equal(t, thirty, *ttl.SecondsAfterCompletion)
+	})
+
+	t.Run("TTLFromWorkflow", func(t *testing.T) {
+		wf := test.LoadWorkflowFromBytes([]byte(succeededWf))
+		wf.Spec.TTLStrategy = &wfv1.TTLStrategy{
+			SecondsAfterCompletion: &ten,
+		}
+		ttl := wf.GetTTLStrategy(defaultTTLStrategy)
+		assert.NotNil(t, ttl)
+		assert.Equal(t, ten, *ttl.SecondsAfterCompletion)
+	})
+
+	t.Run("TTLInWfwithWorkflowTemplate", func(t *testing.T) {
+		wf1 := test.LoadWorkflowFromBytes([]byte(wftRefWithTTLinWF))
+		ttl := wf1.GetTTLStrategy(defaultTTLStrategy)
+		assert.NotNil(t, ttl)
+		assert.Equal(t, ten, *ttl.SecondsAfterCompletion)
+
+		wf1.Spec.TTLStrategy = nil
+		wf1.Status.StoredWorkflowSpec.TTLStrategy = nil
+		ttl = wf1.GetTTLStrategy(nil)
+		assert.Nil(t, ttl)
+	})
+	t.Run("TTLwithWorkflowTemplate", func(t *testing.T) {
+		wf2 := test.LoadWorkflowFromBytes([]byte(wftRefWithTTLinWFT))
+		wf2.Spec.TTLSecondsAfterFinished = nil
+		wf2.Status.StoredWorkflowSpec.TTLSecondsAfterFinished = &twenty
+		ttl := wf2.GetTTLStrategy(defaultTTLStrategy)
+		assert.NotNil(t, ttl)
+		assert.Equal(t, ten, *ttl.SecondsAfterCompletion)
+		wf2.Status.StoredWorkflowSpec.TTLSecondsAfterFinished = nil
+		wf2.Status.StoredWorkflowSpec.TTLStrategy = nil
+		ttl = wf2.GetTTLStrategy(nil)
+		assert.Nil(t, ttl)
+	})
+}
 
-	ttl := getTTLStrategy(wf)
-	assert.NotNil(t, ttl)
-	assert.Equal(t, ten, *ttl.SecondsAfterCompletion)
-
-	wf1 := test.LoadWorkflowFromBytes([]byte(wftRefWithTTLinWF))
-	ttl = getTTLStrategy(wf1)
-	assert.NotNil(t, ttl)
-	assert.Equal(t, ten, *ttl.SecondsAfterCompletion)
-	wf1.Spec.TTLStrategy = nil
-	wf1.Status.StoredWorkflowSpec.TTLStrategy = nil
-	ttl = getTTLStrategy(wf1)
-	assert.Nil(t, ttl)
-
-	wf2 := test.LoadWorkflowFromBytes([]byte(wftRefWithTTLinWFT))
-	ttl = getTTLStrategy(wf2)
-	assert.NotNil(t, ttl)
-	assert.Equal(t, ten, *ttl.SecondsAfterCompletion)
-	wf2.Status.StoredWorkflowSpec.TTLStrategy = nil
-	ttl = getTTLStrategy(wf2)
+func TestGetDefaultTTLStrategy(t *testing.T) {
+	controller := newTTLController()
+	testConfig = config.Config{WorkflowDefaults: &wfv1.Workflow{Spec: wfv1.WorkflowSpec{TTLSecondsAfterFinished: pointer.Int32Ptr(10)}}}
+	ttl := controller.getDefaultTTLStrategy()
+	assert.Equal(t, int32(10), *ttl.SecondsAfterCompletion)
+	testConfig = config.Config{WorkflowDefaults: &wfv1.Workflow{Spec: wfv1.WorkflowSpec{TTLSecondsAfterFinished: pointer.Int32Ptr(20)}}}
+	ttl = controller.getDefaultTTLStrategy()
+	assert.Equal(t, int32(20), *ttl.SecondsAfterCompletion)
+	testConfig = config.Config{WorkflowDefaults: nil}
+	ttl = controller.getDefaultTTLStrategy()
 	assert.Nil(t, ttl)
 }