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

provider/google: ignore certain project services that can't be enabled directly via the api #13730

Merged
merged 1 commit into from
Apr 27, 2017

Conversation

danawillow
Copy link
Contributor

When a project service is enabled, it may also end up enabling another one. We don't yet have a fix to the general problem, but this PR solves a small case of that:
Some of the services (such as dataproc-control.googleapis.com) are only enabled via these side-effects, and can't be enabled or disabled via the api. Explicitly ignore these to avoid issues like #13004.

apiServices = append(apiServices, v.ServiceName)
if _, ok := ignore[v.ServiceName]; !ok {
apiServices = append(apiServices, v.ServiceName)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I could be wrong on this, but I think we could use a DiffSuppressFunc instead of modifying the state for this. It's not a huge deal, but in the unlikely event the API starts letting us control one of these services in the future, Terraform would know it's already enabled instead of thinking it's not yet. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

We talked about this, and there was trouble with the count part of the Set and DiffSuppressFunc. I need to ask about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, and just in case anyone else is wondering, here's what the test output looks like with a DiffSuppressFunc:

--- FAIL: TestAccGoogleProjectServices_ignoreUnenablableServices (260.05s)
	testing.go:280: Step 0 error: After applying this step, the plan was not empty:

		DIFF:

		UPDATE: google_project_services.acceptance
		  services.#:          "10" => "9"
		  services.133405307:  "storage-component.googleapis.com" => "storage-component.googleapis.com"
		  services.2692275209: "dataproc.googleapis.com" => "dataproc.googleapis.com"
		  services.2966512281: "deploymentmanager.googleapis.com" => "deploymentmanager.googleapis.com"
		  services.3010261123: "replicapool.googleapis.com" => "replicapool.googleapis.com"
		  services.3075019877: "replicapoolupdater.googleapis.com" => "replicapoolupdater.googleapis.com"
		  services.3077910291: "resourceviews.googleapis.com" => "resourceviews.googleapis.com"
		  services.3731214611: "compute-component.googleapis.com" => "compute-component.googleapis.com"
		  services.3740470850: "container.googleapis.com" => "container.googleapis.com"
		  services.3875785048: "storage-api.googleapis.com" => "storage-api.googleapis.com"

Copy link
Contributor

Choose a reason for hiding this comment

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

Outcome of the conversation is largely that DiffSuppressFunc and Sets/Lists is largely uncharted territory, and we don't know a ton about its behaviour just yet.

I think @danawillow was going to investigate more, but unless she surfaces information that can make this work cleanly, I think the implementation as it exists is as good as this is going to get, and am fine moving forward with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I took a look but didn't come up with anything nice. Good to merge?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good to merge. 👍

@ghost
Copy link

ghost commented Apr 13, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants