-
Notifications
You must be signed in to change notification settings - Fork 162
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
support param masters in cce cluster #885
support param masters in cce cluster #885
Conversation
docs/resources/cce_cluster.md
Outdated
@@ -145,6 +145,12 @@ versions are available, choose Dashboard > Buy Cluster on the CCE console. Chang | |||
|
|||
* `enterprise_project_id` - (Optional, String, ForceNew) The enterprise project id of the cce cluster. Changing this creates a new cluster. | |||
|
|||
* `masters` - (Optional, List, ForceNew) Advanced configuration of master nodes. Changing this creates a new cluster. Conflicts with `multi_az`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems masters
and multi_az
have the same logical function, so please put them together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
docs/resources/cce_cluster.md
Outdated
@@ -134,7 +134,7 @@ versions are available, choose Dashboard > Buy Cluster on the CCE console. Chang | |||
* `authenticating_proxy_ca` - (Optional, String, ForceNew) CA root certificate provided in the authenticating_proxy mode. The CA root certificate | |||
is encoded to the Base64 format. Changing this parameter will create a new cluster resource. | |||
|
|||
* `multi_az` - (Optional, Bool, ForceNew) Enable multiple AZs for the cluster, only when using HA flavors. Changing this parameter will create a new cluster resource. | |||
* `multi_az` - (Optional, Bool, ForceNew) Enable multiple AZs for the cluster, only when using HA flavors. Changing this parameter will create a new cluster resource. Conflicts with `masters`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
two small tips:
- if the paramter conflicts with others, please use the following template:
This parameter andxxx
are alternative. - In code style guides, maximum line length rules are often set to 100 or 120.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
func resourceClusterMastersV3(d *schema.ResourceData) ([]clusters.MasterSpec, error) { | ||
flavorId := d.Get("flavor_id").(string) | ||
mastersRaw := d.Get("masters").([]interface{}) | ||
if strings.Split(flavorId, ".")[1] == "s1" && len(mastersRaw) != 1 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems the flavor likes cce.dec.s1.small
does not applicable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
return []clusters.MasterSpec{}, fmt.Errorf("Error creating HuaweiCloud Cluster: single-master cluster need 1 az for master node, but got %d", len(mastersRaw)) | ||
} | ||
if strings.Split(flavorId, ".")[1] == "s2" && len(mastersRaw) != 3 { | ||
return []clusters.MasterSpec{}, fmt.Errorf("Error creating HuaweiCloud Cluster: high-availability cluster need 3 az for master nodes, but got %d", len(mastersRaw)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's better to return nil, error when the validation failed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
d793cb7
to
b6ee54e
Compare
Test result:
make testacc TEST='./huaweicloud' TESTARGS='-run TestAccCCEClusterV3_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./huaweicloud -v -run TestAccCCEClusterV3_basic -timeout 360m -parallel 4
=== RUN TestAccCCEClusterV3_basic
=== PAUSE TestAccCCEClusterV3_basic
=== CONT TestAccCCEClusterV3_basic
--- PASS: TestAccCCEClusterV3_basic (422.07s)
PASS
ok github.com/huaweicloud/terraform-provider-huaweicloud/huaweicloud 422.138s