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

Update skip_remote_state_provision default value #1521

Merged
merged 1 commit into from
Oct 28, 2022

Conversation

viniciusdc
Copy link
Contributor

Fixes | Closes | Resolves #

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:

  • Update skip_remote_state_provision default value from True to False

By keeping the skip_remote_state_provision default value as True, we were inadvertently revoking the creation of the terraform state bucket during the first deployments. Which is used during the stage provision phase:

nebari/nebari/deploy.py

Lines 201 to 208 in 77d15e7

if (
config["provider"] not in {"existing", "local"}
and config["terraform_state"]["type"] == "remote"
):
if skip_remote_state_provision:
print("Skipping remote state provision")
else:
provision_01_terraform_state(stage_outputs, config)

I think the reason we didn't spot this sooner as due to until #1519 we still used a mix of the new CLI with the old qhub entry points.

Image of the error caused during deployment on GCP with terraform_state: remote (default) set in the config

nebari deploy -c nebari-config.yaml --dns-provider cloudflare --dns-auto-provision --disable-prompt

image

Even with no mention of --skip-remote-state-provision you can see the log message right at the beginning finalizing the skip.

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

In case you checked yes, did you write tests?

  • Yes
  • No

Documentation

Does your contribution include breaking changes or deprecations?
If so have you updated the documentation?

  • Yes, docstrings
  • Yes, main documentation
  • Yes, deprecation notices

Further comments (optional)

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered and more.

@viniciusdc
Copy link
Contributor Author

viniciusdc commented Oct 28, 2022

There seems to be a problem with pytest running the jupyterhub ssh test. I will handle that in another PR. I ran IT with this branch all working.

@viniciusdc viniciusdc merged commit f44dc0b into release/2022.10.1 Oct 28, 2022
@iameskild iameskild deleted the update-state-provision branch October 28, 2022 03:56
iameskild added a commit that referenced this pull request Nov 2, 2022
* Switch to CalVer (#1501)

* Run CI on branchs release/**

* Update pyproject.toml to use calver

* Update on branch

* Update theme welcome messages to use Nebari (#1503)

* update welcome messge

* remove qhub specifics from theme

* Name change QHub --> Nebari (#1508)

* Name change: .github

* Name change: qhub/

* Name change: tests/

* Name change: tests_deployment/

* Name change: tests_e2e/

* Name change: /

* Rename folder

* Rename releases

* Remove qhub/

* Run pre-commit

* Revert docs links

* Fix infracost artifact names

* Update upgrade command + test

* Updates based on review

* Update step output format

* Fix: Update step output format

* Update nebari/provider/cicd/common.py

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update stage 08 folder names

* Update nebari/schema.py

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update nebari/template/stages/02-infrastructure/azure/variables.tf

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update nebari/template/stages/02-infrastructure/do/variables.tf

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update nebari/template/stages/02-infrastructure/gcp/variables.tf

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update nebari/template/stages/02-infrastructure/gcp/variables.tf

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update nebari/template/stages/03-kubernetes-initialize/variables.tf

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update nebari/template/stages/05-kubernetes-keycloak/variables.tf

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update nebari/template/stages/06-kubernetes-keycloak-configuration/variables.tf

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update nebari/template/stages/06-kubernetes-keycloak-configuration/variables.tf

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update nebari/template/stages/07-kubernetes-services/modules/kubernetes/forwardauth/variables.tf

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update setup.cfg

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update scripts/aws-force-destroy.py

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update setup.cfg

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update setup.cfg

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update setup.cfg

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update setup.cfg

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* Update nebari/constants.py

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

* Update setup.cfg

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* fix flake8 complaint

* Update ref to qhub-dask to nebari-dask

* Update readme

* Change to PYPI_API_TOKEN

Co-authored-by: viniciusdc <vinivdc2009@hotmail.com>
Co-authored-by: Tania Allard <taniar.allard@gmail.com>
Co-authored-by: Vinicius D. Cerutti <51954708+viniciusdc@users.noreply.github.com>

* qhub/initialize: lazy load attributes that require remote information (#1509)

* qhub/initialize: lazy load attributes that require remote information

* Fix broken configuration

* fix renaming

* fix header text

* Update nebari/initialize.py

* Update nebari/initialize.py

* Blackify

Co-authored-by: Amit Kumar <dtu.amit@gmail.com>
Co-authored-by: Nebari-sensei <116370392+nebari-sensei@users.noreply.github.com>

* Update README logo (#1514)

* Add fix, enhancements and pytests for CLI (#1498)

* Add fix, enhancements and pytests for CLI

* No zero-padded month for CalVer release branches

* Update cli test

* Simplify test.yaml

* Remove old CLI + cleanup (#1519)

* Remove old cli

* Update theme

* Remove shim

* display_version: True

* Update `skip_remote_state_provision` default value (#1521)

* Add release notes for 2022.10.1 (#1523)

* Add release notes for 2022.10.1

* Wording

* Update README.md with new docs links

* Update README.md with new docs links

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

* Remove leading `v` for release action (#1526)

* Remove leading v for release tags

* Add note to release checklist

* Merge release branch into main

Co-authored-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Co-authored-by: viniciusdc <vinivdc2009@hotmail.com>
Co-authored-by: Tania Allard <taniar.allard@gmail.com>
Co-authored-by: Vinicius D. Cerutti <51954708+viniciusdc@users.noreply.github.com>
Co-authored-by: Filipe Laíns <filipe.lains@gmail.com>
Co-authored-by: Amit Kumar <dtu.amit@gmail.com>
Co-authored-by: Nebari-sensei <116370392+nebari-sensei@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant