diff --git a/pkg/apis/build/v1alpha1/build_types.go b/pkg/apis/build/v1alpha1/build_types.go index 9a85f196..59d33025 100644 --- a/pkg/apis/build/v1alpha1/build_types.go +++ b/pkg/apis/build/v1alpha1/build_types.go @@ -86,7 +86,7 @@ type BuildSpec struct { // Time after which the build times out. Defaults to 10 minutes. // Specified build timeout should be less than 24h. // Refer Go's ParseDuration documentation for expected format: https://golang.org/pkg/time/#ParseDuration - Timeout string `json:"timeout,omitempty"` + Timeout metav1.Duration `json:"timeout,omitempty"` // If specified, the pod's scheduling constraints // +optional diff --git a/pkg/apis/build/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/build/v1alpha1/zz_generated.deepcopy.go index aeb8b1db..f5bc4f1f 100644 --- a/pkg/apis/build/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/build/v1alpha1/zz_generated.deepcopy.go @@ -145,6 +145,7 @@ func (in *BuildSpec) DeepCopyInto(out *BuildSpec) { (*out)[key] = val } } + out.Timeout = in.Timeout if in.Affinity != nil { in, out := &in.Affinity, &out.Affinity if *in == nil { diff --git a/pkg/builder/interface.go b/pkg/builder/interface.go index 80790954..d58df04c 100644 --- a/pkg/builder/interface.go +++ b/pkg/builder/interface.go @@ -20,6 +20,7 @@ import ( "time" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1alpha1 "github.com/knative/build/pkg/apis/build/v1alpha1" ) @@ -79,23 +80,19 @@ func IsDone(status *v1alpha1.BuildStatus) bool { // IsTimeout returns true if the build's execution time is greater than // specified build spec timeout. -func IsTimeout(status *v1alpha1.BuildStatus, buildTimeout string) bool { +func IsTimeout(status *v1alpha1.BuildStatus, buildTimeout metav1.Duration) bool { var timeout time.Duration var defaultTimeout = 10 * time.Minute - var err error if status == nil { return false } - if buildTimeout == "" { + if buildTimeout.Duration == 0 { // Set default timeout to 10 minute if build timeout is not set timeout = defaultTimeout } else { - timeout, err = time.ParseDuration(buildTimeout) - if err != nil { - return false - } + timeout = buildTimeout.Duration } // If build has not started timeout, startTime should be zero. diff --git a/pkg/controller/build/controller.go b/pkg/controller/build/controller.go index 71d5ff53..0855bdd9 100644 --- a/pkg/controller/build/controller.go +++ b/pkg/controller/build/controller.go @@ -280,7 +280,7 @@ func (c *Controller) syncHandler(key string) error { // Check if build has timed out if builder.IsTimeout(&build.Status, build.Spec.Timeout) { //cleanup operation and update status - timeoutMsg := fmt.Sprintf("Build %q failed to finish within %q", build.Name, build.Spec.Timeout) + timeoutMsg := fmt.Sprintf("Build %q failed to finish within %q", build.Name, build.Spec.Timeout.Duration.String()) if err := op.Terminate(); err != nil { c.logger.Errorf("Failed to terminate pod: %v", err) diff --git a/pkg/controller/build/controller_test.go b/pkg/controller/build/controller_test.go index fa3ee6a0..7a4cfd0e 100644 --- a/pkg/controller/build/controller_test.go +++ b/pkg/controller/build/controller_test.go @@ -67,7 +67,7 @@ func newBuild(name string) *v1alpha1.Build { Namespace: metav1.NamespaceDefault, }, Spec: v1alpha1.BuildSpec{ - Timeout: "20m", + Timeout: metav1.Duration{Duration: 20 * time.Minute}, }, } } @@ -237,7 +237,7 @@ func TestTimeoutFlows(t *testing.T) { t.Errorf("Error parsing duration") } - build.Spec.Timeout = "1s" + build.Spec.Timeout = metav1.Duration{Duration: 1 * time.Second} f := &fixture{ t: t, diff --git a/pkg/webhook/build.go b/pkg/webhook/build.go index bcb13e67..5ac41c6d 100644 --- a/pkg/webhook/build.go +++ b/pkg/webhook/build.go @@ -237,19 +237,13 @@ func validationError(reason, format string, fmtArgs ...interface{}) error { } } -func validateTimeout(timeout string) error { +func validateTimeout(timeout metav1.Duration) error { maxTimeout := time.Duration(24 * time.Hour) - if timeout == "" { - return nil - } else { - timeoutDuration, err := time.ParseDuration(timeout) - if err != nil { - return validationError("InvalidTimeFormat", "invalid build timeout %q err %#v. Refer https://golang.org/pkg/time/#ParseDuration for time format documentation", timeout, err) - } - if timeoutDuration > maxTimeout { - return validationError("InvalidTimeout", "build timeout exceeded 24h") - } + if timeout.Duration > maxTimeout { + return validationError("InvalidTimeout", "build timeout exceeded 24h") + } else if timeout.Duration < 0 { + return validationError("InvalidFormat", "build timeout should be greater than 0") } return nil } diff --git a/pkg/webhook/webhook_test.go b/pkg/webhook/webhook_test.go index c4e7876d..f258f6a1 100644 --- a/pkg/webhook/webhook_test.go +++ b/pkg/webhook/webhook_test.go @@ -21,6 +21,7 @@ import ( "encoding/json" "strings" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/mattbaird/jsonpatch" @@ -207,21 +208,21 @@ func TestValidateBuild(t *testing.T) { }, }, }, { - reason: "invalid build timeout", + reason: "negative build timeout", build: &v1alpha1.Build{ Spec: v1alpha1.BuildSpec{ - Timeout: "garbagetimeout", + Timeout: metav1.Duration{Duration: -48 * time.Hour}, Steps: []corev1.Container{{ Name: "foo", Image: "gcr.io/foo-bar/baz:latest", }}, }, - }, + }, }, { reason: "maximum timeout", build: &v1alpha1.Build{ Spec: v1alpha1.BuildSpec{ - Timeout: "48h", + Timeout: metav1.Duration{Duration: 48 * time.Hour}, Steps: []corev1.Container{{ Name: "foo", Image: "gcr.io/foo-bar/baz:latest", @@ -231,7 +232,7 @@ func TestValidateBuild(t *testing.T) { }, { build: &v1alpha1.Build{ Spec: v1alpha1.BuildSpec{ - Timeout: "1m", + Timeout: metav1.Duration{Duration: 5 * time.Minute}, Steps: []corev1.Container{{ Name: "foo", Image: "gcr.io/foo-bar/baz:latest", diff --git a/test/e2e/simple_test.go b/test/e2e/simple_test.go index 54642160..e6143434 100644 --- a/test/e2e/simple_test.go +++ b/test/e2e/simple_test.go @@ -70,7 +70,7 @@ func TestSimpleBuild(t *testing.T) { Name: buildName, }, Spec: v1alpha1.BuildSpec{ - Timeout: "40s", + Timeout: metav1.Duration{Duration: 40 * time.Second}, Steps: []corev1.Container{{ Image: "busybox", Args: []string{"echo", "simple"}, @@ -132,7 +132,7 @@ func TestBuildLowTimeout(t *testing.T) { Name: buildName, }, Spec: v1alpha1.BuildSpec{ - Timeout: buildTimeout.String(), + Timeout: metav1.Duration{Duration: buildTimeout}, Steps: []corev1.Container{{ Name: "lowtimeoutstep", Image: "ubuntu",