Fix clear_fault/clear_all in FaultAPIsV2 raise exception when alarm is

not found

For the all 6 APIs in FaultAPIsV2, will treat alarm non-exist as normal
case, and exception will not be raised. Exception is raised only if
there is operation failure, such as fmclient fail to connect fmManager,
failure in memory allocation, failure in internal structure parse, etc.

Test:
Run each 6 APIs from FaultAPIs and FaultAPIsV2 with below case:
1. with correct alarm id
2. with wrong alarm id
3. with fmManager cannot be connected (Modify fmManager listen port to
   simulate it)
Confirm FaultAPIs's behavior is not changed.
And FaultAPIsV2 raises exception for operation failure only.
Check Events Suppression function in horizon GUI, it works correctly.

Closes-Bug: 1821112

Change-Id: I8ba122b19964613d90e9d0bf4a25134ff60e5c19
Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>

fm-api/fm_api/fm_api.py

@@ -185,7 +185,13 @@ class FaultAPIs(FaultAPIsBase):
         buff = (sep + self._check_val(alarm_id) + sep +
                 self._check_val(entity_instance_id) + sep)
         try:
-            return fm_core.clear(buff)
+            resp = fm_core.clear(buff)
+            # resp may be True/False/None after FaultAPIsV2 implementation.
+            # to keep FaultAPIs the same as before, return False for None case.
+            if resp is True:
+                return True
+            else:
+                return False
         except (RuntimeError, SystemError, TypeError):
             return False
 
@@ -203,7 +209,13 @@ class FaultAPIs(FaultAPIsBase):
 
     def clear_all(self, entity_instance_id):
         try:
-            return fm_core.clear_all(entity_instance_id)
+            resp = fm_core.clear_all(entity_instance_id)
+            # resp may be True/False/None after FaultAPIsV2 implementation.
+            # to keep FaultAPIs the same as before, return False for None case.
+            if resp is True:
+                return True
+            else:
+                return False
         except (RuntimeError, SystemError, TypeError):
             return False
 
@@ -234,6 +246,10 @@ class FaultAPIs(FaultAPIsBase):
 
 class FaultAPIsV2(FaultAPIsBase):
 
+    # Input: alarm data
+    # Return: Success: uuid for the alarm
+    # Exception: 1. Input Alarm format is not valid
+    #            2. When there is operation failure
     def set_fault(self, data):
         self._check_required_attributes(data)
         self._validate_attributes(data)
@@ -243,14 +259,28 @@ class FaultAPIsV2(FaultAPIsBase):
             raise APIException("Failed to execute set_fault.")
         return uuid
 
+    # Input: alarm_id, entity_instance_id
+    # Return: Success: True
+    #         Alarm doesn't exist: False
+    # Exception: When there is operation failure
     def clear_fault(self, alarm_id, entity_instance_id):
         sep = constants.FM_CLIENT_STR_SEP
         buff = (sep + self._check_val(alarm_id) + sep +
                 self._check_val(entity_instance_id) + sep)
         resp = fm_core.clear(buff)
         if resp is False:
+            # There is operation failure
             raise APIException("Failed to execute clear_fault.")
+        elif resp is None:
+            # alarm is not found
+            return False
+        else:
+            return True
 
+    # Input: alarm_id, entity_instance_id
+    # Return: Success: Alarm
+    #         Alarm doesn't exist: None
+    # Exception: When there is operation failure
     def get_fault(self, alarm_id, entity_instance_id):
         sep = constants.FM_CLIENT_STR_SEP
         buff = (sep + self._check_val(alarm_id) + sep +
@@ -261,11 +291,25 @@ class FaultAPIsV2(FaultAPIsBase):
         else:
             return self._str_to_alarm(resp) if resp else None
 
+    # Input: entity_instance_id
+    # Return: Success: True
+    #         Alarm doesn't exist: False
+    # Exception: When there is operation failure
     def clear_all(self, entity_instance_id):
         resp = fm_core.clear_all(entity_instance_id)
         if resp is False:
+            # There is operation failure
             raise APIException("Failed to execute clear_all.")
+        elif resp is None:
+            # alarm is not found
+            return False
+        else:
+            return True
 
+    # Input: entity_instance_id
+    # Return: Success: Alarm list
+    #         Alarm doesn't exist: None
+    # Exception: When there is operation failure
     def get_faults(self, entity_instance_id):
         resp = fm_core.get_by_eid(entity_instance_id)
         if resp is False:
@@ -278,6 +322,10 @@ class FaultAPIsV2(FaultAPIsBase):
         else:
             return None
 
+    # Input: alarm_id
+    # Return: Success: Alarm list
+    #         Alarm doesn't exist: None
+    # Exception: When there is operation failure
     def get_faults_by_id(self, alarm_id):
         resp = fm_core.get_by_aid(alarm_id)
         if resp is False:

fm-api/fm_api_v2_test.py

@@ -57,16 +57,22 @@ def create():
 
 def delete(alarm_id, instance_id):
     try:
-        ser.clear_fault(alarm_id, instance_id)
+        resp = ser.clear_fault(alarm_id, instance_id)
-        print("Delete fault success")
+        if resp:
+            print("Delete fault success")
+        else:
+            print("Alarm not found")
     except Exception:
         print("Fail to delete fault")
 
 
 def del_all(instance_id):
     try:
-        ser.clear_all(instance_id)
+        resp = ser.clear_all(instance_id)
-        print("Delete faults success")
+        if resp:
+            print("Delete faults success")
+        else:
+            print("Alarms not found")
     except Exception:
         print("Fail to delete faults")
 

fm-common/sources/fmDb.cpp

@@ -128,25 +128,27 @@ bool CFmDBSession::query(const char *db_cmd,fm_db_result_t & result) {
 	return true;
 }
 
-bool CFmDBSession::cmd(const char *db_cmd, bool check_row){
+// return value: -1: if there is PQ operation failure
+//                0: if cmd success and check_row == false
+//              row: row number if cmd success and check_row == true
+int CFmDBSession::cmd(const char *db_cmd, bool check_row){
 	PGresult *res;
-	bool rc = true;
+	int rc = -1;
 
 	if (check_conn() == false){
 		FM_ERROR_LOG("Failed to reconnect: %s", PQerrorMessage(m_conn.pgconn));
-		return false;
+		return rc;
 	}
 
 	res = PQexec(m_conn.pgconn, db_cmd);
 	if (PQresultStatus(res) != PGRES_COMMAND_OK) {
 		FM_ERROR_LOG("Status:(%s)\n", PQresStatus(PQresultStatus(res)));
 		FM_ERROR_LOG("Failed to execute (%s) (%s)", db_cmd, PQresultErrorMessage(res));
-		rc = false;
+	} else if (check_row){
-	}
+		rc = atoi(PQcmdTuples(res));
-	if (rc && check_row){
+		FM_DEBUG_LOG("SQL command returned successful: %d rows affected.\n", rc);
-		int row = atoi(PQcmdTuples(res));
+	} else {
-		FM_DEBUG_LOG("SQL command returned successful: %d rows affected.\n", row);
+		rc = 0;
-		if (row < 1) rc = false;
 	}
 
 	PQclear(res);

fm-common/sources/fmDb.h

@@ -53,7 +53,7 @@ public:
 	bool reconnect();
 
 	bool query(const char *db_cmd,fm_db_result_t & result);
-	bool cmd(const char *db_cmd, bool check_row=true);
+	int  cmd(const char *db_cmd, bool check_row=true);
 	bool params_cmd(fm_db_util_sql_params & sql_params);
 
 	PGconn* get_pgconn(){

fm-common/sources/fmDbAlarm.cpp

@@ -209,7 +209,9 @@ bool CFmDbAlarmOperation::create_alarm(CFmDBSession &sess,CFmDbAlarm &a) {
 	return sess.params_cmd(sql_params);
 }
 
-bool CFmDbAlarmOperation::delete_alarms(CFmDBSession &sess, const char *id) {
+// return value: -1: if cmd fail to execute
+//              row: Deleted row number, maybe 0.
+int CFmDbAlarmOperation::delete_alarms(CFmDBSession &sess, const char *id) {
 	std::string sql;
 
 	fm_db_util_build_sql_delete_all((const char*)FM_ALARM_TABLE_NAME, id, sql);
@@ -217,7 +219,9 @@ bool CFmDbAlarmOperation::delete_alarms(CFmDBSession &sess, const char *id) {
 	return sess.cmd(sql.c_str());
 }
 
-bool CFmDbAlarmOperation::delete_alarm(CFmDBSession &sess, AlarmFilter &af) {
+// return value: -1: if cmd fail to execute
+//              row: Deleted row number, maybe 0.
+int CFmDbAlarmOperation::delete_alarm(CFmDBSession &sess, AlarmFilter &af) {
 	std::string sql;
 
 	fm_db_util_build_sql_delete((const char*)FM_ALARM_TABLE_NAME, &af, sql);

fm-common/sources/fmDbAlarm.h

@@ -51,8 +51,8 @@ class CFmDbAlarmOperation {
 public:
 	bool create_alarm(CFmDBSession &sess, CFmDbAlarm &a);
 
-	bool delete_alarms(CFmDBSession &sess, const char *id);
+	int delete_alarms(CFmDBSession &sess, const char *id);
-	bool delete_alarm(CFmDBSession &sess, AlarmFilter &af);
+	int delete_alarm(CFmDBSession &sess, AlarmFilter &af);
 
 	bool delete_row(CFmDBSession &sess, const char* db_table);
 

fm-common/sources/fmDbUtils.cpp

@@ -565,7 +565,7 @@ bool fm_db_util_get_next_log_id(CFmDBSession &sess, int &id){
 		id = get_oldest_id(sess, FM_EVENT_LOG_TABLE_NAME);
 		fm_db_util_build_sql_delete_row(FM_EVENT_LOG_TABLE_NAME, id, sql);
 		FM_DEBUG_LOG("Delete row (%s)\n", sql.c_str());
-		if (false == sess.cmd(sql.c_str())){
+		if (sess.cmd(sql.c_str()) <= 0){
 			FM_INFO_LOG("Fail to delete the old event log: (%d)", id);
 			pthread_mutex_unlock(&mutex);
 			return false;

fm-common/sources/fmEventSuppression.cpp

@@ -84,7 +84,7 @@ bool CFmEventSuppressionOperation::set_table_notify_listen(CFmDBSession &sess){
 		sql += FM_EVENT_SUPPRESSION_TABLE_NAME;
 		sql += ")";
 
-		if (sess.cmd(sql.c_str(), false) != true){
+		if (sess.cmd(sql.c_str(), false) < 0){
 			FM_INFO_LOG("Failed to set rule CMD: (%s)", sql.c_str());
 			return false;
 		}
@@ -97,5 +97,5 @@ bool CFmEventSuppressionOperation::set_table_notify_listen(CFmDBSession &sess){
 
 	FM_DEBUG_LOG("CMD:(%s)\n", sql.c_str());
 	// no row affected by LISTEN command
-	return sess.cmd(sql.c_str(), false);
+	return sess.cmd(sql.c_str(), false) < 0 ? false : true;
 }

fm-common/sources/fmMsgServer.cpp

@@ -441,9 +441,11 @@ void FmSocketServerProcessor::handle_delete_faults(int fd,
 	void * data = &(rdata[sizeof(SFmMsgHdrT)]);
 	fm_ent_inst_t *pid = (fm_ent_inst_t *)(data);
 	fm_ent_inst_t &id = *pid;
+	int rc = 0;
 
 	hdr->msg_rc = FM_ERR_OK;
-	if (op.delete_alarms(sess,id)){
+	rc = op.delete_alarms(sess,id);
+	if (rc > 0){
 		FM_DEBUG_LOG("Deleted alarms (%s)\n", id);
 		SFmAlarmDataT alarm;
 		memset(&alarm, 0, sizeof(alarm));
@@ -456,10 +458,16 @@ void FmSocketServerProcessor::handle_delete_faults(int fd,
 		req.set = false;
 		req.data = alarm;
 		enqueue_job(req);
-	}else{
+	} else if (rc == 0){
-		FM_INFO_LOG("Fail to Delete alarms (%s)\n", id);
+		hdr->msg_rc = FM_ERR_ENTITY_NOT_FOUND;
+	} else {
 		hdr->msg_rc = FM_ERR_DB_OPERATION_FAILURE;
 	}
+
+	FM_INFO_LOG("Deleted alarms status: (%s) (%s)\n",
+		id,
+		fm_error_from_int((EFmErrorT)hdr->msg_rc).c_str());
+
 	send_response(fd,hdr,NULL,0);
 }
 
@@ -481,7 +489,7 @@ void FmSocketServerProcessor::handle_delete_fault(int fd,
 		hdr->msg_rc = FM_ERR_DB_OPERATION_FAILURE;
 	}else{
 		if (res.size() > 0){
-			if(op.delete_alarm(sess, *filter)){
+			if(op.delete_alarm(sess, *filter) > 0){
 				FM_INFO_LOG("Deleted alarm: (%s) (%s)\n",
 						filter->alarm_id, filter->entity_instance_id);
 				CFmDbAlarm::convert_to(res[0],&alarm);

fm-common/sources/fm_python_mod_main.cpp

@@ -260,6 +260,7 @@ static PyObject * _fm_clear(PyObject * self, PyObject *args) {
 
 	if (rc == FM_ERR_ENTITY_NOT_FOUND) {
 		DEBUG_LOG("No alarm found to clear: (%s) (%s)", af.alarm_id, af.entity_instance_id);
+		Py_RETURN_NONE;
 	} else if (rc == FM_ERR_NOCONNECT) {
 		WARNING_LOG("Failed to connect to FM manager");
 	} else {
@@ -285,11 +286,18 @@ static PyObject * _fm_clear_all(PyObject * self, PyObject *args) {
 	rc = fm_clear_all(&inst_id);
 	if (rc == FM_ERR_OK) {
 		Py_RETURN_TRUE;
+	}
+
+	if (rc == FM_ERR_ENTITY_NOT_FOUND) {
+		DEBUG_LOG("No alarm found to clear with entity id (%s)", inst_id);
+		Py_RETURN_NONE;
+	} else if (rc == FM_ERR_NOCONNECT) {
+		WARNING_LOG("Failed to connect to FM manager");
 	} else {
 		ERROR_LOG("Failed to clear alarms with entity id (%s), error code: (%d)",
 				inst_id, rc);
-		Py_RETURN_FALSE;
 	}
+	Py_RETURN_FALSE;
 }
 
 static PyMethodDef _methods [] = {