Skip to content

Optimize call flow to complete loopback adapter creation #408

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 30, 2019
Merged

Optimize call flow to complete loopback adapter creation #408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d76d76c
Add optimization for loopback adapter creation.
ashvindeodhar Sep 26, 2019
951d5cf
Fix line formatting
ashvindeodhar Sep 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ CNSFILES = \
$(wildcard cns/restserver/*.go) \
$(wildcard cns/routes/*.go) \
$(wildcard cns/service/*.go) \
$(wildcard cns/networkcontainers/*.go) \
$(COREFILES) \
$(CNMFILES)

Expand Down
24 changes: 14 additions & 10 deletions cns/networkcontainers/networkcontainers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,38 +55,42 @@ func NewNetPluginConfiguration(binPath, configPath string) *NetPluginConfigurati
func interfaceExists(iFaceName string) (bool, error) {
_, err := net.InterfaceByName(iFaceName)
if err != nil {
errMsg := fmt.Sprintf("[Azure CNS] Unable to get interface by name %v, %v", iFaceName, err)
errMsg := fmt.Sprintf("[Azure CNS] Unable to get interface by name %s. Error: %v", iFaceName, err)
log.Printf(errMsg)
return false, errors.New(errMsg)
}

log.Printf("[Azure CNS] Found interface by name %s", iFaceName)

return true, nil
}

// Create creates a network container.
func (cn *NetworkContainers) Create(createNetworkContainerRequest cns.CreateNetworkContainerRequest) error {
log.Printf("[Azure CNS] NetworkContainers.Create called")
log.Printf("[Azure CNS] NetworkContainers.Create called for NC: %s", createNetworkContainerRequest.NetworkContainerid)
err := createOrUpdateInterface(createNetworkContainerRequest)
if err == nil {
err = setWeakHostOnInterface(createNetworkContainerRequest.PrimaryInterfaceIdentifier)
}
log.Printf("[Azure CNS] NetworkContainers.Create finished.")
log.Printf("[Azure CNS] NetworkContainers.Create completed for NC: %s with error: %v",
createNetworkContainerRequest.NetworkContainerid, err)

return err
}

// Update updates a network container.
func (cn *NetworkContainers) Update(createNetworkContainerRequest cns.CreateNetworkContainerRequest, netpluginConfig *NetPluginConfiguration) error {
log.Printf("[Azure CNS] NetworkContainers.Update called")
log.Printf("[Azure CNS] NetworkContainers.Update called for NC: %s", createNetworkContainerRequest.NetworkContainerid)
err := updateInterface(createNetworkContainerRequest, netpluginConfig)
log.Printf("[Azure CNS] NetworkContainers.Update finished.")
log.Printf("[Azure CNS] NetworkContainers.Update completed for NC: %s with error: %v",
createNetworkContainerRequest.NetworkContainerid, err)

return err
}

// Delete deletes a network container.
func (cn *NetworkContainers) Delete(networkContainerID string) error {
log.Printf("[Azure CNS] NetworkContainers.Delete called")
log.Printf("[Azure CNS] NetworkContainers.Delete called for NC: %s", networkContainerID)
err := deleteInterface(networkContainerID)
log.Printf("[Azure CNS] NetworkContainers.Delete finished.")
log.Printf("[Azure CNS] NetworkContainers.Delete completed for NC: %s with error: %v", networkContainerID, err)

return err
}

Expand Down
2 changes: 1 addition & 1 deletion cns/networkcontainers/networkcontainers_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func createOrUpdateInterface(createNetworkContainerRequest cns.CreateNetworkCont
return nil
}

func setWeakHostOnInterface(ipAddress string) error {
func setWeakHostOnInterface(ipAddress, ncID string) error {
return nil
}

Expand Down
37 changes: 21 additions & 16 deletions cns/networkcontainers/networkcontainers_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ func createOrUpdateInterface(createNetworkContainerRequest cns.CreateNetworkCont
return nil
}

exists, _ := interfaceExists(createNetworkContainerRequest.NetworkContainerid)

if !exists {
if exists, _ := interfaceExists(createNetworkContainerRequest.NetworkContainerid); !exists {
return createOrUpdateWithOperation(createNetworkContainerRequest, "CREATE")
}

Expand All @@ -40,7 +38,7 @@ func updateInterface(createNetworkContainerRequest cns.CreateNetworkContainerReq
return nil
}

func setWeakHostOnInterface(ipAddress string) error {
func setWeakHostOnInterface(ipAddress, ncID string) error {
Copy link
Contributor

@ninzavivek ninzavivek Sep 27, 2019

Choose a reason for hiding this comment

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

is this function even needed now?

Copy link
Contributor

@ninzavivek ninzavivek Sep 27, 2019

Choose a reason for hiding this comment

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

I mean separately?

Copy link
Contributor

@ninzavivek ninzavivek Sep 27, 2019

Choose a reason for hiding this comment

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

Is there way to asset the lock has been acquired already?

Copy link
Member Author

@ashvindeodhar ashvindeodhar Sep 27, 2019

Choose a reason for hiding this comment

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

asset the lock?
Keeping the function separate because it's still a separate functionality

interfaces, err := net.Interfaces()
if err != nil {
log.Printf("[Azure CNS] Unable to retrieve interfaces on machine. %+v", err)
Expand Down Expand Up @@ -76,7 +74,6 @@ func setWeakHostOnInterface(ipAddress string) error {
}

ethIndexString := strconv.Itoa(targetIface.Index)
log.Printf("[Azure CNS] Going to setup weak host routing for interface with index[%v, %v]\n", targetIface.Index, ethIndexString)

args := []string{"/C", "AzureNetworkContainer.exe", "/logpath", log.GetLogDirectory(),
"/index",
Expand All @@ -88,17 +85,17 @@ func setWeakHostOnInterface(ipAddress string) error {
"/weakhostreceive",
"true"}

log.Printf("[Azure CNS] Going to enable weak host send/receive on interface: %v", args)
log.Printf("[Azure CNS] Going to enable weak host send/receive on interface: %v for NC: %s", args, ncID)
c := exec.Command("cmd", args...)

loopbackOperationLock.Lock()
bytes, err := c.Output()
loopbackOperationLock.Unlock()

if err == nil {
log.Printf("[Azure CNS] Successfully updated weak host send/receive on interface %v.\n", string(bytes))
log.Printf("[Azure CNS] Successfully updated weak host send/receive for NC: %s on interface %v",
ncID, string(bytes))
} else {
log.Printf("[Azure CNS] Received error while enable weak host send/receive on interface. %v - %v", err.Error(), string(bytes))
log.Printf("[Azure CNS] Failed to update weak host send/receive for NC: %s. Error: %v. Output: %v",
ncID, err.Error(), string(bytes))
return err
}

Expand Down Expand Up @@ -140,17 +137,23 @@ func createOrUpdateWithOperation(createNetworkContainerRequest cns.CreateNetwork
"/weakhostreceive",
"true"}

log.Printf("[Azure CNS] Going to create/update network loopback adapter: %v", args)
c := exec.Command("cmd", args...)

loopbackOperationLock.Lock()
log.Printf("[Azure CNS] Going to create/update network loopback adapter: %v", args)
bytes, err := c.Output()
if err == nil {
err = setWeakHostOnInterface(createNetworkContainerRequest.PrimaryInterfaceIdentifier,
createNetworkContainerRequest.NetworkContainerid)
}
loopbackOperationLock.Unlock()

if err == nil {
log.Printf("[Azure CNS] Successfully created network loopback adapter %v.\n", string(bytes))
log.Printf("[Azure CNS] Successfully created network loopback adapter for NC: %s. Output:%v.",
createNetworkContainerRequest.NetworkContainerid, string(bytes))
} else {
log.Printf("Received error while Creating a Network Container %v %v", err.Error(), string(bytes))
log.Printf("Failed to create/update Network Container: %s. Error: %v. Output: %v",
createNetworkContainerRequest.NetworkContainerid, err.Error(), string(bytes))
}

return err
Expand All @@ -174,17 +177,19 @@ func deleteInterface(networkContainerID string) error {
"/operation",
"DELETE"}

log.Printf("[Azure CNS] Going to delete network loopback adapter: %v", args)
c := exec.Command("cmd", args...)

loopbackOperationLock.Lock()
log.Printf("[Azure CNS] Going to delete network loopback adapter: %v", args)
bytes, err := c.Output()
loopbackOperationLock.Unlock()

if err == nil {
Copy link
Contributor

@ninzavivek ninzavivek Sep 27, 2019

Choose a reason for hiding this comment

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

do you think we can just combine this? a single line?

Copy link
Member Author

@ashvindeodhar ashvindeodhar Sep 27, 2019

Choose a reason for hiding this comment

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

Which lines are you suggesting to combine?

log.Printf("[Azure CNS] Successfully deleted network container %v.\n", string(bytes))
log.Printf("[Azure CNS] Successfully deleted network container: %s. Output: %v.",
networkContainerID, string(bytes))
} else {
log.Printf("Received error while deleting a Network Container %v %v", err.Error(), string(bytes))
log.Printf("Failed to delete Network Container: %s. Error:%v. Output:%v",
networkContainerID, err.Error(), string(bytes))
return err
}
return nil
Expand Down