ingress: update nodepool ingress proposal #645
Conversation
|
@zzguang: GitHub didn't allow me to assign the following users: your_reviewer. Note that only openyurtio members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/assign @Fei-Guo @rambohe-ch @kadisi |
| By evaluating all the alternatives above, we think Yurthub naturally can take the responsibility to filter | ||
| the data between Cloud and Edge, so we prefer to Solution 5 currently, thanks @wenjun93 for his great | ||
| suggestions about this proposal, any other suggestions will also be appreciated, if no different opinions, | ||
| we will follow Solution 5 to implement the feature. |
| will be divided into 2 parts: one part is deployed on the Cloud side focusing on admission webhook, the other part | ||
| is deployed to NodePool focusing on service proxy and load balancer. These 2 parts can work collaboratively without | ||
| any conflict by setting the different startup parameters. | ||
| - A new NodePool binding namespace will be created when a NodePool ingress is enabled, then all the nginx ingress |
There was a problem hiding this comment.
A new namespace per NodePool? a lot of new namespaces maybe created.
There was a problem hiding this comment.
Yes, nginx ingress controller will deploy several components (RBAC/deployment/service/configmap/job/...) in the cluster, if all the related components of different NodePools are deployed in one namespace(e.g. kube-system):
1). We need to define the naming rule to differentiate them
2). Even we can differentiate them by different names, it's not very clear to users
So I suggest to create a namespace per pool for all the related components, besides, I think maybe it can be reused by other features(e.g. CoreDNS) in future.
There was a problem hiding this comment.
@zzguang ok, i got it. we may also discuss this point in the next community meeting.
| } | ||
| ``` | ||
| Some other details about the design: | ||
| - The `YurtIngress` CR should be singleton CR which means there should be only one instance in the cluster, |
There was a problem hiding this comment.
Only one CR can be created for the entire cluster, losing the meaning of CRD。
There was a problem hiding this comment.
If we want to operate mutli nodepools ingress simultaneously, it seems we should ensure the CR is singleton to avoid conflicts from different CRs for a same NodePool ingress. Any other suggestions will also be appreciated.
|
建议还是把 ingress-controller 的镜像爆漏出来吧，毕竟我们更喜欢用 traefik。 而担心用户配置的版本和 k8s 不匹配所以不爆漏参数，这个理由不够充分（想要自定义 ingress-controller 的镜像的人本身应该对k8s 就很熟悉了）。理想还是暴漏参数并给予默认值 @zzguang |
|
|
The current YurtIngress solution is based on Nginx ingress controller and doesn't support traefik yet. We think the users care more about the ingress feature itself instead of the ingress controller, so the YurtIngress focus on providing ingress capability for NodePools, and only the basic APIs are exposed at current stage, we can extend other APIs in future according to users requirements. |
zzguang
force-pushed
the
dev
branch
3 times, most recently
from
d70903d
to
b37f332
Compare
Signed-off-by: zhenggu1 <[email protected]>
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rambohe-ch, zzguang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: zhenggu1 <[email protected]>
Signed-off-by: zhenggu1 <[email protected]>
Signed-off-by: zhenggu1 [email protected]
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?
other Note