Alb fix and add Ingress status report #5160

szombi · 2024-02-29T07:26:51Z

The scope of this PR is updating the current ALB solutions based on our current API and introduce ingress status and ingress config in cluster resource and data source.
More info about the ingress status report: https://cloud.ibm.com/docs/containers?topic=containers-ingress-status

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000

Output from acceptance testing:

resource_ibm_container_cluster_test:
=== RUN   TestAccIBMContainerClusterIngressConfig
--- PASS: TestAccIBMContainerClusterIngressConfig (3489.21s)
PASS
ok  

resource_ibm_container_vpc_cluster_test:
=== RUN   TestAccIBMContainerVpcClusterIngressConfig
--- PASS: TestAccIBMContainerVpcClusterIngressConfig (4855.69s)
PASS
ok  

data_source_ibm_container_cluster_test:
=== RUN   TestAccIBMContainerClusterDataSourceIngressConfig
--- PASS: TestAccIBMContainerClusterDataSourceIngressConfig (2171.34s)
PASS
ok  	github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/kubernetes	2171.718s

data_source_ibm_container_vpc_cluster_test:
=== RUN   TestAccIBMContainerVPCClusterDataSourceIngressConfig
--- PASS: TestAccIBMContainerVPCClusterDataSourceIngressConfig (3884.00s)
PASS

resource_ibm_container_vpc_alb_test:
=== RUN   TestAccIBMContainerVPCClusterALBBasic
--- PASS: TestAccIBMContainerVPCClusterALBBasic (3875.44s)
PASS
ok  	github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/kubernetes	3875.678s


resource_ibm_container_vpc_alb_create_test:
=== RUN   TestAccIBMContainerVPCClusterALBCreate
--- PASS: TestAccIBMContainerVPCClusterALBCreate (3784.44s)
PASS
ok  	github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/kubernetes	3784.656s

resource_ibm_container_alb_test:
=== RUN   TestAccIBMContainerALB_Basic
--- PASS: TestAccIBMContainerALB_Basic (2477.09s)
PASS
ok  	github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/kubernetes	2477.306s

resource_ibm_container_alb_create_test:
=== RUN   TestAccIBMContainerALB_Create
--- PASS: TestAccIBMContainerALB_Create (2133.30s)
PASS
ok  	github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/kubernetes	2133.484s

data_source_ibm_container_alb_test:
=== RUN   TestAccIBMContainerALBDataSource_Basic
--- PASS: TestAccIBMContainerALBDataSource_Basic (4404.49s)
PASS
ok  	github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/kubernetes	4404.632s

ibm/service/kubernetes/resource_ibm_container_vpc_alb_create_test.go

ibm/service/kubernetes/data_source_ibm_container_alb.go

ibm/service/kubernetes/data_source_ibm_container_alb_test.go

SzucsAti · 2024-03-14T15:41:56Z

ibm/service/kubernetes/data_source_ibm_container_cluster.go


 	targetEnv, err := getClusterTargetHeader(d, meta)
 	if err != nil {
 		return err
 	}

+	targetEnvV2, err := getVpcClusterTargetHeader(d, meta)


Bit odd to me that we are calling a vpc function from a data source responsible for classic clusters.

+1 I believe this change should go to ibm/service/kubernetes/data_source_ibm_container_vpc_cluster.go

Discussed offline, needed because we want to use a v2 API with SDK.

SzucsAti · 2024-03-14T15:57:57Z

ibm/service/kubernetes/data_source_ibm_container_vpc_cluster_test.go

+// You need to set up env vars:
+// export IBM_CLUSTER_VPC_ID
+// export IBM_CLUSTER_VPC_RESOURCE_GROUP_ID
+// export IBM_CLUSTER_VPC_SUBNET_ID


acctest.go could be updated with these filenames for these fields.

SzucsAti · 2024-03-14T16:35:11Z

ibm/service/kubernetes/data_source_ibm_container_alb.go

+				Computed:    true,
+				Description: "Status of the ALB",
+			},
+			"ingress_image": {


Shouldn't we call this field version?

ibm/service/kubernetes/resource_ibm_container_alb.go

SzucsAti · 2024-03-14T16:47:35Z

ibm/service/kubernetes/resource_ibm_container_alb.go

 	}

-	numOfInstances := "2"
 	if v, ok := d.GetOk("user_ip"); ok {
 		userIP = v.(string)
 	}
 	params := v1.ALBConfig{


params are only used if the user wants to enable the ALB, we should move initializing/configuring the variable there. I also don't see that setting ingress_image has any effect as it's not passed to the params.

ibm/service/kubernetes/resource_ibm_container_cluster.go

Co-authored-by: Attila Szucs <[email protected]>

SzekeresB · 2024-03-18T09:57:49Z

ibm/service/kubernetes/data_source_ibm_container_alb.go

-				Type:        schema.TypeString,
-				Computed:    true,
-				Description: "ALB zone",
+				Deprecated:  "Remove this attribute's configuration as it no longer is used",


Maybe

Suggested change

Deprecated: "Remove this attribute's configuration as it no longer is used",

Deprecated: "This field deprecated and no longer supported",

ibm/service/kubernetes/data_source_ibm_container_alb_test.go

SzekeresB · 2024-03-18T10:05:04Z

ibm/service/kubernetes/data_source_ibm_container_vpc_alb.go

@@ -57,6 +57,11 @@ func DataSourceIBMContainerVPCClusterALB() *schema.Resource {
 				Type:     schema.TypeString,
 				Computed: true,
 			},
+			"ingress_image": {


same version related observation

SzekeresB · 2024-03-18T10:06:51Z

ibm/service/kubernetes/data_source_ibm_container_cluster.go


 	targetEnv, err := getClusterTargetHeader(d, meta)
 	if err != nil {
 		return err
 	}

+	targetEnvV2, err := getVpcClusterTargetHeader(d, meta)


+1 I believe this change should go to ibm/service/kubernetes/data_source_ibm_container_vpc_cluster.go

ibm/service/kubernetes/resource_ibm_container_vpc_cluster_test.go

Co-authored-by: Balázs Szekeres <[email protected]>

ibm/service/kubernetes/resource_ibm_container_cluster.go

SzekeresB

General comment: The description fields are sometimes sentences with/without a . and sometimes just a few words. It would be nice to have them in a uniform way.

SzekeresB · 2024-03-19T12:17:43Z

ibm/service/kubernetes/data_source_ibm_container_alb.go

+			"version": {
+				Type:        schema.TypeString,
+				Computed:    true,
+				Description: "The type of Ingress image that you want to use for your ALB deployment.",


The type is either public or private so I do not think this a valid wording.

Suggested change

Description: "The type of Ingress image that you want to use for your ALB deployment.",

Description: "the version of the ALB image",

Maybe a pointer could be nice to acquire the current supported images, in the description.

And I believe we should change the alb_type description to a more meaningful one like: the type of ALB, available options: public, private but since it is not part of this change I can't comment there.

SzekeresB · 2024-03-19T12:21:33Z

ibm/service/kubernetes/data_source_ibm_container_vpc_alb.go

+			"version": {
+				Type:        schema.TypeString,
+				Computed:    true,
+				Description: "The type of Ingress image that you want to use for your ALB deployment.",


The type is either public or private so I do not think this a valid wording.

Suggested change

Description: "The type of Ingress image that you want to use for your ALB deployment.",

Description: "the version of the ALB image",

Maybe a pointer could be nice to acquire the current supported images, in the description.

SzekeresB · 2024-03-19T12:23:37Z

ibm/service/kubernetes/resource_ibm_container_alb_create.go

@@ -35,7 +35,7 @@ func ResourceIBMContainerAlbCreate() *schema.Resource {
 				Default:     true,
 				Description: "If set to true, the ALB is enabled by default.",
 			},
-			"ingress_image": {
+			"version": {


description update

As it's an already existing field, we can't just rename it. We have to handle the change properly.

SzucsAti · 2024-03-20T09:35:28Z

ibm/service/kubernetes/resource_ibm_container_alb.go


-	numOfInstances := "2"
 	if v, ok := d.GetOk("user_ip"); ok {


This is also only relevant if enable == true, should be moved to the if condition.

SzucsAti · 2024-03-20T09:37:33Z

ibm/service/kubernetes/resource_ibm_container_alb.go

@@ -187,7 +217,6 @@ func resourceIBMContainerALBUpdate(d *schema.ResourceData, meta interface{}) err

 	if d.HasChange("enable") {
 		enable := d.Get("enable").(bool)
-		disableDeployment := d.Get("disable_deployment").(bool)
 		albID := d.Id()
 		params := v1.ALBConfig{


Similarly to the other cases, params is only needed if enable == true, should be moved there.

SzucsAti · 2024-03-20T09:50:38Z

ibm/service/kubernetes/resource_ibm_container_alb_create.go

@@ -35,7 +35,7 @@ func ResourceIBMContainerAlbCreate() *schema.Resource {
 				Default:     true,
 				Description: "If set to true, the ALB is enabled by default.",
 			},
-			"ingress_image": {
+			"version": {


As it's an already existing field, we can't just rename it. We have to handle the change properly.

SzucsAti · 2024-03-20T09:54:38Z

ibm/service/kubernetes/resource_ibm_container_alb_create.go

@@ -91,11 +96,13 @@ func ResourceIBMContainerAlbCreate() *schema.Resource {
 				Type:        schema.TypeString,
 				Computed:    true,
 				Description: "ALB name",
+				Deprecated:  "Remove this attribute's configuration as it no longer is used",


There are some Deprecated descriptions in this file that should be also updated to be consistent.

szombi added 5 commits February 8, 2024 14:22

ALB fixes and add ALB status report

b159c63

ALB fixes and add ALB status report

2fa8cd9

ALB fixes and add ALB status report

1f26307

ALB fixes and add ALB status report

d252635

Merge remote-tracking branch 'origin/master' into alb-fix

1148e0b

szombi changed the title ~~Alb fix~~ Alb fix and add Ingress status report Mar 12, 2024

SzekeresB reviewed Mar 12, 2024

View reviewed changes

ibm/service/kubernetes/resource_ibm_container_vpc_alb_create_test.go Outdated Show resolved Hide resolved

ALB fixes and add ALB status report

20139fa

SzucsAti reviewed Mar 14, 2024

View reviewed changes

szombi and others added 4 commits March 18, 2024 10:49

Update ibm/service/kubernetes/data_source_ibm_container_alb_test.go

2e9d5db

Co-authored-by: Attila Szucs <[email protected]>

Update ibm/service/kubernetes/data_source_ibm_container_alb.go

61a5899

Co-authored-by: Attila Szucs <[email protected]>

Update ibm/service/kubernetes/resource_ibm_container_cluster.go

4a6f60d

Co-authored-by: Attila Szucs <[email protected]>

Update ibm/service/kubernetes/resource_ibm_container_alb.go

4367a94

Co-authored-by: Attila Szucs <[email protected]>

SzekeresB reviewed Mar 18, 2024

View reviewed changes

szombi and others added 5 commits March 18, 2024 11:18

Update ibm/service/kubernetes/data_source_ibm_container_alb_test.go

42d69bc

Co-authored-by: Balázs Szekeres <[email protected]>

Update ibm/service/kubernetes/data_source_ibm_container_alb_test.go

12c680c

Co-authored-by: Balázs Szekeres <[email protected]>

Update ibm/service/kubernetes/resource_ibm_container_vpc_cluster_test.go

a88cc9a

Co-authored-by: Balázs Szekeres <[email protected]>

Update ibm/service/kubernetes/resource_ibm_container_vpc_cluster_test.go

279c71a

Co-authored-by: Balázs Szekeres <[email protected]>

Update ibm/service/kubernetes/resource_ibm_container_vpc_cluster_test.go

d8d7dee

Co-authored-by: Balázs Szekeres <[email protected]>

szombi commented Mar 18, 2024

View reviewed changes

ibm/service/kubernetes/resource_ibm_container_cluster.go Show resolved Hide resolved

szombi added 2 commits March 19, 2024 12:02

ALB fixes and add ALB status report

cba81fa

Merge remote-tracking branch 'origin/master' into alb-fix

c15ef6f

SzekeresB reviewed Mar 19, 2024

View reviewed changes

SzucsAti reviewed Mar 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alb fix and add Ingress status report #5160

Alb fix and add Ingress status report #5160

szombi commented Feb 29, 2024 •

edited

Loading

SzucsAti Mar 14, 2024

SzekeresB Mar 18, 2024

szombi Mar 19, 2024

SzucsAti Mar 14, 2024

szombi Mar 19, 2024

SzucsAti Mar 14, 2024

szombi Mar 19, 2024

SzucsAti Mar 14, 2024

szombi Mar 19, 2024

SzekeresB Mar 18, 2024

szombi Mar 19, 2024

SzekeresB Mar 18, 2024

szombi Mar 19, 2024

SzekeresB Mar 18, 2024

SzekeresB left a comment

SzekeresB Mar 19, 2024

SzekeresB Mar 19, 2024

SzekeresB Mar 19, 2024

SzucsAti Mar 20, 2024 •

edited

Loading

SzucsAti Mar 20, 2024

SzucsAti Mar 20, 2024

SzucsAti Mar 20, 2024 •

edited

Loading

SzucsAti Mar 20, 2024

	Deprecated: "Remove this attribute's configuration as it no longer is used",
	Deprecated: "This field deprecated and no longer supported",

	Description: "The type of Ingress image that you want to use for your ALB deployment.",
	Description: "the version of the ALB image",

Alb fix and add Ingress status report #5160

Are you sure you want to change the base?

Alb fix and add Ingress status report #5160

Conversation

szombi commented Feb 29, 2024 • edited Loading

Community Note

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

SzekeresB left a 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

SzucsAti Mar 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SzucsAti Mar 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

szombi commented Feb 29, 2024 •

edited

Loading

SzucsAti Mar 20, 2024 •

edited

Loading

SzucsAti Mar 20, 2024 •

edited

Loading