Make configuration DB location survive ESXi reboot #1401

msterin · 2017-06-12T23:50:29Z

On reboot all changes in /etc are lost, unless they are backed up.
ESXi has a standard backup for /etc/rc.local.d/local.sh - this file is
backed up and run on boot ,so to persist symlink to shared DB we need to
insert the link creation to local.sh. And when we drop the link, we need
to clean up the local.sh

This is exactly what this change is doing.

Tested - unit test attached. Admin_cli tested manually:

Add:

[root@msterin-esx-136:/usr/lib/vmware/vmdkops/bin] ./vmdkops_admin.py config init --datastore=datastore1 --force                                                                                                                               
Warning: this feature is EXPERIMENTAL                                                                                                                                                                                                          
Creating a symlink to /vmfs/volumes/datastore1/dockvols/vmdkops_config.db at /etc/vmware/vmdkops/auth-db                                                                                                                                       
Updating /etc/rc.local.d/local.sh                                                                                                                                                                                                              
[root@msterin-esx-136:/usr/lib/vmware/vmdkops/bin] cat /etc/rc.local.d/local.sh                                                                                                                                                                
#!/bin/sh                                                                                                                                                                                                                                      
                                                                                                                                                                                                                                               
# local configuration options                                                                                                                                                                                                                  
                                                                                                                                                                                                                                               
# Note: modify at your own risk!  If you do/use anything in this                                                                                                                                                                               
# script that is not part of a stable API (relying on files to be in                                                                                                                                                                           
# specific places, specific tools, specific output, etc) there is a                                                                                                                                                                            
# possibility you will end up with a broken system after patching or                                                                                                                                                                           
# upgrading.  Changes are not supported unless under direction of                                                                                                                                                                              
# VMware support.                                                                                                                                                                                                                              
                                                                                                                                                                                                                                               
# Note: This script will not be run when UEFI secure boot is enabled.                                                                                                                                                                          
                                                                                                                                                                                                                                               
# -- vSphere Docker Volume Service configuration --                                                                                                                                                                                            
#                                                                                                                                                                                                                                              
# Please do not edit this section manually. It is managed by vmdkops_admin.py config command.                                                                                                                                                  
# Note: the code relies on local.sh having "exit 0" at the end.                                                                                                                                                                                
#                                                                                                                                                                                                                                              
                                                                                                                                                                                                                                               
datastore=datastore1                                                                                                                                                                                                                           
                                                                                                                                                                                                                                               
slink=/etc/vmware/vmdkops/auth-db                                                                                                                                                                                                              
shared_dbn=/vmfs/volumes/$datastore/dockvols/vmdkops_config.db                                                                                                                                                                                 
                                                                                                                                                                                                                                               
if [ -d $(basename $slink) ] && [ ! -e $slink  ]                                                                                                                                                                                               
then                                                                                                                                                                                                                                           
    ln -s $shared_db $slink                                                                                                                                                                                                                    
fi                                                                                                                                                                                                                                             
                                                                                                                                                                                                                                               
# -- vSphere Docker Volume Service configuration --                                                                                                                                                                                            
exit 0                                                                                                                                                                                                                                         
[root@msterin-esx-136:/usr/lib/vmware/vmdkops/bin>

remove :

[root@msterin-esx-136:/usr/lib/vmware/vmdkops/bin] ./vmdkops_admin.py config rm --unlink --confirm
Removed link /etc/vmware/vmdkops/auth-db
Updating /etc/rc.local.d/local.sh
[root@msterin-esx-136:/usr/lib/vmware/vmdkops/bin] cat /etc/rc.local.d/local.sh
#!/bin/sh

# local configuration options

# Note: modify at your own risk!  If you do/use anything in this
# script that is not part of a stable API (relying on files to be in
# specific places, specific tools, specific output, etc) there is a
# possibility you will end up with a broken system after patching or
# upgrading.  Changes are not supported unless under direction of
# VMware support.

# Note: This script will not be run when UEFI secure boot is enabled.

exit 0
[root@msterin-esx-136:/usr/lib/vmware/vmdkops/bin]

pshahzeb · 2017-06-13T03:59:27Z

esx_service/cli/local_sh.py

+# This is what we use to identify the our content for DB links..
+CONFIG_DB_TAG = "# -- vSphere Docker Volume Service configuration --"
+
+# This is the content tempate for db links. '{}' will be replaced by datastore name


nit: template

shaominchen

We should have an e2e test to verify this scenario. Can we file a issue to track the task of adding an e2e test?

shaominchen · 2017-06-13T04:44:34Z

esx_service/cli/local_sh.py

+"""
+Support for adding/removing information about config db link from /etc/rc.local.d/local.sh
+Any config stuff we need and configure in /etc/... will be removed on ESX reboot.
+Anything we need to persist between the reboots, needs to be confugured here.


Nit: confugured => configured

shaominchen · 2017-06-13T04:47:29Z

esx_service/cli/local_sh.py

+# We need to insert the content before it.
+END_OF_SCRIPT = "exit 0"
+
+# This is what we use to identify the our content for DB links..


Nit: "the our" => "the"

shaominchen · 2017-06-13T04:49:02Z

esx_service/cli/local_sh.py

+datastore={}
+
+slink=/etc/vmware/vmdkops/auth-db
+shared_dbn=/vmfs/volumes/$datastore/dockvols/vmdkops_config.db


shared_dbn => shared_db?

shaominchen · 2017-06-13T04:50:19Z

esx_service/cli/local_sh.py

+
+# open file and scan it.
+#
+# if we reached "exit 0" add the text section, add the rest of the file and be done


Nit:

Let's keep the tense consistent: reached => reach (since we are using "find" below)

What does "add the rest of the file and be done" mean?

shaominchen · 2017-06-13T04:53:25Z

esx_service/cli/local_sh.py

+        # requested content just in case.
+        if add:
+            sys.stdout.write(content)
+


Nit: remove one empty line

I want 2 newline between functions

shaominchen · 2017-06-13T04:54:14Z

esx_service/cli/local_sh_test.py

+import shutil
+
+class TestLocalShInfo(unittest.TestCase):
+    """ Basic test for saving cofig DB link uising local.sh. The test checks saving


Nit: cofig => config; uising => using; fie => file

Darn, my spellchecker was off. Thanks for the catch. Fixed

shaominchen · 2017-06-13T04:55:35Z

esx_service/cli/local_sh_test.py

+
+    # Basic test: add new content. Replace it. Remove it.
+    # Compare with original content - should be the same.
+    # Also, on neach step check some pattern in the current file


Nit: neach => each?

shaominchen · 2017-06-13T04:56:42Z

esx_service/cli/local_sh_test.py

+        else:
+            print("local.sh update/remove test - All good")
+        os.remove(name)
+


Nit: remove one empty line

thanks Sam, will do (will fix all nits and file an issue)

pshahzeb

Looks good.
couple of nits and a question about inplace update of the file.

pshahzeb · 2017-06-13T04:07:54Z

esx_service/cli/vmdkops_admin.py

@@ -1343,16 +1349,19 @@ def config_rm(args):
                print("Removed link {}".format(link_path))
            except Exception as ex:
                print(" Failed to remove {}: {}".format(link_path, ex))
+            print("Updating /etc/rc.local.d/local.sh")


"/etc/rc.local.d/local.sh" should this be a const?

yes, good catch. will fix

pshahzeb · 2017-06-13T05:00:03Z

esx_service/cli/local_sh.py

+            # First tag - add the content (if needed) and skip till the next tag
+            if add:
+                sys.stdout.write(content)
+            skip_to_tag = True


Just confirming the code flow about how is this piece working when there is already one section with vdvs tag (datastore 1) in the file and then this function is called to add another section with vdvs tag (datastore 2). The new section would be written and old section would be removed?

yes, that's exactly the behavior

govint

Can the ESX service instead use a symbolic link in /usr/lib/vmware/vmdkops instead of a link in /etc/vmware? Given the comments in local.sh regarding its usage.
If needing to still use a link from /etc/vmware can the VIB install a script under /usr/lib/vmware/vmdkops/bin which handles restoring the linkage to the DB and insert an invocation of the script in local.sh - only one line addition to local.sh
Having a script to handle the linkage is easier, any changes to handling links to the DB can be done to the script vs. in local_sh.py.

msterin · 2017-06-13T06:51:34Z

@govint

Can the ESX service instead use a symbolic link in /usr/lib/vmware/vmdkops instead of a link in /etc/vmware?

it can but the result will be the same. VIB is loaded in ramdisk and changes to ramdisk are discarded on reboot

If needing to still use a link from /etc/vmware can the VIB install a script under /usr/lib/vmware/vmdkops/bin which handles restoring the linkage to the DB and insert an invocation of the script in local.sh - only one line addition to local.sh

I did not catch it - can you elaborate ? Keep in mind that all locations we install via VIB are just a tar unpacked into ramdisk, and ramdisk changes are discarded on reboot. There is no system persistency without state.tgz backup (there are plenty of related KBs)

pshahzeb

LGTM

msterin · 2017-06-14T03:36:59Z

please do not merge yet, I need to investigate a potential issue...

pshahzeb · 2017-06-14T09:09:43Z

@msterin curious as to what is the potential issue?

msterin · 2017-06-14T15:09:38Z

~~On my test laptop one of the esxi reboots failed and it is now stuck in booting. I want to investigate before any merge~~

never mind, it was VSAN-related issue - delay in starting vsantraced on rebooot. The vDVS config worked fine.

shaominchen · 2017-06-19T22:12:49Z

esx_service/cli/vmdkops_admin.py

@@ -1323,7 +1331,17 @@ def config_rm(args):
            info = auth.get_info()


Not the current change, but the info variable seems not being used.

shaominchen · 2017-06-19T22:15:27Z

esx_service/cli/vmdkops_admin.py

+                os.remove(auth_data.AUTH_DB_PATH)
+            except:
+                pass
+        return None


Shouldn't this return statement be inside the above "except auth_data.DbAccessError as ex:" block? The current indention seems incorrect as it will return in any case.

yes, there is an indent error and it should be returning only for "except auth_data.DbAccessError as ex:". Thanks for the catch

pdhamdhere · 2017-06-20T17:21:40Z

@msterin Please triage CI failure. This has been open for a while. Let's try to get this in asap.

msterin · 2017-06-20T17:37:02Z

@pdhamdhere - looks test-related. The branch was a bit behind - just rebased and resubmitted.
Will triage when CI is done

msterin · 2017-06-20T20:22:41Z

test fails in docker volume create, for some reason driver name does not seem to have been passed as it says "vol_access_volume_l33x03nwf9uh@datastore1" includes invalid characters for a local volume name

the failure is unrelated to the fix but concerning. @shuklanirdesh82 - any comment ?

Also @shuklanirdesh82 - there is still garbage in the logs (after cron) , I thought it was fixed ?

2017/06/20 18:48:10 Destroying volume [vol_access_volume_l33x03nwf9uh@datastore1]

----------------------------------------------------------------------
FAIL: volume_access_test.go:76: VolumeAccessTestSuite.TestAccessUpdate

volume_access_test.go:91:
    c.Assert(err, IsNil, Commentf(out))
... value *exec.ExitError = &exec.ExitError{ProcessState:(*os.ProcessState)(0xc4201263e0), Stderr:[]uint8(nil)} ("exit status 125")
... docker: Error response from daemon: create vol_access_volume_l33x03nwf9uh@datastore1: "vol_access_volume_l33x03nwf9uh@datastore1" includes invalid characters for a local volume name, only "[a-zA-Z0-9][a-zA-Z0-9_.-]" are allowed. If you intended to pass a host directory, use absolute path.
See 'docker run --help'.

2017/06/20 18:48:11 START: VolumeAccessTestSuite.TestAccessUpdate_R_RW
2017/06/20 18:48:11 Creating volume [vol_access_volume_f0iszigydzxy@datastore1] with options [ -o access=read-only] on VM [192.168.31.81]
2017/06/20 18:48:18 Writing message_by_host1 to file test.txt on volume [vol_access_volume_f0iszigydzxy@datastore1] from VM[192.168.31.81]
2017/06/20 18:48:23 Writing message_by_host2 to file test.txt on volume [vol_access_volume_f0iszigydzxy@datastore1] from VM[192.168.31.85]
2017/06/20 18:48:24 Destroying volume [vol_access_volume_f0iszigydzxy@datastore1]

----------------------------------------------------------------------
FAIL: volume_access_test.go:139: VolumeAccessTestSuite.TestAccessUpdate_R_RW

volume_access_test.go:154:
    c.Assert(strings.Contains(out, errorWriteVolume), Equals, true, Commentf(out))
... obtained bool = false
... expected bool = true
... docker: Error response from daemon: create vol_access_volume_f0iszigydzxy@datastore1: "vol_access_volume_f0iszigydzxy@datastore1" includes invalid characters for a local volume name, only "[a-zA-Z0-9][a-zA-Z0-9_.-]" are allowed. If you intended to pass a host directory, use absolute path.
See 'docker run --help'.
'

pshahzeb · 2017-06-21T03:51:08Z

@msterin @shuklanirdesh82
Looking into this issue about volume creation failed. Filed issue to track this #1448

shuklanirdesh82 · 2017-06-21T06:29:51Z

#1401 (comment)

test fails in docker volume create, for some reason driver name does not seem to have been passed as it says "vol_access_volume_l33x03nwf9uh@datastore1" includes invalid characters for a local volume name

the failure is unrelated to the fix but concerning. @shuklanirdesh82 - any comment ?

We have seen such failure locally once, I asked @pshahzeb to raise an issue to triage further (#1401 (comment)). The failure is unrelated to your PR.

Also @shuklanirdesh82 - there is still garbage in the logs (after cron) , I thought it was fixed ?

Yeah it was fixed. I will check once again what is causing this again. Thanks @msterin for pointing out.

shuklanirdesh82 · 2017-06-21T09:04:42Z

#1401 (comment)
We have seen such failure locally once, I asked @pshahzeb to raise an issue to triage further (#1401 (comment)). The failure is unrelated to your PR.

After triaging further came to know that it is a regression introduced by this PR.

E2E test failure is tricky and not helping to find out root cause easily. The failure observed due to this PR is raising a false impression about volume_access_test failure in fact the failure is with basic_test (it fails to find out the root cause).

I had to checkout the branch locally to generate the VIB and following is my analysis.

conclusion: config rm --local --confirm does nothing and stays the DB as it is.

steps to reproduce:

config init (Single mode)
vmdkops_admin config status
config remove --confirm
vmdkops_admin config status <=== here is the failure config stays as SingleMode

[root@promc-2n-dhcp105-97:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py config init --local
Warning: this feature is EXPERIMENTAL
Creating new DB at /etc/vmware/vmdkops/auth-db
Warning: Local configuration will not survive ESXi reboot. See KB2043564 for details

[root@promc-2n-dhcp105-97:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py status
=== Service: 
Version: 0.14.6732c52-0.0.1
Status: Running
Pid: 925689
Port: 1019
LogConfigFile: /etc/vmware/vmdkops/log_config.json
LogFile: /var/log/vmware/vmdk_ops.log
LogLevel: INFO
=== Authorization Config DB: 
DB_LocalPath: /etc/vmware/vmdkops/auth-db
DB_SharedLocation: N/A
DB_Mode: SingleNode (local DB exists)

[root@promc-2n-dhcp105-97:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py vmgroup ls
Uuid                                  Name      Description                Default_datastore  VM_list  
------------------------------------  --------  -------------------------  -----------------  -------  
11111111-1111-1111-1111-111111111111  _DEFAULT  This is a default vmgroup  _VM_DS                      

[root@promc-2n-dhcp105-97:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py vmgroup create --name=T1 --default-datastore=_VM_DS --vm-list=ubuntu1
vmgroup 'T1' is created. Do not forget to run 'vmgroup vm add' to add vm to vmgroup.
[root@promc-2n-dhcp105-97:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py vmgroup ls
Uuid                                  Name      Description                Default_datastore  VM_list  
------------------------------------  --------  -------------------------  -----------------  -------  
11111111-1111-1111-1111-111111111111  _DEFAULT  This is a default vmgroup  _VM_DS                      
55a13c06-ee8e-4435-ba2a-77894057b208  T1                                   _VM_DS             ubuntu1  

[root@promc-2n-dhcp105-97:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py config rm --local --confirm
[root@promc-2n-dhcp105-97:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py vmgroup ls
Uuid                                  Name      Description                Default_datastore  VM_list  
------------------------------------  --------  -------------------------  -----------------  -------  
11111111-1111-1111-1111-111111111111  _DEFAULT  This is a default vmgroup  _VM_DS                      
55a13c06-ee8e-4435-ba2a-77894057b208  T1                                   _VM_DS             ubuntu1  

[root@promc-2n-dhcp105-97:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py config status
DB_LocalPath: /etc/vmware/vmdkops/auth-db
DB_SharedLocation: N/A
DB_Mode: SingleNode (local DB exists)

[root@promc-2n-dhcp105-97:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py config rm --local --confirm

[root@promc-2n-dhcp105-97:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py config status
DB_LocalPath: /etc/vmware/vmdkops/auth-db
DB_SharedLocation: N/A
DB_Mode: SingleNode (local DB exists)

msterin · 2017-06-21T09:18:42Z

thanks for the analysis.
It sounds like a basic test for "config rm' is either missing or did not catch the issue. Is it on the list for 1.6 ?

shuklanirdesh82 · 2017-06-21T09:20:46Z

It sounds like a basic test for "config rm' is either missing or did not catch the issue.

The step is there but not performing any validation. (https://github.com/vmware/docker-volume-vsphere/blob/master/tests/e2e/basic_test.go#L197)

	// Remove Config DB
	admincli.ConfigRemove(s.esx)

	misc.LogTestEnd(c.TestName())
}

Is it on the list for 1.6 ?

I have just created a new issue. (#1450)

pshahzeb · 2017-06-21T16:48:17Z

Thanks for analysis @shuklanirdesh82 .
I too looked at why error message thrown was about' illegal characters during volume creation'
After going through the steps you mentioned, I think here is what is happening.

steps to reproduce:

config init (Single mode)
vmdkops_admin config status
config remove --confirm
vmdkops_admin config status <=== here is the failure config stays as SingleMode

Since the cleanup didn't complete, VM2 is still a part of different vmgroup (not default)
Durin access update test,

volume v1 is created from VM1.
A message is written from VM1 to a file on volume v1.
Next step is to read the content of the file from VM2.
- As part of this step, we fire a container from VM2 and attach volume v1.
- But since VM2 is still part of different vmgroup, it can't see v1.
- So docker tries to create a local volume with name (which is in format name@datastore) on VM2.
- @ character is invalid for the local volume name and hence the error.

This is what I think is happening.

On reboot all changes in /etc are lost, unless they are backed up. ESXi has a standard backup for /etc/rc.local.d/local.sh - this file is backed up and run on boot ,so to persist symlink to shared DB we need to insert the link crestion to local.sh. And when we drop the link, we need to clean up the local.sh This is exactly what this change is doing

vmwclabot added the cla-not-required label Jun 12, 2017

pshahzeb reviewed Jun 13, 2017

View reviewed changes

shaominchen reviewed Jun 13, 2017

View reviewed changes

pshahzeb reviewed Jun 13, 2017

View reviewed changes

govint reviewed Jun 13, 2017

View reviewed changes

msterin force-pushed the persist_dblink.msterin branch 2 times, most recently from c43cd9a to b30ff67 Compare June 14, 2017 01:22

pshahzeb approved these changes Jun 14, 2017

View reviewed changes

pshahzeb assigned msterin Jun 14, 2017

shaominchen reviewed Jun 19, 2017

View reviewed changes

msterin force-pushed the persist_dblink.msterin branch from 7fca0f3 to 6732c52 Compare June 20, 2017 17:36

shuklanirdesh82 mentioned this pull request Jun 21, 2017

Invalid characters in name error during volume creation. #1448

Closed

shuklanirdesh82 mentioned this pull request Jun 21, 2017

Need to add more verification steps for basic_test #1450

Closed

Mark Sterin added 2 commits June 21, 2017 12:02

fixed typos and review comments

45ed101

msterin force-pushed the persist_dblink.msterin branch 2 times, most recently from a46ba38 to bc15906 Compare June 21, 2017 22:13

msterin force-pushed the persist_dblink.msterin branch from bc15906 to db1db20 Compare June 21, 2017 23:26

Allow to remove broken configuration.

a244904

msterin force-pushed the persist_dblink.msterin branch from db1db20 to a244904 Compare June 22, 2017 04:31

msterin merged commit 3ed8c16 into master Jun 22, 2017

shuklanirdesh82 deleted the persist_dblink.msterin branch November 10, 2017 05:31

		@@ -1323,7 +1331,17 @@ def config_rm(args):
		info = auth.get_info()

Make configuration DB location survive ESXi reboot #1401

Make configuration DB location survive ESXi reboot #1401

Conversation

msterin commented Jun 12, 2017 • edited Loading

Choose a reason for hiding this comment

shaominchen 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

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

pshahzeb 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

Choose a reason for hiding this comment

govint left a comment • edited Loading

Choose a reason for hiding this comment

msterin commented Jun 13, 2017

pshahzeb left a comment

Choose a reason for hiding this comment

msterin commented Jun 14, 2017

pshahzeb commented Jun 14, 2017

msterin commented Jun 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdhamdhere commented Jun 20, 2017

msterin commented Jun 20, 2017

msterin commented Jun 20, 2017

pshahzeb commented Jun 21, 2017

shuklanirdesh82 commented Jun 21, 2017 • edited Loading

shuklanirdesh82 commented Jun 21, 2017 • edited Loading

msterin commented Jun 21, 2017

shuklanirdesh82 commented Jun 21, 2017

pshahzeb commented Jun 21, 2017

msterin commented Jun 12, 2017 •

edited

Loading

govint left a comment •

edited

Loading

msterin commented Jun 14, 2017 •

edited

Loading

shuklanirdesh82 commented Jun 21, 2017 •

edited

Loading

shuklanirdesh82 commented Jun 21, 2017 •

edited

Loading