doc | ibmcloud: Inconsistency in default values for cos_service_instance_name

[stevenhorsman]

From #18 (comment):

In the next section the doc says:

cos_service_instance_name is the COS Service Instance Name, optional, default to value based on your cluster_name > tfvar if not set, ${cluster_name}-cos-service-instance.

I think we should make the defaults of these line up, otherwise it will get confusing.

We now have inconsistency in the default values of the cos_service_instance_name in the two sections it is used:

• In creating the COS instance we have:

  cos_service_instance_name in variables.tf has the default value

• In building the pod VM image and uploading it we have:

  cos_service_instance_name is the COS Service Instance Name, optional, default to value based on your cluster_name tfvar if not set, ${cluster_name}-cos-service-instance.

This is not a good user experience, so we have a few choices:

• Either get rid of the defaults and stop them being optional altogether
• Change both defaults to cos-image-instance. I don't think there is a requirement for the cos service instance name to be unique across users, so this should work
• Change both defaults to <cluster_name>-cos-image-instance. Unless there is a way to read the cluster_name value from the cluster terraform.tfvars, I don't think this is a good solution, as there isn't another reason for cluster name to be required in the cos instance creation, so it's letting a user not supply one value by making them supply another
• Any other solution someone smarter than me can think of?

[stevenhorsman]

@yoheiueda - you have more knowledge of terraform than me, so do you have an opinion on which option is best?

[yoheiueda]

@stevenhorsman I prefer changing the default value to cos-image-instance.

I found the description of the default value <cluster_name>-cos-image-instance at

cloud-api-adaptor/ibmcloud/README.md

Line 202 in 471c0ba

> - `cos_service_instance_name` is the COS Service Instance Name, optional, default to value based on your `cluster_name` tfvar if not set, `${cluster_name}-cos-service-instance`.

But the terraform configuration files uses a null value for the variable default value.

cloud-api-adaptor/ibmcloud/terraform/podvm-build/variables.tf

Line 23 in 471c0ba

default = null

So, do we need to update the both files?

[stevenhorsman]

@yoheiueda - thanks for the input. That matches what I'd expect.

I think that we need to update to the doc as you said and add a new default into

cloud-api-adaptor/ibmcloud/terraform/podvm-build/variables.tf

Line 23 in 471c0ba

default = null

and remove the logic in

cloud-api-adaptor/ibmcloud/terraform/podvm-build/main.tf

Line 12 in 471c0ba

cos_service_instance_name = var.cos_service_instance_name != null ? var.cos_service_instance_name : "${var.cluster_name}-cos-service-instance"

which currently does the defaulting.

BTW - I think Jonah is probably going to be looking at this issue soon.

[Jonah-F]

README requires change to manual running of ansible playbook as current command would not work because that playbook no longer exists.

> $ ansible-playbook -i ./inventory -u root ./playbook.yml

needs to be

> $ ansible-playbook -i ./inventory -u root ./kube-playbook.yml && ansible-playbook -i ./inventory -u root ./kata-playbook.yml

[stevenhorsman]

issue accidentally closed by the wrong issue being labelled in the PR #37 commit, so re-opening.

[stevenhorsman]

Closed with #29 merging