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

feat(helm): add liveness and readiness for deployment webserver #20567

Merged
merged 11 commits into from
Aug 8, 2022

Conversation

jplanckeel
Copy link
Contributor

@jplanckeel jplanckeel commented Jun 30, 2022

SUMMARY

We want add readiness and liveness in helmchart because we need liveness to restart container automatically when server crash and readiness for to delay start.

Is best practice in K8s to have liveness and readiness. :)

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

image

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️
We hope to see you in our Slack community too!

Copy link
Member

@craig-rueda craig-rueda left a comment

Choose a reason for hiding this comment

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

Pls bump the version in Chart.yaml

@craig-rueda
Copy link
Member

Still needs a version bump

@jplanckeel
Copy link
Contributor Author

Still needs a version bump

We can automate bump version, in my entreprise we automate bump version with label bump:minor, bump:major, bump;patch. is more easier and reduce error to bump version.

@nytai
Copy link
Member

nytai commented Jul 12, 2022

You need to update the schema to get CI passing

"$schema": "http://json-schema.org/draft-04/schema#",

@jplanckeel
Copy link
Contributor Author

jplanckeel commented Jul 20, 2022

Hi, i updated values schema, can you merge this PR ?

@gforien
Copy link
Contributor

gforien commented Jul 25, 2022

Hi, any update on this ?

Copy link
Contributor

@wiktor2200 wiktor2200 left a comment

Choose a reason for hiding this comment

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

There is wrong order in variable name.

helm/superset/templates/deployment.yaml Outdated Show resolved Hide resolved
@wiktor2200
Copy link
Contributor

I've got also one more question. Is not liveness/readiness probe also needed for rest Superset's elements (worker and celerybeat)?

Co-authored-by: wiktor2200 <wiktor2200@users.noreply.github.com>
@jplanckeel
Copy link
Contributor Author

I've got also one more question. Is not liveness/readiness probe also needed for rest Superset's elements (worker and celerybeat)?

we can do this step by step, the PR is already open for several weeks.

For the workers, do we have a status endpoint or a command to check the functioning? same for celery

@craig-rueda craig-rueda merged commit 554ed64 into apache:master Aug 8, 2022
@jplanckeel jplanckeel deleted the feat/helm/add-k8s-probes branch February 28, 2023 15:35
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants