Fix e2e tests to work with CAPZ

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"

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
 		}

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!")
 	}
 }
 

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
 		}
 

tests/e2e/e2e_test.go

@@ -35,14 +35,20 @@ import (
 
 const (
 	reportDirEnv     = "CCM_JUNIT_REPORT_DIR"
+	artifactsDirEnv  = "ARTIFACTS"
 	defaultReportDir = "_report/"
 )
 
 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 != "" {

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))

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
+			}
 		}
 	}