Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clusterset delete policy #53

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ldpliu
Copy link
Contributor

@ldpliu ldpliu commented Apr 22, 2022

Signed-off-by: ldpliu [email protected]

@openshift-ci openshift-ci bot requested review from deads2k and mdelder April 22, 2022 07:38
@openshift-ci
Copy link

openshift-ci bot commented Apr 22, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ldpliu
To complete the pull request process, please assign qiujian16 after the PR has been reviewed.
You can assign the PR to them by writing /assign @qiujian16 in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ldpliu
Copy link
Contributor Author

ldpliu commented Apr 22, 2022

/assign @qiujian16 @elgnay

Copy link
Member

@qiujian16 qiujian16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might consider whether a clusterset owns a cluster, instead of a deletion policy

The proposed work enhances the managedClusterSet API to handle managedClusterSet Delete policy.

## Motivation
Currently, when we add a cluster to a clusterset, we will add a label `cluster.open-cluster-management.io/clusterset:<Clusterset Name>` to the cluster. But when the clusterset is deleted, this label will not be removed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please clarify why this is a problem

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this need more clarification on use cases.


## Proposal Detail
Actually, we always use clusterset as a group of clusters. So when deleting a clusterset, there are some options for the delete policy.
1. When clusterset deleted, delete the clusterset labels in managedClusters.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think this is an unnatural behavior to remove a thing not managed by it.


```
### Details
Notes: the `DeletePolicy` only works for the `ClusterSelector.SelectorType` is `LegacyClusterSetLabel`. It means the clusterset uses the label `cluster.open-cluster-management.io/clusterset:<Clusterset Name>` to select target clusters. So In the doc, we only cover the clusterset which `ClusterSelector.SelectorType` is `LegacyClusterSetLabel`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reason?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants