search

kubernetes-sigs / cluster-api-provider-nested

Cluster API Provider for Nested Clusters
Apache License 2.0
298 stars 65 forks source link

❓ [VC] Why pod with nodeName is not supported for now? #337

Closed LuBingtan closed 1 year ago

LuBingtan commented 1 year ago

User Story

As a user, sometimes I would like to create a pod has nodeName set in spec. But it seems not supported for now. https://github.com/kubernetes-sigs/cluster-api-provider-nested/blob/99d6c2293dd0228efa32127a4af8856f1b837efd/virtualcluster/pkg/syncer/resources/pod/dws.go#L174-L182

I wonder what is the reason for doing this? What problems would arise if this is allowed?

Detailed Description

Anything else you would like to add:

/kind feature

LuBingtan commented 1 year ago

cc @Fei-Guo @wondywang @christopherhein

Fei-Guo commented 1 year ago

The reason is clearly mentioned in the comment. VC does not support DeamonSet out of the box. And DeamonSet Pod is the most common case where nodename is specified in the PodSpec.

LuBingtan commented 1 year ago

The reason is clearly mentioned in the comment. VC does not support DeamonSet out of the box. And DeamonSet Pod is the most common case where nodename is specified in the PodSpec.

Thanks for your reply!

I was wondering, is it reasonable to add a feature-gate to allow these pods?

In our scenario, there are such pods with nodeName set (not daemonset) before introducing VC, and we don't want to change them for now.

Fei-Guo commented 1 year ago

1) The behavior of letting vPod have pre-set nodename has not been tested. 2) I got the similar request before. Usually, the user will fork the project and add nodename support under their own risk. 3) By allowing this, I would suggest fully exposing the node objects in the super cluster to virtual clusters (of course, another hack of the code).

LuBingtan commented 1 year ago
  1. The behavior of letting vPod have pre-set nodename has not been tested.
  2. I got the similar request before. Usually, the user will fork the project and add nodename support under their own risk.
  3. By allowing this, I would suggest fully exposing the node objects in the super cluster to virtual clusters (of course, another hack of the code).

Thanks for the suggestion! I'll try it out /close

k8s-ci-robot commented 1 year ago

@LuBingtan: Closing this issue.

In response to [this](https://github.com/kubernetes-sigs/cluster-api-provider-nested/issues/337#issuecomment-1418536402): >> 1. The behavior of letting vPod have pre-set nodename has not been tested. >> 2. I got the similar request before. Usually, the user will fork the project and add nodename support under their own risk. >> 3. By allowing this, I would suggest fully exposing the node objects in the super cluster to virtual clusters (of course, another hack of the code). > >Thanks for the suggestion! I'll try it out >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.