chore: introduce the v1 version CRDs #7757
Conversation
leon-inf
requested review from
nayutah,
ldming,
heng4fun,
free6om,
wangyelei,
Y-Rookie and
weicao
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7757 +/- ##
==========================================
+ Coverage 63.30% 63.35% +0.05%
==========================================
Files 339 339
Lines 41438 41438
==========================================
+ Hits 26234 26255 +21
+ Misses 12870 12853 -17
+ Partials 2334 2330 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
apis/apps/v1/cluster_types.go
Outdated
| // Cluster by default. | ||
| // | ||
| // +optional | ||
| Namespace string `json:"namespace,omitempty"` |
There was a problem hiding this comment.
as discussed, references a Cluster in a different namespace is forbidden because of the security consideration (referencing a Secret in another ns). so may be we can delete this field
| // The name of the credential (SystemAccount) to reference. | ||
| // | ||
| // +kubebuilder:validation:Required | ||
| Name string `json:"name"` |
There was a problem hiding this comment.
seems named to serviceAccount is more consistent and self-explanatory
| // of the Component providing the service in the referenced Cluster. | ||
| // | ||
| // +optional | ||
| Credential *ServiceRefCredentialSelector `json:"credential,omitempty"` |
| // Extends the ServiceSpec.Selector by allowing the specification of a sharding name, which is defined in | ||
| // `cluster.spec.shardingSpecs[*].name`, to be used as a selector for the service. | ||
| // Note that this and the `componentSelector` are mutually exclusive and cannot be set simultaneously. | ||
| // | ||
| // +optional | ||
| ShardingSelector string `json:"shardingSelector,omitempty"` | ||
|
|
||
| // Extends the ServiceSpec.Selector by allowing the specification of a component, to be used as a selector for the service. | ||
| // Note that this and the `shardingSelector` are mutually exclusive and cannot be set simultaneously. | ||
| // | ||
| // +optional | ||
| ComponentSelector string `json:"componentSelector,omitempty"` |
There was a problem hiding this comment.
can these two fields get merged?
for example, there is only a single field componentSelector. if it equals to a name specified in cluster.spec.shardingSpecs[*].name, then it's a sharding selector.
There was a problem hiding this comment.
This way, if I don't use the sharding feature, I won't need to understand the shardingSelector field.
| // FailedClusterPhase represents all components are in `Failed` phase, indicates that the cluster is unavailable. | ||
| FailedClusterPhase ClusterPhase = "Failed" | ||
|
|
||
| // AbnormalClusterPhase represents some components are in `Failed` or `Abnormal` phase, indicates that the cluster | ||
| // is in a fragile state and troubleshooting is required. | ||
| AbnormalClusterPhase ClusterPhase = "Abnormal" |
There was a problem hiding this comment.
should mark one as deprecated
| // | ||
| // +kubebuilder:validation:Required | ||
| // +kubebuilder:validation:MaxLength=64 | ||
| CompDef string `json:"compDef"` |
There was a problem hiding this comment.
add field like:
// +optional
IsSharding bool `json:isSharding,omitempty`
default value is false
There was a problem hiding this comment.
MinShardCount int32 `json:minShardCount,omitempty`
MaxShardCount int32 `json:maxShardCount,omitempty`
There was a problem hiding this comment.
IsAccountSecretShared bool `json:isAccountSecretShared,omitempty`
| // The value will be presented in the following format: FQDN1,FQDN2,... | ||
| // | ||
| // +optional | ||
| PodFQDNs *VarOption `json:"podFQDNs,omitempty"` |
There was a problem hiding this comment.
Currently, except the load balancer IP, the provided addresses (e.g., pod FQDN, service host) are mostly DNS domain names or parts of them.
There should be a way to provide a list of node hostnames, pod IPs, or service IPs that disclose the components' pods reside on. This way, in scenarios like hostNetwork or when there is no service (e.g., Kuaishou), IP addresses can be constructed accordingly.
Node or pod IPs cannot be obtained during the provisioning stage, so dynamic modification and notification for the resolution of vars values need to be supported first.
| // | ||
| // +kubebuilder:default=false | ||
| // +optional | ||
| Initialization bool `json:"initialization,omitempty"` |
There was a problem hiding this comment.
better renamed to isSystemInitializationAccount
| // Specifies the statement template used to delete the account. | ||
| // | ||
| // +optional | ||
| Deletion *string `json:"deletion,omitempty"` |
There was a problem hiding this comment.
// TODO (v1.1 or later)
password mutation sql stmt, for secret rotation
| // | ||
| // +kubebuilder:default=false | ||
| // +optional | ||
| Votable bool `json:"votable,omitempty"` |
There was a problem hiding this comment.
This field is used when memberUpdateStrategy is 'BestEffort'.
Suggest to rename to ParticipatesInQuorum bool, // Indicates if pods with this role participate in forming the quorum.
There was a problem hiding this comment.
The original name of this field was 'votable', which doesn't seem appropriate.
For example, if we have a leader and two loggers, they are all votable, but we can't stop the leader. Only the leader is in the quorum.
| // +kubebuilder:default=false | ||
| // +optional | ||
| Writable bool `json:"writable,omitempty"` | ||
|
|
There was a problem hiding this comment.
could add more fields to describe a role's abilities or properties, like:
Unique bool, Indicates if there could be only one pod that has such role in the component.Required bool, Indicates whether at least one replica with this role is required for the component to be considered running.UpdatePriority int, Describes the priority for updating pods with this role. When updating, all pods are sorted by this priority and updated accordingly.
There was a problem hiding this comment.
In this way, there is no need to inference which role is leader in controller's code.
There was a problem hiding this comment.
One more field:
SwitchoverBeforeUpdate bool, // Indicates if a switchover operation is needed before updating.
| // - KB_SERVICE_PORT: The port used by the database service. | ||
| // - KB_SERVICE_USER: The username with the necessary permissions to interact with the database service. | ||
| // - KB_SERVICE_PASSWORD: The corresponding password for KB_SERVICE_USER to authenticate with the database service. | ||
| // - KB_PRIMARY_POD_FQDN: The FQDN of the primary Pod within the replication group. |
There was a problem hiding this comment.
“KB_PRIMARY_POD_FQDN“ reveals a concept "primary"
There was a problem hiding this comment.
sounds the "primary" pod should be determined by the action itself.
| // | ||
| // The container executing this action has access to following environment variables: | ||
| // | ||
| // - KB_POD_FQDN: The FQDN of the Pod whose role is being assessed. |
There was a problem hiding this comment.
The parameter is unnecessary once action is executed in kb-agent
| // - KB_SWITCHOVER_CANDIDATE_FQDN: The FQDN of the new leader candidate's pod, which may not be specified (empty). | ||
| // - KB_LEADER_POD_IP: The IP address of the current leader's pod prior to the switchover. | ||
| // - KB_LEADER_POD_NAME: The name of the current leader's pod prior to the switchover. | ||
| // - KB_LEADER_POD_FQDN: The FQDN of the current leader's pod prior to the switchover. |
There was a problem hiding this comment.
Switchover refers to transferring the current pod's role to another pod candidate with a different role.
The parameters should include the roles of the current pod and the candidate pod(s).
| // Specifies the builtin handler name to use to probe the role of the main container. | ||
| // Available handlers include: mysql, postgres, mongodb, redis, etcd, kafka. | ||
| // Use CustomHandler to define a custom role probe function if none of the built-in handlers meet the requirement. | ||
| // | ||
| // +optional | ||
| BuiltinHandler *string `json:"builtinHandlerName,omitempty"` |
| type AddonInstallSpecItem struct { | ||
| // Specifies the number of replicas. | ||
| // | ||
| // +optional | ||
| Replicas *int32 `json:"replicas,omitempty"` | ||
|
|
||
| // Indicates whether the Persistent Volume is enabled or not. | ||
| // | ||
| // +optional | ||
| PVEnabled *bool `json:"persistentVolumeEnabled,omitempty"` | ||
|
|
||
| // Specifies the name of the storage class. | ||
| // | ||
| // +optional | ||
| StorageClass string `json:"storageClass,omitempty"` | ||
|
|
||
| // Specifies the tolerations in a JSON array string format. | ||
| // | ||
| // +optional | ||
| Tolerations string `json:"tolerations,omitempty"` | ||
|
|
||
| // Specifies the resource requirements. | ||
| // | ||
| // +optional | ||
| Resources ResourceRequirements `json:"resources,omitempty"` | ||
| } |
There was a problem hiding this comment.
do not need these fields any more once Addon is true "addon" (which is a collection of cd, cmpd, cm etc). This will be done in 1.0
| type CliPlugin struct { | ||
| // Specifies the name of the plugin. | ||
| // | ||
| // +kubebuilder:validation:Required | ||
| Name string `json:"name"` | ||
|
|
||
| // Defines the index repository of the plugin. | ||
| // | ||
| // +kubebuilder:validation:Required | ||
| IndexRepository string `json:"indexRepository"` | ||
|
|
||
| // Provides a brief description of the plugin. | ||
| // | ||
| // +optional | ||
| Description string `json:"description,omitempty"` | ||
| } |
There was a problem hiding this comment.
no addon is using, can be deleted
No description provided.