[devel] [PATCH 0/2] Review Request for osaf: update etcd v2 plugin [#3003]

[Gary Lee]

Summary: osaf: update etcdv2 and sample plugins [#3003]
Review request for Ticket(s): 3003
Peer Reviewer(s): Hans, Minh 
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-3003
Base revision: c68d843d99087cebe15c9c67f46b9045fb753059
Personal repository: git://git.code.sf.net/u/userid-2226215/review


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesn
 OpenSAF servicesn
 Core libraries  n
 Samples y 
 Tests   n
 Other   n 


Comments (indicate scope for each "y" above):
-

revision d3dba70d552d21717bf83a17c5f7ef4e653f230a
Author: Gary Lee 
Date:   Thu, 24 Jan 2019 12:11:36 +1100

osaf: update sample plugin [#3003]



revision 26f675ea92b6b4ba9045182d083dcf607e6d123a
Author: Gary Lee 
Date:   Thu, 24 Jan 2019 12:11:36 +1100

osaf: update etcd v2 plugin [#3003]

'etcdctl watch' will return if connection to the etcd server is lost.
If that occurs, send a 'fake' takeover request to rded so rded
will reboot the node. This is in alignment with the etcd v3 plugin.



Complete diffstat:
--
 src/osaf/consensus/plugins/etcd.plugin   | 29 +
 src/osaf/consensus/plugins/sample.plugin | 20 +++-
 2 files changed, 36 insertions(+), 13 deletions(-)


Testing Commands:
-
*** LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES ***


Testing, Expected Results:
--
*** PASTE COMMAND OUTPUTS / TEST RESULTS ***


Conditions of Submission:
-
ack from any reviewer

Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  y 
powerpc n  n
powerpc64   n  n


Reviewer Checklist:
---
[Submitters: make sure that your review doesn't trigger any checkmarks!]


Your checkin has not passed review because (see checked entries):

___ Your RR template is generally incomplete; it has too many blank entries
that need proper data filled in.

___ You have failed to nominate the proper persons for review and push.

___ Your patches do not have proper short+long header

___ You have grammar/spelling in your header that is unacceptable.

___ You have exceeded a sensible line length in your headers/comments/text.

___ You have failed to put in a proper Trac Ticket # into your commits.

___ You have incorrectly put/left internal data in your comments/files
(i.e. internal bug tracking tool IDs, product names etc)

___ You have not given any evidence of testing beyond basic build tests.
Demonstrate some level of runtime or other sanity testing.

___ You have ^M present in some of your files. These have to be removed.

___ You have needlessly changed whitespace or added whitespace crimes
like trailing spaces, or spaces before tabs.

___ You have mixed real technical changes with whitespace and other
cosmetic code cleanup changes. These have to be separate commits.

___ You need to refactor your submission into logical chunks; there is
too much content into a single commit.

___ You have extraneous garbage in your review (merge commits etc)

___ You have giant attachments which should never have been sent;
Instead you should place your content in a public tree to be pulled.

___ You have too many commits attached to an e-mail; resend as threaded
commits, or place in a public tree for a pull.

___ You have resent this content multiple times without a clear indication
of what has changed between each re-send.

___ You have failed to adequately and individually address all of the
comments and change requests that were proposed in the initial review.

___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc)

___ Your computer have a badly configured date and time; confusing the
the threaded patch review.

___ Your changes affect IPC mechanism, and you don't present any results
for in-service upgradability test.

___ Your changes affect user manual and documentation, your patch series
do not contain the patch that updates the Doxygen manual.



___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 1/2] osaf: update etcd v2 plugin [#3003]

[Gary Lee]

'etcdctl watch' will return if connection to the etcd server is lost.
If that occurs, send a 'fake' takeover request to rded so rded
will reboot the node. This is in alignment with the etcd v3 plugin.
---
 src/osaf/consensus/plugins/etcd.plugin | 29 +
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/src/osaf/consensus/plugins/etcd.plugin 
b/src/osaf/consensus/plugins/etcd.plugin
index f62cc89..f88a7e7 100644
--- a/src/osaf/consensus/plugins/etcd.plugin
+++ b/src/osaf/consensus/plugins/etcd.plugin
@@ -17,7 +17,9 @@
 # backward compatible. This plugin may need to be adapted.
 
 readonly keyname="opensaf_consensus_lock"
+readonly takeover_request="takeover_request"
 readonly directory="/opensaf/"
+readonly node_name_file="/etc/opensaf/node_name"
 readonly etcd_options="--no-sync"
 readonly etcd_timeout="5s"
 
@@ -27,7 +29,8 @@ readonly etcd_timeout="5s"
 #   $1 - 
 # returns:
 #   0 - success,  is echoed to stdout
-#   non-zero - failure
+#   1 - invalid param
+#   other - failure
 get() {
   readonly key="$1"
 
@@ -36,7 +39,7 @@ get() {
 echo "$value"
 return 0
   else
-return 1
+return 2
   fi
 }
 
@@ -73,7 +76,8 @@ setkey() {
 # returns:
 #   0 - success
 #   1 - already exists
-#   2 or above - other failure
+#   2 - invalid param
+#   3 or above - other failure
 create_key() {
   readonly key="$1"
   readonly value="$2"
@@ -90,7 +94,7 @@ create_key() {
 fi
   fi
 
-  return 2
+  return 3
 }
 
 # set
@@ -103,7 +107,8 @@ create_key() {
 #   $4 - 
 # returns:
 #   0 - success
-#   non-zero - failure
+#   1 - invalid param
+#   other - failure
 setkey_match_prev() {
   readonly key="$1"
   readonly value="$2"
@@ -115,7 +120,7 @@ setkey_match_prev() {
   then
 return 0
   else
-return 1
+return 2
   fi
 }
 
@@ -158,7 +163,8 @@ lock() {
 return 0
   fi
 
-  if current_owner=$(etcdctl get "$directory$keyname")
+  if current_owner=$(etcdctl $etcd_options --timeout $etcd_timeout \
+get "$directory$keyname")
   then
 # see if we already hold the lock
 if [ "$current_owner" = "$owner" ]; then
@@ -252,7 +258,14 @@ watch() {
 echo "$value"
 return 0
   else
-return 1
+# etcd down?
+if [ "$key" = "$takeover_request" ]; then
+  hostname=`cat $node_name_file`
+  echo "$hostname SC-0 1000 UNDEFINED"
+  return 0
+else
+  return 1
+fi
   fi
 }
 
-- 
2.7.4



___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

[devel] [PATCH 2/2] osaf: update sample plugin [#3003]

[Gary Lee]

---
 src/osaf/consensus/plugins/sample.plugin | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/osaf/consensus/plugins/sample.plugin 
b/src/osaf/consensus/plugins/sample.plugin
index fc4c54c..cadb9e0 100644
--- a/src/osaf/consensus/plugins/sample.plugin
+++ b/src/osaf/consensus/plugins/sample.plugin
@@ -17,6 +17,8 @@
 # backward compatible.
 
 readonly keyname="opensaf_consensus_lock"
+readonly takeover_request="takeover_request"
+readonly node_name_file="/etc/opensaf/node_name"
 
 # get
 #   retrieve  of  from key-value store
@@ -24,7 +26,8 @@ readonly keyname="opensaf_consensus_lock"
 #   $1 - 
 # returns:
 #   0 - success,  is echoed to stdout
-#   non-zero - failure
+#   1 - invalid param
+#   other - failure
 get() {
   readonly key="$1"
   ...
@@ -56,7 +59,8 @@ setkey() {
 # returns:
 #   0 - success
 #   1 - already exists
-#   2 or above - other failure
+#   2 - invalid param
+#   3 or above - other failure
 create_key() {
   readonly key="$1"
   readonly value="$2"
@@ -74,7 +78,8 @@ create_key() {
 #   $4 - 
 # returns:
 #   0 - success
-#   non-zero - failure
+#   1 - invalid param
+#   other - failure
 setkey_match_prev() {
   readonly key="$1"
   readonly value="$2"
@@ -101,7 +106,8 @@ erase() {
 #   $2 - , will automatically unlock after  seconds
 # returns:
 #   0 - success
-#   non-zero - failure
+#   1 - the lock is owned by someone else
+#   2 or above - other failure
 lock() {
   readonly owner="$1"
   readonly timeout="$2"
@@ -129,7 +135,7 @@ lock_owner() {
 # returns:
 #   0 - success
 #   1 - the lock is owned by someone else
-#   2 or above - other failure#
+#   2 or above - other failure
 unlock() {
   readonly owner="$1"
   readonly forced=${2:-false}
@@ -146,6 +152,10 @@ unlock() {
 watch() {
   readonly key="$1"
   ..
+  # if  is $takeover_request and we have lost connectivity to the
+  # consensus service, a fake takeover_request can be returned to force
+  # rded to fence this node. Eg:
+  # "$hostname SC-0 1000 UNDEFINED"
 }
 
 # argument parsing
-- 
2.7.4



___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] Review request ntf: update document for limit of logger buffer [#2994]

[Lennart Lund]

Hi Canh,

Ack

Thanks
Lennart

From: Canh Van Truong 
Sent: den 23 januari 2019 07:13
To: Lennart Lund ; Minh Hon Chau 

Cc: opensaf-devel@lists.sourceforge.net
Subject: RE: Review request ntf: update document for limit of logger buffer 
[#2994]

Thanks Lennart

I have updated your comments. If no more comments, the document will be pushed.

Regards
Canh

From: Lennart Lund mailto:lennart.l...@ericsson.com>>
Sent: Friday, January 18, 2019 3:32 PM
To: Canh Van Truong 
mailto:canh.v.tru...@dektech.com.au>>; Minh Hon 
Chau mailto:minh.c...@dektech.com.au>>
Cc: 
opensaf-devel@lists.sourceforge.net;
 Lennart Lund mailto:lennart.l...@ericsson.com>>
Subject: RE: Review request ntf: update document for limit of logger buffer 
[#2994]


Hi Canh,



Ack, with some minor editorial comments given in the copied text below. See 
comments within [ ]. Text within { } should be removed and replaced by the text 
in the following [ ]



Text copied from the document with comments:

--

Alarm notification log buffer size. Valid values are 10 to 5000 stored 
notifications.
{Some alarm}[All] notifications are logged using the OpenSAF log service. NTF 
has a buffer to store alarm notifications to be logged later in case the log 
service returns TRY AGAIN when NTF writes the log record. When the log service 
is available again (returns OK) all alarm notifications in the buffer will be 
written before the NTF service can service any new alarm notification-send 
requests [incorrect usage of -. Suggest; alarm notification send-requests]. If 
the buffer is big this may take some time and may cause the requests of NTF 
client {is}[to] time out.
If the buffer is full, NTF will return TRY AGAIN when alarm notification-send 
request [see previous comment about -] is called. {The sending}[Send] requests 
of other type[s] [of] notifications are handled normally.



Thanks
Lennart

From: Canh Van Truong 
mailto:canh.v.tru...@dektech.com.au>>
Sent: den 18 januari 2019 08:43
To: Minh Hon Chau mailto:minh.c...@dektech.com.au>>; 
Lennart Lund mailto:lennart.l...@ericsson.com>>
Cc: 
opensaf-devel@lists.sourceforge.net
Subject: RE: Review request ntf: update document for limit of logger buffer 
[#2994]

Hi aMinh

I have updated your comments. Please help check and push the document if there 
are no more comments.

Thanks
Canh

From: Minh Hon Chau mailto:minh.c...@dektech.com.au>>
Sent: Friday, January 18, 2019 7:25 AM
To: Lennart Lund mailto:lennart.l...@ericsson.com>>; 
Canh Van Truong 
mailto:canh.v.tru...@dektech.com.au>>
Cc: 
opensaf-devel@lists.sourceforge.net
Subject: Re: Review request ntf: update document for limit of logger buffer 
[#2994]


Hi Canh,

A very minor comment, that we may have to mention it's for alarm notif only. 
Thanks, Minh
On 11/1/19 11:30 pm, Lennart Lund wrote:
Hi Canh,

I should have Acked the document in my previous answer. Just fix any minor 
things and push.

Thanks
Lennart

From: Canh Van Truong 

Sent: den 11 januari 2019 12:01
To: Lennart Lund ; 
Minh Hon Chau 
Cc: 
opensaf-devel@lists.sourceforge.net
Subject: RE: Review request ntf: update document for limit of logger buffer 
[#2994]

Hi Lennart

Yes, I add new column for "Default value".

Regards
Canh
From: Lennart Lund 
Sent: Friday, January 11, 2019 5:33 PM
To: Canh Van Truong 
; Minh Hon 
Chau 
Cc: 
opensaf-devel@lists.sourceforge.net
Subject: RE: Review request ntf: update document for limit of logger buffer 
[#2994]

Hi Canh,

I have one minor comment in the attached document. Also, it may be better if 
the table has three columns "Environment Variable, Default value and Comment"

Thanks
Lennart

From: Canh Van Truong 
mailto:canh.v.tru...@dektech.com.au>>
Sent: den 11 januari 2019 10:18
To: Lennart Lund mailto:lennart.l...@ericsson.com>>; 
Minh Hon Chau mailto:minh.c...@dektech.com.au>>
Cc: 
opensaf-devel@lists.sourceforge.net
Subject: RE: Review request ntf: update document for limit of logger buffer 
[#2994]

Thanks Lennart,

I have updated with your comments.
Please give the comments if there is something need to be updated.


Regards
Canh

From: Lennart Lund mailto:lennart.l...@ericsson.com>>
Sent: Thursday, January 10, 2019 9:07 PM
To: Canh Van Truong 
mailto:canh.v.tru...@dektech.com.au>>; Minh Hon 
Chau mailto:minh.c...@dektech.com.au>>
Cc: 
opensaf-devel@lists.sourceforge.net
Subject: RE: Review request ntf: update document for limit of logger buffer 
[#2994]

Hi Canh,

The following text describing