From fc698d2e61a3eeefedea0eae65acbbe4b99238c7 Mon Sep 17 00:00:00 2001 From: sanposhiho Date: Sat, 9 Jan 2021 01:44:06 +0900 Subject: [PATCH 1/7] Add config for set timeout on wait approval --- pkg/config/deployment.go | 11 +++++++++-- pkg/config/deployment_terraform_test.go | 1 + 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/config/deployment.go b/pkg/config/deployment.go index 813007d02b..8c006fa02f 100644 --- a/pkg/config/deployment.go +++ b/pkg/config/deployment.go @@ -66,13 +66,13 @@ type DeploymentCommitMatcher struct { // DeploymentPipeline represents the way to deploy the application. // The pipeline is triggered by changes in any of the following objects: -// - Target PodSpec (Target can be Deployment, DaemonSet, StatefullSet) +// - Target PodSpec (Target can be Deployment, DaemonSet, StatefulSet) // - ConfigMaps, Secrets that are mounted as volumes or envs in the deployment. type DeploymentPipeline struct { Stages []PipelineStage `json:"stages"` } -// PiplineStage represents a single stage of a pipeline. +// PipelineStage represents a single stage of a pipeline. // This is used as a generic struct for all stage type. type PipelineStage struct { Id string @@ -133,6 +133,9 @@ func (s *PipelineStage) UnmarshalJSON(data []byte) error { if len(gs.With) > 0 { err = json.Unmarshal(gs.With, s.WaitApprovalStageOptions) } + if s.WaitApprovalStageOptions.Timeout <= 0 { + s.WaitApprovalStageOptions.Timeout = defaultWaitApprovalTimeout + } case model.StageAnalysis: s.AnalysisStageOptions = &AnalysisStageOptions{} if len(gs.With) > 0 { @@ -220,9 +223,13 @@ type WaitStageOptions struct { // WaitStageOptions contains all configurable values for a WAIT_APPROVAL stage. type WaitApprovalStageOptions struct { + // number of seconds to timeout wait approval + Timeout int64 `json:"timeout"` Approvers []string `json:"approvers"` } +var defaultWaitApprovalTimeout int64 = 600 + // AnalysisStageOptions contains all configurable values for a K8S_ANALYSIS stage. type AnalysisStageOptions struct { // How long the analysis process should be executed. diff --git a/pkg/config/deployment_terraform_test.go b/pkg/config/deployment_terraform_test.go index 2b63cb7a72..61a1cf0d88 100644 --- a/pkg/config/deployment_terraform_test.go +++ b/pkg/config/deployment_terraform_test.go @@ -90,6 +90,7 @@ func TestTerraformDeploymentConfig(t *testing.T) { Name: model.StageWaitApproval, WaitApprovalStageOptions: &WaitApprovalStageOptions{ Approvers: []string{"foo", "bar"}, + Timeout: defaultWaitApprovalTimeout, // set default timeout value when nothing sets on config }, }, { From a3915669ae022492ff9cb468279404e9a478691a Mon Sep 17 00:00:00 2001 From: sanposhiho Date: Sat, 9 Jan 2021 01:44:49 +0900 Subject: [PATCH 2/7] Return failure when timeout --- pkg/app/piped/executor/waitapproval/waitapproval.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/app/piped/executor/waitapproval/waitapproval.go b/pkg/app/piped/executor/waitapproval/waitapproval.go index 3ec1c0f976..7ee7e3f939 100644 --- a/pkg/app/piped/executor/waitapproval/waitapproval.go +++ b/pkg/app/piped/executor/waitapproval/waitapproval.go @@ -55,6 +55,8 @@ func (e *Executor) Execute(sig executor.StopSignal) model.StageStatus { ) defer ticker.Stop() + timer := time.NewTimer(time.Duration(e.StageConfig.WaitApprovalStageOptions.Timeout) * time.Second) + e.LogPersister.Info("Waiting for an approval...") for { select { @@ -73,6 +75,9 @@ func (e *Executor) Execute(sig executor.StopSignal) model.StageStatus { default: return model.StageStatus_STAGE_FAILURE } + case <-timer.C: + e.LogPersister.Info("Timeout wait approval") + return model.StageStatus_STAGE_FAILURE } } } From c5f82fd57af21159117103b03dcad63559ddab3b Mon Sep 17 00:00:00 2001 From: sanposhiho Date: Mon, 11 Jan 2021 22:59:47 +0900 Subject: [PATCH 3/7] Fix comment --- pkg/config/deployment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/deployment.go b/pkg/config/deployment.go index 8c006fa02f..15c391bb85 100644 --- a/pkg/config/deployment.go +++ b/pkg/config/deployment.go @@ -223,7 +223,7 @@ type WaitStageOptions struct { // WaitStageOptions contains all configurable values for a WAIT_APPROVAL stage. type WaitApprovalStageOptions struct { - // number of seconds to timeout wait approval + // The maximum length of time to wait before giving up. Timeout int64 `json:"timeout"` Approvers []string `json:"approvers"` } From b733c0e1d89c12965f88aa683d9cce10f353265f Mon Sep 17 00:00:00 2001 From: sanposhiho Date: Mon, 11 Jan 2021 23:09:09 +0900 Subject: [PATCH 4/7] Use Duration instead of int64 --- pkg/config/deployment.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/config/deployment.go b/pkg/config/deployment.go index 15c391bb85..bdcf7307c1 100644 --- a/pkg/config/deployment.go +++ b/pkg/config/deployment.go @@ -17,6 +17,7 @@ package config import ( "encoding/json" "fmt" + "time" "github.com/pipe-cd/pipe/pkg/model" ) @@ -224,11 +225,11 @@ type WaitStageOptions struct { // WaitStageOptions contains all configurable values for a WAIT_APPROVAL stage. type WaitApprovalStageOptions struct { // The maximum length of time to wait before giving up. - Timeout int64 `json:"timeout"` + Timeout Duration `json:"timeout"` Approvers []string `json:"approvers"` } -var defaultWaitApprovalTimeout int64 = 600 +var defaultWaitApprovalTimeout = Duration(6 * time.Hour) // AnalysisStageOptions contains all configurable values for a K8S_ANALYSIS stage. type AnalysisStageOptions struct { From cc58dc219b8e1bdbe84fa5b1daed326de1dcd84c Mon Sep 17 00:00:00 2001 From: sanposhiho Date: Mon, 11 Jan 2021 23:10:03 +0900 Subject: [PATCH 5/7] Fix error message --- pkg/app/piped/executor/waitapproval/waitapproval.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/app/piped/executor/waitapproval/waitapproval.go b/pkg/app/piped/executor/waitapproval/waitapproval.go index 7ee7e3f939..f2f4e0b908 100644 --- a/pkg/app/piped/executor/waitapproval/waitapproval.go +++ b/pkg/app/piped/executor/waitapproval/waitapproval.go @@ -76,7 +76,7 @@ func (e *Executor) Execute(sig executor.StopSignal) model.StageStatus { return model.StageStatus_STAGE_FAILURE } case <-timer.C: - e.LogPersister.Info("Timeout wait approval") + e.LogPersister.Errorf("Timed out %v", e.StageConfig.WaitApprovalStageOptions.Timeout) return model.StageStatus_STAGE_FAILURE } } From 7790acc5ac588bb2e157614ddd5b32f426739986 Mon Sep 17 00:00:00 2001 From: sanposhiho Date: Tue, 12 Jan 2021 14:48:28 +0900 Subject: [PATCH 6/7] Move default value definition --- pkg/config/deployment.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/config/deployment.go b/pkg/config/deployment.go index bdcf7307c1..b281c27147 100644 --- a/pkg/config/deployment.go +++ b/pkg/config/deployment.go @@ -22,6 +22,8 @@ import ( "github.com/pipe-cd/pipe/pkg/model" ) +var defaultWaitApprovalTimeout = Duration(6 * time.Hour) + type GenericDeploymentSpec struct { // Forcibly use QuickSync or Pipeline when commit message matched the specified pattern. CommitMatcher DeploymentCommitMatcher `json:"commitMatcher"` @@ -229,8 +231,6 @@ type WaitApprovalStageOptions struct { Approvers []string `json:"approvers"` } -var defaultWaitApprovalTimeout = Duration(6 * time.Hour) - // AnalysisStageOptions contains all configurable values for a K8S_ANALYSIS stage. type AnalysisStageOptions struct { // How long the analysis process should be executed. From a07728e5ce77848c765383d4f007e4cbd86e7e37 Mon Sep 17 00:00:00 2001 From: sanposhiho Date: Tue, 12 Jan 2021 14:48:41 +0900 Subject: [PATCH 7/7] Move comment --- pkg/config/deployment_terraform_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/config/deployment_terraform_test.go b/pkg/config/deployment_terraform_test.go index 61a1cf0d88..24a4c715dd 100644 --- a/pkg/config/deployment_terraform_test.go +++ b/pkg/config/deployment_terraform_test.go @@ -90,7 +90,8 @@ func TestTerraformDeploymentConfig(t *testing.T) { Name: model.StageWaitApproval, WaitApprovalStageOptions: &WaitApprovalStageOptions{ Approvers: []string{"foo", "bar"}, - Timeout: defaultWaitApprovalTimeout, // set default timeout value when nothing sets on config + // Use defaultWaitApprovalTimeout on unset timeout value for WaitApprovalStage. + Timeout: defaultWaitApprovalTimeout, }, }, {