diff --git a/pkg/network/aci_network.go b/pkg/network/aci_network.go index cc485833..64dd93e0 100644 --- a/pkg/network/aci_network.go +++ b/pkg/network/aci_network.go @@ -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 @@ -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 @@ -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 } @@ -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 } } @@ -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 diff --git a/pkg/network/mock_aci_network_test.go b/pkg/network/mock_aci_network_test.go deleted file mode 100644 index 7b9ae4de..00000000 --- a/pkg/network/mock_aci_network_test.go +++ /dev/null @@ -1,81 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: aci_network.go - -// Package network is a generated GoMock package. -package network - -import ( - context "context" - reflect "reflect" - - armnetwork "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v2" - gomock "github.com/golang/mock/gomock" - auth "github.com/virtual-kubelet/azure-aci/pkg/auth" -) - -// MockProviderNetworkInterface is a mock of ProviderNetworkInterface interface. -type MockProviderNetworkInterface struct { - ctrl *gomock.Controller - recorder *MockProviderNetworkInterfaceMockRecorder -} - -// MockProviderNetworkInterfaceMockRecorder is the mock recorder for MockProviderNetworkInterface. -type MockProviderNetworkInterfaceMockRecorder struct { - mock *MockProviderNetworkInterface -} - -// NewMockProviderNetworkInterface creates a new mock instance. -func NewMockProviderNetworkInterface(ctrl *gomock.Controller) *MockProviderNetworkInterface { - mock := &MockProviderNetworkInterface{ctrl: ctrl} - mock.recorder = &MockProviderNetworkInterfaceMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockProviderNetworkInterface) EXPECT() *MockProviderNetworkInterfaceMockRecorder { - return m.recorder -} - -// CreateACISubnet mocks base method. -func (m *MockProviderNetworkInterface) CreateACISubnet(ctx context.Context, subnetsClient *armnetwork.SubnetsClient) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateACISubnet", ctx, subnetsClient) - ret0, _ := ret[0].(error) - return ret0 -} - -// CreateACISubnet indicates an expected call of CreateACISubnet. -func (mr *MockProviderNetworkInterfaceMockRecorder) CreateACISubnet(ctx, subnetsClient interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateACISubnet", reflect.TypeOf((*MockProviderNetworkInterface)(nil).CreateACISubnet), ctx, subnetsClient) -} - -// GetACISubnet mocks base method. -func (m *MockProviderNetworkInterface) GetACISubnet(ctx context.Context, subnetsClient *armnetwork.SubnetsClient) (armnetwork.Subnet, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetACISubnet", ctx, subnetsClient) - ret0, _ := ret[0].(armnetwork.Subnet) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetACISubnet indicates an expected call of GetACISubnet. -func (mr *MockProviderNetworkInterfaceMockRecorder) GetACISubnet(ctx, subnetsClient interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetACISubnet", reflect.TypeOf((*MockProviderNetworkInterface)(nil).GetACISubnet), ctx, subnetsClient) -} - -// GetSubnetClient mocks base method. -func (m *MockProviderNetworkInterface) GetSubnetClient(ctx context.Context, azConfig *auth.Config) (*armnetwork.SubnetsClient, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetSubnetClient", ctx, azConfig) - ret0, _ := ret[0].(*armnetwork.SubnetsClient) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetSubnetClient indicates an expected call of GetSubnetClient. -func (mr *MockProviderNetworkInterfaceMockRecorder) GetSubnetClient(ctx, azConfig interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSubnetClient", reflect.TypeOf((*MockProviderNetworkInterface)(nil).GetSubnetClient), ctx, azConfig) -} diff --git a/pkg/provider/aci.go b/pkg/provider/aci.go index 74943766..95f7a3a9 100644 --- a/pkg/provider/aci.go +++ b/pkg/provider/aci.go @@ -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 @@ -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 != "" { @@ -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) @@ -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, @@ -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 && diff --git a/pkg/provider/config.go b/pkg/provider/config.go index 6174765b..17bc2ac8 100644 --- a/pkg/provider/config.go +++ b/pkg/provider/config.go @@ -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 == "" {