Skip to content

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

Merged
craig-rueda merged 11 commits into
apache:masterfrom
jplanckeel:feat/helm/add-k8s-probes
Aug 8, 2022
Merged

feat(helm): add liveness and readiness for deployment webserver#20567
craig-rueda merged 11 commits into
apache:masterfrom
jplanckeel:feat/helm/add-k8s-probes

Conversation

@jplanckeel

@jplanckeel jplanckeel commented Jun 30, 2022

Copy link
Copy Markdown
Contributor

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

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

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!

Comment thread helm/superset/templates/deployment.yaml Outdated
Comment thread helm/superset/templates/deployment.yaml Outdated
Comment thread helm/superset/values.yaml Outdated

@craig-rueda craig-rueda left a comment

Copy link
Copy Markdown
Member

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

Comment thread helm/superset/templates/deployment.yaml Outdated
@craig-rueda

Copy link
Copy Markdown
Member

Still needs a version bump

@jplanckeel

Copy link
Copy Markdown
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

nytai commented Jul 12, 2022

Copy link
Copy Markdown
Member

You need to update the schema to get CI passing

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

@jplanckeel

jplanckeel commented Jul 20, 2022

Copy link
Copy Markdown
Contributor Author

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

@gforien

gforien commented Jul 25, 2022

Copy link
Copy Markdown
Contributor

Hi, any update on this ?

@wiktor2200 wiktor2200 left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread helm/superset/templates/deployment.yaml Outdated
@wiktor2200

Copy link
Copy Markdown
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
Copy Markdown
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 the 🚢 2.1.3 First shipped in 2.1.3 label Feb 18, 2024
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 First shipped in 2.1.0 and removed 🚢 2.1.3 First shipped in 2.1.3 labels Mar 13, 2024
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
…he#20567)

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

* feat(helm): add example in values

* feat(helm): move config probes under supersetNode

* feat(helm): bump chart.yaml

* fix(helm): remove default values in template and use values.yaml

* fix(git): bump chart verison

* fix(json):  update the schema to get CI passing

* fix(helm): correct path for values readiness

Co-authored-by: wiktor2200 <wiktor2200@users.noreply.github.com>

Co-authored-by: Jeremy PLANCKEEL <jplanckeel.externe@bedrockstreaming.com>
Co-authored-by: Craig Rueda <craig@craigrueda.com>
Co-authored-by: wiktor2200 <wiktor2200@users.noreply.github.com>
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 First shipped in 2.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants