feat: pause duration as string with time unit

.codecov.yml

@@ -6,4 +6,6 @@ coverage:
         threshold: 0.1
     project:
       default:
-        threshold: 0.1
+        threshold: 0.1
+ignore:
+- "pkg/apis/rollouts/v1alpha1"

docs/features/canary.md

@@ -36,9 +36,32 @@ spec:
       steps:
       - setWeight: 10
       - pause:
-          duration: 3600 # 1 hour
+          duration: "1h" # 1 hour
       - setWeight: 20
-      - pause: {}
+      - pause: {} # pause indefinitely
+```
+
+### Pause Duration
+Pause duration can be specied with an optional time unit suffix. Valid time units are "s", "m", "h". Defaults to "s" if not specified. Values less than zero are not allowed. 
+
+```yaml
+spec:
+  strategy:
+    canary:
+      steps:
+        - pause: { duration: 10 }  # 10 seconds
+        - pause: { duration: 10s } # 10 seconds
+        - pause: { duration: 10m } # 10 minutes
+        - pause: { duration: 10h } # 10 hours
+        - pause: { duration: -10 } # invalid spec!
+        - pause: {}                # pause indefinitely
+```
+
+If no `duration` specified for a pause step the rollout will be paused indefinitely. To unpause use the [argo kubectl plugin](kubectl-plugin.md) `promote` command. 
+
+```shell
+# promote to the next step
+kubectl argo rollouts promote <rollout>
 ```
 
 ## Mimicking Rolling Update
@@ -59,11 +82,12 @@ spec:
 `maxSurge` defines the maximum number of replicas the rollout can create to move to the correct ratio set by the last setWeight. Max Surge can either be an integer or percentage as a string (i.e. "20%")
 
 Defaults to "25%".
+
 ### maxUnavailable
 The maximum number of pods that can be unavailable during the update. Value can be an absolute number (ex: 5) or a percentage of desired pods (ex: 10%). This can not be 0 if MaxSurge is 0.
     // Defaults to 0
 
-### CanaryService
+### canaryService
 `canaryService` references a Service that will be modified to send traffic to only the canary ReplicaSet. This allows users to only hit the canary ReplicaSet.
 
 Defaults to an empty string

docs/features/index.md

@@ -63,7 +63,7 @@ spec:
       - setWeight: 20 
         # Pauses the rollout for an hour
       - pause:
-          duration: 3600 # One hour
+          duration: "1h" # One hour
       - setWeight: 40
         # Sets .spec.paused to true and waits until the field is changed back
       - pause: {} 

hack/update-codegen.sh

@@ -6,11 +6,10 @@ set -o pipefail
 # code-generator does work with go.mod but makes assumptions about the project living in `$GOPATH/src`. 
 # To work around this and support any location: 
 #   create a temporary directory, use this as an output base, and copy everything back once generated.
-
+export GOPATH=$(go env GOPATH) # export gopath so it's available to generate scripts
 SCRIPT_ROOT="$( cd "$( dirname "${BASH_SOURCE[0]}" )/.." >/dev/null 2>&1 && pwd )"
 CODEGEN_VERSION=$(go list -m k8s.io/code-generator | awk '{print $2}' | head -1)
-CODEGEN_PKG=$(echo `go env GOPATH`"/pkg/mod/k8s.io/code-generator@${CODEGEN_VERSION}")
-
+CODEGEN_PKG="${GOPATH}/pkg/mod/k8s.io/code-generator@${CODEGEN_VERSION}"
 TEMP_DIR=$(mktemp -d)
 cleanup() {
     rm -rf ${TEMP_DIR}

hack/update-openapigen.sh

@@ -4,9 +4,10 @@ set -o errexit
 set -o nounset
 set -o pipefail
 
+export GOPATH=$(go env GOPATH) # export gopath do generator output goes to the correct location
 PROJECT_ROOT=$(cd $(dirname "$0")/.. ; pwd)
 CODEGEN_VERSION=$(go list -m k8s.io/kube-openapi | awk '{print $2}' | head -1)
-CODEGEN_PKG=$(echo `go env GOPATH`"/pkg/mod/k8s.io/kube-openapi@${CODEGEN_VERSION}")
+CODEGEN_PKG="${GOPATH}/pkg/mod/k8s.io/kube-openapi@${CODEGEN_VERSION}"
 VERSION="v1alpha1"
 
 go run ${CODEGEN_PKG}/cmd/openapi-gen/openapi-gen.go \

manifests/crds/rollout-crd.yaml

@@ -276,8 +276,9 @@ spec:
                           pause:
                             properties:
                               duration:
-                                format: int32
-                                type: integer
+                                anyOf:
+                                - type: integer
+                                - type: string
                             type: object
                           setWeight:
                             format: int32

manifests/install.yaml

@@ -8251,8 +8251,9 @@ spec:
                           pause:
                             properties:
                               duration:
-                                format: int32
-                                type: integer
+                                anyOf:
+                                - type: integer
+                                - type: string
                             type: object
                           setWeight:
                             format: int32

manifests/namespace-install.yaml

@@ -8251,8 +8251,9 @@ spec:
                           pause:
                             properties:
                               duration:
-                                format: int32
-                                type: integer
+                                anyOf:
+                                - type: integer
+                                - type: string
                             type: object
                           setWeight:
                             format: int32

pkg/apis/rollouts/v1alpha1/types.go

@@ -1,6 +1,9 @@
 package v1alpha1
 
 import (
+	"strconv"
+	"time"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -321,7 +324,42 @@ const (
 type RolloutPause struct {
 	// Duration the amount of time to wait before moving to the next step.
 	// +optional
-	Duration *int32 `json:"duration,omitempty"`
+	Duration *intstr.IntOrString `json:"duration,omitempty"`
+}
+
+// DurationSeconds converts the pause duration to seconds
+// If Duration is nil 0 is returned
+// if Duration values is string and does not contain a valid unit -1 is returned
+func (p RolloutPause) DurationSeconds() int32 {
+	if p.Duration != nil {
+		if p.Duration.Type == intstr.String {
+			s, err := strconv.Atoi(p.Duration.StrVal)
+			if err != nil {
+				d, err := time.ParseDuration(p.Duration.StrVal)
+				if err != nil {
+					return -1
+				}
+				return int32(d.Seconds())
+			}
+			// special case where no unit was specified
+			return int32(s)
+		}
+		return p.Duration.IntVal
+	}
+	return 0
+}
+
+// DurationFromInt creates duration in seconds from int value
+func DurationFromInt(i int) *intstr.IntOrString {
+	d := intstr.FromInt(i)
+	return &d
+}
+
+// DurationFromString creates duration from string
+// value must be a string representation of an int with optional time unit (see time.ParseDuration)
+func DurationFromString(s string) *intstr.IntOrString {
+	d := intstr.FromString(s)
+	return &d
 }
 
 // PauseReason reasons that the rollout can pause

pkg/apis/rollouts/v1alpha1/types_test.go

@@ -0,0 +1,24 @@
+package v1alpha1
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRolloutPauseDuration(t *testing.T) {
+	rp := RolloutPause{}
+	assert.Equal(t, int32(0), rp.DurationSeconds())
+	rp.Duration = DurationFromInt(10)
+	assert.Equal(t, int32(10), rp.DurationSeconds())
+	rp.Duration = DurationFromString("10")
+	assert.Equal(t, int32(10), rp.DurationSeconds())
+	rp.Duration = DurationFromString("10s")
+	assert.Equal(t, int32(10), rp.DurationSeconds())
+	rp.Duration = DurationFromString("1h")
+	assert.Equal(t, int32(3600), rp.DurationSeconds())
+	rp.Duration = DurationFromString("1ms")
+	assert.Equal(t, int32(0), rp.DurationSeconds())
+	rp.Duration = DurationFromString("1z")
+	assert.Equal(t, int32(-1), rp.DurationSeconds())
+}

pkg/kubectl-argo-rollouts/cmd/list/list_test.go

@@ -35,7 +35,7 @@ func newCanaryRollout() *v1alpha1.Rollout {
 						},
 						{
 							Pause: &v1alpha1.RolloutPause{
-								Duration: pointer.Int32Ptr(60),
+								Duration: v1alpha1.DurationFromInt(60),
 							},
 						},
 						{

pkg/kubectl-argo-rollouts/info/info_test.go

@@ -28,7 +28,7 @@ func newCanaryRollout() *v1alpha1.Rollout {
 						},
 						{
 							Pause: &v1alpha1.RolloutPause{
-								Duration: pointer.Int32Ptr(60),
+								Duration: v1alpha1.DurationFromInt(60),
 							},
 						},
 						{

rollout/canary.go

@@ -147,7 +147,7 @@ func (c *RolloutController) reconcileCanaryPause(roCtx *canaryContext) bool {
 	if currentStep.Pause.Duration == nil {
 		return true
 	}
-	c.checkEnqueueRolloutDuringWait(rollout, cond.StartTime, *currentStep.Pause.Duration)
+	c.checkEnqueueRolloutDuringWait(rollout, cond.StartTime, currentStep.Pause.DurationSeconds())
 	return true
 }
 

rollout/canary_test.go

@@ -798,7 +798,7 @@ func TestSyncRolloutWaitAddToQueue(t *testing.T) {
 			SetWeight: int32Ptr(10),
 		}, {
 			Pause: &v1alpha1.RolloutPause{
-				Duration: int32Ptr(10),
+				Duration: v1alpha1.DurationFromInt(10),
 			},
 		},
 	}
@@ -838,7 +838,7 @@ func TestSyncRolloutIgnoreWaitOutsideOfReconciliationPeriod(t *testing.T) {
 		},
 		{
 			Pause: &v1alpha1.RolloutPause{
-				Duration: int32Ptr(int32(3600)), //1 hour
+				Duration: v1alpha1.DurationFromInt(3600), //1 hour
 			},
 		},
 	}
@@ -877,7 +877,7 @@ func TestSyncRolloutWaitIncrementStepIndex(t *testing.T) {
 		},
 		{
 			Pause: &v1alpha1.RolloutPause{
-				Duration: int32Ptr(5),
+				Duration: v1alpha1.DurationFromInt(5),
 			},
 		}, {
 			Pause: &v1alpha1.RolloutPause{},
@@ -1111,7 +1111,7 @@ func TestResumeRolloutAfterPauseDuration(t *testing.T) {
 		},
 		{
 			Pause: &v1alpha1.RolloutPause{
-				Duration: pointer.Int32Ptr(60),
+				Duration: v1alpha1.DurationFromInt(60),
 			},
 		},
 		{
@@ -1159,7 +1159,7 @@ func TestNoResumeAfterPauseDurationIfUserPaused(t *testing.T) {
 		},
 		{
 			Pause: &v1alpha1.RolloutPause{
-				Duration: pointer.Int32Ptr(60),
+				Duration: v1alpha1.DurationFromInt(60),
 			},
 		},
 		{
@@ -1200,7 +1200,7 @@ func TestHandleNilNewRSOnScaleAndImageChange(t *testing.T) {
 		},
 		{
 			Pause: &v1alpha1.RolloutPause{
-				Duration: pointer.Int32Ptr(60),
+				Duration: v1alpha1.DurationFromInt(60),
 			},
 		},
 		{

rollout/pause.go

@@ -143,7 +143,7 @@ func (pCtx *pauseContext) CompletedPauseStep(pause v1alpha1.RolloutPause) bool {
 	if pause.Duration != nil {
 		now := metav1.Now()
 		if pauseCondition != nil {
-			expiredTime := pauseCondition.StartTime.Add(time.Duration(*pause.Duration) * time.Second)
+			expiredTime := pauseCondition.StartTime.Add(time.Duration(pause.DurationSeconds()) * time.Second)
 			if now.After(expiredTime) {
 				pCtx.log.Info("Rollout has waited the duration of the pause step")
 				return true

utils/conditions/conditions.go

@@ -339,7 +339,7 @@ func VerifyRolloutSpec(rollout *v1alpha1.Rollout, prevCond *v1alpha1.RolloutCond
 			if step.SetWeight != nil && (*step.SetWeight < 0 || *step.SetWeight > 100) {
 				return newInvalidSpecRolloutCondition(prevCond, InvalidSpecReason, InvalidSetWeightMessage)
 			}
-			if step.Pause != nil && step.Pause.Duration != nil && *step.Pause.Duration < 0 {
+			if step.Pause != nil && step.Pause.DurationSeconds() < 0 {
 				return newInvalidSpecRolloutCondition(prevCond, InvalidSpecReason, InvalidDurationMessage)
 			}
 		}

utils/conditions/rollouts_test.go

@@ -368,7 +368,19 @@ func TestVerifyRolloutSpecCanary(t *testing.T) {
 			name: "Pause duration is not less than 0",
 			steps: []v1alpha1.CanaryStep{{
 				Pause: &v1alpha1.RolloutPause{
-					Duration: pointer.Int32Ptr(-1),
+					Duration: v1alpha1.DurationFromInt(-1),
+				},
+			}},
+
+			notValid: true,
+			reason:   InvalidSpecReason,
+			message:  InvalidDurationMessage,
+		},
+		{
+			name: "Pause duration invalid unit",
+			steps: []v1alpha1.CanaryStep{{
+				Pause: &v1alpha1.RolloutPause{
+					Duration: v1alpha1.DurationFromString("10z"),
 				},
 			}},