sap-oc / crowbar-openstack

Openstack deployment for Crowbar
3 stars 1 forks source link

Make nas_mount_options configurable #40

Closed mkoderer closed 6 years ago

mkoderer commented 7 years ago

Issue: os_brick lib uses 4.1 per default. This causes issues with permissions in the host systems. We need to set something like: nas_mount_options = vers=3

The Pull request that fixes this issue is here: #42

matelakat commented 7 years ago

Not sure if it is related, but this question is also around NFS mount options: http://lists.openstack.org/pipermail/openstack-dev/2017-May/117466.html

matelakat commented 7 years ago

It seems that nas_mount_options https://github.com/sap-oc/crowbar-openstack/blob/stable/sap/3.0/chef/cookbooks/cinder/templates/default/cinder.conf.erb#L1802 is only added to the config, when we use a different backend, remotefs: https://github.com/sap-oc/crowbar-openstack/blob/stable/sap/3.0/chef/cookbooks/cinder/templates/default/cinder.conf.erb#L1761 Also looking at the code for cinder, it seems that brick client is only using nas_mount_options to override nfs_mount_options: https://github.com/openstack/cinder/blob/liberty-eol/cinder/volume/drivers/nfs.py#L112 So using nfs_mount_options seems to be a better place. I don't get why nfs_shares_config was not taken into account, we have the config there.

snschee commented 7 years ago

Hi Mate, so far we use nfs mount options in nfs_shares_config. This works quite well. But when we want to retire a share we've to remove it from the nfs_shares_config. But when there are still Cinder volumes on this share then we still have to mount this share. At this moment there are no longer any mount options than the "hard coded" defaults which are NFS version 4.1. Otherwise we have to specify the defaults in Cinder config file to override the coded defaults. Br Sebastian

matelakat commented 7 years ago

first draft here: https://github.com/sap-oc/crowbar-openstack/pull/42 needs to be tested

matelakat commented 7 years ago

@snschee the fix will add the nfs_mount_options, however, it seems that on the compute side the options passed to mount depend on the values of nfs_shares_config, for which we have no entries at all. So that's kind of a risk, if a compute needs to re-mount that share, it might not see the flags? This is something that we would need to check. I learned that there is another option libvirt.nfs_mount_options that might be used by nova on the compute side. Is this scenario likely to happen (volume from a retired share be re-mounted on compute)

snschee commented 7 years ago

I think this will happen absolutely really good hint - we should investigate this behavior So far I thought these options will always (also when this is in Cinder config) be transmitted to Nova compute

matelakat commented 7 years ago

@snschee some development on the mailing list in our topic: http://lists.openstack.org/pipermail/openstack-dev/2017-May/117721.html

snschee commented 7 years ago

@matelakat good point to know that there is a newer, prefered way to integrate NAS shares. As far as I understand it, it do not help in our case when retiring a share which still have Cinder volumes and I think it's getting worser than before (I believe). Reading of the nfs_shares_config is a dynamic process - no restart of Cinder volume needed. But rereading the cinder.conf needs a restart of the Cinder volume service.

matelakat commented 7 years ago

@snschee And I also have the feeling that with the new config options, we'll have a 1:1 mapping in terms of backend:share, so I'm not sure how our case would be modeled? could you try https://github.com/sap-oc/crowbar-openstack/pull/42 and see what happens when the volume needs to be re-mounted on the compute side?

snschee commented 7 years ago

@matelakat This would not be a problem, because we use different shares and even different SVMs (NetApp Storage Virtual Machines) per backend. I'll take the task to test the patch also in conjunction with Nova compute and retired share.

mkoderer commented 7 years ago

@snschee can you also check for the compute node setting

snschee commented 7 years ago

@matelakat @mkoderer tested the whole scenario in lab the provided patch works fine for Cinder volume host, but as already expected my Mate we also need the nfs_mount_options option in nova compute in libvirt section as well

matelakat commented 7 years ago

@snschee adding that then.

matelakat commented 7 years ago

The PR is now including nova changes as well, and was tested locally with the NFS Driver.

matelakat commented 7 years ago

FTR, this issue is related in C7 (in relation with NFS driver): https://github.com/crowbar/crowbar-openstack/pull/972

matelakat commented 7 years ago

@snschee what would be the next step? Development and Testing is done on our side, would you like to try it yourself?

vuntz commented 6 years ago

Pull request for SOC 6 CD is merged.