Skip to content

feat(helm): add RollingUpdate parameters#20931

Merged
craig-rueda merged 3 commits into
apache:masterfrom
gforien:feat/helm/add-rollingupdate-parameters
Sep 22, 2022
Merged

feat(helm): add RollingUpdate parameters#20931
craig-rueda merged 3 commits into
apache:masterfrom
gforien:feat/helm/add-rollingupdate-parameters

Conversation

@gforien
Copy link
Copy Markdown
Contributor

@gforien gforien commented Aug 1, 2022

SUMMARY

We want to handle the RollingUpdate parameters for the supersetNode and supersetWorker.

How

  • Add the parameters in the deployment.yaml and deployment-worker.yaml templates
  • Get default values from K8S documentation to ensure no change
  • Put default values in values.yaml
  • Update JSON schema
  • Bump chart version

TESTING INSTRUCTIONS

helm lint
helm template . superset-release

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

Comment thread helm/superset/Chart.yaml Outdated
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.

This should be 0.7.1

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 make the strategy configurable. You can just add the whole strategy section into values.yaml and then call toYaml here.

@dpgaspar
Copy link
Copy Markdown
Member

Hi @gforien would be great if you could rebase and address the requested changes. Thank you!

@gforien
Copy link
Copy Markdown
Contributor Author

gforien commented Aug 30, 2022

Hi @dpgaspar
Sorry for the delay. I will do this in the upcoming days

@gforien gforien force-pushed the feat/helm/add-rollingupdate-parameters branch from 661b8d1 to f54187c Compare September 21, 2022 08:38
@gforien
Copy link
Copy Markdown
Contributor Author

gforien commented Sep 21, 2022

Hi 👋
I have rebased my branch, and addressed the requested changes
Thank you for your patience

@gforien
Copy link
Copy Markdown
Contributor Author

gforien commented Sep 21, 2022

@craig-rueda @dpgaspar

Comment thread helm/superset/Chart.yaml Outdated
email: craig@craigrueda.com
url: https://github.com/craig-rueda
version: 0.7.2
version: 0.7.3
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.

Prob need to bump this one more time

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Did not know which one you would merge first :)

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

LGTM

@gforien gforien force-pushed the feat/helm/add-rollingupdate-parameters branch from f54187c to 809696a Compare September 21, 2022 20:53
@craig-rueda craig-rueda merged commit 3f8e9a5 into apache:master Sep 22, 2022
@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
Co-authored-by: gforien <gforien.externe@bedrockstreaming.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/S 🚢 2.1.0 First shipped in 2.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants