-
Notifications
You must be signed in to change notification settings - Fork 664
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
generic_config_updater: Filename changed & VLAN validator added #1919
Conversation
2) Read conf file from install dir 3) Drop empty keys & tables upon jsonpatch.JsonPatch.apply to be in sync with redis update 4) Prefix service_validator module path with "generic_updater"
2) Added vlan validator 3) Added test code for vlan validator
for key in set(old_vlan.keys()).union(set(upd_vlan.keys())): | ||
if (old_vlan.get(key, {}).get("dhcp_servers", []) != | ||
upd_vlan.get(key, {}).get("dhcp_servers", [])): | ||
return _service_restart("dhcp_relay") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NICE! We just restart dhcp at the first occurrence of mismatch between dhcp_servers in old vs updated.
@@ -39,6 +39,9 @@ | |||
}, | |||
"DHCP_SERVER": { | |||
"services_to_validate": [ "dhcp-relay" ] | |||
}, | |||
"VLAN": { | |||
"services_to_validate": [ "vlan-service" ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the design doc https://github.com/Azure/SONiC/blob/master/doc/config-generic-update-rollback/Json_Change_Application_Design.md#3212-metadata. It fits better as restart-command
than services_to_validate
#Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not blindly restart.
For example, in case of VLAN table update, we call vlan_validator, which may or may not restart dhcp_sevice.
Again a validator, could potentially restart more than one and possibly none too, as in vlan case.
So I prefer this name validator, which is table specific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the intelligence of restarting dhcp_service for vlan-service. However, the validator is design as read-only operations for the purpose of validation, including validating services are completely restarted and healthy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get your last comments. But Design doc updated.
#### What I did Change filename per review comments from last PR. Removed print statement from change_applier Added vlan validator & test code for the same
What I did
Change filename per review comments from last PR.
Removed print statement from change_applier
Added vlan validator & test code for the same
How I did it
How to verify it
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)