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

Netops refactor #5117

Merged
merged 2 commits into from
Apr 8, 2024
Merged

Netops refactor #5117

merged 2 commits into from
Apr 8, 2024

Conversation

holmanb
Copy link
Member

@holmanb holmanb commented Mar 28, 2024

Additional Context

This is a small step towards making cloud-init use reusable networking interfaces for better cross-distro abstractions and easier maintenance.

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

@holmanb
Copy link
Member Author

holmanb commented Mar 28, 2024

@cjp256 I think that you originally requested the iproute2 refactor to be multi-line for easier reading? Would you mind reviewing this commit which contains that change?

@holmanb holmanb force-pushed the holmanb/netops-refactor branch 3 times, most recently from 343f2b8 to dbe1243 Compare March 29, 2024 00:13
@holmanb holmanb force-pushed the holmanb/netops-refactor branch from dbe1243 to b32fdcc Compare March 29, 2024 00:17
Copy link
Contributor

@cjp256 cjp256 left a comment

Choose a reason for hiding this comment

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

LGTM, nice change - thanks!

cloudinit/net/activators.py Outdated Show resolved Hide resolved
@aciba90 aciba90 self-assigned this Apr 3, 2024
Copy link
Contributor

@aciba90 aciba90 left a comment

Choose a reason for hiding this comment

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

LGTM. Nice refactoring!

As the Merge Type is set to rebase & merge, would you like to merge your last commit into the previous ones?

Various different activators, datasources, and networking code
implementations make use of manual iproute2 calls, which has led to
much code duplication in the codebase.

This is a small step towards replacing distro assumptions at call sites
with common interfaces, which will simplify future refactors for more
distro-agnostic code. These same abstractions will also enable simpler
testing.
@holmanb holmanb force-pushed the holmanb/netops-refactor branch from 9195ba9 to 4d91563 Compare April 8, 2024 20:07
@holmanb
Copy link
Member Author

holmanb commented Apr 8, 2024

LGTM. Nice refactoring!

As the Merge Type is set to rebase & merge, would you like to merge your last commit into the previous ones?

Done, thanks for the review @aciba90!

@holmanb holmanb merged commit 01027e5 into canonical:main Apr 8, 2024
25 of 27 checks passed
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.

3 participants