Skip to content

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