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

Jump to bottom

chore: Graceful shutdown for API Server (redo) (#18642) #21224

Merged
merged 3 commits into from
Dec 17, 2024
Merged

chore: Graceful shutdown for API Server (redo) (#18642) #21224

merged 3 commits into from
Dec 17, 2024

Conversation

andrii-korotkov-verkada
Copy link
Contributor

Closes #18642

Implements a graceful shutdown the the API server. Without this, ArgoCD API server will eventually return 502 during rolling update. However, healthcheck would return 503 if the server is terminating.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • The title of the PR conforms to the Toolchain Guide
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).
  • My new feature complies with the feature status guidelines.
  • I have added a brief description of why this PR is necessary and/or what this PR solves.
  • Optional. My organization is added to USERS.md.
  • Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

@andrii-korotkov-verkada @leoluz @crenshaw-dev
fix: Graceful shutdown for the API server (argoproj#18642) (argoproj#…
0e9d075 
…20981)

* fix: Graceful shutdown for the API server (argoproj#18642)

Closes argoproj#18642

Implements a graceful shutdown the the API server. Without this, ArgoCD API server will eventually return 502 during rolling update. However, healthcheck would return 503 if the server is terminating.

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
Co-authored-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* Init server only once, but keep re-initializing listeners

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>

* Check error for SetParamInSettingConfigMap as needed after fresh master

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>

* Prevent a data race

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>

* Remove unused variable, don't pass lock when not necessary

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>

* Try overriding URL instead of additional URLs

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>

* Use a more specific url

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>

---------

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
Co-authored-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
@andrii-korotkov-verkada
Use a custom signal for graceful restart
aaf25af 
Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
@andrii-korotkov-verkada andrii-korotkov-verkada requested a review from a team as a code owner December 17, 2024 17:48
@bunnyshell Bunnyshell
Copy link

bunnyshell bot commented Dec 17, 2024 •
edited
Loading

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

  • 🚀 /bns:deploy to deploy the environment

@andrii-korotkov-verkada
Re-run tests
c85f3d6 
Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
@codecov Codecov
Copy link

codecov bot commented Dec 17, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 71.07438% with 35 lines in your changes missing coverage. Please review.

Project coverage is 55.17%. Comparing base (46bfc10) to head (c85f3d6).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
server/server.go 79.80% 17 Missing and 4 partials ⚠️
cmd/argocd-server/commands/argocd_server.go 0.00% 8 Missing ⚠️
util/cache/redis.go 20.00% 3 Missing and 1 partial ⚠️
util/session/state.go 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #21224      +/-   ##
==========================================
+ Coverage   55.03%   55.17%   +0.14%     
==========================================
  Files         338      338              
  Lines       57075    57167      +92     
==========================================
+ Hits        31410    31543     +133     
+ Misses      22968    22923      -45     
- Partials     2697     2701       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

leoluz
leoluz reviewed Dec 17, 2024
View reviewed changes
Copy link
Collaborator

@leoluz leoluz left a comment

Choose a reason for hiding this comment

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

Tks for the fix.
Added a minor comment.

server/server.go Show resolved Hide resolved
leoluz
leoluz approved these changes Dec 17, 2024
View reviewed changes
Copy link
Collaborator

@leoluz leoluz left a comment

Choose a reason for hiding this comment

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

LGTM

@leoluz leoluz merged commit 45e4886 into argoproj:master Dec 17, 2024
27 checks passed
@leoluz
Copy link
Collaborator

leoluz commented Dec 17, 2024

/cherry-pick release-2.14

gcp-cherry-pick-bot bot pushed a commit that referenced this pull request Dec 17, 2024
@andrii-korotkov-verkada @leoluz @crenshaw-dev
chore: Graceful shutdown for API Server (#18642) (#21224)
1966506 
* fix: Graceful shutdown for the API server (#18642) (#20981)

* fix: Graceful shutdown for the API server (#18642)

Closes #18642

Implements a graceful shutdown the the API server. Without this, ArgoCD API server will eventually return 502 during rolling update. However, healthcheck would return 503 if the server is terminating.

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
Co-authored-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* Init server only once, but keep re-initializing listeners

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>

* Check error for SetParamInSettingConfigMap as needed after fresh master

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>

* Prevent a data race

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>

* Remove unused variable, don't pass lock when not necessary

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>

* Try overriding URL instead of additional URLs

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>

* Use a more specific url

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>

---------

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
Co-authored-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* Use a custom signal for graceful restart

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>

* Re-run tests

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>

---------

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
Co-authored-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
@gcp-cherry-pick-bot gcp-cherry-pick-bot bot mentioned this pull request Dec 17, 2024
Merged
@andrii-korotkov-verkada andrii-korotkov-verkada deleted the 18642-graceful-shutdown-for-api-server-redo branch December 17, 2024 22:55
pasha-codefresh pushed a commit that referenced this pull request Dec 18, 2024
@gcp-cherry-pick-bot @andrii-korotkov-verkada
@leoluz @crenshaw-dev
chore: Graceful shutdown for API Server (#18642) (#21224) (#21229)
018014c 
* fix: Graceful shutdown for the API server (#18642) (#20981)

* fix: Graceful shutdown for the API server (#18642)

Closes #18642

Implements a graceful shutdown the the API server. Without this, ArgoCD API server will eventually return 502 during rolling update. However, healthcheck would return 503 if the server is terminating.





* Init server only once, but keep re-initializing listeners



* Check error for SetParamInSettingConfigMap as needed after fresh master



* Prevent a data race



* Remove unused variable, don't pass lock when not necessary



* Try overriding URL instead of additional URLs



* Use a more specific url



---------





* Use a custom signal for graceful restart



* Re-run tests



---------

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
Co-authored-by: Andrii Korotkov <137232734+andrii-korotkov-verkada@users.noreply.github.com>
Co-authored-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leoluz leoluz leoluz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement graceful shutdown in all Argo CD components
2 participants
@andrii-korotkov-verkada @leoluz