Skip to content

Make teamId in cluster name optional #2001

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

Merged
merged 4 commits into from
Aug 24, 2022
Merged

Make teamId in cluster name optional #2001

merged 4 commits into from
Aug 24, 2022

Conversation

FxKu
Copy link
Member

@FxKu FxKu commented Aug 16, 2022

Teams of cluster change. Although the name of the cluster is immutable, it would still be helpful to change teamId field without the need to create a PostgresTeam resource.

On start up the operator calls add_cluster for all Postgresql resource it finds. When the new enable_team_id_clustername_prefix is enabled and cluster name does not start with team ID, add_cluster will return an error message and the cluster is not added to the operator's internal cluster list.

The Postgresql resource itself still gets created via K8s API. We have no controller in between to block this. Because the resource is there we can patch it's PostgresClusterStatus field in status subresource to invalid. Each sync on the cluster will try to call add_cluster again because it does not exist in the operator#s internal list. When the option is turned off and operator is restarted, the cluster is synced like very other cluster.

This PR keeps teamId as an required field of the cluster manifest (because we want to keep cluster ownership from teams) and it deprecates the cluster's internal ClusterName field which used to store the cluster name without the teamId prefix. We did not use the ClusterName anywhere else.

@@ -53,8 +53,7 @@ Those parameters are grouped under the `metadata` top-level key.
These parameters are grouped directly under the `spec` key in the manifest.

* **teamId**
name of the team the cluster belongs to. Changing it after the cluster
creation is not supported. Required field.
name of the team the cluster belongs to. Required field.
Copy link
Member

Choose a reason for hiding this comment

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

Should inform about settings and implications. it may still be impossible.

Copy link
Member Author

Choose a reason for hiding this comment

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

it's now possible to change it later. When enable_team_id_clustername_prefix is enabled you will only get an error on sync that cluster name does not match the { TEAM-ID }-{... format with status being set to inavlid.

tmp2.Status = PostgresStatus{PostgresClusterStatus: ClusterStatusInvalid}
} else if err := validateCloneClusterDescription(tmp2.Spec.Clone); err != nil {

if err := validateCloneClusterDescription(tmp2.Spec.Clone); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

how is this related to clone? maybe better function name? or is this indeed about cloning?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, this is a validation of the clone section. Existed also before.

@FxKu
Copy link
Member Author

FxKu commented Aug 23, 2022

👍

1 similar comment
@idanovinda
Copy link
Member

👍

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