Skip to content
This repository has been archived by the owner on Jun 17, 2023. It is now read-only.

Set disable_on_destroy=true for API management. #91

Closed
wants to merge 1 commit into from

Conversation

mrtyler
Copy link
Collaborator

@mrtyler mrtyler commented Jun 1, 2018

I have an exekube project. It's pretty straightforward for now, basically a copy of https://github.com/exekube/demo-apps-project/.

I am using exekube:0.3.1-google with some trivial changes.

When I do this with a new cluster:

$ xk gcp-project-init
$ xk up live/dev/infra
$ xk down live/dev/infra

I get this error every time I re-run xk up live/dev/infra:

* google_project_service.services.0: Error enabling service:
Error enabling service ["compute.googleapis.com"] for project "xk-tyler11":
Error waiting for api to enable: googleapi:
Error 403: The caller does not have permission, forbidden

I can work around it by running xk gcloud services disable on each service in ${var.project_services} here:

for i in compute.googleapis.com container.googleapis.com containerregistry.googleapis.com cloudkms.googleapis.com dns.googleapis.com cloudresourcemanager.googleapis.com iam.googleapis.com ; do
    xk gcloud services disable $i
done

I can also fix it by setting disable_on_destroy = true in the terraform code, as I have done in this PR.

What do you think of this approach?

Slightly separate but related question:

The design today seems to be xk down live/dev/infra is run only rarely, even if xk down; xk up is run frequently. However, I think there is a difference between enabling GCP APIs -- which creates no resources and (AFAIK) costs nothing -- and managing networks, static IPs, and DNS Zones -- which are visible resources and have small (but non-zero) costs. All of these things are managed by a single module today, gke-network.

What do you think about moving the API management out of the gke-network module and into its own module? Since (AIUI) API management happens asynchronously on Google's end, it can require a few runs of xk up live/dev/infra before everything converges[1]. Hence, it might be desirable to run the API management code less often, or at least separately from the network/IP/DNS management code. Splitting gke-network's responsibilities might also enable a different decision on the disable_on_destroy question above.

[1] The errors look like this:

* google_project_service.services.1: Error enabling service:
Error enabling service ["container.googleapis.com"] for project "xk-tyler11":
googleapi: Error 400: Precondition check failed., failedPrecondition

Without this, API management code fails after up/down/up.
@ilyasotkov
Copy link
Collaborator

What do you think about moving the API management out of the gke-network module and into its own module?

That's a good idea.

I agree that (as it currently is) enabling all APIs at the top of gke-network module doesn't make much sense. The most logical decision would perhaps be enabling APIs near the resources that require them, but that would be a bad experience since enabling an API can take a long time and we don't get appropriate feedback from Terraform.

@mrtyler
Copy link
Collaborator Author

mrtyler commented Jun 6, 2018

I agree that putting API enabling near where the API is used makes sense and provides better cohesion. However, I also think that API enabling is kind of a weird special case because of slow/asynchronous updates.

FYI I'm not sure if/when we'll need to pull API enabling out of gke-network, but I have no immediate plans to work on such a refactor. I'll update this PR if that changes.

In the meantime, it's up to you whether to merge this PR as-is (I think the situation is better for exekube users with this change than without it) or to wait for a more serious fix.

Thanks Ilya :)

@ilyasotkov
Copy link
Collaborator

I've now separated API management into its own gcp-api-mgmt module, and set disable_on_destroy=true.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants