Change in vdsm[master]: [WIP] net.api: expose cancel_rollback()

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has uploaded a new change for review.

Change subject: [WIP] net.api: expose cancel_rollback()
......................................................................

[WIP] net.api: expose cancel_rollback()

When connectivityCheck=True is passed to the setupNetworks verb, an
automatically roll back is performed, unless unless cancel_rollback()
command is requested by the client. Note that Vdsm does not have an
explicit cancel_rollback verb. cancel_rollback is called as a side
effect of the ping verb.

Change-Id: If42316ac6ecd694e398534e3e87b915aeb4f1f38
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M lib/vdsm/constants.py.in
M lib/vdsm/network/api.py
M lib/vdsm/network/connectivity.py
M vdsm/API.py
4 files changed, 9 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/99/79299/1

diff --git a/lib/vdsm/constants.py.in b/lib/vdsm/constants.py.in
index 32c87d9..7ad3eb1 100644
--- a/lib/vdsm/constants.py.in
+++ b/lib/vdsm/constants.py.in
@@ -80,7 +80,6 @@
 P_VDSM_CONF = '@CONFDIR@/'  # NOQA: E501 (potentially long line)
 P_VDSM_KEYS = '/etc/pki/vdsm/keys/'
 
-P_VDSM_CLIENT_LOG = &#39;@VDSMRUNDIR(a)/client.log&#39;  # NOQA: E501 (potentially long
line)
 P_VDSM_LOG = '@VDSMLOGDIR@'  # NOQA: E501 (potentially long line)
 P_VDSM_NODE_ID = '/etc/vdsm/vdsm.id'
 
diff --git a/lib/vdsm/network/api.py b/lib/vdsm/network/api.py
index cb7210f..8c4113e 100644
--- a/lib/vdsm/network/api.py
+++ b/lib/vdsm/network/api.py
@@ -27,6 +27,7 @@
 
 from vdsm import hooks
 
+from vdsm.common.fileutils import touch_file
 from vdsm.network import netswitch
 from vdsm.network import sourceroute
 from vdsm.network.configurators.ifcfg import ConfigWriter
@@ -281,3 +282,7 @@
 
 def remove_ovs_port(bridge, port):
     netswitch.configurator.ovs_remove_port(bridge, port)
+
+
+def cancel_rollback():
+    touch_file(constants.P_VDSM_CLIENT_LOG)
diff --git a/lib/vdsm/network/connectivity.py b/lib/vdsm/network/connectivity.py
index 3b2402d..11e5601 100644
--- a/lib/vdsm/network/connectivity.py
+++ b/lib/vdsm/network/connectivity.py
@@ -30,6 +30,7 @@
 from .errors import ConfigNetworkError
 
 CONNECTIVITY_TIMEOUT_DEFAULT = 4
+P_VDSM_CLIENT_LOG = constants.P_VDSM_RUN + 'client.log'
 
 
 def _get_connectivity_timeout(options):
@@ -50,7 +51,7 @@
     start = time.time()
     while timeout >= 0:
         try:
-            if os.stat(constants.P_VDSM_CLIENT_LOG).st_mtime > start:
+            if os.stat(P_VDSM_CLIENT_LOG).st_mtime > start:
                 return True
         except OSError as e:
             if e.errno == errno.ENOENT:
diff --git a/vdsm/API.py b/vdsm/API.py
index fc928a7..056f61e 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -22,6 +22,7 @@
 
 import os
 
+from vdsm.network.api import cancel_rollback
 from vdsm.network.errors import ConfigNetworkError
 
 from vdsm import commands
@@ -78,13 +79,6 @@
 
 
 throttledlog.throttle('getAllVmStats', 100)
-
-
-def updateTimestamp():
-    # The setup API uses this log file to determine if this host is still
-    # accessible.  We use a file (rather than an event) because setup is
-    # performed by a separate, root process.
-    fileutils.touch_file(constants.P_VDSM_CLIENT_LOG)
 
 
 class APIBase(object):
@@ -1237,7 +1231,7 @@
 
     def ping(self):
         "Ping the server. Useful for tests"
-        updateTimestamp()
+        cancel_rollback()
         return {'status': doneCode}
 
     def getCapabilities(self):


-- 
To view, visit https://gerrit.ovirt.org/79299
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: If42316ac6ecd694e398534e3e87b915aeb4f1f38
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has posted comments on this change.

Change subject: [WIP] net.api: expose cancel_rollback()
......................................................................


Patch Set 3: Verified+1

Ran 70 tests in 419.325s

OK (SKIP=1)

-- 
To view, visit https://gerrit.ovirt.org/79299
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If42316ac6ecd694e398534e3e87b915aeb4f1f38
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has posted comments on this change.

Change subject: [WIP] net.api: expose cancel_rollback()
......................................................................


Patch Set 3:

ci please build

-- 
To view, visit https://gerrit.ovirt.org/79299
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If42316ac6ecd694e398534e3e87b915aeb4f1f38
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: net.api: expose cancel_rollback()

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has posted comments on this change.

Change subject: net.api: expose cancel_rollback()
......................................................................


Patch Set 4: Verified+1

copying score

-- 
To view, visit https://gerrit.ovirt.org/79299
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If42316ac6ecd694e398534e3e87b915aeb4f1f38
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: net.api: expose updateTimestamp as cancel_rollback()

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has posted comments on this change.

Change subject: net.api: expose updateTimestamp as cancel_rollback()
......................................................................


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/79299/4/lib/vdsm/network/api.py
File lib/vdsm/network/api.py:

PS4, Line 288: cancel_rollback
>  This is a bit strange, the function name does not tell what it does,
> but wh 
In my opinion, "connectivity" is only an implementation detail for
this verb.

A user may have another way to check whether a pending setupNetworks is correct (e.g, when
run from the localhost). It is really not the business of vdsm to tell him that he can
avoid rollback only if Engine pings the host.

setupNetworks() should have declared an auto_rollback=True and rollback_timeout=; come to
think of it, the connectivity module should be renamed, as well as P_VDSM_CLIENT_LOG.


-- 
To view, visit https://gerrit.ovirt.org/79299
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If42316ac6ecd694e398534e3e87b915aeb4f1f38
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

Change in vdsm[master]: net.api: expose updateTimestamp as cancel_rollback()

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has posted comments on this change.

Change subject: net.api: expose updateTimestamp as cancel_rollback()
......................................................................


Patch Set 5: Code-Review-1

I think that the args of setupNetworks and connectivity.py should be renamed:

 connectivityCheck -> auto_rollback
 connectivityTimeout -> rollback_timeout
 connectivity.py -> rollback
 client.log -> cancel_rollback_request

-- 
To view, visit https://gerrit.ovirt.org/79299
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If42316ac6ecd694e398534e3e87b915aeb4f1f38
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: net: move API.updateTimestamp to net.connectivity.confirm

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has posted comments on this change.

Change subject: net: move API.updateTimestamp to net.connectivity.confirm
......................................................................


Patch Set 4:

(2 comments)

https://gerrit.ovirt.org/#/c/79299/4/lib/vdsm/network/api.py
File lib/vdsm/network/api.py:

PS4, Line 288: cancel_rollback
>  The connectivity module is responsible of checking connectivity, it
> has no  
ok, I'm shelving my grand plans for now. removing P_CLIENT_LOG out of
vdsm.constants is more important to me now.


Line 285:     netswitch.configurator.ovs_remove_port(bridge, port)
Line 286: 
Line 287: 
Line 288: def cancel_rollback():
Line 289:     touch_file(connectivity.P_VDSM_CLIENT_LOG)
>  How abut pushing in the implementation to the connectivity module?
Done


-- 
To view, visit https://gerrit.ovirt.org/79299
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If42316ac6ecd694e398534e3e87b915aeb4f1f38
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

Change in vdsm[master]: net: move API.updateTimestamp to net.connectivity.confirm

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has posted comments on this change.

Change subject: net: move API.updateTimestamp to net.connectivity.confirm
......................................................................


Patch Set 6:

ci please build

-- 
To view, visit https://gerrit.ovirt.org/79299
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If42316ac6ecd694e398534e3e87b915aeb4f1f38
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: net: move API.updateTimestamp to net.connectivity.confirm

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has posted comments on this change.

Change subject: net: move API.updateTimestamp to net.connectivity.confirm
......................................................................


Patch Set 6: Verified+1

passed ost
http://jenkins.ovirt.org/view/oVirt%20system%20tests/job/ovirt-system-tes...

-- 
To view, visit https://gerrit.ovirt.org/79299
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If42316ac6ecd694e398534e3e87b915aeb4f1f38
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: net: move API.updateTimestamp to net.connectivity.confirm

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has submitted this change and it was merged. (
https://gerrit.ovirt.org/79299 )

Change subject: net: move API.updateTimestamp to net.connectivity.confirm
......................................................................


net: move API.updateTimestamp to net.connectivity.confirm

When connectivityCheck=True is passed to the setupNetworks verb, an
automatically roll back is performed, unless connectivity is confirmed
by the client. Note that Vdsm does not have an explicit
connectivity_confirm verb. connectivity_confirm is called as a side
effect of the ping verb.

Change-Id: If42316ac6ecd694e398534e3e87b915aeb4f1f38
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M lib/vdsm/API.py
M lib/vdsm/constants.py.in
M lib/vdsm/network/api.py
M lib/vdsm/network/connectivity.py
4 files changed, 14 insertions(+), 11 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Dan Kenigsberg: Verified
  Edward Haas: Looks good to me, approved



-- 
To view, visit https://gerrit.ovirt.org/79299
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: If42316ac6ecd694e398534e3e87b915aeb4f1f38
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;