Set exec timeout repo branch #1650
Conversation
Signed-off-by: Sarfaraz Ahamad <[email protected]>
Signed-off-by: Sarfaraz Ahamad <[email protected]>
Signed-off-by: Sarfaraz Ahamad <[email protected]>
| }) | ||
|
|
||
| t.Run("ExecTimeout set", func(t *testing.T) { | ||
| logf.SetLogger(ZapLogger(true)) | ||
| a := makeTestArgoCD() | ||
| timeout := 600 | ||
| a.Spec.Repo.ExecTimeout = &timeout | ||
| timeout := "600m" |
| timeout := 600 | ||
| a.Spec.Repo.ExecTimeout = &timeout | ||
| timeout := "600m" | ||
| a.Spec.Repo.ExecTimeout = timeout |
There was a problem hiding this comment.
Can do a direct assignment a.Spec.Repo.ExecTimeout = 600s
| @@ -419,7 +419,7 @@ func TestReconcileArgoCD_reconcileRepoDeployment_env(t *testing.T) { | |||
|
|
|||
| // Check that the env vars are set, Count is 2 because of the default REDIS_PASSWORD env var | |||
| assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Env, 2) | |||
| assert.Contains(t, deployment.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{Name: "ARGOCD_EXEC_TIMEOUT", Value: "600s"}) | |||
| assert.Contains(t, deployment.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{Name: "ARGOCD_EXEC_TIMEOUT", Value: "600m"}) | |||
There was a problem hiding this comment.
Let's keep the same value as 600s instead of switching to minute.
| @@ -1672,7 +1672,7 @@ spec: | |||
| type: object | |||
| type: array | |||
| execTimeout: | |||
| description: ExecTimeout specifies the timeout in seconds for | |||
| description: ExecTimeout specifies the timeout in minutes for | |||
There was a problem hiding this comment.
It can be seconds, minutes or hours based on the suffix user provides. This description needs a change.
|
@anandf - What are you thoughts on this? To me, looks like a breaking change. Might result into upgrade failures for users who have this field set in their ArgoCD CR.
logger.go:42: 14:19:11 | 1-011_validate_repo_exec_timeout/2-change-exec-timeout | starting test step 2-change-exec-timeout
case.go:254: failed in step 2-change-exec-timeout
case.go:256: ArgoCD.argoproj.io "argocd" is invalid: [spec.repo.execTimeout: Invalid value: "integer": spec.repo.execTimeout in body must be of type string: "integer", <nil>: Invalid value: "null": some validation rules were not checked because the object was invalid; correct the existing errors to complete validation]
|
Signed-off-by: Sarfaraz Ahamad <[email protected]>
…goproj.io_argocds.yaml Signed-off-by: Sarfaraz Ahamad <[email protected]>
sarfarazgit
force-pushed
the
Set_execTimeoutRepo_branch
branch
from
870a4bb to
3038f5b
Compare
| @@ -454,7 +454,7 @@ type ArgoCDRepoSpec struct { | |||
| Version string `json:"version,omitempty"` | |||
|
|
|||
| // ExecTimeout specifies the timeout in seconds for tool execution | |||
| ExecTimeout *int `json:"execTimeout,omitempty"` | |||
| ExecTimeout string `json:"execTimeout,omitempty"` | |||
There was a problem hiding this comment.
For including the validation, let's define an alias for string called Duration
// +kubebuilder:validation:Pattern:="^(0|(([0-9]+)y)?(([0-9]+)w)?(([0-9]+)d)?(([0-9]+)h)?(([0-9]+)m)?(([0-9]+)s)?(([0-9]+)ms)?)$"
type Duration string
Detailed example can be found here. https://github.com/prometheus-operator/prometheus-operator/blob/dff3575c55ee9e6423907e4bfdcb5ac4b8fe9c89/pkg/apis/monitoring/v1/types.go#L45
sarfarazgit
force-pushed
the
Set_execTimeoutRepo_branch
branch
from
923d0af to
5308b55
Compare
Signed-off-by: Sarfaraz Ahamad <[email protected]>
Signed-off-by: Sarfaraz Ahamad <[email protected]>
Signed-off-by: Sarfaraz Ahamad <[email protected]>
What type of PR is this?
What does this PR do / why we need it:
Have you updated the necessary documentation?
Which issue(s) this PR fixes:
Fixes #?
How to test changes / Special notes to the reviewer: