Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue:3563885:UFM Enterprise network reports #123

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Abeer-Moghrabi
Copy link
Contributor

@Abeer-Moghrabi Abeer-Moghrabi commented Aug 14, 2023

What

This plugin will assist users in monitoring events related to changes in the Fabric Topology, particularly in the context of a large fabric.
This plugin will monitor the event.log and store any new events linked to topology changes. These events will be accessible through a REST API, which will allow specifying the event type and desired time interval.
UFM has the following events linked to topology changes:

• Link Up/Down
• Node Up/Down
• Switch Up/Down
• Director Switch Up/Down

For more information please find HLD

Why ?

To assist users in monitoring events related to changes in the Fabric Topology, particularly in the context of a large fabric.

Testing ?

CI tests (not implemented yet)

Special triggers

Use the following phrases as comments to trigger different runs

  • bot:retest rerun CI
  • bot:upgrade run additional update tests

#
# This software product is governed by the End User License Agreement
# provided with the software product.
# @author: Anan Al-Aghbar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to author in all files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

if fname.endswith(".gz"):
return gzip.open(fname, mode, encoding='utf8')
else:
return open(fname, mode)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from where you imported this function? it seems you forget the import statement

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The open() function is a built-in function that is part of the core Python language, so you do not need to import it

@@ -0,0 +1,17 @@
class Event:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add the copyrights

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

+ fix author name
+ handel exception in openEventFile function

LABEL maintainer="[email protected]"

ARG PLUGIN_NAME
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value for plugin name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

out_dir=$4
random_hash=$5
keep_image=$6
prefix="mellanox"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still mellanox?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, all other plugins use mellanox as prefix

Copy link
Contributor

@alextabachnik alextabachnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

take a look at my comments

@@ -65,12 +74,14 @@ def parse_event_log_files(self):
files.append(EventsConstants.EVENT_LOG)
for file in files:
fname = os.path.join(EventsConstants.EVENT_LOGS_DIRECTORY, file)
Logger.log_message(f"Parsing file {fname}", LOG_LEVELS.INFO)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If file not exists this message is incorrect. Message should be moved under the if, but need to mention list of files you are going to parse.

self.last_position = self._get_last_position()
self.event_mgr = EventsHistoryMgr.getInstance()

def onChange(self, event):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will you get last position if file was replaced by logrotate?

Copy link
Contributor

@alextabachnik alextabachnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to be handled two aspects:

  1. Possibility of rotate of log files by logrotate - so you will need to take info from both old and new files.
  2. Aging of collected data - you just create new events and keep them in memory - so memory utilization will grow all the time. Need to clean collected events or to compress them.


RUN apt-get update && apt-get -y install supervisor python3.9 python3-pip vim curl sudo

RUN python3.9 -m pip install -r ${BASE_PATH}/${SRC_BASE_DIR}/src/${PLUGIN_NAME}/requirements.txt

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not python3 (which is usually symlink to python3.x available)


[program:events_history_service]
directory=/opt/ufm/ufm_plugin_events_history
command=python3.9 /opt/ufm/ufm_plugin_events_history/events_history_plugin/src/events_history_plugin/app.py

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python3?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants