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
net-convert: use yaml.dump for debugging python NetworkState obj #1484
Conversation
Mostly a unittest fix, as most systems which want to render systemd-networkd config artifacts already have /etc/systemd/network. Avoid calling os.dirname on self.network_conf_dir because then we only create the parent directory /etc/systemd and cannot write /etc/systemd/network/10-cloud-init-eth0.network
When debugging python's NetworkState intance we cannot use safeyaml.dumps because that leverages the yaml.SafeDumper which does not allow rendering python objects. Use yamls.dump instead. LP: #1975907
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.
LGTM. Thanks for fixing both bugs and add unit tests.
|
||
util.ensure_dir(os.path.dirname(fp_nwkd)) | ||
util.ensure_dir(network_dir) |
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 discovery here!
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 have one optional inline nit, but I don't think it currently changes anything functionally, so feel free to disregard. Lgtm!
"""Assert proper output-kind artifacts are written.""" | ||
network_data = tmpdir.join("network_data") | ||
network_data.write(SAMPLE_NET_V1) | ||
distro = "fedora" if output_kind == "sysconfig" else "ubuntu" |
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.
Nit: Since fedora just switched to the network-manager renderer, maybe we should use a slower-moving distro for the sysconfig distro, such as CentOS?
Proposed Commit Message
Additional Context
Test Steps
Checklist: