Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[yurthub]improve list/watch node pool resource in serviceTopology filter. #421

Closed
rambohe-ch opened this issue Aug 10, 2021 · 10 comments · Fixed by #454
Closed

[yurthub]improve list/watch node pool resource in serviceTopology filter. #421

rambohe-ch opened this issue Aug 10, 2021 · 10 comments · Fixed by #454
Assignees
Labels
kind/enhancement kind/enhancement kind/feature kind/feature kind/good-first-issue kind/good-first-issue

Comments

@rambohe-ch
Copy link
Member

What would you like to be added:
Improve list/watch node pool resource, only list/watch the node pool instance which the edge node belongs to.

Why is this needed:
In order to reduce the network traffic between cloud and edge.

others
/kind feature

@rambohe-ch rambohe-ch added kind/enhancement kind/enhancement kind/good-first-issue kind/good-first-issue kind/feature kind/feature labels Aug 10, 2021
@neo502721
Copy link
Member

I want to do some things of this, can you assign to me ?

@rambohe-ch
Copy link
Member Author

/assign @neo502721

@neo502721
Copy link
Member

hi@rambohe-ch, I am trying add TweakListOptions to the Openyurt SharedInformer. But the LabelSelector confulse me. Is the edge nodepool's lable is "default-edge-nodepool=true"?

@rambohe-ch
Copy link
Member Author

rambohe-ch commented Aug 15, 2021

hi@rambohe-ch, I am trying add TweakListOptions to the Openyurt SharedInformer. But the LabelSelector confulse me. Is the edge nodepool's lable is "default-edge-nodepool=true"?

@neo502721 you can add an new parameter for defining nodePool name in yurthub. if nodePool name is set by user, we can add the nodePool name with node labels "apps.openyurt.io/nodepool" in TweakListOptions. and if nodePool name is not set, we can keep the current SharedInformer for all nodePools.

node labels for nodePool, you can reference code here: https://github.com/openyurtio/openyurt/blob/master/pkg/yurthub/filter/servicetopology/handler.go#L167

@rambohe-ch
Copy link
Member Author

hi@rambohe-ch, I am trying add TweakListOptions to the Openyurt SharedInformer. But the LabelSelector confulse me. Is the edge nodepool's lable is "default-edge-nodepool=true"?

@neo502721 How about define the new parameter name of yurthub is --nodepool-name?

@neo502721
Copy link
Member

hi@rambohe-ch, I am trying add TweakListOptions to the Openyurt SharedInformer. But the LabelSelector confulse me. Is the edge nodepool's lable is "default-edge-nodepool=true"?

@neo502721 How about define the new parameter name of yurthub is --nodepool-name?

Do you means add a new parameter named nodepool-name to yurthub in the convert process?

@rambohe-ch
Copy link
Member Author

hi@rambohe-ch, I am trying add TweakListOptions to the Openyurt SharedInformer. But the LabelSelector confulse me. Is the edge nodepool's lable is "default-edge-nodepool=true"?

@neo502721 How about define the new parameter name of yurthub is --nodepool-name?

Do you means add a new parameter named nodepool-name to yurthub in the convert process?

Only add nodepool-name parameter for yurthub and optimize nodepool list/watch in serviceTopology in yurthub.
I think that we can keep the yurtctl without modification.

@rambohe-ch
Copy link
Member Author

hi@rambohe-ch, I am trying add TweakListOptions to the Openyurt SharedInformer. But the LabelSelector confulse me. Is the edge nodepool's lable is "default-edge-nodepool=true"?

@neo502721 LabelSelector can be set as --labelSelector=metadata.name={nodePool-name} and you can get nodePoolName from yurthub startup parameter.

@neo502721
Copy link
Member

Hi @rambohe-ch, I study the nodepool carefully. And found the nodepopl's type is defiene by the spec.type. Type can be Edge or Cloud defined by the yurtappmanager apis. The serviceTopology's nodepoolLister instantiation by the yurtinformers.SharedInformerFactory. So I want to add the TweakListOptions sepc.type=Edge to the yurtinformerFactory in yurthub cmd options comlete process.
That is to say, in the function createSharedInformers:
yurtSharedInformerFactory := yurtinformers.NewSharedInformerFactoryWithOptions(yurtClient, 24*time.Hour, yurtinformers.WithTweakListOptions(func(options *metav1.ListOptions) { options.FieldSelector = fields.OneTermEqualSelector("sepc.type", "Edge").String() }))
If this solution do not match the feature. I will try to find another way such as in the servicetopology filter's handler.

@rambohe-ch
Copy link
Member Author

rambohe-ch commented Aug 27, 2021

Hi @rambohe-ch, I study the nodepool carefully. And found the nodepopl's type is defiene by the spec.type. Type can be Edge or Cloud defined by the yurtappmanager apis. The serviceTopology's nodepoolLister instantiation by the yurtinformers.SharedInformerFactory. So I want to add the TweakListOptions sepc.type=Edge to the yurtinformerFactory in yurthub cmd options comlete process.
That is to say, in the function createSharedInformers:
yurtSharedInformerFactory := yurtinformers.NewSharedInformerFactoryWithOptions(yurtClient, 24*time.Hour, yurtinformers.WithTweakListOptions(func(options *metav1.ListOptions) { options.FieldSelector = fields.OneTermEqualSelector("sepc.type", "Edge").String() }))
If this solution do not match the feature. I will try to find another way such as in the servicetopology filter's handler.

@neo502721 Thank you for your feedback.
There are maybe a lot of edge type nodePool in OpenYurt cluster, so traffic reduction is not obvious.
how about add an new parameter --nodepool-name for yurthub, so the createSharedInformers can like as following:

yurtSharedInformerFactory := yurtinformers.NewSharedInformerFactoryWithOptions(yurtClient, 24*time.Hour, 
yurtinformers.WithTweakListOptions(func(options *metav1.ListOptions) { 
    options.LabelSelector = labels.OneTermEqualSelector("metadata.name", {--ndoepool-name}).String()
}))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement kind/enhancement kind/feature kind/feature kind/good-first-issue kind/good-first-issue
Projects
None yet
2 participants