Add emc vnx block driver #392

jiangyutan · 2020-11-23T03:15:03Z

add emc vnx storage driver to community

codecov · 2020-11-23T03:17:20Z

Codecov Report

Merging #392 (063cdfa) into v0.8.0-maint (4d8429d) will increase coverage by 0.38%.
The diff coverage is 68.32%.

@@               Coverage Diff                @@
##           v0.8.0-maint     #392      +/-   ##
================================================
+ Coverage         66.88%   67.27%   +0.38%     
================================================
  Files                88       99      +11     
  Lines              6025     7490    +1465     
  Branches            690      914     +224     
================================================
+ Hits               4030     5039    +1009     
- Misses             1784     2129     +345     
- Partials            211      322     +111

Impacted Files	Coverage Δ
delfin/drivers/utils/navicli_client.py	`37.03% <37.03%> (ø)`
delfin/drivers/utils/ssh_client.py	`42.16% <42.68%> (+0.50%)`	⬆️
delfin/drivers/hitachi/vsp/rest_handler.py	`55.97% <55.97%> (ø)`
...in/drivers/dell_emc/vnx_block/component_handler.py	`68.07% <68.07%> (ø)`
delfin/drivers/dell_emc/vnx_block/navi_handler.py	`68.19% <68.19%> (ø)`
delfin/drivers/dell_emc/vnx_block/alert_handler.py	`70.93% <70.93%> (ø)`
delfin/drivers/ibm/storwize_svc/ssh_handler.py	`72.18% <72.18%> (ø)`
delfin/drivers/hitachi/vsp/vsp_stor.py	`75.30% <75.30%> (ø)`
delfin/drivers/utils/tools.py	`76.92% <76.92%> (ø)`
delfin/drivers/ibm/storwize_svc/storwize_svc.py	`96.15% <96.15%> (ø)`
... and 17 more

kumarashit · 2020-11-24T06:07:36Z

Use proper template and put PR description

delfin/drivers/dell_emc/vnx_block/component_handler.py

kumarashit · 2020-11-25T18:38:27Z

delfin/drivers/dell_emc/vnx_block/component_handler.py

+                if used_map:
+                    used_cap = used_map.get('used_cap')
+                    if used_cap == 0:
+                        status = constants.StorageStatus.ABNORMAL


Why the status be abnormal if used_cap is 0?

The state of vnx block storage is determined by whether the usage and the remaining amount can be fetched. The usage is the amount allocated or used by all Lun. If the remaining amount or usage is 0, it means that the command line may not be able to obtain the stored related information, so it is set to abnormal

But then the storage status being abnormal is misleading. It means the storage system is down

kumarashit · 2020-11-25T18:41:43Z

delfin/drivers/dell_emc/vnx_block/component_handler.py

+        else:
+            # If no data is returned, it indicates that there
+            # may be a problem with the network or the device.
+            # Default return OFFLINE


If the data is not available, it doesn't mean storage is OFFLINE. It maybe a temporary network issue from Client. This is misleading

kumarashit · 2020-11-25T18:45:42Z

delfin/drivers/dell_emc/vnx_block/component_handler.py

+            return self.handler_volume(volumes, pool_ids, storage_id)
+
+        except Exception as e:
+            err_msg = "Failed to get list volumes from EmcVnxStor: %s" % (


Suggested change

err_msg = "Failed to get list volumes from EmcVnxStor: %s" % (

err_msg = "Failed to get list of volumes from EmcVnxStor: %s" % (

kumarashit · 2020-11-25T18:48:07Z

delfin/drivers/dell_emc/vnx_block/navi_handler.py

@@ -0,0 +1,469 @@
+# Copyright 2020 The SODA Authors.
+# Copyright (c) 2016 Huawei Technologies Co., Ltd.


Why this copyright?

delfin/drivers/dell_emc/vnx_block/navi_handler.py

kumarashit · 2020-11-25T18:53:03Z

delfin/drivers/dell_emc/vnx_block/navi_handler.py

+    def get_agent(self):
+        """get agent info"""
+        agent_model = {}
+        try:


How about moving the whole try catch block into common function and here simply call that common function. This common function can return result

Implementation method self.navi_ Exe is a public method, but different resource contents return different processing.

Yes, i got that..i was saying that there is lots of code repetition which can be avoided by simply getting the result and processing separately

kumarashit · 2020-11-25T18:54:58Z

delfin/drivers/utils/navicli_client.py

+            re = out.strip()
+            if re:
+                result = re
+                if 'Caller not privileged' in result:


To avoid multiple if elif, can we not keep a map of string and the exception?

Many exceptions are judged by the returned value content, while some information in the returned content is to change, such as IP, etc., so only a part of the keywords can be intercepted to judge, so map and other methods cannot be used at present.

kumarashit

LGTM

delfin/drivers/dell_emc/vnx_block/navi_handler.py

After redis config change restart service

edit

bf50dc4

kumarashit requested changes Nov 25, 2020

View reviewed changes

kumarashit previously approved these changes Nov 26, 2020

View reviewed changes

delfin/drivers/dell_emc/vnx_block/navi_handler.py Show resolved Hide resolved

jiangyutan dismissed kumarashit’s stale review via 0fe3268 November 27, 2020 01:26

modify bug,squash all commit

a46a28d

jiangyutan closed this Dec 7, 2020

jiangyutan deleted the vnx_block20201106 branch December 7, 2020 02:13

amanr032 pushed a commit to amanr032/delfin that referenced this pull request Dec 5, 2022

Merge pull request sodafoundation#392 from joseph-v/fix_redis_restart

e89abe8

After redis config change restart service

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add emc vnx block driver #392

Add emc vnx block driver #392

jiangyutan commented Nov 23, 2020 •

edited

Loading

codecov bot commented Nov 23, 2020 •

edited

Loading

kumarashit commented Nov 24, 2020

kumarashit Nov 25, 2020

jiangyutan Nov 26, 2020

kumarashit Nov 26, 2020

jiangyutan Nov 26, 2020

kumarashit Nov 25, 2020

jiangyutan Nov 26, 2020

kumarashit Nov 25, 2020

jiangyutan Nov 26, 2020

kumarashit Nov 25, 2020

jiangyutan Nov 26, 2020

kumarashit Nov 25, 2020

jiangyutan Nov 26, 2020

kumarashit Nov 26, 2020

jiangyutan Nov 26, 2020

kumarashit Nov 25, 2020

jiangyutan Nov 26, 2020

kumarashit Nov 26, 2020

jiangyutan Nov 27, 2020

kumarashit left a comment

	err_msg = "Failed to get list volumes from EmcVnxStor: %s" % (
	err_msg = "Failed to get list of volumes from EmcVnxStor: %s" % (

		@@ -0,0 +1,469 @@
		# Copyright 2020 The SODA Authors.
		# Copyright (c) 2016 Huawei Technologies Co., Ltd.

Add emc vnx block driver #392

Add emc vnx block driver #392

Conversation

jiangyutan commented Nov 23, 2020 • edited Loading

codecov bot commented Nov 23, 2020 • edited Loading

Codecov Report

kumarashit commented Nov 24, 2020

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

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

kumarashit left a comment

Choose a reason for hiding this comment

jiangyutan commented Nov 23, 2020 •

edited

Loading

codecov bot commented Nov 23, 2020 •

edited

Loading