Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

kubefed: remove FederatedIngress feature #1284

Closed
hectorj2f opened this issue Sep 28, 2020 · 14 comments
Closed

kubefed: remove FederatedIngress feature #1284

hectorj2f opened this issue Sep 28, 2020 · 14 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@hectorj2f
Copy link
Contributor

hectorj2f commented Sep 28, 2020

Kubefed v2 should be only focused on the federation of resources across kubefed clusters. In the Kubernetes ecosystem there are other third party tools (such as service meshes) that already provide support to the DNS based federated ingress.

As a consequence, kubefed won't create anymore IngressDNSRecord objects https://github.com/kubernetes-sigs/kubefed/blob/master/docs/ingressdns-with-externaldns.md).

Why is this needed:

Therefore, we need to remove any related work as part of kubefed to keep the source code clean and focused on satisfying the federation of resources.

/kind feature

@hectorj2f hectorj2f self-assigned this Sep 28, 2020
@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 28, 2020
@RainbowMango
Copy link
Contributor

In the Kubernetes ecosystem there are other third party tools that already provide support to the DNS based federated ingress.

I'm not familiar with this feature. Which third-party tools we are talking about here?

@RainbowMango
Copy link
Contributor

cc @shashidharatd @irfanurrehman

@irfanurrehman
Copy link
Contributor

@hectorj2f This feature is already behind a feature flag and if we don't want to support it in future, it can always remain so, that is in an alpha state. If we foresee that this can burden us as maintenance debt, I would suggest to remove it when we reach that point (for example failing/flaky/broken tests which none of us wants to fix). If there are other more pressing issues/problems this is causing us now, I am quite curious to know.

@irfanurrehman
Copy link
Contributor

I did miss saying that if it is enabled by default now, we can certainly change that to keep it disbled in the default deploy configuration.

@hectorj2f
Copy link
Contributor Author

One of the intentions of this issue was to call the attention of any potential user of this feature. Based on the current reactions, I cannot see anyone relying on this feature at this moment and finding this action as a blocker to use kubefed. With that said, I am happy to follow a specific procedure such as disabling it first, and then delete it at some point in the future.

@RainbowMango I don't think there is any user using kubefed to just manage DNS records of Kubernetes Ingress resources created in other clusters. For sure, I can provide more context on which components can satisfy this similar scenario. I'd personally search for DNS discovery services or gateway-connected services for a multi-clusters env. I realize we should provide a more detailed information that describes which steps to follow to achieve the same result using these tools.

@irfanurrehman It sounds good to me to disable now, as part of a short term solution, and then delete it in the future. The main reason is to keep the source code focused on the federation and management of resources instead of having extra features that aim to address some networking gaps. This could also be a blocker to add new features to kubefed while having to support these ones.
I am not sure how much people is relying on this feature. We didn't have any bug since long time about this feature. Do you know if there is currently people using it ?.
Regarding to your point about removing it when failing/flaky/broken tests are discovered, it is a bit tricky :) because there is not much test coverage nowadays. I am sure that limiting the scope of kubefed to what people seems more interesting would help maintaining and improving kubefed, rather than trying to solve multiple problems at once.

@shashidharatd
Copy link
Contributor

LGTM for disabling and eventually remove from the code. This was an experimental feature and i don't know anybody using it. I agree that its a burden to keep maintaining the feature in alpha state.
Also it is high time to revisit the features in alpha state (https://github.com/kubernetes-sigs/kubefed#features) and evaluate whether to promote them to beta or drop them. keeping the features in alpha for long time does not give confidence to user apart from just playing with it for learning purpose.

@swiftslee
Copy link
Contributor

One of the intentions of this issue was to call the attention of any potential user of this feature. Based on the current reactions, I cannot see anyone relying on this feature at this moment and finding this action as a blocker to use kubefed. With that said, I am happy to follow a specific procedure such as disabling it first, and then delete it at some point in the future.

@RainbowMango I don't think there is any user using kubefed to just manage DNS records of Kubernetes Ingress resources created in other clusters. For sure, I can provide more context on which components can satisfy this similar scenario. I'd personally search for DNS discovery services or gateway-connected services for a multi-clusters env. I realize we should provide a more detailed information that describes which steps to follow to achieve the same result using these tools.

@irfanurrehman It sounds good to me to disable now, as part of a short term solution, and then delete it in the future. The main reason is to keep the source code focused on the federation and management of resources instead of having extra features that aim to address some networking gaps. This could also be a blocker to add new features to kubefed while having to support these ones.
I am not sure how much people is relying on this feature. We didn't have any bug since long time about this feature. Do you know if there is currently people using it ?.
Regarding to your point about removing it when failing/flaky/broken tests are discovered, it is a bit tricky :) because there is not much test coverage nowadays. I am sure that limiting the scope of kubefed to what people seems more interesting would help maintaining and improving kubefed, rather than trying to solve multiple problems at once.

I‘m using this feature for muti-cluster dns discovery with external-dns and coredns.
Hope the community keep the feature.

@hectorj2f
Copy link
Contributor Author

@yuswift As a short term solution, we would disable it by default. But I'll share my thoughts during the next sig meeting about these features and their possible future.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 8, 2021
@hectorj2f
Copy link
Contributor Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 8, 2021
@hectorj2f hectorj2f moved this from To do to In progress in kubefed - beta Jan 29, 2021
@conquerorAlex
Copy link

with latest version, or v0.6.1, is this feature still available? In another word, user guide (https://github.com/kubernetes-sigs/kubefed/blob/master/docs/ingress-service-dns-with-coredns.md) is unavailable, is that right?
I have done the same work according the ingress-service-dns-with-coredns.md, but got empty spec in dnsendpoint and status. And dig failed in dnstools container.

@swiftslee
Copy link
Contributor

with latest version, or v0.6.1, is this feature still available? In another word, user guide (https://github.com/kubernetes-sigs/kubefed/blob/master/docs/ingress-service-dns-with-coredns.md) is unavailable, is that right?
I have done the same work according the ingress-service-dns-with-coredns.md, but got empty spec in dnsendpoint and status. And dig failed in dnstools container.

This feature has been disabled by default within v0.5.0 and will be removed in the future(maybe v0.6.2). If you want to try the feature, use v.0.4.1 or before.

@hectorj2f hectorj2f moved this from In progress to Done in kubefed - beta Mar 16, 2021
@hectorj2f
Copy link
Contributor Author

@conquerorAlex Yes, @yuswift is right about the plans for this feature.

Are you still experiencing the issue with v0.5.0? Did you open any issue about it ?

@swiftslee
Copy link
Contributor

Hi @hectorj2f I think we need to reopen this issue cause we can still create federatedingress in v0.7.0. Shall we create a validating webhook for federatedingress or mutatingwebhook for federatedtypeconfig which disables the propagation?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
Development

No branches or pull requests

8 participants