Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set exec timeout repo branch #1650

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Set exec timeout repo branch #1650

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0866a5d
Adding execTimeout change in yaml
sarfarazgit Jan 28, 2025
8f6f636
Adding execTimeout field chnge type as string
sarfarazgit Jan 28, 2025
798ddb5
Adding execTimeout value in seconed in yaml
sarfarazgit Jan 28, 2025
9490d55
Changing minutes to seconed in the execTimeout value
sarfarazgit Jan 29, 2025
3038f5b
Adding description to the deploy/olm-catalog/argocd-operator/0.9.1/ar…
sarfarazgit Jan 29, 2025
5308b55
Adding some changes seconed to minutes or hours to this branch
sarfarazgit Feb 4, 2025
283c9c0
Adding validation code duration
sarfarazgit Feb 4, 2025
5afcc05
Merge branch 'argoproj-labs:master' into Set_execTimeoutRepo_branch
sarfarazgit Feb 4, 2025
4b25473
Adding execTimeout code to this branch
sarfarazgit Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/v1alpha1/argocd_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


// Env lets you specify environment for repo server pods
Env []corev1.EnvVar `json:"env,omitempty"`
Expand Down
5 changes: 0 additions & 5 deletions api/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion api/v1beta1/argocd_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,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"`

// Env lets you specify environment for repo server pods
Env []corev1.EnvVar `json:"env,omitempty"`
Expand Down
5 changes: 0 additions & 5 deletions api/v1beta1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bundle/manifests/argocd-operator.clusterserviceversion.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ metadata:
capabilities: Deep Insights
categories: Integration & Delivery
certified: "false"
createdAt: "2024-12-20T09:48:39Z"
createdAt: "2025-01-28T06:26:16Z"
description: Argo CD is a declarative, GitOps continuous delivery tool for Kubernetes.
operators.operatorframework.io/builder: operator-sdk-v1.35.0
operators.operatorframework.io/project_layout: go.kubebuilder.io/v4
Expand Down
4 changes: 2 additions & 2 deletions bundle/manifests/argoproj.io_argocds.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1868,7 +1868,7 @@ spec:
execTimeout:
description: ExecTimeout specifies the timeout in seconds for
tool execution
type: integer
type: string
extraRepoCommandArgs:
description: |-
Extra Command arguments allows users to pass command line arguments to repo server workload. They get added to default command line arguments provided
Expand Down Expand Up @@ -15091,7 +15091,7 @@ spec:
execTimeout:
description: ExecTimeout specifies the timeout in seconds for
tool execution
type: integer
type: string
extraRepoCommandArgs:
description: |-
Extra Command arguments allows users to pass command line arguments to repo server workload. They get added to default command line arguments provided
Expand Down
4 changes: 2 additions & 2 deletions config/crd/bases/argoproj.io_argocds.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1857,7 +1857,7 @@ spec:
execTimeout:
description: ExecTimeout specifies the timeout in seconds for
tool execution
type: integer
type: string
extraRepoCommandArgs:
description: |-
Extra Command arguments allows users to pass command line arguments to repo server workload. They get added to default command line arguments provided
Expand Down Expand Up @@ -15080,7 +15080,7 @@ spec:
execTimeout:
description: ExecTimeout specifies the timeout in seconds for
tool execution
type: integer
type: string
extraRepoCommandArgs:
description: |-
Extra Command arguments allows users to pass command line arguments to repo server workload. They get added to default command line arguments provided
Expand Down
6 changes: 4 additions & 2 deletions controllers/argocd/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -918,8 +918,10 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argoproj.ArgoCD, useTLSFor
})
// Environment specified in the CR take precedence over everything else
repoEnv = argoutil.EnvMerge(repoEnv, proxyEnvVars(), false)
if cr.Spec.Repo.ExecTimeout != nil {
repoEnv = argoutil.EnvMerge(repoEnv, []corev1.EnvVar{{Name: "ARGOCD_EXEC_TIMEOUT", Value: fmt.Sprintf("%ds", *cr.Spec.Repo.ExecTimeout)}}, true)
if _, err := time.ParseDuration(cr.Spec.Repo.ExecTimeout); err != nil {
log.Info(fmt.Sprintf("unable to found the correct string value for execTimeout: %s", cr.Spec.Repo.ExecTimeout))
} else {
repoEnv = argoutil.EnvMerge(repoEnv, []corev1.EnvVar{{Name: "ARGOCD_EXEC_TIMEOUT", Value: cr.Spec.Repo.ExecTimeout}}, true)
}

AddSeccompProfileForOpenShift(r.Client, &deploy.Spec.Template.Spec)
Expand Down
22 changes: 11 additions & 11 deletions controllers/argocd/deployment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,8 @@ func TestReconcileArgoCD_reconcile_ServerDeployment_env(t *testing.T) {
Value: "FOO",
},
}
timeout := 600
a.Spec.Repo.ExecTimeout = &timeout
timeout := "600"
a.Spec.Repo.ExecTimeout = timeout

resObjs := []client.Object{a}
subresObjs := []client.Object{a}
Expand Down Expand Up @@ -336,8 +336,8 @@ func TestReconcileArgoCD_reconcileRepoDeployment_env(t *testing.T) {
Value: "FOO",
},
}
timeout := 600
a.Spec.Repo.ExecTimeout = &timeout
timeout := "600m"
a.Spec.Repo.ExecTimeout = timeout

resObjs := []client.Object{a}
subresObjs := []client.Object{a}
Expand All @@ -359,14 +359,14 @@ func TestReconcileArgoCD_reconcileRepoDeployment_env(t *testing.T) {
assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Env, 4)
assert.Contains(t, deployment.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{Name: "FOO", Value: "BAR"})
assert.Contains(t, deployment.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{Name: "BAR", Value: "FOO"})
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"})
})

t.Run("ExecTimeout set", func(t *testing.T) {
logf.SetLogger(ZapLogger(true))
a := makeTestArgoCD()
timeout := 600
a.Spec.Repo.ExecTimeout = &timeout
timeout := "600m"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it not be 600s ?

a.Spec.Repo.ExecTimeout = timeout

resObjs := []client.Object{a}
subresObjs := []client.Object{a}
Expand All @@ -386,14 +386,14 @@ 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"})
})

t.Run("ExecTimeout set with env set explicitly", func(t *testing.T) {
logf.SetLogger(ZapLogger(true))
a := makeTestArgoCD()
timeout := 600
a.Spec.Repo.ExecTimeout = &timeout
timeout := "600m"
a.Spec.Repo.ExecTimeout = timeout
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can do a direct assignment a.Spec.Repo.ExecTimeout = 600s

a.Spec.Repo.Env = []corev1.EnvVar{
{
Name: "ARGOCD_EXEC_TIMEOUT",
Expand All @@ -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"})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep the same value as 600s instead of switching to minute.

})
t.Run("ExecTimeout not set", func(t *testing.T) {
logf.SetLogger(ZapLogger(true))
Expand Down
2 changes: 1 addition & 1 deletion deploy/olm-catalog/argocd-operator/0.14.0/argocd-operator.v0.14.0.clusterserviceversion.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ metadata:
capabilities: Deep Insights
categories: Integration & Delivery
certified: "false"
createdAt: "2024-12-20T09:48:39Z"
createdAt: "2025-01-28T06:26:16Z"
description: Argo CD is a declarative, GitOps continuous delivery tool for Kubernetes.
operators.operatorframework.io/builder: operator-sdk-v1.35.0
operators.operatorframework.io/project_layout: go.kubebuilder.io/v4
Expand Down
4 changes: 2 additions & 2 deletions deploy/olm-catalog/argocd-operator/0.14.0/argoproj.io_argocds.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1868,7 +1868,7 @@ spec:
execTimeout:
description: ExecTimeout specifies the timeout in seconds for
tool execution
type: integer
type: string
extraRepoCommandArgs:
description: |-
Extra Command arguments allows users to pass command line arguments to repo server workload. They get added to default command line arguments provided
Expand Down Expand Up @@ -15091,7 +15091,7 @@ spec:
execTimeout:
description: ExecTimeout specifies the timeout in seconds for
tool execution
type: integer
type: string
extraRepoCommandArgs:
description: |-
Extra Command arguments allows users to pass command line arguments to repo server workload. They get added to default command line arguments provided
Expand Down
2 changes: 1 addition & 1 deletion deploy/olm-catalog/argocd-operator/0.9.1/argoproj.io_argocds.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be seconds, minutes or hours based on the suffix user provides. This description needs a change.

tool execution
type: integer
extraRepoCommandArgs:
Expand Down
2 changes: 1 addition & 1 deletion tests/k8s/1-011_validate_repo_exec_timeout/02-change-exec-timeout.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ metadata:
name: argocd
spec:
repo:
execTimeout: 300
execTimeout: 300s
Loading