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

add driver interfaces for Port, Controller, NAS(FS, QTree, Share, Quota) #597

Merged
merged 18 commits into from
Jun 10, 2021

Conversation

guankc
Copy link
Contributor

@guankc guankc commented Jun 7, 2021

this PR is to add driver interfaces for Port, Controller, NAS(FS, QTree, Share, Quota)

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Release note:

tanjiangyu-ghca and others added 3 commits June 7, 2021 11:38
…ota)

 add driver interfaces for Port, Controller, NAS(FS, QTree, Share, Quota)
@codecov
Copy link

codecov bot commented Jun 7, 2021

Codecov Report

Merging #597 (8bfd069) into master (73fe3a1) will decrease coverage by 0.04%.
The diff coverage is 67.02%.

@@            Coverage Diff             @@
##           master     #597      +/-   ##
==========================================
- Coverage   72.13%   72.09%   -0.05%     
==========================================
  Files         141      141              
  Lines       12211    12462     +251     
  Branches     1446     1480      +34     
==========================================
+ Hits         8809     8985     +176     
- Misses       2909     2976      +67     
- Partials      493      501       +8     
Impacted Files Coverage Δ
delfin/drivers/utils/ssh_client.py 19.79% <10.00%> (+0.31%) ⬆️
delfin/drivers/netapp/dataontap/netapp_handler.py 68.45% <63.91%> (-4.39%) ⬇️
delfin/drivers/netapp/dataontap/cluster_mode.py 93.18% <100.00%> (+15.90%) ⬆️
delfin/drivers/netapp/dataontap/constants.py 100.00% <100.00%> (ø)
delfin/drivers/utils/tools.py 90.38% <100.00%> (+0.18%) ⬆️

@@ -241,15 +242,17 @@ def put(self, conn):
super(SSHPool, self).put(conn)

def do_exec(self, command_str):
result = None
result = ''
eventlet.monkey_patch()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why add monkey_patch() here, we do this at the entry of every process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

18/5000
This module needs to set this place before setting the timeout

Copy link
Collaborator

@ThisIsClark ThisIsClark Jun 7, 2021

Choose a reason for hiding this comment

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

What would happend if we didn't call eventlet.monkey_patch() here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the place to mark this part of the code as a green thread. You don't have to wait for the execution to complete. If the set time exceeds, you can proceed directly.

disks_map = {}
physical_array = physicals_info.split('\r\n')
for i in range(2, len(physical_array), 2):
physicals_list.append(physical_array[i].split())
for disk_str in disks_array[1:]:
speed = physical_type = firmware = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why change default value from None to '-'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No local synchronization, has been modified

ThisIsClark
ThisIsClark previously approved these changes Jun 8, 2021
Copy link
Collaborator

@ThisIsClark ThisIsClark left a comment

Choose a reason for hiding this comment

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

LGTM

if controller_map['Health'] == 'true' \
else constants.ControllerStatus.OFFLINE
controller_model = {
'name': controller_map['e'],
Copy link
Member

Choose a reason for hiding this comment

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

what is value for key 'e' ? do you have an exmplae of command output and parsed map ?

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 sample is:
Node: cl-01
Owner:
Location:
Model: SIMBOX
Serial Number: 4082368-50-7
Asset Tag: -
Uptime: 3 days 21:20
NVRAM System ID: 4082368507
System ID: 4082368507
Vendor: NetApp
Health: true
Eligibility: true
Differentiated Services: false
All-Flash Optimized: false

Here the first Nod is removed as a split string, so 'e' is used as the key

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mappings have been added for easy reading

Copy link
Collaborator

@wisererik wisererik left a comment

Choose a reason for hiding this comment

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

LGTM

@ThisIsClark ThisIsClark merged commit 93b3b01 into sodafoundation:master Jun 10, 2021
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.

5 participants