-
Notifications
You must be signed in to change notification settings - Fork 127
Cluster controller first iteration #1688
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
base: feature/database-controllers
Are you sure you want to change the base?
Changes from all commits
4e0ed11
2132309
90ebcc0
800e3d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| /* | ||
| Copyright 2026. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package v4 | ||
|
|
||
| import ( | ||
| corev1 "k8s.io/api/core/v1" | ||
| "k8s.io/apimachinery/pkg/api/resource" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| ) | ||
|
|
||
| // ClusterSpec defines the desired state of Cluster. | ||
| type ClusterSpec struct { | ||
| // This field is IMMUTABLE after creation. | ||
| // +kubebuilder:validation:Required | ||
| // +kubebuilder:validation:MinLength=1 | ||
| // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="class is immutable" | ||
| Class string `json:"class"` | ||
|
|
||
| // Storage overrides the storage size from ClusterClass. | ||
| // Example: "5Gi" | ||
| // +optional | ||
| // +kubebuilder:validation:XValidation:rule="self == null || oldSelf == null || quantity(self).compareTo(quantity(oldSelf)) >= 0",message="storage size can only be increased" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I understand this rule correctly, self=null allows situation when user adds the attribute with value and then remove it. IN that case we would rely on class provided value which might be lower. IMO we should not allow such behaviour and once set attribute should not be removed
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But it also checks, if the quantity is not lower then set previously, so will check but should be fine.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I dont this is the way it works. You put || between rules so it means that either of this rule succeed the whole validation pass i.e:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have added separate validation methods to check values inherited from ClusterClass, as, IMO, using CEL I can check only against old/new values from Cluster, not ClusterClass
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will update the logic, you are correct.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And will update for PG version as well |
||
| Storage *resource.Quantity `json:"storage,omitempty"` | ||
|
|
||
| // Instances overrides the number of PostgreSQL instances from ClusterClass. | ||
| // +optional | ||
| // +kubebuilder:validation:Minimum=1 | ||
| // +kubebuilder:validation:Maximum=10 | ||
| Instances *int32 `json:"instances,omitempty"` | ||
|
|
||
| // PostgresVersion overrides the PostgreSQL version from ClusterClass. | ||
| // Example: "16" | ||
| // +optional | ||
| PostgresVersion *string `json:"postgresVersion,omitempty"` | ||
|
|
||
| // Resources overrides CPU/memory resources from ClusterClass. | ||
| // +optional | ||
| Resources *corev1.ResourceRequirements `json:"resources,omitempty"` | ||
|
|
||
| // PostgreSQL overrides PostgreSQL engine parameters from ClusterClass. | ||
| // Maps to postgresql.conf settings. | ||
| // +optional | ||
| PostgreSQLConfig map[string]string `json:"postgresqlConfig,omitempty"` | ||
|
|
||
| // PgHBA contains pg_hba.conf host-based authentication rules. | ||
| // Defines client authentication and connection security (cluster-wide). | ||
| // Example: ["hostssl all all 0.0.0.0/0 scram-sha-256"] | ||
| // +optional | ||
| PgHBA []string `json:"pgHBA,omitempty"` | ||
| } | ||
|
|
||
| // ClusterStatus defines the observed state of Cluster. | ||
| type ClusterStatus struct { | ||
| // Phase represents the current phase of the Cluster. | ||
| // Values: "Pending", "Provisioning", "Failed", "Ready", "Deleting" | ||
| // +optional | ||
| Phase string `json:"phase,omitempty"` | ||
|
|
||
| // Conditions represent the latest available observations of the Cluster's state. | ||
| // +optional | ||
| Conditions []metav1.Condition `json:"conditions,omitempty"` | ||
|
|
||
| // ProvisionerRef contains reference to the provisioner resource managing this cluster. | ||
| // Right now, only CNPG is supported. | ||
| // +optional | ||
| ProvisionerRef *corev1.ObjectReference `json:"provisionerRef,omitempty"` | ||
|
|
||
| // CNPG config status, for reference when getting cluster status. | ||
| // +optional | ||
| CNPGConfigStatus *CNPGConfig `json:"cnpgConfigStatus,omitempty"` | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We probably shouldnt return in our Cluster status Specific to CNPG directly - the aim of this abstraction is to allow lower level pg operator swap so the Status ( and input) should be generic
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, but we'd probably need to have some more details when describing our cluster, i.e, image version, storage size, etc, which are available in clusters.postgresql.cnpg.io, that's why I put it here (not working yet)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you have most of this info when you describe your Cluster CR as you should see merged params ( from class and cluster CR itself). Here we should keep the status not static config |
||
| } | ||
|
|
||
| // +kubebuilder:object:root=true | ||
| // +kubebuilder:subresource:status | ||
| // +kubebuilder:resource:scope=Namespaced | ||
| // +kubebuilder:printcolumn:name="Class",type=string,JSONPath=`.spec.class` | ||
| // +kubebuilder:printcolumn:name="Phase",type=string,JSONPath=`.status.phase` | ||
| // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` | ||
|
|
||
| // Cluster is the Schema for the clusters API. | ||
| type Cluster struct { | ||
| metav1.TypeMeta `json:",inline"` | ||
| metav1.ObjectMeta `json:"metadata,omitempty"` | ||
|
|
||
| Spec ClusterSpec `json:"spec,omitempty"` | ||
| Status ClusterStatus `json:"status,omitempty"` | ||
| } | ||
|
|
||
| // +kubebuilder:object:root=true | ||
|
|
||
| // ClusterList contains a list of Cluster. | ||
| type ClusterList struct { | ||
| metav1.TypeMeta `json:",inline"` | ||
| metav1.ListMeta `json:"metadata,omitempty"` | ||
| Items []Cluster `json:"items"` | ||
| } | ||
|
|
||
| func init() { | ||
| SchemeBuilder.Register(&Cluster{}, &ClusterList{}) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I initially named the CRD's ClusterClass, Cluster and Database but when I look at this right now I think we can do better. Maybe we can rename ClusterClass to DatabaseClusterClass, Cluster to DatabaseCluster and leave Database as is?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should discuss it, I have some ideas as well. |
||
| } | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We aren't comparing it to the value inherited from ClusterClass, is there any mechanism that would also prohibit the following scenerio? Would CMPG block this behaviour?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Storage is not downgradable, but good point, will check, did I understand you correctly, @limak9182 , that you are describing a situation when user created a default cluster, and then updated spec?