Kdump Remote ssh yang enhancements#19540
Conversation
|
@venkatmahalingam @ganglyu pls review this code PR. Thanks |
| type string; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
would you please add unit test?
|
@ganglyu can you pls help review this PR. |
muhammadalihussnain
left a comment
There was a problem hiding this comment.
@venkatmahalingam Please review the Code PR
|
@venkatmahalingam Hi! we have updated the code. Please Help us review the Code PRs. Thanks |
| leaf ssh_path { | ||
| description "Remote ssh private key path"; | ||
| type string; | ||
| } |
There was a problem hiding this comment.
Could you update these new changes to doc/Configuration.md? #Closed
There was a problem hiding this comment.
updated, please review
| "memory": "0M-2G:256M,2G-4G:256M,4G-8G:384M,8G-:448M" | ||
| "memory": "0M-2G:256M,2G-4G:256M,4G-8G:384M,8G-:448M", | ||
| "remote": "true", | ||
| "ssh_string" : "username@ipadress", |
There was a problem hiding this comment.
This can't be the sample config, please use the actual configuration e.g user1@10.0.0.2 and actual private key path, this is the problem of using the generic string type, see if you can use string pattern for user name and ip address.
There was a problem hiding this comment.
Noted, Updating as per suggestion
|
/azpw run Azure.sonic-buildimage |
|
/azpw run |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azpw run |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@venkatmahalingam please help review updated changes. Thanks in advance! |
|
@qiluo-msft please help merge this PR |
|
Hi @qiluo-msft, please review to help merge this PR. Regards |
|
Hi @qiluo-msft PR has been reviewed by the reviewer with all checks passed, please help merge this. |
|
@qiluo-msft help merge this PR, pending for long |
|
@liushilongbuaa can you please check the conflict? Thanks. |
|
/azpw ms_conflict |
1 similar comment
|
/azpw ms_conflict |
| "Enable or Disable the Kdump remote ssh mechanism"; | ||
| } | ||
|
|
||
| leaf ssh_string { |
There was a problem hiding this comment.
Default key added in hostcfgd doesn't match the Yang model (https://github.com/muhammadalihussnain/sonic-host-services/blob/3417b5f8a010901131a4646685efb3e61822ef2a/scripts/hostcfgd#L1142) for both ssh_string and ssh_key. I would suggest to update the default keys in hostcfgd both to smaller case.
There was a problem hiding this comment.
Thanks for sharing your suggestion.
As we have hostcfg code merged to master, we have two options available to modify in the YANG model.
- Replace names in Upper case, leaf SSH_KEY & leaf SSH_PATH, with the current ones leaf ssh_string & leaf ssh_path.
or - Add deviations ensuring compatibility with hostcfgd by setting defaults, keeping lowercase of leaf ssh_string & leaf ssh_path
What would you suggest? Thanks
There was a problem hiding this comment.
I would suggest to change both SSH_KEY and SSH_PATH to lowercase in hostcfgd to make it consistent with other config variables.
|
HI @ram25794 , we have made the changes in the new PR per your suggestions. Kindly review and help to merge PR |
Why I did it
Added support for Kdump remote SSH feature in existing sonic-kdump.yang
How I did it
Modified sonic-kdump.yang file to add support for ssh , ssh_connection and ssh_path strings
How to verify it
Updated yang model with required changes for feature