Skip to content

Conversation

@CloudWise-Lukemiao
Copy link
Contributor

feature iotdb operator

Copy link

@phamour phamour left a comment

Choose a reason for hiding this comment

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

another problem related to the design: validating webhooks are not used to prohibit the situation where datanode replicas > the number of worker nodes thus extra pods would just stuck in pending status with a scheduling failure event.

Copy link

Choose a reason for hiding this comment

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

most of the types defined here seem to exist in k8s go-client, for instance, the one for resource requirements may be replaced directly by ResourceRequirements in corev1. Others like the volume claim template also have official implementation. are there any reasons to redefine those types here ?

storageClassName: "standard"
resources:
requests:
storage: 1Gi No newline at end of file
Copy link

Choose a reason for hiding this comment

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

newline at EOF?

persistentVolumeReclaimPolicy: Delete
hostPath:
path: /data/k8s_iotdb/data/confignode1
type: DirectoryOrCreate No newline at end of file
Copy link

Choose a reason for hiding this comment

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

newline at EOF?

persistentVolumeReclaimPolicy: Delete
hostPath:
path: /data/k8s_iotdb/data/datanode1
type: DirectoryOrCreate No newline at end of file
Copy link

Choose a reason for hiding this comment

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

newline at EOF?

spec:
capacity:
storage: 1Gi
storageClassName: standard
Copy link

Choose a reason for hiding this comment

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

for me personally setting a storage class name in PV could be confusing

spec:
capacity:
storage: 1Gi
storageClassName: standard
Copy link

Choose a reason for hiding this comment

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

for me personally setting a storage class name in PV could be confusing

name: iotdb-datanode
spec:
image: apache/iotdb:1.3.2-datanode
replicas: 2
Copy link

Choose a reason for hiding this comment

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

it might be more reasonable to let the whole sample deploy a standalone IoTDB cluster. then the datanode replicas could be 1 and the datanode pv2 could be ignored.

controllerutil.SetControllerReference(configNode, statefulset, r.Scheme)
return statefulset
} else {
//todo add check
Copy link

Choose a reason for hiding this comment

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

if only one template is allowed, why not just make the field to accept a single value instead of a list ?

}

func (r *ConfigNodeReconciler) constructServicesForConfigNode(configNode *iotdbv1.ConfigNode) ([]corev1.Service, error) {
// 创建Headless Service
Copy link

Choose a reason for hiding this comment

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

English comments should be preferred ?


services := []corev1.Service{*headlessService}

// 检查是否需要创建NodePort Service
Copy link

Choose a reason for hiding this comment

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

English comments should be preferred ?

Copy link

@phamour phamour left a comment

Choose a reason for hiding this comment

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

lgtm

@CritasWang CritasWang merged commit d5a37e8 into apache:master Dec 10, 2024
1 check passed
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