Copilot commented on code in PR #322:
URL:
https://github.com/apache/cloudstack-terraform-provider/pull/322#discussion_r3795788659
##########
cloudstack/resource_cloudstack_kubernetes_cluster.go:
##########
@@ -340,6 +354,14 @@ func resourceCloudStackKubernetesClusterCreate(d
*schema.ResourceData, meta inte
p.SetCniconfigdetails(cniConfigDetailsFormatted)
}
+ if externalLoadBalancerIPAddress, ok :=
d.GetOk("externalloadbalanceripaddress"); ok {
+
p.SetExternalloadbalanceripaddress(externalLoadBalancerIPAddress.(string))
+ }
+
+ if enableCSI, ok := d.GetOk("enablecsi"); ok {
+ p.SetEnablecsi(enableCSI.(bool))
+ }
Review Comment:
Using d.GetOk for an Optional bool prevents callers from explicitly setting
the value to false (GetOk returns ok=false for false). That means
enablecsi=false in config won't be sent to the API. Use GetOkExists so "set to
false" is distinguishable from "unset".
##########
cloudstack/resource_cloudstack_kubernetes_cluster.go:
##########
@@ -214,6 +214,20 @@ func resourceCloudStackKubernetesCluster()
*schema.Resource {
ForceNew: true,
Description: "An optional map of node roles to
instance templates. If not specified, system VM template will be used. Valid
roles are: worker, control, etcd",
},
+
+ "externalloadbalanceripaddress": {
+ Type: schema.TypeString,
+ Optional: true,
+ ForceNew: true,
+ Description: "The external load balancer IP
address for HA clusters (multiple control nodes) on Shared networks",
+ },
+
+ "enablecsi": {
+ Type: schema.TypeBool,
+ Optional: true,
+ ForceNew: true,
+ Description: "Enable CloudStack CSI (Container
Storage Interface) for the Kubernetes cluster",
+ },
Review Comment:
The new schema attribute names are not snake_case (e.g.,
"externalloadbalanceripaddress", "enablecsi"), while this resource otherwise
predominantly uses snake_case (e.g., control_nodes_size, ip_address,
cni_configuration_id). For Terraform UX and consistency, consider exposing
these as external_load_balancer_ip_address and enable_csi, and map them to the
CloudStack API fields internally.
##########
cloudstack/resource_cloudstack_kubernetes_cluster.go:
##########
@@ -340,6 +354,14 @@ func resourceCloudStackKubernetesClusterCreate(d
*schema.ResourceData, meta inte
p.SetCniconfigdetails(cniConfigDetailsFormatted)
}
+ if externalLoadBalancerIPAddress, ok :=
d.GetOk("externalloadbalanceripaddress"); ok {
+
p.SetExternalloadbalanceripaddress(externalLoadBalancerIPAddress.(string))
+ }
+
+ if enableCSI, ok := d.GetOk("enablecsi"); ok {
+ p.SetEnablecsi(enableCSI.(bool))
+ }
Review Comment:
PR description notes this should be merged only after updating the
cloudstack-go dependency/tag; this change introduces calls/fields
(SetExternalloadbalanceripaddress/SetEnablecsi,
Externalloadbalanceripaddress/Enablecsi) that are not present unless the
provider's github.com/apache/cloudstack-go/v2 version is bumped accordingly.
Please include the go.mod/go.sum update in this PR (or remove these references
until the dependency is updated) so the branch builds.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
