Bug 1417857

Summary: Missing and Wrong documentation for "Persistent Storage Using Azure Disk"
Product: OpenShift Container Platform Reporter: Takayoshi Tanaka <tatanaka>
Component: DocumentationAssignee: brice <bfallonf>
Status: CLOSED CURRENTRELEASE QA Contact: Wenqi He <wehe>
Severity: unspecified Docs Contact: Vikram Goyal <vigoyal>
Priority: unspecified    
Version: 3.4.1CC: aos-bugs, bfallonf, eparis, hchen, jokerman, mmccomas, nhashimo, tlonoy, wsun
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-05-30 01:25:58 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Takayoshi Tanaka 2017-01-31 10:22:44 UTC
Document URL: 
https://docs.openshift.com/container-platform/3.4/install_config/configuring_azure.html
https://docs.openshift.com/container-platform/3.4/install_config/persistent_storage/persistent_storage_azure.html

Section Number and Name: 
Configuring Masters
Configuring Nodes
Creating the Persistent Volume

Describe the issue: 
There are many issues around them when the customer tries to set up Persistent Storage Using Azure Disk.

1)  prerequisite 
Node name which is retrieved by "oc get node" must be the same of Azure VM name. It appears implementation limitation of kuberenetes Azure Cloud Provider.

When cloud-provider is specified as "azure" in node-config.yaml, kuberenetes tries to search ExternalID
https://github.com/kubernetes/kubernetes/blob/v1.4.8/pkg/kubelet/kubelet_node_status.go#L218-L230

Azure Cloud Provider in kubernetes tries to find the VM whose name is node name 
https://github.com/kubernetes/kubernetes/blob/v1.4.8/pkg/cloudprovider/providers/azure/azure_instances.go#L43-L71
https://github.com/kubernetes/kubernetes/blob/v1.4.8/pkg/cloudprovider/providers/azure/azure_wrap.go#L41-L56

If the Azure VM name is different from the OpenShift node name, the node falls into "NotReady" status.
https://github.com/openshift/origin/blob/v1.4.1/pkg/cmd/server/kubernetes/node_config.go#L220-L222

2) master-config and node-config
The indent in master-config.yaml and node-config.yaml are wrong. It's pointed by the upstream PR.
https://github.com/openshift/origin/issues/12498

3) azure.conf
The document specifies "/etc/azure/azure.conf" as an example for config.yaml, but there is no description about azure.conf itself. I found the below azure.conf works well.

```
tenantId: <>
subscriptionId: <>
aadClientId: <>
aadClientSecret: <>
aadTenantId: <same as tenantID>
resourceGroup: <Azure Resource Group name which Azure VM belongs to>
location: <compact style azure region e.g. southeastasia>
```

Ι found this format with referring to kubernetes code.
https://github.com/kubernetes/kubernetes/blob/master/pkg/cloudprovider/providers/azure/azure.go

If "/etc/azure/azure.conf" is missing, atomic-openshift-node.service failed to start.
If some of the parameters are missing, node falls into "NotReady" status.


Suggestions for improvement: 
1) Add prerequisite if my understanding is correct.

2) Fix the indent.

3) Add sample azure.conf file. Also, it's good to refer to "parameters are required" and to point the Azure document how to set up a service principal.
e.g.) https://docs.microsoft.com/en-us/azure/azure-resource-manager/resource-group-create-service-principal-portal


Additional information: 
It helps us to describe the test environment for this feature (Azure VM names, node names, VHD name and URL and so on)

Comment 4 brice 2017-05-02 05:35:19 UTC
Takayoshi,

I've created a PR for this that incorporates your suggestions:

https://github.com/openshift/openshift-docs/pull/4302

Not sure if there's more info to come. I'm finding it hard to find people who know about OpenShift+Azure. 

Please let me know if you have any suggestions for what's here. If there's nothing else, at minimum, I can get your suggestions into the documentation.

Comment 5 Takayoshi Tanaka 2017-05-02 07:51:07 UTC
Thanks! I'll look into it and make a comment if I have.

Comment 6 brice 2017-05-11 04:21:21 UTC
I've updated the PR:

https://github.com/openshift/openshift-docs/pull/4302

I'll put this onto QA.

For QA:

Can I please verify that the prerequisites I've added are correct, and if there's not any more, and also if the information about the /etc/azure/azure.conf file is correct as well. Thank you!

Comment 7 Wenqi He 2017-05-11 05:58:08 UTC
@hchen Could you please also take look of the doc? I could not 100% sure whether the doc is all correct. Actually, QE also need a detailed doc so that I can perform my testing. All the work I have done is from my every step of try. Thanks.

Comment 8 Wenqi He 2017-05-11 06:06:47 UTC
My azure.conf is like below:
{
  "aadClientID" : ...,
  "aadClientSecret" : ...,
  "subscriptionID" : ...,
  "tenantID" : ...,
  "resourceGroup": ...,
}

Maybe we could point out the aadTenantId and location are optional parameters.

Comment 9 hchen 2017-05-12 12:36:12 UTC
Please see my comment on the PR

Comment 10 brice 2017-05-15 03:08:03 UTC
Thanks, Wenqi and Huamin. I've updated the PR. I'm not sure about the JSON vs YAML format. Please let me know if there's anything else.

Comment 11 Wenqi He 2017-05-15 03:19:36 UTC
(In reply to brice from comment #10)
> Thanks, Wenqi and Huamin. I've updated the PR. I'm not sure about the JSON
> vs YAML format. Please let me know if there's anything else.

I have commented on the PR to provide the example of the JSON file format, you can take a look of it. Thanks

Comment 12 Wenqi He 2017-05-16 05:51:20 UTC
Tested with config file in yaml format, it also work. Changing the status to verified. Thanks.

Comment 13 brice 2017-05-16 23:34:57 UTC
Wenqi,

Good to hear. I was confused about which to use, because YAML was suggested to me. Thanks for the review. I'll move this along.

Comment 14 openshift-github-bot 2017-05-17 23:32:30 UTC
Commit pushed to master at https://github.com/openshift/openshift-docs

https://github.com/openshift/openshift-docs/commit/340ae5707d170fb525f03bbeda2928a2f4fd717b
Merge pull request #4302 from bfallonf/azure_1417857

Bug 1417857 Added more into the azure docs