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

Fix broken AWS, set minimum desired size to 1, enable 0 scaling #1304

Merged
merged 1 commit into from
Jun 9, 2022
Merged

Fix broken AWS, set minimum desired size to 1, enable 0 scaling #1304

merged 1 commit into from
Jun 9, 2022

Conversation

tylerpotts
Copy link
Contributor

@tylerpotts tylerpotts commented Jun 1, 2022 •
edited
Loading

Fixes | Closes | Resolves # 1302

Please remove anything marked as optional that you don't need to fill in.
Choose one of the keywords preceding to refer to the issue this PR solves, followed by the issue number (e.g Fixes # 666).
If there is no issue, remove the line. Remove this note after reading.

Changes introduced in this PR:

Types of changes

What types of changes does your PR introduce?

Put an x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features to not work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

Testing

Requires testing

  • Yes
  • No

@costrouc @viniciusdc I think this justifies a minor release prior to the Nebari rename. Scaling to 0 is a major improvement, and this currently fixes a bug which prevents successful AWS deployment.

@tylerpotts
Copy link
Contributor Author

This brings a tear to my eye 🥲

image

@tylerpotts
Copy link
Contributor Author

Note that when I tried to deploy this on AWS I got an error. It was working when I had these changes on top of the 0.4.1 release, so I suspect the error isn't coming from this code.

Sadly I didn't capture the error message. If I deploy off of main and see the error again I'll post it.

@trallard trallard added type: bug 🐛 Something isn't working needs: review 👀 This PR is complete and ready for reviewing provider: AWS labels Jun 6, 2022
@trallard trallard requested review from costrouc and aktech June 7, 2022 10:31
viniciusdc
viniciusdc reviewed Jun 7, 2022
View reviewed changes
qhub/stages/input_vars.py
@@ -95,7 +95,7 @@ def stage_02_infrastructure(stage_outputs, config):
{
"name": key,
"min_size": value["min_nodes"],
"desired_size": value["min_nodes"],
"desired_size": max(value["min_nodes"], 1),
Copy link
Contributor

Choose a reason for hiding this comment

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

To follow the pattern I think we can update the min_node value directly from the init template here, and defined the values as we did for GCP in there.. What are your thoughts on this @tylerpotts ?

@viniciusdc viniciusdc added status: in review 👀 This PR is currently being reviewed by the team and removed needs: review 👀 This PR is complete and ready for reviewing labels Jun 8, 2022
viniciusdc
viniciusdc approved these changes Jun 8, 2022
View reviewed changes
Copy link
Contributor

@viniciusdc viniciusdc left a comment

Choose a reason for hiding this comment

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

LGTM! this will fix the current issues regarding the AWS deployments, thanks @tylerpotts

@iameskild
Copy link
Member

The failing CI Test QHub Provider is all the result of this PR being opened from a fork repo.

Thanks for this fix @tylerpotts 🎉

@viniciusdc viniciusdc added type: maintenance 🛠 Day-to-day maintenance tasks area: terraform 💾 status: merge-ready and removed status: in review 👀 This PR is currently being reviewed by the team labels Jun 9, 2022
@iameskild iameskild merged commit 03c8b6d into nebari-dev:main Jun 9, 2022
@iameskild iameskild mentioned this pull request Jun 9, 2022
Closed
18 tasks
@iameskild
Copy link
Member

closes #1302

iameskild pushed a commit that referenced this pull request Jun 9, 2022
@tylerpotts
Fix broken AWS, set minimum desired size to 1, enable 0 scaling (#1304)
906606e
iameskild added a commit that referenced this pull request Jun 9, 2022
@iameskild @tylerpotts @viniciusdc
v0.4.2 release notes (#1323)
9a1baba 
* Update minio, postgresql chart repo location (#1308)

* Update minio, postgresql charts

* Update bitnami index url

* Update url

Co-authored-by: Eskild Eriksen <eskild@MBP21.local>

* Add release notes, postmoderm

* Fix broken AWS, set minimum desired size to 1, enable 0 scaling (#1304)

* Update release notes

* clean up

* Update RELEASE.md

Co-authored-by: Vinicius D. Cerutti <51954708+viniciusdc@users.noreply.github.com>

* Update RELEASE.md

Co-authored-by: Vinicius D. Cerutti <51954708+viniciusdc@users.noreply.github.com>

* Update RELEASE.md

Co-authored-by: Vinicius D. Cerutti <51954708+viniciusdc@users.noreply.github.com>

* Update RELEASE.md

Co-authored-by: Vinicius D. Cerutti <51954708+viniciusdc@users.noreply.github.com>

Co-authored-by: Eskild Eriksen <eskild@MBP21.local>
Co-authored-by: iameskild <eskild@double.io>
Co-authored-by: Tyler <49161327+tylerpotts@users.noreply.github.com>
Co-authored-by: Vinicius D. Cerutti <51954708+viniciusdc@users.noreply.github.com>
@viniciusdc viniciusdc mentioned this pull request Jun 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@viniciusdc viniciusdc viniciusdc approved these changes

@costrouc costrouc Awaiting requested review from costrouc

@aktech aktech Awaiting requested review from aktech

Assignees

@costrouc costrouc

@aktech aktech

Labels
area: terraform 💾 provider: AWS type: bug 🐛 Something isn't working type: maintenance 🛠 Day-to-day maintenance tasks
Projects
🪴 Nebari Project Management
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tylerpotts @iameskild @viniciusdc @costrouc @aktech @trallard