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

Add the feature of resource-level region[1/2] #616

Merged
merged 1 commit into from
Nov 2, 2020
Merged

Add the feature of resource-level region[1/2] #616

merged 1 commit into from
Nov 2, 2020

Conversation

qzyu92
Copy link
Contributor

@qzyu92 qzyu92 commented Oct 29, 2020

Intro

Based on provider-level region, this commit add an optional param named region in each resource and datasource.
User can manage cloud-resources in different regions just in a single tf file by confing region param in resource or datasource.
But, plase read the following attentions:

  1. Resource-level region only supports AK/SK authentication.
  2. If you set region in resource or datasource but authed by user-name/pwd or token, then the resource-level region must be the same with the provider-level region. Or you will get an error.

Test

Have commited a test case in resource_huaweicloud_vpc_test.go , named TestAccVpcV1_WithCustomRegion, and there is the test script and result.

func tesstAccVpcV1_WithCustomRegion(name1 string, name2 string, region string) string {
	return fmt.Sprintf(`
resource "huaweicloud_vpc" "test1" {
  name = "%s"
  cidr = "192.168.0.0/16"
}

resource "huaweicloud_vpc" "test2" {    
  name = "%s"
  region = "%s"
  cidr = "192.168.0.0/16"
}
`, name1, name2, region)
}
root@ecs-yuqizi:~/go/src/github.com/terraform-provider-huaweicloud# make testacc TEST='./huaweicloud/' TESTARGS='-run TestAccVpcV1_WithCustomRegion'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./huaweicloud/ -v -run TestAccVpcV1_WithCustomRegion -timeout 360m
=== RUN   TestAccVpcV1_WithCustomRegion
--- PASS: TestAccVpcV1_WithCustomRegion (53.66s)
PASS
ok      github.com/huaweicloud/terraform-provider-huaweicloud/huaweicloud       53.722s

@ShiChangkuo
Copy link
Collaborator

@qzyu92 Thanks for your effort. I suggest you can also update the docs along with the patch.

projectID, ok := RegionProjectIDMap.Load(region)
if !ok {
// Not find in the map, then try to query and store.
err := c.loadUserProjects(client, region)
Copy link
Collaborator

Choose a reason for hiding this comment

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

as resources are created in parallel, we must have a mutex here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'm working on it. Will fix it in the following patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added a mutex around here, and there is no more duplicate http request in my local test, please review about it.

Name: region,
}
sc := new(golangsdk.ServiceClient)
sc.Endpoint = fmt.Sprintf("https://iam.%s/v3/", c.Cloud)
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggest to set Endpoint by c.IdentityEndpoint

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, and I have modified about this.

Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

ForceNew is useless in data source

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got, I will fix 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.

Fix compeleted.

"region": {
Type: schema.TypeString,
Optional: true,
Computed: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

ForceNew must be true when region has changed for resource.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, I will push a patch and fix 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.

Fix compeleted.

@qzyu92
Copy link
Contributor Author

qzyu92 commented Oct 30, 2020

@qzyu92 Thanks for your effort. I suggest you can also update the docs along with the patch.

@qzyu92 qzyu92 closed this Oct 30, 2020
@qzyu92 qzyu92 reopened this Oct 30, 2020
@qzyu92
Copy link
Contributor Author

qzyu92 commented Oct 30, 2020

@ShiChangkuo The docs have been modified, and will push another request for it.

@qzyu92 qzyu92 changed the title Add the feature of resource-level region Add the feature of resource-level region[1/2] Oct 31, 2020
@qzyu92
Copy link
Contributor Author

qzyu92 commented Oct 31, 2020

@ShiChangkuo The modification of docs refs #622

Comment on lines 237 to 241
resource "huaweicloud_vpc" "test2" {
name = "%s"
region = "%s"
cidr = "192.168.0.0/16"
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

please format the resource with HCL style

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have fixed this by modifing it’s indentation into two blanks.

}

func (c *Config) newServiceClientByName(client *golangsdk.ProviderClient, catalog ServiceCatalog, region string) (*golangsdk.ServiceClient, error) {
if catalog.Name == "" || catalog.Version == "" {
return nil, fmt.Errorf("must specify the service name and api version")
}

// Resource-level region only supports AK/SK authentication.
if region != c.Region && (c.AccessKey == "" || c.SecretKey == "") {
return nil, fmt.Errorf("Resource-level region must be same as Provider-level region when not AK/SK authentication if Resource-level region setted")
Copy link
Member

Choose a reason for hiding this comment

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

s/setted/set

Copy link
Member

Choose a reason for hiding this comment

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

"Resource-level region must be same as Provider-level region when using non AK/SK authentication" seems better :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, that seems better truly.

Add region to datasource

Add region to resource

Add resource-level region test case for `resource_huaweicloud_vpc`
@ShiChangkuo ShiChangkuo merged commit b7902f6 into huaweicloud:master Nov 2, 2020
@qzyu92 qzyu92 deleted the region branch November 3, 2020 08:57
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.

3 participants