-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
refactor network commands #4757
refactor network commands #4757
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: baude The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
move core of network commands from pkg/adapter to pkg/network to assist with api development and remote podman commands. Signed-off-by: baude <[email protected]>
036392f
to
93350b9
Compare
this is blocking apiv2 work. please review and merge |
} | ||
|
||
// InspectNetwork reads a CNI config and returns its configuration | ||
func InspectNetwork(name string) (map[string]interface{}, error) { |
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.
Can we return anything better than this? I know that CNI doesn't bother exporting most types, but can we potentially provide some sort of struct defining what contents could be?
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.
It is really too loosely defined to do much more. I don't like it either.
@baude I mean, it is Sunday of shutdown 😄 |
Ack. LGTM then.
…On Mon, Dec 30, 2019, 12:23 Brent Baude ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pkg/network/network.go
<#4757 (comment)>:
> + return errors.Wrapf(err, "failed to get live network names")
+ }
+ if util.StringInSlice(interfaceName, liveNetworkNames) {
+ if err := RemoveInterface(interfaceName); err != nil {
+ return errors.Wrapf(err, "failed to delete the network interface %q", interfaceName)
+ }
+ }
+ // Remove the configuration file
+ if err := os.Remove(cniPath); err != nil {
+ return errors.Wrapf(err, "failed to remove network configuration file %q", cniPath)
+ }
+ return nil
+}
+
+// InspectNetwork reads a CNI config and returns its configuration
+func InspectNetwork(name string) (map[string]interface{}, error) {
It is really too loosely defined to do much more. I don't like it either.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#4757>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB3AOCB7OOEB2N5WS4OHV2TQ3IU77ANCNFSM4KARK66Q>
.
|
/lgtm |
move core of network commands from pkg/adapter to pkg/network to assist
with api development and remote podman commands.
Signed-off-by: baude [email protected]