Skip to content
This repository has been archived by the owner on Mar 15, 2024. It is now read-only.

Allow GCP hosted clusters to be deployed #24

Merged
merged 6 commits into from
Sep 12, 2022

Conversation

d-nagy
Copy link
Contributor

@d-nagy d-nagy commented Jul 13, 2022

Fix for issue #23

Copy link
Contributor

@jamesrwilkinson jamesrwilkinson left a comment

Choose a reason for hiding this comment

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

We'll want to remove this from the validation but this alone won't allow for GCP hosted clusters to be deployed. We would get an error as the GCP disk storage type PD-SSD is not included in the enum V3StorageType in the api client.

We will need to update the Go API client to support the new GCP resources added in the API.

I'll regenerate the api client from the latest api spec - we might also need to make some changes inside of the hosted cluster resource.

@d-nagy
Copy link
Contributor Author

d-nagy commented Jul 13, 2022

Ah, thanks for the information @JamesWilkinsonCB, that makes sense. I forgot about the disk changes.

One thing too is that for PD-SSDs we cannot specify IOPS - we'd need to check how the API handles this (whether we can just specify 0 or whether we need to omit the IOPS parameter entirely)

@jamesrwilkinson
Copy link
Contributor

One thing too is that for PD-SSDs we cannot specify IOPS - we'd need to check how the API handles this (whether we can just specify 0 or whether we need to omit the IOPS parameter entirely)

I think we will want to omit this parameter as it looks like it's no longer a required field. We can also remove the required tag from this field in the Schema.

I've regenerated the client in this PR, which will allow us to add GCP support to the provider.

@krugster krugster requested a review from a team July 21, 2022 13:17
- Added test for GCP Hosted Cluster
- Added GCP Compute/Regions to Validation
- Added GCP configuraiton to Hosted Cluster resource documentation
- Moved Building the Provider section further down
- Updated example usage to pull provider from the Terraform registry
@jamesrwilkinson jamesrwilkinson self-assigned this Aug 19, 2022
ConorNevin
ConorNevin previously approved these changes Sep 5, 2022
jamesrwilkinson
jamesrwilkinson previously approved these changes Sep 6, 2022
README.md Outdated
After the provider has been built locally it must be placed in the user plugins directory so it can be discovered by the Terraform CLI. Please execute the following command to move the provider binary to this directory:

```sh
$ mv terraform-provider-couchbasecapella ~/.terraform.d/plugins/local/couchbasecapella/<VERSION>/<OS_ARCH>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think using local works here:

» ls -1 ~/.terraform.d/plugins/local/couchbasecapella/1.0.0/
darwin_amd64

» cat versions.tf
terraform {
  required_providers {
    couchbasecapella = {
      source  = "local/couchbasecapella"
      version = "1.0.0"
    }
  }
}

» terraform init

Initializing the backend...

Initializing provider plugins...
- Finding local/couchbasecapella versions matching "1.0.0"...
╷
│ Error: Failed to query available provider packages
│
│ Could not retrieve the list of available versions for provider local/couchbasecapella: provider registry registry.terraform.io does not have a provider named
│ registry.terraform.io/local/couchbasecapella

IIRC the hostname when building locally is kind of required. If you omit it, it will use the default registry and it fails:

If hostname is omitted, Terraform will use the Terraform Registry hostname as the default hostname.

Usually setting this to github.com is a good practice, as it matches the hostname/namespace from the repository on GitHub.

Also, the folder needs to be created first, maybe we can set up in another PR a target on the Makefile to automate this and improve the development flow (I can help with this)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I agree, that would be much better in the Makefile. We can just run the one command then. I'll create a branch off of this one and we can make a PR for Makefile/changes to docs

@jamesrwilkinson jamesrwilkinson dismissed stale reviews from ConorNevin and themself via ed02656 September 12, 2022 09:56
@jamesrwilkinson
Copy link
Contributor

I've fixed the issue with the commands listed for building the provider in the README - I'll create a new PR for the changes to the Makefile that can automate this.

@jamesrwilkinson jamesrwilkinson merged commit 42779b4 into main Sep 12, 2022
@jamesrwilkinson jamesrwilkinson deleted the fix/gcp_hosted_clusters branch September 12, 2022 10:06
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.

4 participants