Change in vdsm[master]: Add --force option to upgrades

Assaf Muller has uploaded a new change for review.

Change subject: Add --force option to upgrades
......................................................................

Add --force option to upgrades

The --force option will ignore the existance of upgrade files
in /var/lib/vdsm/upgrade/

Each upgrade will still only run if it thinks it should run.

Change-Id: I2e6b84bdd14b9cd3921cea187b2654be22989334
Signed-off-by: Assaf Muller 
---
M lib/vdsm/tool/unified_persistence.py
M lib/vdsm/tool/upgrade.py
M lib/vdsm/tool/upgrade_300_networks.py
3 files changed, 17 insertions(+), 3 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/50/22750/1

diff --git a/lib/vdsm/tool/unified_persistence.py b/lib/vdsm/tool/unified_persistence.py
index d1b3d0a..a2d92cf 100644
--- a/lib/vdsm/tool/unified_persistence.py
+++ b/lib/vdsm/tool/unified_persistence.py
@@ -137,7 +137,7 @@

@expose('upgrade-to-unified-persistence')
-def unified_persistence():
+def unified_persistence(*args):
     """
     Upgrade host networking persistence from ifcfg to unified if the
     persistence model is set as unified in /usr/lib64/python2.X/site-packages/
diff --git a/lib/vdsm/tool/upgrade.py b/lib/vdsm/tool/upgrade.py
index c3e92bc..8c9f57b 100644
--- a/lib/vdsm/tool/upgrade.py
+++ b/lib/vdsm/tool/upgrade.py
@@ -17,6 +17,7 @@
 # Refer to the README and COPYING files for full details of the license
 #

+import argparse
 from functools import wraps
 import logging
 import logging.config
@@ -116,6 +117,18 @@
         self._detachUpgradeLog()

+def _parse_args():
+    parser = argparse.ArgumentParser('vdsm-tool')
+    parser.add_argument(
+        '--force',
+        dest='force',
+        default=False,
+        action='store_true',
+        help='Run the upgrade again, even if it was ran before',
+    )
+    return parser.parse_args(sys.argv[2:])
+
+
 def upgrade(upgradeName):
     """
     Used as a decorator for upgrades. Runs the upgrade with an Upgrade
@@ -126,7 +139,8 @@
         @wraps(f)
         def wrapper(*args, **kwargs):
             with Upgrade(upgradeName) as upgrade:
-                if upgrade.isNeeded():
+                cliArguments = _parse_args()
+                if cliArguments.force or upgrade.isNeeded():
                     upgrade.log.debug("Running upgrade %s", upgradeName)
                     try:
                         f(*args, **kwargs)
diff --git a/lib/vdsm/tool/upgrade_300_networks.py b/lib/vdsm/tool/upgrade_300_networks.py
index 0f754fd..41a49b5 100644
--- a/lib/vdsm/tool/upgrade_300_networks.py
+++ b/lib/vdsm/tool/upgrade_300_networks.py
@@ -55,7 +55,7 @@

@expose('upgrade-3.0.0-networks')
-def upgrade_networks():
+def upgrade_networks(*args):
     """
     Since ovirt-3.0, Vdsm uses libvirt networks (with names vdsm-*) to store
     its own networks. Older Vdsms did not have those defined, and used only

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Add --force option to upgrades
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6394/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5501/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6297/ : SUCCESS

Assaf Muller has posted comments on this change.

Change subject: Add --force option to upgrades
......................................................................

Patch Set 1: Verified+1

Verification:
VDSM's invocation of the upgrades doesn't use --force
Using --force properly ignores the upgrade files. Not setting it properly checks for the existence of the upgrade files.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Add --force option to upgrades
......................................................................

Patch Set 1: Code-Review-1

What is it good for? When should it be used? Please explain in the commit message.

Assaf Muller has posted comments on this change.

Change subject: Add --force option to upgrades
......................................................................

Patch Set 1:

It's useful in two cases:
1) For development purposes (Of course this would have been convenient 3 months ago~)
2) More importantly - For support cases. If for whatever reason we'd want a user to run this upgrade again, it's more friendly to tell him to run 'vdsm-tool upgradeName --force' then to manually delete some weird file first.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Add --force option to upgrades
......................................................................

Patch Set 1:

Could you think of a more explicit name? "--force" gave me very little clues on what it is going to do, and which barriers it is going to break.

Yaniv Bronhaim has posted comments on this change.

Change subject: Add --force option to upgrades
......................................................................

Patch Set 1:

(3 comments)

....................................................
File lib/vdsm/tool/upgrade.py
Line 117:         self._detachUpgradeLog()
Line 118: 
Line 119: 
Line 120: def _parse_args():
Line 121:     parser = argparse.ArgumentParser('vdsm-tool')
it will print:

usage: vdsm-tool [--force]

you should add the function's name
Line 122:     parser.add_argument(
Line 123:         '--force',
Line 124:         dest='force',
Line 125:         default=False,

Line 125:         default=False,
Line 126:         action='store_true',
Line 127:         help='Run the upgrade again, even if it was ran before',
Line 128:     )
Line 129:     return parser.parse_args(sys.argv[2:])
sys.argv? iirc you don't call it only by vdsm-tool call.. shouldn't it be the args sent to the command? pass it..
Line 130: 
Line 131: 
Line 132: def upgrade(upgradeName):
Line 133:     """

....................................................
File lib/vdsm/tool/upgrade_300_networks.py
Line 64:     """
Line 65:     networks = netinfo.networks()
Line 66:     bridges = netinfo.bridges()
Line 67: 
Line 68:     if isNeeded(networks, bridges):
should be "and not --force"?

Change in vdsm[master]: Add --run-again option to upgrades

Assaf Muller has posted comments on this change.

Change subject: Add --run-again option to upgrades
......................................................................

Patch Set 1:

(3 comments)

....................................................
File lib/vdsm/tool/upgrade.py
Line 117:         self._detachUpgradeLog()
Line 118: 
Line 119: 
Line 120: def _parse_args():
Line 121:     parser = argparse.ArgumentParser('vdsm-tool')
Done
Line 122:     parser.add_argument(
Line 123:         '--force',
Line 124:         dest='force',
Line 125:         default=False,

Line 125:         default=False,
Line 126:         action='store_true',
Line 127:         help='Run the upgrade again, even if it was ran before',
Line 128:     )
Line 129:     return parser.parse_args(sys.argv[2:])
Sorry I'm not following - The upgrades should and can only be called via vdsm-tool?
Line 130: 
Line 131: 
Line 132: def upgrade(upgradeName):
Line 133:     """

....................................................
File lib/vdsm/tool/upgrade_300_networks.py
Line 64:     """
Line 65:     networks = netinfo.networks()
Line 66:     bridges = netinfo.bridges()
Line 67: 
Line 68:     if isNeeded(networks, bridges):
I wanted an option that could skip the upgrade file check, but wouldn't meddle with an upgrade's logic. I'll rename the option.

Change in vdsm[master]: Add --run-again option to upgrades

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Add --run-again option to upgrades
......................................................................

Patch Set 2:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6442/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5549/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6355/ : SUCCESS

Change in vdsm[master]: Add --run-again option to upgrades

Assaf Muller has posted comments on this change.

Change subject: Add --run-again option to upgrades
......................................................................

Patch Set 2: Verified+1

Change in vdsm[master]: Add --run-again option to upgrades

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Add --run-again option to upgrades
......................................................................

Patch Set 2: Code-Review+1

(I'd like to wait for Yaniv's ACK before approving this)

Change in vdsm[master]: Add --run-again option to upgrades

Yaniv Bronhaim has posted comments on this change.

Change subject: Add --run-again option to upgrades
......................................................................

Patch Set 1:

(1 comment)

....................................................
File lib/vdsm/tool/upgrade.py
Line 125:         default=False,
Line 126:         action='store_true',
Line 127:         help='Run the upgrade again, even if it was ran before',
Line 128:     )
Line 129:     return parser.parse_args(sys.argv[2:])
are you asking?

if that's the case the parse is alright.. but I thought the upgrade is being called not only by vdsm-tool verb, but also during vdsm run
Line 130: 
Line 131: 
Line 132: def upgrade(upgradeName):
Line 133:     """

Change in vdsm[master]: Add --run-again option to upgrades

Yaniv Bronhaim has posted comments on this change.

Change subject: Add --run-again option to upgrades
......................................................................

Patch Set 2:

(1 comment)

Wow!... thanks for waiting for me. sorry to miss that so long.

I thought @upgrade decorator can be used also for NOT vdsm-tool verbs, and can be called without inline arguments. If that's not the case I'm ok with that

....................................................
File lib/vdsm/tool/upgrade_300_networks.py
Line 28: 
Line 29: sys.path.append("/usr/share/vdsm")
Line 30: from netconf import ifcfg
Line 31: 
Line 32: UPGRADE_NAME = 'upgrade-3.0.0-networks'
this change is not related..
Line 33: 
Line 34: 
Line 35: def isNeeded(networks, bridges):
Line 36:     return (MANAGEMENT_NETWORK not in networks and

Change in vdsm[master]: Add --run-again option to upgrades

Assaf Muller has posted comments on this change.

Change subject: Add --run-again option to upgrades
......................................................................

Patch Set 1:

(1 comment)

....................................................
File lib/vdsm/tool/upgrade.py
Line 125:         default=False,
Line 126:         action='store_true',
Line 127:         help='Run the upgrade again, even if it was ran before',
Line 128:     )
Line 129:     return parser.parse_args(sys.argv[2:])
The two upgrades are currently only being called with vdsm-tool, as a prestart task.
Line 130: 
Line 131: 
Line 132: def upgrade(upgradeName):
Line 133:     """

Change in vdsm[master]: Add --run-again option to upgrades

Yaniv Bronhaim has posted comments on this change.

Change subject: Add --run-again option to upgrades
......................................................................

Patch Set 2: Code-Review+1

Change in vdsm[master]: Add --run-again option to upgrades

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Add --run-again option to upgrades
......................................................................

Patch Set 2: Code-Review+2

Change in vdsm[master]: Add --run-again option to upgrades

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Add --run-again option to upgrades
......................................................................

Add --run-again option to upgrades

The --run-again option will ignore the existance of upgrade files
in /var/lib/vdsm/upgrade/

Each upgrade will still only run if it thinks it should run.

The --run-again option has two use cases:
1) It's easier to tell users to run an upgrade again with
   --run-again, then it is to tell him/her to delete some random
   file first, and then run the upgrade again. This will make
   life slightly easier when tickets start popping up.
2) During development of upgrades

Change-Id: I2e6b84bdd14b9cd3921cea187b2654be22989334
Signed-off-by: Assaf Muller 
Reviewed-on: http://gerrit.ovirt.org/22750
Reviewed-by: Yaniv Bronhaim 
Reviewed-by: Dan Kenigsberg 
---
M lib/vdsm/tool/unified_persistence.py
M lib/vdsm/tool/upgrade.py
M lib/vdsm/tool/upgrade_300_networks.py
3 files changed, 23 insertions(+), 7 deletions(-)

Approvals:
  Yaniv Bronhaim: Looks good to me, but someone else must approve
  Assaf Muller: Verified
  Dan Kenigsberg: Looks good to me, approved