🔨 fix: update annotation lb name immediately after creating + annotat…

…ion target-node-labels not work

pkg/ingress/controller/controller.go

@@ -486,6 +486,12 @@ func (c *Controller) ensureIngress(oldIng, ing *nwv1.Ingress) error {
 		}
 	}
 
+	ingressKey := fmt.Sprintf("%s/%s", ing.Namespace, ing.Name)
+	ing, err := c.updateIngressAnnotation(ingressKey)
+	if err != nil {
+		return err
+	}
+
 	lb, err := c.ensureCompareIngress(oldIng, ing)
 	if err != nil {
 		c.isReApplyNextTime = true
@@ -499,9 +505,10 @@ func (c *Controller) ensureIngress(oldIng, ing *nwv1.Ingress) error {
 	return err
 }
 
-func (c *Controller) updateIngressStatus(ing *nwv1.Ingress, lb *lObjects.LoadBalancer) (*nwv1.Ingress, error) {
-	// get the latest version of ingress before update
-	ingressKey := fmt.Sprintf("%s/%s", ing.Namespace, ing.Name)
+// when create new ingress, you should update the load balancer name annotation immediately,
+// avoid the case user update this annotation before load balancer is created
+// then webhook will not allow to update this annotation (just allow when this annotation is nil)
+func (c *Controller) updateIngressAnnotation(ingressKey string) (*nwv1.Ingress, error) {
 	latestIngress, err := utils.GetIngress(c.ingressLister, ingressKey)
 	if err != nil {
 		logrus.Errorf("Failed to get the latest version of ingress %s", ingressKey)
@@ -510,8 +517,25 @@ func (c *Controller) updateIngressStatus(ing *nwv1.Ingress, lb *lObjects.LoadBal
 	if latestIngress.ObjectMeta.Annotations == nil {
 		latestIngress.ObjectMeta.Annotations = map[string]string{}
 	}
-	// latestIngress.ObjectMeta.Annotations[ServiceAnnotationLoadBalancerID] = lb.UUID
-	latestIngress.ObjectMeta.Annotations[ServiceAnnotationLoadBalancerName] = lb.Name
+	if _, ok := latestIngress.ObjectMeta.Annotations[ServiceAnnotationLoadBalancerName]; !ok {
+		latestIngress.ObjectMeta.Annotations[ServiceAnnotationLoadBalancerName] = utils.GenerateLBName(c.getClusterID(), latestIngress.Namespace, latestIngress.Name, consts.RESOURCE_TYPE_INGRESS)
+		newObj, err := c.kubeClient.NetworkingV1().Ingresses(latestIngress.Namespace).Update(context.TODO(), latestIngress, apimetav1.UpdateOptions{})
+		if err != nil {
+			return nil, err
+		}
+		return newObj, nil
+	}
+	return latestIngress, nil
+}
+
+func (c *Controller) updateIngressStatus(ing *nwv1.Ingress, lb *lObjects.LoadBalancer) (*nwv1.Ingress, error) {
+	// get the latest version of ingress before update
+	ingressKey := fmt.Sprintf("%s/%s", ing.Namespace, ing.Name)
+	latestIngress, err := utils.GetIngress(c.ingressLister, ingressKey)
+	if err != nil {
+		logrus.Errorf("Failed to get the latest version of ingress %s", ingressKey)
+		return nil, vErrors.ErrIngressNotFound
+	}
 
 	newIng := latestIngress.DeepCopy()
 	newState := new(nwv1.IngressLoadBalancerStatus)
@@ -703,6 +727,9 @@ func (c *Controller) DeleteLoadbalancer(ing *nwv1.Ingress) error {
 			// ensure default pool have the same member
 			dpool, err := vngcloudutil.FindPoolByName(c.vLBSC, c.getProjectID(), lbID, consts.DEFAULT_NAME_DEFAULT_POOL)
 			if err != nil {
+				if err == vErrors.ErrNotFound {
+					return true
+				}
 				klog.Errorln("error when find default pool", err)
 				return false
 			}
@@ -857,19 +884,19 @@ func (c *Controller) inspectIngress(ing *nwv1.Ingress) (*Expander, error) {
 		ingressInspect.LbOptions.Name = serviceConf.LoadBalancerName
 	}
 
-	nodeObjs, err := utils.ListNodeWithPredicate(c.nodeLister, serviceConf.TargetNodeLabels)
-	if len(nodeObjs) < 1 {
+	nodesAfterFilter, err := utils.ListNodeWithPredicate(c.nodeLister, serviceConf.TargetNodeLabels)
+	if len(nodesAfterFilter) < 1 {
 		klog.Errorf("No nodes found in the cluster")
 		return nil, vErrors.ErrNoNodeAvailable
 	}
 	if err != nil {
 		klog.Errorf("Failed to retrieve current set of nodes from node lister: %v", err)
 		return nil, err
 	}
-	membersAddr := utils.GetNodeMembersAddr(nodeObjs)
+	membersAddr := utils.GetNodeMembersAddr(nodesAfterFilter)
 
 	// get subnetID of this ingress
-	providerIDs := utils.GetListProviderID(nodeObjs)
+	providerIDs := utils.GetListProviderID(nodesAfterFilter)
 	if len(providerIDs) < 1 {
 		klog.Errorf("No nodes found in the cluster")
 		return nil, vErrors.ErrNoNodeAvailable

pkg/utils/utils.go

@@ -314,6 +314,23 @@ func ListNodeWithPredicate(nodeLister corelisters.NodeLister, nodeLabels map[str
 	return filtered, nil
 }
 
+// FilterByNodeLabel filters the given list of nodes by the given node labels.
+func FilterByNodeLabel(nodes []*apiv1.Node, nodeLabels map[string]string) []*apiv1.Node {
+	var filtered []*apiv1.Node
+	for _, node := range nodes {
+		if node == nil {
+			continue
+		}
+		if node.Labels == nil {
+			continue
+		}
+		if labels.Set(nodeLabels).AsSelector().Matches(labels.Set(node.Labels)) {
+			filtered = append(filtered, node)
+		}
+	}
+	return filtered
+}
+
 func getNodeConditionPredicate(node *apiv1.Node) bool {
 	// We add the master to the node list, but its unschedulable.  So we use this to filter
 	// the master.

pkg/vngcloud/vlb.go

@@ -21,6 +21,7 @@ import (
 	"github.com/vngcloud/vngcloud-go-sdk/vngcloud/services/loadbalancer/v2/loadbalancer"
 	"github.com/vngcloud/vngcloud-go-sdk/vngcloud/services/loadbalancer/v2/pool"
 	lCoreV1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/informers"
@@ -205,6 +206,10 @@ func (c *vLB) ensureLoadBalancer(
 	}()
 
 	serviceKey := fmt.Sprintf("%s/%s", pService.Namespace, pService.Name)
+	pService, err := c.updateServiceAnnotation(pService)
+	if err != nil {
+		return nil, err
+	}
 	oldIngExpander, _ := c.inspectService(nil, pNodes)
 	if oldService, ok := c.serviceCache[serviceKey]; ok {
 		oldIngExpander, _ = c.inspectService(oldService, pNodes)
@@ -244,13 +249,26 @@ func (c *vLB) ensureLoadBalancer(
 	return lbStatus, nil
 }
 
-func (c *vLB) createLoadBalancerStatus(pService *lCoreV1.Service, lb *lObjects.LoadBalancer) *lCoreV1.LoadBalancerStatus {
+// when create new ingress, you should update the load balancer name annotation immediately,
+// avoid the case user update this annotation before load balancer is created
+// then webhook will not allow to update this annotation (just allow when this annotation is nil)
+// func (c *vLB) updateServiceAnnotation(serviceKey string) (*nwv1.Ingress, error)
+func (c *vLB) updateServiceAnnotation(pService *lCoreV1.Service) (*lCoreV1.Service, error) {
 	if pService.ObjectMeta.Annotations == nil {
 		pService.ObjectMeta.Annotations = map[string]string{}
 	}
-	// pService.ObjectMeta.Annotations[ServiceAnnotationLoadBalancerID] = lb.UUID
-	pService.ObjectMeta.Annotations[ServiceAnnotationLoadBalancerName] = lb.Name
+	if _, ok := pService.ObjectMeta.Annotations[ServiceAnnotationLoadBalancerName]; !ok {
+		pService.ObjectMeta.Annotations[ServiceAnnotationLoadBalancerName] = utils.GenerateLBName(c.getClusterID(), pService.Namespace, pService.Name, consts.RESOURCE_TYPE_SERVICE)
+		newObj, err := c.kubeClient.CoreV1().Services(pService.Namespace).Update(context.Background(), pService, metav1.UpdateOptions{})
+		if err != nil {
+			return nil, err
+		}
+		return newObj, nil
+	}
+	return pService, nil
+}
 
+func (c *vLB) createLoadBalancerStatus(pService *lCoreV1.Service, lb *lObjects.LoadBalancer) *lCoreV1.LoadBalancerStatus {
 	status := &lCoreV1.LoadBalancerStatus{}
 	// Default to IP
 	status.Ingress = []lCoreV1.LoadBalancerIngress{{IP: lb.Address}}
@@ -490,7 +508,12 @@ func (c *vLB) inspectService(pService *lCoreV1.Service, pNodes []*lCoreV1.Node)
 		ingressInspect.LbOptions.Name = serviceConf.LoadBalancerName
 	}
 
-	membersAddr := utils.GetNodeMembersAddr(pNodes)
+	nodesAfterFilter := utils.FilterByNodeLabel(pNodes, serviceConf.TargetNodeLabels)
+	if len(nodesAfterFilter) < 1 {
+		klog.Errorf("No nodes found in the cluster")
+		return nil, vErrors.ErrNoNodeAvailable
+	}
+	membersAddr := utils.GetNodeMembersAddr(nodesAfterFilter)
 
 	// get subnetID of this ingress
 	providerIDs := utils.GetListProviderID(pNodes)