kubernetes-sigs / kubebuilder-declarative-pattern

A toolkit for building declarative operators with kubebuilder
Apache License 2.0
252 stars 84 forks source link

New declarative method `WithNamespace` #305

Closed yuwenma closed 1 year ago

yuwenma commented 1 year ago

Problems

In my case, my deployment manifests have both cluster scoped resources (e.g. CustomResourceDefinition, ClusterRoleBinding) and namespace scoped resources(e.g. Deployment, Service). I want to assign a namespace value to the namespace scoped resources. I also want the declarativeObject to be the ownerRef of both kinds, so that I can have the deployment manifests be easily cleaned up by just deleting the declarativeObject.

Available features

Current k-d-p provides 3 handy features:

My current solutions

Right now, I have two options but neither are handy enough:

Option 1:

Set the declarativeObject as namespace scoped, this will auto-assign the namespace to deployment manifests. It has two problems:

  1. I have to create the namespace object before applying the declarativeObject object and cannot rely on my k-d-p operator to guarantee the existence of the namespace.
  2. I have to manage the cluster scoped resources separately, because namespace scoped resource can not be the ownerRef if cluster resources.

Option 2:

The declarativeObject is cluster scoped. It can be the ownerRef of all manifests and I can add the namespace existence logic in my k-d-p operator. But I have to implement my own declarative methods to update the namespace.

Proposals

Add a new declarative method which does not rely on the declarativeObject but can assign given namespace to the deployment manifests (it's different than skipping the namespace as what WithPreserveNamespace does, but assigning default namespace). Even better, if the namespace object does not exist in the cluster, I want it to be created by request.

New declarative method

WithNamespace(namespace string, createIfNotExist bool) 

This method assigns namespace namespace to any namespace scoped deployment manifests, only if their current namespace is empty (basically the same behavior of current setNamespaces but using a dedicated namespace value). namespace is the dedicated namespace value, createIfNotExist guarantees the existence of the namespace object before applying any other resources.

Please share your thoughts!

I think the use case I described can be very common for users who need k-d-p. And I'm just proposing a simple solution that is good enough for my use case. I think the behavior of WithNamespace can also be extended to update all the namespace-scoped deployment manifests rather than merely the unassigned ones. But I don't have a strong opinion on this.

Besides updating the metadata.namespace, we can also extend the setNamespace to update spec/conversion/webhook/clientConfig/service/namespace in CustomResourceDefinition, spec/service/namespace in APIService, etc.

justinsb commented 1 year ago

I think this could be a great feature. I was imagining that normally when the "parent" object is cluster-scoped, that the namespace-scoped objects would be in well-known namespaces (i.e. you can't really have multiple instances). But when this isn't the case, I can see WithNamespace being useful.

I think WithNamespace could be a "normal" ObjectTransform, so I think you can write it in your controller and then we can promote it into the shared library if/when other people need it (or just to encourage them to use it because we think it's a good idea)?

Are there things you need in k-d-p or from k-d-p to enable you to write this (e.g. some utility functions)? If so I think that could be a great place to start, and then we can look at the end result and add that?

yuwenma commented 1 year ago

Thank you for the explanation! Yeah, my use case does not have the objects in well-known namespaces, and I see the "parent" object contains namespace logics (e.g. setting default, ownerRef filtering), so I think k-d-p may actually want this new method. Happy to share my WithNamespace version to k-d-p, and yes it is currently called as a ObjectTransform . The downside is that the ObjectTransform is always called after other builtin functions, so it gives less flexibility for certain cases.

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

k8s-ci-robot commented 1 year ago

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to [this](https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern/issues/305#issuecomment-1624469536): >The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. > >This bot triages issues according to the following rules: >- After 90d of inactivity, `lifecycle/stale` is applied >- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied >- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed > >You can: >- Reopen this issue with `/reopen` >- Mark this issue as fresh with `/remove-lifecycle rotten` >- Offer to help out with [Issue Triage][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close not-planned > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ 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.