Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion network/hnswrapper/hnsv2wrapperfake.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func (f Hnsv2wrapperFake) GetNetworkByName(networkName string) (*hcn.HostCompute
if network, ok := f.Cache.networks[networkName]; ok {
return network.GetHCNObj(), nil
}
return &hcn.HostComputeNetwork{}, nil
return nil, hcn.NetworkNotFoundError{}
Copy link
Member

Choose a reason for hiding this comment

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

just make sure cni code doesn't dereference pointer without checking for error

Copy link
Contributor Author

@aegal aegal Apr 9, 2022

Choose a reason for hiding this comment

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

this is the hns mock wrapper and only used by test code, no real cni code uses this type so there's no concerns.

}

func (f Hnsv2wrapperFake) GetNetworkByID(networkID string) (*hcn.HostComputeNetwork, error) {
Expand Down
26 changes: 20 additions & 6 deletions network/network_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package network

import (
"encoding/json"
"errors"
"fmt"
"strconv"
"strings"
Expand Down Expand Up @@ -310,14 +311,27 @@ func (nm *networkManager) newNetworkImplHnsV2(nwInfo *NetworkInfo, extIf *extern
return nil, err
}

// Create the HNS network.
log.Printf("[net] Creating hcn network: %+v", hcnNetwork)
hnsResponse, err := hnsv2.CreateNetwork(hcnNetwork)
// check if network exists, only create the network does not exist
hnsResponse, err := hnsv2.GetNetworkByName(hcnNetwork.Name)
Copy link
Member

Choose a reason for hiding this comment

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

can you also log if network exists

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, added it when err is empty


if err != nil {
return nil, fmt.Errorf("Failed to create hcn network: %s due to error: %v", hcnNetwork.Name, err)
}
// if network not found, create the HNS network.
if errors.As(err, &hcn.NetworkNotFoundError{}) {
Copy link
Member

Choose a reason for hiding this comment

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

can you add ut for testing these cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

log.Printf("[net] Creating hcn network: %+v", hcnNetwork)
hnsResponse, err = hnsv2.CreateNetwork(hcnNetwork)

if err != nil {
return nil, fmt.Errorf("Failed to create hcn network: %s due to error: %v", hcnNetwork.Name, err)
}

log.Printf("[net] Successfully created hcn network with response: %+v", hnsResponse)
log.Printf("[net] Successfully created hcn network with response: %+v", hnsResponse)
} else {
// we can't validate if the network already exists, don't continue
return nil, fmt.Errorf("Failed to create hcn network: %s, failed to query for existing network with error: %v", hcnNetwork.Name, err)
}
} else {
log.Printf("[net] Network with name %s already exists", hcnNetwork.Name)
}

var vlanid int
opt, _ := nwInfo.Options[genericData].(map[string]interface{})
Expand Down
39 changes: 38 additions & 1 deletion network/network_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"testing"

"github.com/Azure/azure-container-networking/network/hnswrapper"

"github.com/Microsoft/hcsshim/hcn"
)

func TestNewAndDeleteNetworkImplHnsV2(t *testing.T) {
Expand All @@ -20,7 +22,7 @@ func TestNewAndDeleteNetworkImplHnsV2(t *testing.T) {

// this hnsv2 variable overwrites the package level variable in network
// we do this to avoid passing around os specific objects in platform agnostic code
hnsv2 = hnswrapper.Hnsv2wrapperFake{}
hnsv2 = hnswrapper.NewHnsv2wrapperFake()

nwInfo := &NetworkInfo{
Id: "d3e97a83-ba4c-45d5-ba88-dc56757ece28",
Expand All @@ -47,3 +49,38 @@ func TestNewAndDeleteNetworkImplHnsV2(t *testing.T) {
t.Fatal(err)
}
}

func TestSuccesfulNetworkCreationWhenAlreadyExists(t *testing.T) {
nm := &networkManager{
ExternalInterfaces: map[string]*externalInterface{},
}

// this hnsv2 variable overwrites the package level variable in network
// we do this to avoid passing around os specific objects in platform agnostic code
hnsv2 = hnswrapper.NewHnsv2wrapperFake()

network := &hcn.HostComputeNetwork{
Name: "azure-vlan1-172-28-1-0_24",
}

_, err := hnsv2.CreateNetwork(network)

// network name is derived from network info id
nwInfo := &NetworkInfo{
Id: "azure-vlan1-172-28-1-0_24",
MasterIfName: "eth0",
Mode: "bridge",
}

extInterface := &externalInterface{
Name: "eth0",
Subnets: []string{"subnet1", "subnet2"},
}

_, err = nm.newNetworkImplHnsV2(nwInfo, extInterface)

if err != nil {
fmt.Printf("+%v", err)
t.Fatal(err)
}
}