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.

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

Use pure python entrypoints to unbreak skaffold debugging #501

Merged
merged 6 commits into from
Jun 22, 2021

Conversation

j-windsor
Copy link
Contributor

@j-windsor j-windsor commented Jun 9, 2021

Fixes #500.

Change summary:

  • Reverts Python/Flask - Replaces gunicorn with uWSGI  #463 and bumps Gunicorn version to address the concern that prompted the switch to uwsgi.
  • loadgenerator now calls locust directly instead of from a bash script. Dockerfile was modified to support debugging locust files.

Debugging will still not work until this is fixed on the skaffold side: https://github.com/GoogleContainerTools/container-debug-support/issues/83

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jun 9, 2021
@askmeegs
Copy link
Collaborator

askmeegs commented Jun 9, 2021

Hi Jay, could you please provide a bit more context on why this PR is needed?
Gunicorn gave us some issues related to readiness/liveness probes and that was what prompted the move from Gunicorn to UWSGI.

#447

It does seem like the underlying bug that caused our gunicorn issues -- benoitc/gunicorn#1913 -- has been fixed, and gunicorn has put out an updated release. but I'd rather not do another backend overhaul unless there's a concrete benefit behind doing so.

@j-windsor
Copy link
Contributor Author

Hi @askmeegs! I provided some rationale and encountered error messages in the linked issue, but the TLDR is that running skaffold debug or running debug in Cloud Code will not work with this sample. Skaffold currently just fails and exits (@briandealwis has a bug to allow it to fail silently instead: GoogleContainerTools/container-debug-support#81), and it would be much more ideal if users can actually link their debugger to this sample, and if it worked out of the box with skaffold and Cloud Code.

@@ -1,4 +1,4 @@
# Copyright 2021 Google LLC
# Copyright 2019 Google LLC
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: license should be 2021

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated all touched files to have the correct copyright year.

@j-windsor
Copy link
Contributor Author

Thanks for the review @kelsk! I addressed your comment. PTAL.

@kelsk
Copy link
Contributor

kelsk commented Jun 16, 2021

@j-windsor perfect, thanks!

@askmeegs For a little more context, the main goal of configuring Bank of Anthos to be debuggable using Cloud Code/skaffold debug is to let folks use the Cloud Code debug feature without additional setup.

If it's not feasible to revert back to using gunicorn in the near future, maybe we could keep this version in a specific branch and have that branch cloned when a user opens BoA in Cloud Shell. (We could add a one-liner instruction for users to clone the specific branch if they want to try debugging on Cloud Code in their IDE.) WDYT?

Copy link
Collaborator

@askmeegs askmeegs left a comment

Choose a reason for hiding this comment

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

Tested locally in a GKE cluster, and a cluster with Anthos Service Mesh. LGTM.

No readiness probe issues observed. (which was the gunicorn bug that caused us to move to uwsgi - ). Will need to keep an eye on this. #447

@j-windsor
Copy link
Contributor Author

Thanks @askmeegs and @kelsk! Feel free to merge whenever.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

python services do not work with skaffold debug, cause status check failure.
3 participants