diff --git a/pkg/consts/consts.go b/pkg/consts/consts.go index 8d68168e6e..d522a7759d 100644 --- a/pkg/consts/consts.go +++ b/pkg/consts/consts.go @@ -83,6 +83,10 @@ const ( // NodeLabelRole specifies the role of a node NodeLabelRole = "kubernetes.io/role" + // MasterNodeRoleLabel specifies is the master node label for a node + MasterNodeRoleLabel = "node-role.kubernetes.io/master" + // ControlPlaneNodeRoleLabel specifies is the control-plane node label for a node + ControlPlaneNodeRoleLabel = "node-role.kubernetes.io/control-plane" // NicFailedState is the failed state of a nic NicFailedState = "Failed" diff --git a/pkg/provider/azure_standard.go b/pkg/provider/azure_standard.go index f92318f89d..40073fa6d1 100644 --- a/pkg/provider/azure_standard.go +++ b/pkg/provider/azure_standard.go @@ -148,14 +148,20 @@ func (az *Cloud) getAzureLoadBalancerName(clusterName string, vmSetName string, return lbNamePrefix } -// isMasterNode returns true if the node has a master role label. -// The master role is determined by looking for: -// * a kubernetes.io/role="master" label -func isMasterNode(node *v1.Node) bool { +// isControlPlaneNode returns true if the node has a control-plane role label. +// The control-plane role is determined by looking for: +// * a node-role.kubernetes.io/control-plane or node-role.kubernetes.io/master="" label +func isControlPlaneNode(node *v1.Node) bool { + if _, ok := node.Labels[consts.ControlPlaneNodeRoleLabel]; ok { + return true + } + // include master role labels for k8s < 1.19 + if _, ok := node.Labels[consts.MasterNodeRoleLabel]; ok { + return true + } if val, ok := node.Labels[consts.NodeLabelRole]; ok && val == "master" { return true } - return false } @@ -662,7 +668,7 @@ func (as *availabilitySet) getAgentPoolAvailabilitySets(vms []compute.VirtualMac agentPoolAvailabilitySets = &[]string{} for nx := range nodes { nodeName := (*nodes[nx]).Name - if isMasterNode(nodes[nx]) { + if isControlPlaneNode(nodes[nx]) { continue } asID, ok := vmNameToAvailabilitySetID[nodeName] @@ -916,7 +922,7 @@ func (as *availabilitySet) EnsureHostsInPool(service *v1.Service, nodes []*v1.No hostUpdates := make([]func() error, 0, len(nodes)) for _, node := range nodes { localNodeName := node.Name - if as.useStandardLoadBalancer() && as.excludeMasterNodesFromStandardLB() && isMasterNode(node) { + if as.useStandardLoadBalancer() && as.excludeMasterNodesFromStandardLB() && isControlPlaneNode(node) { klog.V(4).Infof("Excluding master node %q from load balancer backendpool %q", localNodeName, backendPoolID) continue } diff --git a/pkg/provider/azure_standard_test.go b/pkg/provider/azure_standard_test.go index 8a429de536..b0eff89d83 100644 --- a/pkg/provider/azure_standard_test.go +++ b/pkg/provider/azure_standard_test.go @@ -50,27 +50,45 @@ const ( primary = "primary" ) -func TestIsMasterNode(t *testing.T) { - if isMasterNode(&v1.Node{}) { +func TestIsControlPlaneNode(t *testing.T) { + if isControlPlaneNode(&v1.Node{}) { t.Errorf("Empty node should not be master!") } - if isMasterNode(&v1.Node{ + if isControlPlaneNode(&v1.Node{ ObjectMeta: meta.ObjectMeta{ Labels: map[string]string{ consts.NodeLabelRole: "worker", }, }, }) { - t.Errorf("Node labelled 'worker' should not be master!") + t.Errorf("Node labelled 'worker' should not be control plane!") } - if !isMasterNode(&v1.Node{ + if !isControlPlaneNode(&v1.Node{ ObjectMeta: meta.ObjectMeta{ Labels: map[string]string{ consts.NodeLabelRole: "master", }, }, }) { - t.Errorf("Node should be master!") + t.Errorf("Node with kubernetes.io/role: \"master\" label should be control plane!") + } + if !isControlPlaneNode(&v1.Node{ + ObjectMeta: meta.ObjectMeta{ + Labels: map[string]string{ + consts.MasterNodeRoleLabel: "", + }, + }, + }) { + t.Errorf("Node with node-role.kubernetes.io/master: \"\" label should be control plane!") + } + if !isControlPlaneNode(&v1.Node{ + ObjectMeta: meta.ObjectMeta{ + Labels: map[string]string{ + consts.ControlPlaneNodeRoleLabel: "", + }, + }, + }) { + t.Errorf("Node with node-role.kubernetes.io/control-plane: \"\" label should be control plane!") } } diff --git a/pkg/provider/azure_vmss.go b/pkg/provider/azure_vmss.go index 1d884f1e88..41f67f99c8 100644 --- a/pkg/provider/azure_vmss.go +++ b/pkg/provider/azure_vmss.go @@ -712,7 +712,7 @@ func (ss *ScaleSet) listScaleSetVMs(scaleSetName, resourceGroup string) ([]compu func (ss *ScaleSet) getAgentPoolScaleSets(nodes []*v1.Node) (*[]string, error) { agentPoolScaleSets := &[]string{} for nx := range nodes { - if isMasterNode(nodes[nx]) { + if isControlPlaneNode(nodes[nx]) { continue } @@ -1061,7 +1061,7 @@ func (ss *ScaleSet) ensureVMSSInPool(service *v1.Service, nodes []*v1.Node, back // multiple standard load balancers and the basic load balancer doesn't if ss.useStandardLoadBalancer() && !ss.EnableMultipleStandardLoadBalancers { for _, node := range nodes { - if ss.excludeMasterNodesFromStandardLB() && isMasterNode(node) { + if ss.excludeMasterNodesFromStandardLB() && isControlPlaneNode(node) { continue } @@ -1204,7 +1204,7 @@ func (ss *ScaleSet) EnsureHostsInPool(service *v1.Service, nodes []*v1.Node, bac for _, node := range nodes { localNodeName := node.Name - if ss.useStandardLoadBalancer() && ss.excludeMasterNodesFromStandardLB() && isMasterNode(node) { + if ss.useStandardLoadBalancer() && ss.excludeMasterNodesFromStandardLB() && isControlPlaneNode(node) { klog.V(4).Infof("Excluding master node %q from load balancer backendpool %q", localNodeName, backendPoolID) continue } @@ -1487,8 +1487,10 @@ func (ss *ScaleSet) EnsureBackendPoolDeleted(service *v1.Service, backendPoolID, nodeResourceGroup, nodeVMSS, nodeInstanceID, nodeVMSSVM, err := ss.ensureBackendPoolDeletedFromNode(nodeName, backendPoolID) if err != nil { - klog.Errorf("EnsureBackendPoolDeleted(%s): backendPoolID(%s) - failed with error %v", getServiceName(service), backendPoolID, err) - allErrs = append(allErrs, err) + if !errors.Is(err, ErrorNotVmssInstance) { // Do nothing for the VMAS nodes. + klog.Errorf("EnsureBackendPoolDeleted(%s): backendPoolID(%s) - failed with error %v", getServiceName(service), backendPoolID, err) + allErrs = append(allErrs, err) + } continue } diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 557f202f4a..cd85115e50 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -35,6 +35,7 @@ import ( const ( reportDirEnv = "CCM_JUNIT_REPORT_DIR" + artifactsDirEnv = "ARTIFACTS" defaultReportDir = "_report/" ) @@ -42,7 +43,12 @@ func TestAzureTest(t *testing.T) { RegisterFailHandler(Fail) reportDir := os.Getenv(reportDirEnv) if reportDir == "" { - reportDir = defaultReportDir + artifactsDir := os.Getenv(artifactsDirEnv) + if artifactsDir != "" { + reportDir = artifactsDir + } else { + reportDir = defaultReportDir + } } var r []Reporter if reportDir != "" { diff --git a/tests/e2e/network/ensureloadbalancer.go b/tests/e2e/network/ensureloadbalancer.go index 3301424286..88ae139049 100644 --- a/tests/e2e/network/ensureloadbalancer.go +++ b/tests/e2e/network/ensureloadbalancer.go @@ -151,13 +151,13 @@ var _ = Describe("Ensure LoadBalancer", func() { }() By("Waiting for exposure of internal service with specific IP") err = utils.WaitUpdateServiceExposure(cs, ns.Name, testServiceName, ip1, true) + Expect(err).NotTo(HaveOccurred()) list, errList := cs.CoreV1().Events(ns.Name).List(context.TODO(), metav1.ListOptions{}) Expect(errList).NotTo(HaveOccurred()) utils.Logf("Events list:") for i, event := range list.Items { utils.Logf("%d. %v", i, event) } - Expect(err).NotTo(HaveOccurred()) ip2, err := utils.SelectAvailablePrivateIP(tc) Expect(err).NotTo(HaveOccurred()) @@ -199,14 +199,14 @@ var _ = Describe("Ensure LoadBalancer", func() { By("Waiting for exposure of the original service without assigned lb private IP") ip1, err := utils.WaitServiceExposure(cs, ns.Name, testServiceName) + Expect(err).NotTo(HaveOccurred()) + Expect(ip1).NotTo(Equal(targetIP)) list, errList := cs.CoreV1().Events(ns.Name).List(context.TODO(), metav1.ListOptions{}) Expect(errList).NotTo(HaveOccurred()) utils.Logf("Events list:") for i, event := range list.Items { utils.Logf("%d. %v", i, event) } - Expect(err).NotTo(HaveOccurred()) - Expect(ip1).NotTo(Equal(targetIP)) By("Updating service to bound to specific public IP") utils.Logf("will update IP to %s, %v", targetIP, len(targetIP)) diff --git a/tests/e2e/network/network_security_group.go b/tests/e2e/network/network_security_group.go index 7d8b7c3726..e3d759741a 100644 --- a/tests/e2e/network/network_security_group.go +++ b/tests/e2e/network/network_security_group.go @@ -93,9 +93,9 @@ var _ = Describe("Network security group", func() { By("Validating ip exists in Security Group") port := fmt.Sprintf("%v", nginxPort) - nsg, err := tc.GetClusterSecurityGroup() + nsgs, err := tc.GetClusterSecurityGroups() Expect(err).NotTo(HaveOccurred()) - Expect(validateUnsharedSecurityRuleExists(nsg, ip, port)).To(BeTrue(), "Security rule for service %s not exists", serviceName) + Expect(validateUnsharedSecurityRuleExists(nsgs, ip, port)).To(BeTrue(), "Security rule for service %s not exists", serviceName) By("Validating network security group working") var code int @@ -124,9 +124,9 @@ var _ = Describe("Network security group", func() { Expect(utils.DeleteService(cs, ns.Name, serviceName)).NotTo(HaveOccurred()) isDeleted := false for i := 1; i <= 30; i++ { - nsg, err := tc.GetClusterSecurityGroup() + nsgs, err := tc.GetClusterSecurityGroups() Expect(err).NotTo(HaveOccurred()) - if !validateUnsharedSecurityRuleExists(nsg, ip, port) { + if !validateUnsharedSecurityRuleExists(nsgs, ip, port) { utils.Logf("Target rule successfully deleted") isDeleted = true break @@ -144,15 +144,22 @@ var _ = Describe("Network security group", func() { _ = createAndExposeDefaultServiceWithAnnotation(cs, serviceName, ns.Name, labels, annotation, ports) By("Validating if the corresponding IP prefix existing in nsg") - nsg, err := tc.GetClusterSecurityGroup() + nsgs, err := tc.GetClusterSecurityGroups() Expect(err).NotTo(HaveOccurred()) - rules := nsg.SecurityRules - Expect(len(*rules)).NotTo(Equal(0)) var found bool - for _, rule := range *rules { - if rule.SourceAddressPrefix != nil && strings.Contains(*rule.SourceAddressPrefix, "AzureCloud") { - found = true + for _, nsg := range nsgs { + rules := nsg.SecurityRules + if rules == nil { + continue + } + for _, rule := range *rules { + if rule.SourceAddressPrefix != nil && strings.Contains(*rule.SourceAddressPrefix, "AzureCloud") { + found = true + break + } + } + if found { break } } @@ -174,9 +181,9 @@ var _ = Describe("Network security group", func() { Expect(err).NotTo(HaveOccurred()) By("Checking if there is a deny_all rule") - nsg, err := tc.GetClusterSecurityGroup() + nsgs, err := tc.GetClusterSecurityGroups() Expect(err).NotTo(HaveOccurred()) - found := validateDenyAllSecurityRuleExists(nsg, internalIP) + found := validateDenyAllSecurityRuleExists(nsgs, internalIP) Expect(found).ToNot(BeTrue()) By("Deleting the service") @@ -193,79 +200,87 @@ var _ = Describe("Network security group", func() { Expect(err).NotTo(HaveOccurred()) By("Checking if there is a LoadBalancerSourceRanges rule") - nsg, err = tc.GetClusterSecurityGroup() + nsgs, err = tc.GetClusterSecurityGroups() Expect(err).NotTo(HaveOccurred()) - found = validateLoadBalancerSourceRangesRuleExists(nsg, internalIP, "1.2.3.4/32", "1.2.3.4_32") + found = validateLoadBalancerSourceRangesRuleExists(nsgs, internalIP, "1.2.3.4/32", "1.2.3.4_32") Expect(found).To(BeTrue()) By("Checking if there is a deny_all rule") - found = validateDenyAllSecurityRuleExists(nsg, internalIP) + found = validateDenyAllSecurityRuleExists(nsgs, internalIP) Expect(found).To(BeTrue()) }) }) -func validateUnsharedSecurityRuleExists(nsg *aznetwork.SecurityGroup, ip string, port string) bool { - if nsg == nil || nsg.SecurityRules == nil { - return false - } - for _, securityRule := range *nsg.SecurityRules { - if strings.EqualFold(to.String(securityRule.DestinationAddressPrefix), ip) && strings.EqualFold(to.String(securityRule.DestinationPortRange), port) { - utils.Logf("Find target security rule") - return true +func validateUnsharedSecurityRuleExists(nsgs []aznetwork.SecurityGroup, ip string, port string) bool { + for _, nsg := range nsgs { + if nsg.SecurityRules == nil { + continue + } + for _, securityRule := range *nsg.SecurityRules { + if strings.EqualFold(to.String(securityRule.DestinationAddressPrefix), ip) && strings.EqualFold(to.String(securityRule.DestinationPortRange), port) { + utils.Logf("Found target security rule") + return true + } } } return false } -func validateSharedSecurityRuleExists(nsg *aznetwork.SecurityGroup, ips []string, port string) bool { - if nsg == nil || nsg.SecurityRules == nil { - return false - } - for _, securityRule := range *nsg.SecurityRules { - if strings.EqualFold(to.String(securityRule.DestinationPortRange), port) { - isFind := true - for _, ip := range ips { - if !utils.StringInSlice(ip, *securityRule.DestinationAddressPrefixes) { - utils.Logf("Find target security rule") - isFind = false - break +func validateSharedSecurityRuleExists(nsgs []aznetwork.SecurityGroup, ips []string, port string) bool { + for _, nsg := range nsgs { + if nsg.SecurityRules == nil { + continue + } + for _, securityRule := range *nsg.SecurityRules { + if strings.EqualFold(to.String(securityRule.DestinationPortRange), port) { + found := true + for _, ip := range ips { + if !utils.StringInSlice(ip, *securityRule.DestinationAddressPrefixes) { + found = false + break + } + } + if found { + utils.Logf("Found target security rule") + return true } - } - if isFind { - return true } } } return false } -func validateLoadBalancerSourceRangesRuleExists(nsg *aznetwork.SecurityGroup, ip, sourceAddressPrefix, ipRangesSuffix string) bool { - if nsg == nil || nsg.SecurityRules == nil { - return false - } - for _, securityRule := range *nsg.SecurityRules { - if securityRule.Access == aznetwork.SecurityRuleAccessAllow && - strings.EqualFold(to.String(securityRule.DestinationAddressPrefix), ip) && - strings.HasSuffix(to.String(securityRule.Name), ipRangesSuffix) && - strings.EqualFold(to.String(securityRule.SourceAddressPrefix), sourceAddressPrefix) { - return true +func validateLoadBalancerSourceRangesRuleExists(nsgs []aznetwork.SecurityGroup, ip, sourceAddressPrefix, ipRangesSuffix string) bool { + for _, nsg := range nsgs { + if nsg.SecurityRules == nil { + continue + } + for _, securityRule := range *nsg.SecurityRules { + if securityRule.Access == aznetwork.SecurityRuleAccessAllow && + strings.EqualFold(to.String(securityRule.DestinationAddressPrefix), ip) && + strings.HasSuffix(to.String(securityRule.Name), ipRangesSuffix) && + strings.EqualFold(to.String(securityRule.SourceAddressPrefix), sourceAddressPrefix) { + return true + } } } return false } -func validateDenyAllSecurityRuleExists(nsg *aznetwork.SecurityGroup, ip string) bool { - if nsg == nil || nsg.SecurityRules == nil { - return false - } - for _, securityRule := range *nsg.SecurityRules { - if securityRule.Access == aznetwork.SecurityRuleAccessDeny && - strings.EqualFold(to.String(securityRule.DestinationAddressPrefix), ip) && - strings.HasSuffix(to.String(securityRule.Name), "deny_all") && - strings.EqualFold(to.String(securityRule.SourceAddressPrefix), "*") { - return true +func validateDenyAllSecurityRuleExists(nsgs []aznetwork.SecurityGroup, ip string) bool { + for _, nsg := range nsgs { + if nsg.SecurityRules == nil { + continue + } + for _, securityRule := range *nsg.SecurityRules { + if securityRule.Access == aznetwork.SecurityRuleAccessDeny && + strings.EqualFold(to.String(securityRule.DestinationAddressPrefix), ip) && + strings.HasSuffix(to.String(securityRule.Name), "deny_all") && + strings.EqualFold(to.String(securityRule.SourceAddressPrefix), "*") { + return true + } } } diff --git a/tests/e2e/network/service_annotations.go b/tests/e2e/network/service_annotations.go index 0e857291f5..6089ba6270 100644 --- a/tests/e2e/network/service_annotations.go +++ b/tests/e2e/network/service_annotations.go @@ -316,11 +316,11 @@ var _ = Describe("Service with annotation", func() { By("Validate shared security rule exists") port := fmt.Sprintf("%v", nginxPort) - nsg, err := tc.GetClusterSecurityGroup() + nsgs, err := tc.GetClusterSecurityGroups() Expect(err).NotTo(HaveOccurred()) ipList := []string{ip1, ip2} - Expect(validateSharedSecurityRuleExists(nsg, ipList, port)).To(BeTrue(), "Security rule for service %s not exists", serviceName) + Expect(validateSharedSecurityRuleExists(nsgs, ipList, port)).To(BeTrue(), "Security rule for service %s not exists", serviceName) }) It("should support service annotation `service.beta.kubernetes.io/azure-pip-tags`", func() { @@ -476,7 +476,7 @@ var _ = Describe("[[Multi-Nodepool]][VMSS]", func() { vmssNames := sets.NewString() var resourceGroupName string for i, node := range nodes { - if utils.IsMasterNode(&nodes[i]) { + if utils.IsControlPlaneNode(&nodes[i]) { continue } providerID := node.Spec.ProviderID diff --git a/tests/e2e/network/standard_lb.go b/tests/e2e/network/standard_lb.go index f3ec7cda7a..9b5da620a1 100644 --- a/tests/e2e/network/standard_lb.go +++ b/tests/e2e/network/standard_lb.go @@ -108,7 +108,7 @@ var _ = Describe("[StandardLoadBalancer] Standard load balancer", func() { var found bool for _, nic := range *NICList { found = false - if strings.Split(*nic.Name, "-")[1] == "master" { + if strings.Split(*nic.Name, "-")[1] == "master" || strings.Contains(*nic.Name, "control-plane") { continue } for _, ipConfig := range *nic.IPConfigurations { diff --git a/tests/e2e/utils/network_utils.go b/tests/e2e/utils/network_utils.go index 93c2d649ca..d4f4824772 100644 --- a/tests/e2e/utils/network_utils.go +++ b/tests/e2e/utils/network_utils.go @@ -32,9 +32,7 @@ import ( clientset "k8s.io/client-go/kubernetes" ) -const k8sVNetPrefix = "k8s-vnet-" - -// getVirtualNetworkList is a wapper around listing VirtualNetwork +// getVirtualNetworkList returns the list of virtual networks in the cluster resource group. func (azureTestClient *AzureTestClient) getVirtualNetworkList() (result aznetwork.VirtualNetworkListResultPage, err error) { Logf("Getting virtual network list") vNetClient := azureTestClient.createVirtualNetworksClient() @@ -51,38 +49,27 @@ func (azureTestClient *AzureTestClient) getVirtualNetworkList() (result aznetwor return } -// GetClusterVirtualNetwork gets the only vnet of the cluster +// GetClusterVirtualNetwork returns the cluster's virtual network. func (azureTestClient *AzureTestClient) GetClusterVirtualNetwork() (virtualNetwork aznetwork.VirtualNetwork, err error) { vNetList, err := azureTestClient.getVirtualNetworkList() if err != nil { return } - k8sVNetCount := 0 - returnIndex := 0 - for i, vNet := range vNetList.Values() { - if strings.HasPrefix(to.String(vNet.Name), k8sVNetPrefix) { - Logf("found one k8s virtual network %s", to.String(vNet.Name)) - k8sVNetCount++ - returnIndex = i - } else { - Logf("found other virtual network %s, skip", to.String(vNet.Name)) - } - } - switch k8sVNetCount { + switch len(vNetList.Values()) { case 0: - err = fmt.Errorf("Found no virtual network of the corresponding cluster in resource group") + err = fmt.Errorf("found no virtual network in resource group %s", azureTestClient.GetResourceGroup()) return case 1: - virtualNetwork = vNetList.Values()[returnIndex] + virtualNetwork = vNetList.Values()[0] return default: - err = fmt.Errorf("Found more than one virtual network of the corresponding cluster in resource group") + err = fmt.Errorf("found more than one virtual network in resource group %s", azureTestClient.GetResourceGroup()) return } } -// CreateSubnet will create a new subnet in certain virtual network +// CreateSubnet creates a new subnet in the specified virtual network. func (azureTestClient *AzureTestClient) CreateSubnet(vnet aznetwork.VirtualNetwork, subnetName *string, prefix *string) error { Logf("creating a new subnet %s, %s", *subnetName, *prefix) subnetParameter := (*vnet.Subnets)[0] @@ -93,7 +80,7 @@ func (azureTestClient *AzureTestClient) CreateSubnet(vnet aznetwork.VirtualNetwo return err } -// DeleteSubnet delete a subnet with retry +// DeleteSubnet deletes a subnet with retry. func (azureTestClient *AzureTestClient) DeleteSubnet(vnetName string, subnetName string) error { subnetClient := azureTestClient.createSubnetsClient() return wait.PollImmediate(poll, singleCallTimeout, func() (bool, error) { @@ -115,14 +102,14 @@ func (azureTestClient *AzureTestClient) DeleteSubnet(vnetName string, subnetName Logf("subnet %s has been deleted", subnetName) return true, nil } - Logf("encountered unexpected error %v during deleting subnet %s", err, subnetName) + Logf("encountered unexpected error %v while deleting subnet %s", err, subnetName) return true, nil }) } -// GetNextSubnetCIDR obtains a new ip address which has no overlapping with other subnet +// GetNextSubnetCIDR obtains a new ip address which has no overlap with existing subnets. func GetNextSubnetCIDR(vnet aznetwork.VirtualNetwork) (string, error) { - if len((*vnet.AddressSpace.AddressPrefixes)) == 0 { + if len(*vnet.AddressSpace.AddressPrefixes) == 0 { return "", fmt.Errorf("vNet has no prefix") } vnetCIDR := (*vnet.AddressSpace.AddressPrefixes)[0] @@ -134,7 +121,7 @@ func GetNextSubnetCIDR(vnet aznetwork.VirtualNetwork) (string, error) { return getNextSubnet(vnetCIDR, existSubnets) } -// getSecurityGroupList is a wrapper around listing VirtualNetwork +// getSecurityGroupList returns the list of security groups in the cluster resource group. func (azureTestClient *AzureTestClient) getSecurityGroupList() (result aznetwork.SecurityGroupListResultPage, err error) { Logf("Getting virtual network list") securityGroupsClient := azureTestClient.CreateSecurityGroupsClient() @@ -152,20 +139,20 @@ func (azureTestClient *AzureTestClient) getSecurityGroupList() (result aznetwork return } -// GetClusterSecurityGroup gets the only vnet of the cluster -func (azureTestClient *AzureTestClient) GetClusterSecurityGroup() (ret *aznetwork.SecurityGroup, err error) { +// GetClusterSecurityGroups gets the security groups of the cluster. +func (azureTestClient *AzureTestClient) GetClusterSecurityGroups() (ret []aznetwork.SecurityGroup, err error) { securityGroupsList, err := azureTestClient.getSecurityGroupList() if err != nil { return } Logf("got sg list, length = %d", len(securityGroupsList.Values())) - // Assume there is only one cluster in one resource group - if len(securityGroupsList.Values()) != 1 { - err = fmt.Errorf("Found no or more than 1 virtual network in resource group same as cluster name") + if len(securityGroupsList.Values()) != 0 { + ret = securityGroupsList.Values() return } - ret = &securityGroupsList.Values()[0] + + err = fmt.Errorf("could not find the cluster security group in resource group %s", azureTestClient.GetResourceGroup()) return } @@ -174,6 +161,7 @@ func CreateLoadBalancerServiceManifest(name string, annotation map[string]string return &v1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: name, + Namespace: namespace, Annotations: annotation, }, Spec: v1.ServiceSpec{ @@ -251,6 +239,15 @@ func SelectAvailablePrivateIP(tc *AzureTestClient) (string, error) { return "", fmt.Errorf("failed to find a subnet in vNet %s", to.String(vNet.Name)) } subnet := to.String((*vNet.Subnets)[0].AddressPrefix) + if len(*vNet.Subnets) > 1 { + for _, sn := range *vNet.Subnets { + // if there is more than one subnet, select the first one we find. + if !strings.Contains(*sn.Name, "controlplane") { + subnet = *sn.AddressPrefix + break + } + } + } ip, _, err := net.ParseCIDR(subnet) if err != nil { return "", fmt.Errorf("failed to parse subnet CIDR in vNet %s: %w", to.String(vNet.Name), err) diff --git a/tests/e2e/utils/node_utils.go b/tests/e2e/utils/node_utils.go index a2795463e3..3a91c9e4a1 100644 --- a/tests/e2e/utils/node_utils.go +++ b/tests/e2e/utils/node_utils.go @@ -32,9 +32,11 @@ import ( ) const ( - nodeLabelRole = "kubernetes.io/role" - typeLabel = "type" - agentpoolLabelKey = "agentpool" + masterNodeRoleLabel = "node-role.kubernetes.io/master" + controlPlaneNodeRoleLabel = "node-role.kubernetes.io/control-plane" + nodeLabelRole = "kubernetes.io/role" + typeLabel = "type" + agentpoolLabelKey = "agentpool" // GPUResourceKey is the key of the GPU in the resource map of a node GPUResourceKey = "nvidia.com/gpu" @@ -68,7 +70,7 @@ func GetAgentNodes(cs clientset.Interface) ([]v1.Node, error) { } ret := make([]v1.Node, 0) for i, node := range nodesList.Items { - if !isMasterNode(&nodesList.Items[i]) && !isVirtualKubeletNode(&nodesList.Items[i]) { + if !IsControlPlaneNode(&nodesList.Items[i]) && !isVirtualKubeletNode(&nodesList.Items[i]) { ret = append(ret, node) } } @@ -120,7 +122,7 @@ func GetMaster(cs clientset.Interface) (*v1.Node, error) { return nil, err } for i, node := range nodesList.Items { - if isMasterNode(&nodesList.Items[i]) { + if IsControlPlaneNode(&nodesList.Items[i]) { return &node, nil } } @@ -235,10 +237,17 @@ func WaitAutoScaleNodes(cs clientset.Interface, targetNodeCount int, isScaleDown return err } -// isMasterNode returns true if the node has a master role label. -// The master role is determined by looking for: -// * a kubernetes.io/role="master" label -func isMasterNode(node *v1.Node) bool { +// IsControlPlaneNode returns true if the node has a control-plane role label. +// The control-plane role is determined by looking for: +// * a node-role.kubernetes.io/control-plane or node-role.kubernetes.io/master="" label +func IsControlPlaneNode(node *v1.Node) bool { + if _, ok := node.Labels[controlPlaneNodeRoleLabel]; ok { + return true + } + // include master role labels for k8s < 1.19 + if _, ok := node.Labels[masterNodeRoleLabel]; ok { + return true + } if val, ok := node.Labels[nodeLabelRole]; ok && val == "master" { return true } diff --git a/tests/e2e/utils/service_utils.go b/tests/e2e/utils/service_utils.go index d11afc6e2a..01d96191c9 100644 --- a/tests/e2e/utils/service_utils.go +++ b/tests/e2e/utils/service_utils.go @@ -148,5 +148,8 @@ func extractSuffix() string { prefix := ExtractDNSPrefix() url := c.Clusters[prefix].Server suffix := url[strings.Index(url, "."):] + if strings.Contains(suffix, ":") { + suffix = suffix[:strings.Index(suffix, ":")] + } return suffix } diff --git a/tests/e2e/utils/utils.go b/tests/e2e/utils/utils.go index 32bd8b660f..7eccd732b0 100644 --- a/tests/e2e/utils/utils.go +++ b/tests/e2e/utils/utils.go @@ -21,6 +21,7 @@ import ( "fmt" "os" "reflect" + "strings" "time" v1 "k8s.io/api/core/v1" @@ -170,7 +171,8 @@ func IsRetryableAPIError(err error) bool { // ExtractDNSPrefix obtains the cluster DNS prefix func ExtractDNSPrefix() string { c := obtainConfig() - return c.CurrentContext + parts := strings.Split(c.CurrentContext, "@") + return parts[len(parts)-1] } // Load config from file @@ -190,17 +192,6 @@ func StringInSlice(s string, list []string) bool { return false } -// IsMasterNode returns true if the node has a master role label. -// The master role is determined by looking for: -// * a kubernetes.io/role="master" label -func IsMasterNode(node *v1.Node) bool { - if val, ok := node.Labels[nodeLabelRole]; ok && val == "master" { - return true - } - - return false -} - // HandleVMNotFoundErr returns true if the input error is errVMNotFound or nil func HandleVMNotFoundErr(err error) bool { return err == nil || reflect.DeepEqual(err, errVMNotFound)