-
Notifications
You must be signed in to change notification settings - Fork 296
xe-reset-networking: Allow the user to perform a network reset without renaming the management interface #6852
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -91,6 +91,7 @@ if __name__ == "__main__": | |||||||||||
| parser.add_option("--gateway", help="Gateway for new management interface", dest="gateway", default='') | ||||||||||||
| parser.add_option("--gateway-v6", help="IPv6 Gateway for new management interface", dest="gateway_v6", default='') | ||||||||||||
| parser.add_option("--dns", help="DNS server for new management interface", dest="dns", default='') | ||||||||||||
| parser.add_option("--reset-interface-name-rules", help="Reset the management interface name", dest="reset_interface_name_rules", action="store_const", const=True, default=True) | ||||||||||||
| (options, args) = parser.parse_args() | ||||||||||||
|
|
||||||||||||
| # Determine pool role | ||||||||||||
|
|
@@ -174,6 +175,10 @@ if __name__ == "__main__": | |||||||||||
| if not os.access('/tmp/fist_network_reset_no_warning', os.F_OK): | ||||||||||||
| configuration = [] | ||||||||||||
| configuration.append("Management interface: " + device) | ||||||||||||
| if not options.reset_interface_name_rules: | ||||||||||||
| configuration.append("Rename interface: Yes") | ||||||||||||
| else: | ||||||||||||
| configuration.append("Rename interface: No") | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
| configuration.append("IP configuration mode: " + options.mode) | ||||||||||||
| configuration.append("IPv6 configuration mode:" + options.mode_v6) | ||||||||||||
| if vlan != None: | ||||||||||||
|
|
@@ -288,8 +293,9 @@ Type 'no' to cancel. | |||||||||||
| if rename_script_exists: | ||||||||||||
| # Reset the domain 0 network interface naming configuration | ||||||||||||
| # back to a fresh-install state for the currently-installed | ||||||||||||
| # hardware. | ||||||||||||
| os.system(f"{RENAME_SCRIPT} --reset-to-install") | ||||||||||||
| # hardware (unless prevented by the user). | ||||||||||||
| if not options.reset_interface_name_rules: | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
| os.system(f"{RENAME_SCRIPT} --reset-to-install") | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This script already deletes networkd.db, writes new management.conf, update inventory file. And create
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure I understand 100% the question. Based on my understanding and the testing I have done, Currently, the management network interface will be renamed back to what it would have been if a fresh installation was to be performed. However, for some use cases, the user might have already renamed the interface and would like to prevent the renaming when doing If you mean something different and I can help somehow, please let me know.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to have a better understand of the issue. Can you show the use case in detail? How does the user rename the interface? Does it change the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For example, we had a user with a host which had interfaces with certain names that needed to be renamed to be part of a bond interface before joining an XCP-ng pool. They renamed the relevant interface by using Also, others report that renaming is some times useful to retain consistency across the hosts in the same pool.
Hmm, I don't have a strong opinion.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you. I understand the issue now.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. Let me know if the (force-pushed) log message is ok. Btw,I noticed (Python 3.6.8) an unexpected behavior that is related with parameters that contain any dashes, most probably related to This is obviously part of another discussion, but if we want to avoid this behavior for this parameter with a quick fix, we can use a name without dashes. Let me know your thoughts.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah there are some errors in the new commit.
Based on this, I suggest the option name
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the args parse, I think it is not urgent. It's legacy behavior and there is a message with all the configurations to be confirmed by the user.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the explanation. Not sure we are on the same page. Although I get your description on the path forward, to be honest, I am not really aware of how the transition for the new networkd will take place in relation to e.g. network reset. If I get it correctly, All in all, my original commit was about providing the user the ability to prevent resetting the interface name in the current (pre-transition) state. On your side you suggest to port this ability to the new state, correct? If so, maybe it is better to merge the initial PR to the "old" codebase (as you initially suggested) and maybe proceed afterwards with another PR to port it to the "new" codebase.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||
|
|
||||||||||||
| # Reboot | ||||||||||||
| os.system("mount -o remount,rw / && reboot -f") | ||||||||||||
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.