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

refactor: Update network methods for unit test #532

Merged
merged 1 commit into from
Apr 13, 2023
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
104 changes: 57 additions & 47 deletions pkg/network/aci_network.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,6 @@ var (
subnetAction = "Microsoft.Network/virtualNetworks/subnets/action"
)

type ProviderNetworkInterface interface {
GetSubnetClient(ctx context.Context, azConfig *auth.Config) (*aznetworkv2.SubnetsClient, error)
GetACISubnet(ctx context.Context, subnetsClient *aznetworkv2.SubnetsClient) (aznetworkv2.Subnet, error)
CreateACISubnet(ctx context.Context, subnetsClient *aznetworkv2.SubnetsClient) error
}

type ProviderNetwork struct {
VnetSubscriptionID string
VnetName string
Expand All @@ -60,6 +54,25 @@ func (pn *ProviderNetwork) SetVNETConfig(ctx context.Context, azConfig *auth.Con
ctx, span := trace.StartSpan(ctx, "network.SetVNETConfig")
defer span.End()

err := pn.validateNetworkConfig(ctx, azConfig)
if err != nil {
return err
}

if pn.SubnetName != "" {
if err := pn.setupNetwork(ctx, azConfig); err != nil {
return fmt.Errorf("error setting up network: %v", err)
}

if kubeDNSIP := os.Getenv("KUBE_DNS_IP"); kubeDNSIP != "" {
log.G(ctx).Debug("kube DNS IP env variable KUBE_DNS_IP is set")
pn.KubeDNSIP = kubeDNSIP
}
}
return nil
}

func (pn *ProviderNetwork) validateNetworkConfig(ctx context.Context, azConfig *auth.Config) error {
// the VNET subscription ID by default is authentication subscription ID.
// We need to override when using cross subscription virtual network resource
pn.VnetSubscriptionID = azConfig.AuthConfig.SubscriptionID
Expand Down Expand Up @@ -99,17 +112,6 @@ func (pn *ProviderNetwork) SetVNETConfig(ctx context.Context, azConfig *auth.Con
}
pn.SubnetCIDR = subnetCIDR
}

if pn.SubnetName != "" {
if err := pn.setupNetwork(ctx, azConfig); err != nil {
return fmt.Errorf("error setting up network: %v", err)
}

if kubeDNSIP := os.Getenv("KUBE_DNS_IP"); kubeDNSIP != "" {
log.G(ctx).Debug("kube DNS IP env variable KUBE_DNS_IP is set")
pn.KubeDNSIP = kubeDNSIP
}
}
return nil
}

Expand All @@ -133,36 +135,9 @@ func (pn *ProviderNetwork) setupNetwork(ctx context.Context, azConfig *auth.Conf
}

if err == nil {
if currentSubnet.Properties.AddressPrefix != nil {
if pn.SubnetCIDR == "" {
pn.SubnetCIDR = *currentSubnet.Properties.AddressPrefix
}
if pn.SubnetCIDR != *currentSubnet.Properties.AddressPrefix {
return fmt.Errorf("found subnet '%s' using different CIDR: '%s'. desired: '%s'", pn.SubnetName, *currentSubnet.Properties.AddressPrefix, pn.SubnetCIDR)
}
if currentSubnet.Properties.RouteTable != nil {
return fmt.Errorf("unable to delegate subnet '%s' to Azure Container Instance since it references the route table '%s'", pn.SubnetName, *currentSubnet.Properties.RouteTable.ID)
}
if currentSubnet.Properties.ServiceAssociationLinks != nil {
for _, l := range currentSubnet.Properties.ServiceAssociationLinks {
if l.Properties != nil && l.Properties.LinkedResourceType != nil {
if *l.Properties.LinkedResourceType == subnetDelegationService {
createSubnet = false
break
} else {
return fmt.Errorf("unable to delegate subnet '%s' to Azure Container Instance as it is used by other Azure resource: '%v'", pn.SubnetName, l)
}
}
}
} else {
for _, d := range currentSubnet.Properties.Delegations {
if d.Properties != nil && d.Properties.ServiceName != nil &&
*d.Properties.ServiceName == subnetDelegationService {
createSubnet = false
break
}
}
}
createSubnet, err = pn.shouldCreateSubnet(currentSubnet, createSubnet)
if err != nil {
return err
}
}

Expand All @@ -179,6 +154,41 @@ func (pn *ProviderNetwork) setupNetwork(ctx context.Context, azConfig *auth.Conf
return nil
}

func (pn *ProviderNetwork) shouldCreateSubnet(currentSubnet aznetworkv2.Subnet, createSubnet bool) (bool, error) {
if currentSubnet.Properties.AddressPrefix != nil {
if pn.SubnetCIDR == "" {
pn.SubnetCIDR = *currentSubnet.Properties.AddressPrefix
}
if pn.SubnetCIDR != *currentSubnet.Properties.AddressPrefix {
return false, fmt.Errorf("found subnet '%s' using different CIDR: '%s'. desired: '%s'", pn.SubnetName, *currentSubnet.Properties.AddressPrefix, pn.SubnetCIDR)
}
if currentSubnet.Properties.RouteTable != nil {
return false, fmt.Errorf("unable to delegate subnet '%s' to Azure Container Instance since it references the route table '%s'", pn.SubnetName, *currentSubnet.Properties.RouteTable.ID)
}
if currentSubnet.Properties.ServiceAssociationLinks != nil {
for _, l := range currentSubnet.Properties.ServiceAssociationLinks {
if l.Properties != nil && l.Properties.LinkedResourceType != nil {
if *l.Properties.LinkedResourceType == subnetDelegationService {
createSubnet = false
break
} else {
return false, fmt.Errorf("unable to delegate subnet '%s' to Azure Container Instance as it is used by other Azure resource: '%v'", pn.SubnetName, l)
}
}
}
} else {
for _, d := range currentSubnet.Properties.Delegations {
if d.Properties != nil && d.Properties.ServiceName != nil &&
*d.Properties.ServiceName == subnetDelegationService {
createSubnet = false
break
}
}
}
}
return createSubnet, nil
}

func (pn *ProviderNetwork) GetACISubnet(ctx context.Context, subnetsClient *aznetworkv2.SubnetsClient) (aznetworkv2.Subnet, error) {
response, err := subnetsClient.Get(ctx, pn.VnetResourceGroup, pn.VnetName, pn.SubnetName, nil)
var respErr *azcore.ResponseError
Expand Down
81 changes: 0 additions & 81 deletions pkg/network/mock_aci_network_test.go

This file was deleted.

18 changes: 9 additions & 9 deletions pkg/provider/aci.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ type ACIProvider struct {
configL corev1listers.ConfigMapLister
podsL corev1listers.PodLister
enabledFeatures *featureflag.FlagIdentifier
providernetwork network.ProviderNetwork
providerNetwork network.ProviderNetwork

resourceGroup string
region string
Expand Down Expand Up @@ -201,12 +201,12 @@ func NewACIProvider(ctx context.Context, config string, azConfig auth.Config, az
if azConfig.AKSCredential != nil {
p.resourceGroup = azConfig.AKSCredential.ResourceGroup
p.region = azConfig.AKSCredential.Region
p.providernetwork.VnetName = azConfig.AKSCredential.VNetName
p.providernetwork.VnetResourceGroup = azConfig.AKSCredential.VNetResourceGroup
p.providerNetwork.VnetName = azConfig.AKSCredential.VNetName
p.providerNetwork.VnetResourceGroup = azConfig.AKSCredential.VNetResourceGroup
}

if p.providernetwork.VnetResourceGroup == "" {
p.providernetwork.VnetResourceGroup = p.resourceGroup
if p.providerNetwork.VnetResourceGroup == "" {
p.providerNetwork.VnetResourceGroup = p.resourceGroup
}
// If the log analytics file has been specified, load workspace credentials from the file
if logAnalyticsAuthFile := os.Getenv("LOG_ANALYTICS_AUTH_LOCATION"); logAnalyticsAuthFile != "" {
Expand Down Expand Up @@ -259,11 +259,11 @@ func NewACIProvider(ctx context.Context, config string, azConfig auth.Config, az
return nil, err
}

if err := p.providernetwork.SetVNETConfig(ctx, &azConfig); err != nil {
if err := p.providerNetwork.SetVNETConfig(ctx, &azConfig); err != nil {
return nil, err
}

if p.providernetwork.SubnetName != "" {
if p.providerNetwork.SubnetName != "" {
// windows containers don't support kube-proxy nor realtime metrics
if p.operatingSystem != string(azaciv2.OperatingSystemTypesWindows) {
err = p.setACIExtensions(ctx)
Expand Down Expand Up @@ -358,7 +358,7 @@ func (p *ACIProvider) CreatePod(ctx context.Context, pod *v1.Pod) error {
})
}
}
if len(ports) > 0 && p.providernetwork.SubnetName == "" {
if len(ports) > 0 && p.providerNetwork.SubnetName == "" {
cg.Properties.IPAddress = &azaciv2.IPAddress{
Ports: ports,
Type: &util.ContainerGroupIPAddressTypePublic,
Expand All @@ -379,7 +379,7 @@ func (p *ACIProvider) CreatePod(ctx context.Context, pod *v1.Pod) error {
"CreationTimestamp": &podCreationTimestamp,
}

p.providernetwork.AmendVnetResources(ctx, *cg, pod, p.clusterDomain)
p.providerNetwork.AmendVnetResources(ctx, *cg, pod, p.clusterDomain)

// windows containers don't support kube-proxy nor realtime metrics
if cg.Properties.OSType != nil &&
Expand Down
2 changes: 1 addition & 1 deletion pkg/provider/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (p *ACIProvider) loadConfig(r io.Reader) error {

// default subnet name
if config.SubnetName != "" {
p.providernetwork.SubnetName = config.SubnetName
p.providerNetwork.SubnetName = config.SubnetName
}
if config.SubnetCIDR != "" {
if config.SubnetName == "" {
Expand Down