karmada-io / karmada

Open, Multi-Cloud, Multi-Cluster Kubernetes Orchestration
https://karmada.io
Apache License 2.0
4.12k stars 806 forks source link

Add labels on the namespace created by karmada #2862

Closed Rains6 closed 9 months ago

Rains6 commented 1 year ago

What happened: When I add a cluster to karmada, karmada will create a namespace on my cluster. I hope there will be a label indicating that the namespace was created by karmada to avoid accidental deletion.

What you expected to happen: There will be a label indicating that the namespace was created by karmada to avoid accidental deletion.

How to reproduce it (as minimally and precisely as possible): Add a cluster to karmada

Anything else we need to know?: none

Environment:

image

Poor12 commented 1 year ago

I wonder if you want a label for ns which stored the secret of the cluster or all the ns that karmada created, such as karmada-es-member1, karmada-system. This question is a little similar to https://github.com/karmada-io/karmada/issues/1350.

RainbowMango commented 1 year ago

/remove-kind bug /kind feature

RainbowMango commented 1 year ago

I've tested it on my side:

# kubectl get ns karmada-cluster -o yaml 
apiVersion: v1
kind: Namespace
metadata:
  creationTimestamp: "2022-11-15T07:16:00Z"
  labels:
    kubernetes.io/metadata.name: karmada-cluster
  name: karmada-cluster
  resourceVersion: "966"
  uid: 95bf6df1-a5c3-481a-a0b8-71ff1b20d3dc
spec:
  finalizers:
  - kubernetes
status:
  phase: Active

I wonder who and where added this label kubernetes.io/metadata.name: karmada-cluster?

RainbowMango commented 1 year ago

I get it. The label is added by Kubernetes.

There will be a label indicating that the namespace was created by karmada

That makes sense, but if we don't use it for searching, we can introduce an annotation. Can you propose one?

Rains6 commented 1 year ago

I get it. The label is added by Kubernetes.

There will be a label indicating that the namespace was created by karmada

That makes sense, but if we don't use it for searching, we can introduce an annotation. Can you propose one?

For example, karmada.io/metadata.name: karmada-cluster ?

Zhuzhenghao commented 1 year ago

@RainbowMango Can I add karmada.io/metadata.name: karmada-cluster to namespace?

RainbowMango commented 1 year ago

Thanks for your contribution. I see the #2888. But for now, I don't think the name(karmada.io/metadata.name) is the final decision yet. I'll think about it after we cut the new release.

Zhuzhenghao commented 1 year ago

Okk. After cutting the new release,you can notify me of the confirmed name, I will change the name(karmada.io/metadata.name).

Zhuzhenghao commented 1 year ago

@RainbowMango hi,Big Brother, what is the final decision?

RainbowMango commented 1 year ago

I don't have enough time on this issue recently, you know, we are in the planning phase, so proposing features is my highest priority right now. I tagged this issue with milestone 1.5, so it won't be forgotten.

Zhuzhenghao commented 1 year ago

Thank you, I'll keep track of this issue and work on a solution for milestone 1.5.

RainbowMango commented 1 year ago

There will be a label indicating that the namespace was created by karmada to avoid accidental deletion.

@Rains6 I have a question for your use case, If you just want to indicate the namespace is created by Karmada, then the name karmada-cluster would be enough.

If you want to avoid accidental deletion, adding a label or annotation can't help.

So, could you please give more details about accidental deletion?

Rains6 commented 1 year ago

There will be a label indicating that the namespace was created by karmada to avoid accidental deletion.

@Rains6 I have a question for your use case, If you just want to indicate the namespace is created by Karmada, then the name karmada-cluster would be enough.

If you want to avoid accidental deletion, adding a label or annotation can't help.

So, could you please give more details about accidental deletion?

When deploying karmada, we do not use karmada-cluster as the namespace to be deployed. Adding a label or annotation enables the frontend to identify that the namespace is created by karmada, preventing users from deleting the namespace on the frontend page.

RainbowMango commented 1 year ago

I'm still feeling a little bit confused as you mentioned When I add a cluster to karmada, karmada will create a namespace on my cluster., seems this scenario is related to you wanting to add a label on the namespace created by karmadactl join:

    --cluster-namespace='karmada-cluster':
    Namespace in the control plane where member cluster secrets are stored.

It makes sense to me that you probably do not use the default namespace name and want a label on the customized namespace. For this case, we can add an app.kubernetes.io/managed-by: karmada label which will be introduced by #3262.

We can do it after #3262, does this makes sense to you? @Rains6

Rains6 commented 1 year ago

I'm still feeling a little bit confused as you mentioned When I add a cluster to karmada, karmada will create a namespace on my cluster., seems this scenario is related to you wanting to add a label on the namespace created by karmadactl join:

    --cluster-namespace='karmada-cluster':
  Namespace in the control plane where member cluster secrets are stored.

It makes sense to me that you probably do not use the default namespace name and want a label on the customized namespace. For this case, we can add an app.kubernetes.io/managed-by: karmada label which will be introduced by #3262.

We can do it after #3262, does this makes sense to you? @Rains6

Yes, I think it'll solve this question.

XiShanYongYe-Chang commented 11 months ago

Hi, everyone, as #3262 has been merged, I think we can continue to complete this issue.

We introduced the label karmada.io/managed: true to indicate the resource create by Karmada.

Hi @Zhuzhenghao, would you like to go ahead and finish it?

XiShanYongYe-Chang commented 9 months ago

/help

karmada-bot commented 9 months ago

@XiShanYongYe-Chang: This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to [this](https://github.com/karmada-io/karmada/issues/2862): >/help 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.
XiShanYongYe-Chang commented 9 months ago

Hi @zhy76, would you like to have a try with this issue?

zhy76 commented 9 months ago

Hi @zhy76, would you like to have a try with this issue?

Sure!

zhy76 commented 9 months ago

/assign

zhy76 commented 9 months ago

I see there is already a label karmada.io/managed:true added into namespace,see:https://github.com/karmada-io/karmada/blob/6ef427a98e4c669c6834f0e62a400876c4b7a1c3/pkg/controllers/namespace/namespace_sync_controller.go#L150 So is it still need to do this issue?

XiShanYongYe-Chang commented 9 months ago

Hi @zhy76 The namespace mentioned in the issue refers to the namespace created by Karmada after a cluster joins. This namespace is used to store related authentication credentials, etc., and is specified by the parameter --cluster-namespace.