import node-pool #2138

xiaokouliu · 2023-09-20T10:20:10Z

import node pool
fix the bug of node pool created by custom image
add deletion protection

andrew-tx · 2023-09-22T05:28:17Z

tencentcloud/resource_tc_kubernetes_node_pool.go

-			//	Optional:    true,
-			//	Description: "Indicates whether the node pool deletion protection is enabled.",
-			//},
+			"deletion_protection": {


这里调整了默认值吗？存量的会有change吧

这里新增deletion_protection字段，之前仅在code写了，但没有使用

这里本地要测试一下：老的provider升级到这个provider后，用户本地会不会有diff产生。

andrew-tx · 2023-09-22T05:33:21Z

tencentcloud/resource_tc_kubernetes_node_pool.go

+				Type:     schema.TypeString,
+				Optional: true,
+				Default:  "GENERAL",
+				DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {


这里为啥要加这段逻辑呢

修复自定镜像时，不能指定node_os_type参数值的

andrew-tx · 2023-09-22T05:33:48Z

tencentcloud/resource_tc_kubernetes_node_pool.go

 	)
 	if len(items) != 2 {
 		return fmt.Errorf("resource_tc_kubernetes_node_pool id  is broken")
 	}
 	clusterId := items[0]
 	nodePoolId := items[1]

+	if _, ok := d.GetOkExists("cluster_id"); !ok {
+		fmt.Printf("xxxxxx")


这个打印语句要去掉吧

andrew-tx · 2023-09-22T05:48:26Z

tencentcloud/resource_tc_kubernetes_node_pool.go

@@ -1105,6 +1126,9 @@ func resourceKubernetesNodePoolRead(d *schema.ResourceData, meta interface{}) er
 	_ = d.Set("node_count", AutoscalingAddedTotal+ManuallyAddedTotal)
 	_ = d.Set("auto_scaling_group_id", nodePool.AutoscalingGroupId)
 	_ = d.Set("launch_config_id", nodePool.LaunchConfigurationId)
+	if _, ok := d.GetOkExists("unschedulable"); !ok {


这个是新加的字段吗

unschedulable字段不是新增字段的，unschedulable字段设置默认值0，所以需要在纳管逻辑中添加该字段

andrew-tx · 2023-09-22T05:49:50Z

tencentcloud/resource_tc_kubernetes_node_pool.go

@@ -1221,8 +1250,11 @@ func resourceKubernetesNodePoolRead(d *schema.ResourceData, meta interface{}) er
 		}

 		if launchCfg.SecurityGroupIds != nil {
-			launchConfig["security_group_ids"] = helper.StringsInterfaces(launchCfg.SecurityGroupIds)
-			launchConfig["orderly_security_group_ids"] = helper.StringsInterfaces(launchCfg.SecurityGroupIds)
+			if _, ok := d.GetOk("security_group_ids"); ok {


这里加了if else 会有问题吧

security_group_ids是Computed: true，把这个属性忽略了，恢复一下

tencentcloud/resource_tc_kubernetes_node_pool.go

andrew-tx · 2023-09-22T06:03:17Z

tencentcloud/resource_tc_kubernetes_node_pool.go

+			nodeConfig["gpu_args"] = []map[string]interface{}{gpuArgs}
+		}
+		nodeConfigs = append(nodeConfigs, nodeConfig)
+		_ = d.Set("node_config", nodeConfigs)


node_config也是新增字段是吧

node_config不是新增字段，是新增的读取信息的逻辑，仅对纳管使用

andrew-tx · 2023-09-22T06:04:39Z

.changelog/2138.txt

@@ -0,0 +1,3 @@
+```release-note:enhancement


新加的功能是不是要补充下e2e测试

测试大佬测试过

测试大佬测试过

需要在 tencentcloud/resource_tc_kubernetes_node_pool_test.go 中补充e2e自动化测试覆盖

andrew-tx · 2023-09-26T10:56:36Z

tencentcloud/resource_tc_kubernetes_node_pool.go

+	}
+
+	if importFlag {
+		if dMap, ok := helper.InterfacesHeadMap(d, "node_config"); ok {


这段逻辑要去掉吧

andrew-tx · 2023-09-26T11:03:39Z

tencentcloud/resource_tc_kubernetes_node_pool.go

 		// If not check, the diff between computed and default empty value leads to force replacement
 		if _, ok := d.GetOk("multi_zone_subnet_policy"); ok {
 			_ = d.Set("multi_zone_subnet_policy", asg.MultiZoneSubnetPolicy)
 		}
 	}
+	if v, ok := d.GetOkExists("delete_keep_instance"); ok {


这里为啥需要加 if 判断呢

lyu571 · 2023-09-26T12:34:27Z

.changelog/2138.txt

@@ -0,0 +1,3 @@
+```release-note:enhancement
+resource/tencentcloud_kubernetes_node_pool: import node_pool


这个changelog什么意思呢？重新写下吧

lyu571 · 2023-09-26T12:46:14Z

tencentcloud/resource_tc_kubernetes_node_pool.go

+			"deletion_protection": {
+				Type:     schema.TypeBool,
+				Optional: true,
+				Default:  false,


你们接口不传DeletionProtection，本身默认就是false吧。
如果是，这里不要加Default和DiffSuppressFunc，现在加了无形增加代码复杂度。
DiffSuppressFunc本身就是用于正常方案失效时的“兜底”选择。

lyu571 · 2023-09-26T12:47:31Z

tencentcloud/resource_tc_kubernetes_node_pool.go

-			//	Optional:    true,
-			//	Description: "Indicates whether the node pool deletion protection is enabled.",
-			//},
+			"deletion_protection": {


这里本地要测试一下：老的provider升级到这个provider后，用户本地会不会有diff产生。

lyu571 · 2023-09-26T12:52:52Z

tencentcloud/resource_tc_kubernetes_node_pool.go

 	)
 	if len(items) != 2 {
 		return fmt.Errorf("resource_tc_kubernetes_node_pool id  is broken")
 	}
 	clusterId := items[0]
 	nodePoolId := items[1]

+	if _, ok := d.GetOkExists("cluster_id"); !ok {
+		importFlag = true


1.这是要干什么？
2.cluster_id本身是required

andrew-tx

LGTM

lyu571 · 2023-09-27T07:09:32Z

tencentcloud/resource_tc_kubernetes_node_pool.go

 	)
 	if len(items) != 2 {
 		return fmt.Errorf("resource_tc_kubernetes_node_pool id  is broken")
 	}
 	clusterId := items[0]
 	nodePoolId := items[1]

+	if v, ok := d.GetOk("import_flag"); ok {
+		importFlag = v.(bool)


设置为false操作要在这个if里操作。否则非import流程会多一个import_flag。
d.Set("import_flag", false)

lyu571 · 2023-09-27T07:11:17Z

tencentcloud/resource_tc_kubernetes_node_pool.go

@@ -617,9 +626,23 @@ func resourceTencentCloudKubernetesNodePool() *schema.Resource {
 				Computed:    true,
 				Description: "The auto scaling group ID.",
 			},
+			"import_flag": {


这里不要把import_flag暴露出来

lyu571 · 2023-09-27T08:18:40Z

LGTM

andrew-tx reviewed Sep 22, 2023

View reviewed changes

tencentcloud/resource_tc_kubernetes_node_pool.go Show resolved Hide resolved

andrew-tx reviewed Sep 22, 2023

View reviewed changes

andrew-tx reviewed Sep 26, 2023

View reviewed changes

lyu571 reviewed Sep 26, 2023

View reviewed changes

andrew-tx approved these changes Sep 27, 2023

View reviewed changes

lyu571 reviewed Sep 27, 2023

View reviewed changes

import node-pool

b5787a3

lyu571 merged commit 703420f into tencentcloudstack:master Sep 27, 2023

andrew-tx mentioned this pull request Sep 27, 2023

Revert "import node-pool" #2161

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

import node-pool #2138

import node-pool #2138

xiaokouliu commented Sep 20, 2023 •

edited

Loading

andrew-tx Sep 22, 2023

xiaokouliu Sep 22, 2023

lyu571 Sep 26, 2023

andrew-tx Sep 22, 2023

xiaokouliu Sep 22, 2023

andrew-tx Sep 22, 2023

andrew-tx Sep 22, 2023

xiaokouliu Sep 22, 2023

andrew-tx Sep 22, 2023

xiaokouliu Sep 22, 2023

andrew-tx Sep 22, 2023

xiaokouliu Sep 22, 2023

andrew-tx Sep 22, 2023

xiaokouliu Sep 22, 2023

andrew-tx Sep 22, 2023

andrew-tx Sep 26, 2023

andrew-tx Sep 26, 2023

lyu571 Sep 26, 2023

lyu571 Sep 26, 2023

lyu571 Sep 26, 2023

lyu571 Sep 26, 2023

andrew-tx left a comment

lyu571 Sep 27, 2023

lyu571 Sep 27, 2023

lyu571 commented Sep 27, 2023

		@@ -0,0 +1,3 @@
		```release-note:enhancement
		resource/tencentcloud_kubernetes_node_pool: import node_pool

import node-pool #2138

import node-pool #2138

Conversation

xiaokouliu commented Sep 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrew-tx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lyu571 commented Sep 27, 2023

xiaokouliu commented Sep 20, 2023 •

edited

Loading