-
Notifications
You must be signed in to change notification settings - Fork 163
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
[Feature Request] New bcs instance resource supported #1064
[Feature Request] New bcs instance resource supported #1064
Conversation
docs/resources/bcs_instance.md
Outdated
```hcl | ||
data "huaweicloud_availability_zones" "test" {} | ||
|
||
resource "huaweicloud_cce_cluster" "test" { |
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.
suggest to use data source
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.
This example now uses data source to support the cluster info.
docs/resources/bcs_instance.md
Outdated
resource "huaweicloud_bcs_instance" "test" { | ||
depends_on = [huaweicloud_cce_node.test] | ||
|
||
name = {Resource_Name} |
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.
suggest to use var.instance_name
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.
Has been updated as a variable
docs/resources/bcs_instance.md
Outdated
* `cluster_type` - (Required, String, ForceNew) Specifies the type of the cluster. | ||
Valid value is `cce`. Changing this will create a new instance. |
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.
if this parameter only supports cce
, we can omit it or set it defaults to cce
.
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.
cluster_type has been removed and always set with cce
.
docs/resources/bcs_instance.md
Outdated
Changing this will create a new instance. | ||
|
||
* `orderer_node_num` - (Required, Int, ForceNew) Specifies the number of peers in the orderer organizaion. | ||
Refer to Table 1 for the configuration of the `orderer_node_num`. |
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.
where can I get the link if Table 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.
The description has been updated and use link to point the table.
Target: []string{"Normal"}, | ||
Refresh: blockchainStateRefreshFunc(client, instanceID), | ||
Timeout: d.Timeout(schema.TimeoutCreate), | ||
Delay: 15 * time.Second, |
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.
shall we wait for more time?
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.
According to the creation time of the basic version (The simplest configuration of instance creation time: 2m50s±20s), it is now set to 150s.
"name": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
}, |
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.
can we get the cce cluster name by id?
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.
the cluster_info
struct is updated to cluster_id
parameter.
return nil | ||
} | ||
|
||
func resourceBCSInstanceV2Update(d *schema.ResourceData, meta interface{}) error { |
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.
the function can be deleted
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.
Related notes added.
) | ||
|
||
//Huawei@123 | ||
const salt_password string = "JDYkcGxkUm9uUE55UTEweGRCUyRxbWFDRFluTFhPVjNCakVraGc5YUdhUTBxeWFocmNDbzZ4YS9rVWkyYVB2S" + |
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.
hard coding is not recommended, can we use key_pair?
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 updated
fabric_version = "1.4" | ||
blockchain_type = "private" | ||
enterprise_project_id = "%s" | ||
password = "Huawei@123" |
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.
can we using random_password
?
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.
I see that the acc test does not support random providers, the message is:
- Could not satisfy plugin requirements:
Plugin reinitialization required. Please run "terraform init".
Plugins are external binaries that Terraform uses to access and manipulate
resources. The configuration provided requires plugins which can't be located,
don't satisfy the version constraints, or are otherwise incompatible.
Terraform automatically discovers provider requirements from your
configuration, including providers used in child modules. To see the
requirements and constraints from each module, run "terraform providers".
- provider "random" is not available
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.
The configuration of random password is :
resource "random_password" "instance_password" {
length = 12
special = true
min_lower = 1
min_upper = 1
override_special = "!@$%%^-_=+[{}]:,./?"
}
return &schema.Resource{ | ||
Create: resourceBCSInstanceV2Create, | ||
Read: resourceBCSInstanceV2Read, | ||
Update: resourceBCSInstanceV2Update, |
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.
Since there is nothing to do in update func, this line is unnecessary
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.
This function is used to update delete_obs
and delete_storage
, and the related notes added.
Schema: map[string]*schema.Schema{ | ||
"name": { | ||
Type: schema.TypeString, | ||
Required: true, |
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.
The type of 'channels' is list, so change the param 'name' won't cause forcenew. Is this your intention
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.
thanks, forceNew
is added
docs/resources/bcs_instance.md
Outdated
|
||
* `edition` - (Required, Int, ForceNew) Specifies Service edition of the BCS instance. | ||
Valid values are `1`, `2` and `4`. | ||
For the specifications corresponding to these three versions, please refer to the [specification table](#jump). |
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.
does this can jump to the specification table?
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.
sure, it can jump to the specification table (I mark it with jump
) at the end of Argument Reference.
docs/resources/bcs_instance.md
Outdated
Changing this will create a new instance. | ||
|
||
* `orderer_node_num` - (Required, Int, ForceNew) Specifies the number of peers in the orderer organizaion. | ||
For the number of orderer nodes, please refer to the [specification table](#jump). |
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.
Is the specification table shown the max number of orderer nodes?
docs/resources/bcs_instance.md
Outdated
For the number of orderer nodes, please refer to the [specification table](#jump). | ||
Changing this will create a new instance. | ||
|
||
* `cluster_id` - (Required, String, ForceNew) Specifies the ID of the CCE cluster to attach to the BCS instance. |
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.
suggest to named as cce_cluster_id
Computed: true, | ||
ForceNew: true, | ||
}, | ||
"eip_enable": { |
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.
in schema the default value of eip_enable
is false, but it is defaults to true in docs.
* `type` - (Optional, String, ForceNew) Specifies the type of SFS turbo. | ||
Changing this creates a new instance. | ||
|
||
The `block_info` block supports: |
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.
should add the description of block_info
} | ||
|
||
func resourceBCSInstanceV2Create(d *schema.ResourceData, meta interface{}) error { | ||
var FALSE bool = false |
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.
s/FALSE/newCluster/g
What this PR does / why we need it:
New BCS instance resource supported:
TODO:
Which issue this PR fixes:
(optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged)fixes #1023
Special notes for your reviewer:
Release note:
PR Checklist
Acceptance Steps Performed
BCS client test
Basic test
Kafka consensus