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

Fix delete default namespace #73

Merged
merged 4 commits into from
Sep 3, 2019
Merged

Conversation

lgfa29
Copy link
Contributor

@lgfa29 lgfa29 commented Aug 30, 2019

Currently if you reference the default namespace in your configuration Terraform will fail to destroy it because the API will return a 500 error:

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  - destroy

Terraform will perform the following actions:

  # nomad_namespace.default will be destroyed
  - resource "nomad_namespace" "default" {
      - description = "Default shared namespace" -> null
      - id          = "default" -> null
      - name        = "default" -> null
    }

Plan: 0 to add, 0 to change, 1 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

nomad_namespace.default: Destroying... [id=default]

Error: error deleting namespace "default": Unexpected response code: 500 (can not delete default namespace)

This error can prevent other resources from being destroyed if they depend on this namespace.

This PR prevents this error from happening by not trying to delete the default namespace. Instead it resets the quota and description attributes to their initial values.

Fixes #72 .

@lgfa29 lgfa29 force-pushed the fix-delete-default-namespace branch from 02486d4 to fc111aa Compare August 30, 2019 21:30
@lgfa29 lgfa29 requested a review from cgbaker August 30, 2019 21:32
cgbaker
cgbaker previously approved these changes Aug 30, 2019
Copy link
Contributor

@cgbaker cgbaker left a comment

Choose a reason for hiding this comment

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

manually tested, works great.
minor nitpicks, but lgtm!

return nil
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Co-Authored-By: Chris Baker <1675087+cgbaker@users.noreply.github.com>
@ghost ghost added size/M and removed size/S labels Aug 30, 2019
@cgbaker cgbaker self-requested a review September 3, 2019 15:24
Copy link
Contributor

@cgbaker cgbaker left a comment

Choose a reason for hiding this comment

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

👍

@lgfa29 lgfa29 merged commit 2fb1021 into master Sep 3, 2019
@lgfa29 lgfa29 deleted the fix-delete-default-namespace branch September 4, 2019 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provider should not attempt to delete default namespace
2 participants