Calculating and updtating cluster and volume alert_count in each cluster sync #599

GowthamShanmugam · 2018-03-28T17:18:50Z

tendrl-bug-id: #598

Signed-off-by: GowthamShanmugasundaram gshanmug@redhat.com

GowthamShanmugam · 2018-03-28T17:19:08Z

@shtripat @shirshendu @r0h4n please review

GowthamShanmugam · 2018-03-28T17:19:22Z

Tendrl/monitoring-integration/issues/373

GowthamShanmugam · 2018-03-28T17:20:47Z

only provisioning node will do alert_count update

codecov · 2018-03-28T17:24:10Z

Codecov Report

Merging #599 into master will decrease coverage by 0.02%.
The diff coverage is 7.69%.

@@            Coverage Diff             @@
##           master     #599      +/-   ##
==========================================
- Coverage   39.66%   39.63%   -0.03%     
==========================================
  Files          70       70              
  Lines        2607     2614       +7     
  Branches      374      376       +2     
==========================================
+ Hits         1034     1036       +2     
- Misses       1542     1547       +5     
  Partials       31       31

Impacted Files	Coverage Δ
tendrl/gluster_integration/sds_sync/__init__.py	`29.12% <7.69%> (-0.02%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2abb431...4787a92. Read the comment docs.

r0h4n · 2018-03-29T04:26:11Z

tendrl/gluster_integration/sds_sync/__init__.py

+def find_volume_id():
+ alert_counts = {}
+ volumes = etcd_utils.read(
+ "clusters/%s/Volumes" % NS.tendrl_context.integration_id


use Volume.load_all()

i will change this

shtripat · 2018-03-29T05:51:35Z

tendrl/gluster_integration/sds_sync/__init__.py

+
+def find_volume_id():
+ alert_counts = {}
+ volumes = etcd_utils.read(


Why not use NS.gluster.objects.Volume().load_all() here ? I feel all you need is volume names then form the dict.
Also call the function as get_volume_alert_counts

shtripat · 2018-03-29T05:52:50Z

tendrl/gluster_integration/sds_sync/__init__.py

+ severity = ["WARNING", "CRITICAL"]
+ try:
+ alert_counts = find_volume_id()
+ alerts_arr = NS.tendrl.objects.ClusterAlert(


Is it possible to load all the cluster alerts using tags only? Don't you need integration_id here? Does it default to NS.tendrl_context.integration_id in object constructor?

@shtripat for the cluster_alert object is a subclass of alert object, so for an alert object integration_id is not a mandatory field. So for cluster alerts, integration_id is present in tags only. While forming patch cluster alert object will take integration_id from tags only. That is why i am passing only tags with integration_id.

https://github.com/Tendrl/commons/blob/master/tendrl/commons/objects/cluster_alert/__init__.py

shtripat · 2018-03-29T05:53:52Z

tendrl/gluster_integration/sds_sync/__init__.py

+ if alert.resource in NS.gluster.objects.VolumeAlertCounters(
+ )._defs['relationship'][alert.alert_type.lower()]:
+ vol_name = alert.tags.get('volume_name', None)
+ if vol_name:


Add both the conditions in one if condition as if vol_name and vol_name in alert_counts.keys()

i will change this

shtripat · 2018-03-29T05:58:41Z

tendrl/gluster_integration/sds_sync/__init__.py

+ alert_count=cluster_alert_count
+ ).save()
+ # Update volume alert count
+ for volume in alert_counts:


I understand alert_counts is a dict. If so you should use something like

for vol_name, alert_det in alert_counts.iter_items(): NS.gluster.objects.VolumeAlertCounters( integration_id=NS.tendrl_context.integration_id, alert_count=alert_det['alert_count'], volume_id=alert_det['vol_id'] ).save()

Does this work. Even I feel the dict created in find_volume_id functions could be with keys as volume ids and value as counters. Why you need volume name as keys? Do you really use that anywhere?

the alert object contains volume name only present in tags, to compare this alert with a particular volume i need volume name. that why i am creating dict with the volume name. During alert iteration, i am just comparing volume name from alert object with newly formed dict to increase the alert count.

GowthamShanmugam · 2018-03-29T08:15:24Z

@shtripat changes done

shtripat · 2018-03-29T11:43:57Z

tendrl/gluster_integration/sds_sync/__init__.py

- continue
+ volumes = NS.gluster.objects.Volume().load_all()
+ for volume in volumes:
+ alert_counts[volume.name] = {}


you can do the whole thing in one line

alert_counts[volume.name] = {"vol_id": volume.vol_id, "alert_count": 0}

GowthamShanmugam · 2018-04-03T11:07:07Z

@shtripat changes done

shtripat · 2018-04-03T14:30:06Z

tendrl/gluster_integration/sds_sync/__init__.py

+ severity = ["WARNING", "CRITICAL"]
+ try:
+ alert_counts = get_volume_alert_counts()
+ alerts_arr = NS.tendrl.objects.ClusterAlert(


alerts is good enough name :)

shtripat · 2018-04-03T14:32:12Z

Looks fine. One small suggestion.
Also have you verified these changes?

GowthamShanmugam · 2018-04-04T05:31:06Z

@shtripat changes done, i verified

shtripat

looks fine

r0h4n

please resolve conflicts

…ter sync tendrl-bug-id: Tendrl#598 Signed-off-by: GowthamShanmugasundaram <gshanmug@redhat.com>

tendrl-bug-id: Tendrl#598 Signed-off-by: GowthamShanmugasundaram <gshanmug@redhat.com>

GowthamShanmugam · 2018-04-07T06:03:27Z

Tendrl/ui#867

GowthamShanmugam requested review from shtripat and a team as code owners March 28, 2018 17:18

r0h4n suggested changes Mar 29, 2018

View reviewed changes

shtripat reviewed Mar 29, 2018

View reviewed changes

GowthamShanmugam force-pushed the volume_alert_count_fix branch from ff21148 to e30cbe5 Compare April 3, 2018 11:06

shtripat reviewed Apr 3, 2018

View reviewed changes

shtripat reviewed Apr 4, 2018

View reviewed changes

r0h4n suggested changes Apr 5, 2018

View reviewed changes

GowthamShanmugam added 3 commits April 5, 2018 15:37

Calculating and updtating cluster and volume alert_count in each clus…

c1fda51

…ter sync tendrl-bug-id: Tendrl#598 Signed-off-by: GowthamShanmugasundaram <gshanmug@redhat.com>

Modified code as per review comment

2c91b7e

tendrl-bug-id: Tendrl#598 Signed-off-by: GowthamShanmugasundaram <gshanmug@redhat.com>

Modified code as per review comment

4787a92

tendrl-bug-id: Tendrl#598 Signed-off-by: GowthamShanmugasundaram <gshanmug@redhat.com>

GowthamShanmugam force-pushed the volume_alert_count_fix branch from 3595e93 to 4787a92 Compare April 5, 2018 10:07

GowthamShanmugam mentioned this pull request Apr 7, 2018

indices in case of clear alert Tendrl/ui#867

Closed

r0h4n approved these changes Apr 9, 2018

View reviewed changes

r0h4n merged commit 209923a into Tendrl:master Apr 9, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Calculating and updtating cluster and volume alert_count in each cluster sync #599

Calculating and updtating cluster and volume alert_count in each cluster sync #599

GowthamShanmugam commented Mar 28, 2018

GowthamShanmugam commented Mar 28, 2018

GowthamShanmugam commented Mar 28, 2018

GowthamShanmugam commented Mar 28, 2018

codecov bot commented Mar 28, 2018 •

edited

Loading

r0h4n Mar 29, 2018

GowthamShanmugam Mar 29, 2018

shtripat Mar 29, 2018

GowthamShanmugam Mar 29, 2018

shtripat Mar 29, 2018

GowthamShanmugam Mar 29, 2018

GowthamShanmugam Mar 29, 2018

shtripat Mar 29, 2018

shtripat Mar 29, 2018

GowthamShanmugam Mar 29, 2018

shtripat Mar 29, 2018

GowthamShanmugam Mar 29, 2018

shtripat Mar 29, 2018

GowthamShanmugam commented Mar 29, 2018

shtripat Mar 29, 2018

GowthamShanmugam commented Apr 3, 2018

shtripat Apr 3, 2018

shtripat commented Apr 3, 2018

GowthamShanmugam commented Apr 4, 2018

shtripat left a comment

r0h4n left a comment

GowthamShanmugam commented Apr 7, 2018 •

edited

Loading

Calculating and updtating cluster and volume alert_count in each cluster sync #599

Calculating and updtating cluster and volume alert_count in each cluster sync #599

Conversation

GowthamShanmugam commented Mar 28, 2018

GowthamShanmugam commented Mar 28, 2018

GowthamShanmugam commented Mar 28, 2018

GowthamShanmugam commented Mar 28, 2018

codecov bot commented Mar 28, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GowthamShanmugam commented Mar 29, 2018

Choose a reason for hiding this comment

GowthamShanmugam commented Apr 3, 2018

Choose a reason for hiding this comment

shtripat commented Apr 3, 2018

GowthamShanmugam commented Apr 4, 2018

shtripat left a comment

Choose a reason for hiding this comment

r0h4n left a comment

Choose a reason for hiding this comment

GowthamShanmugam commented Apr 7, 2018 • edited Loading

codecov bot commented Mar 28, 2018 •

edited

Loading

GowthamShanmugam commented Apr 7, 2018 •

edited

Loading