[feature request]yurtappdaemon need a node pool type selector

[donychen1134]

What would you like to be added: Add selector to select node pool by its type for yurtappdaemon CRD.

Why is this needed: When the time developing pool-coordinator feature, we found that pool-coordinator should be deploy to the node pool which type is edge. But yurtappdaemon only supports to select node pool by label. If we add node selector to the podSpec of yurtappdaemon, we will see that the pool-coordinator in cloud type of node pool will always be the pending status.

others /kind feature

[rambohe-ch]

@kadisi Do you have any comments about this feature request?

[donychen1134]

Maybe we could add a new struct as the NodeSelector of yurtappdaemon.

Like this:

// NodePoolSelector includes nodepool type and a label selector of node pool.
type NodePoolSelector struct {
    Type     NodePoolType          `json:"type"`
    Selector *metav1.LabelSelector `json:"selector"`
}

// YurtAppDaemonSpec defines the desired state of YurtAppDaemon.
type YurtAppDaemonSpec struct {
    Selector             *metav1.LabelSelector `json:"selector"`
    WorkloadTemplate     WorkloadTemplate      `json:"workloadTemplate,omitempty"`
    NodePoolSelector     NodePoolSelector      `json:"nodepoolSelector"`
    RevisionHistoryLimit *int32                `json:"revisionHistoryLimit,omitempty"`
}

In yurtappdaemon controller, we can select node pool type by field selector. Rewrite function getNameToNodePools maybe useful.

func (r *ReconcileYurtAppDaemon) getNameToNodePools(instance *unitv1alpha1.YurtAppDaemon) (map[string]unitv1alpha1.NodePool, error) {
    klog.V(4).Infof("YurtAppDaemon [%s/%s] prepare to get associated nodepools",
        instance.Namespace, instance.Name)

    // get the label selector of node pool
    nodepoolLabelSelector, err := metav1.LabelSelectorAsSelector(instance.Spec.NodePoolSelector.Selector)
    if err != nil {
        return nil, err
    }

    // get the field selector of node pool
    var nodepoolFieldSelector fields.Selector
    switch instance.Spec.NodePoolSelector.Type {
    case unitv1alpha1.Edge, unitv1alpha1.Cloud:
        nodepoolFieldSelector = fields.OneTermEqualSelector("spec.nodepoolSelector.type", string(instance.Spec.NodePoolSelector.Type))
    case "":
        // if no type value presented, nodepoolFieldSelector will be nil. But won't return error.
    default:
        return nil, fmt.Errorf("illegal node pool type %s", instance.Spec.NodePoolSelector.Type)
    }

    nodepools := unitv1alpha1.NodePoolList{}
    if err := r.Client.List(context.TODO(), &nodepools, &client.ListOptions{
        LabelSelector: nodepoolLabelSelector,
        FieldSelector: nodepoolFieldSelector,
    }); err != nil {
        klog.Errorf("YurtAppDaemon [%s/%s] Fail to get NodePoolList", instance.GetNamespace(),
            instance.GetName())
        return nil, nil
    }

    indexs := make(map[string]unitv1alpha1.NodePool)
    for i, v := range nodepools.Items {
        indexs[v.GetName()] = v
        klog.V(4).Infof("YurtAppDaemon [%s/%s] get %d's associated nodepools %s",
            instance.Namespace, instance.Name, i, v.Name)

    }

    return indexs, nil
}

[kadisi]

@donychen1134 @rambohe-ch I don't recommend NodePoolSelector. Selector is already an atomic operation that determines which nodes to deploy to. Whether to deploy to an Edge nodepool or not needs to be designed at a higher level.

[donychen1134]

@rambohe-ch @kadisi Maybe we could add a mutating webhook to node pool. When node pool is created or updated, webhook should add a label to nodel pool resource whose value is node pool type. Then we could use selector to chose 'edge' type node pool.

[rambohe-ch]

@rambohe-ch @kadisi Maybe we could add a mutating webhook to node pool. When node pool is created or updated, webhook should add a label to nodel pool resource whose value is node pool type. Then we could use selector to chose 'edge' type node pool.

@donychen1134 It's a good idea to add Label for nodePool in webhook. agree +1

@kadisi please take a look.

[kadisi]

@rambohe-ch @kadisi Maybe we could add a mutating webhook to node pool. When node pool is created or updated, webhook should add a label to nodel pool resource whose value is node pool type. Then we could use selector to chose 'edge' type node pool.

@donychen1134 It's a good idea to add Label for nodePool in webhook. agree +1

@kadisi please take a look.

@donychen1134 @rambohe-ch good idea. but please confirm the label key.

[donychen1134]

@rambohe-ch @kadisi Maybe use openyurt.io/node-pool-type as node pool label?

[rambohe-ch]

@rambohe-ch @kadisi Maybe use openyurt.io/node-pool-type as node pool label?

@donychen1134 agree +1