[GitHub] [skywalking-swck] hanahmily commented on a diff in pull request #70: [Feature] Add BanyanDB CRD & Controllers

[GitBox <gi...@apache.org>]

hanahmily commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r933092034


##########
operator/apis/operator/v1alpha1/banyandb_types.go:
##########
@@ -0,0 +1,66 @@
+package v1alpha1
+
+import (
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+type BanyanDBSpec struct {
+
+	// Version of BanyanDB.
+	// +kubebuilder:validation:Required
+	Version string `json:"version"`
+
+	// Image is the BanyanDB Docker image to deploy.
+	// +kubebuilder:validation:Required
+	Image string `json:"image,omitempty"`
+
+	// BanyanDB startup parameters
+	// +kubebuilder:validation:Optional
+	Config map[string]string `json:"config"`
+
+	// BanyanDB Service
+	// +kubebuilder:validation:Optional
+	Service Service `json:"service,omitempty"`
+
+	// BanyanDB Storage
+	// +kubebuilder:validation:Optional
+	Storage corev1.PersistentVolumeClaim `json:"storage,omitempty"`

Review Comment:
   It should be an array. BanyanDB support placing files into separate storage.



##########
operator/apis/operator/v1alpha1/banyandb_types.go:
##########
@@ -0,0 +1,66 @@
+package v1alpha1
+
+import (
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+type BanyanDBSpec struct {
+
+	// Version of BanyanDB.
+	// +kubebuilder:validation:Required
+	Version string `json:"version"`
+
+	// Image is the BanyanDB Docker image to deploy.
+	// +kubebuilder:validation:Required
+	Image string `json:"image,omitempty"`
+
+	// BanyanDB startup parameters
+	// +kubebuilder:validation:Optional
+	Config map[string]string `json:"config"`
+
+	// BanyanDB Service
+	// +kubebuilder:validation:Optional
+	Service Service `json:"service,omitempty"`

Review Comment:
   It should be two different items: HTTP and gRPC. Please add a subnode to config TLS relevant parameters.



##########
operator/apis/operator/v1alpha1/banyandb_types.go:
##########
@@ -0,0 +1,66 @@
+package v1alpha1
+
+import (
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+type BanyanDBSpec struct {
+
+	// Version of BanyanDB.
+	// +kubebuilder:validation:Required
+	Version string `json:"version"`
+
+	// Image is the BanyanDB Docker image to deploy.
+	// +kubebuilder:validation:Required
+	Image string `json:"image,omitempty"`
+
+	// BanyanDB startup parameters
+	// +kubebuilder:validation:Optional
+	Config map[string]string `json:"config"`
+
+	// BanyanDB Service
+	// +kubebuilder:validation:Optional
+	Service Service `json:"service,omitempty"`
+
+	// BanyanDB Storage
+	// +kubebuilder:validation:Optional
+	Storage corev1.PersistentVolumeClaim `json:"storage,omitempty"`
+}
+
+type BanyanDBState int8
+
+const (
+	Ready BanyanDBState = iota
+	Reconciling
+	Fail
+)
+
+type BanyanDBStatus struct {

Review Comment:
   An `AvailablePods` field is crucial here



##########
operator/apis/operator/v1alpha1/banyandb_types.go:
##########
@@ -0,0 +1,66 @@
+package v1alpha1
+
+import (
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+type BanyanDBSpec struct {

Review Comment:
   We need some specs here:
   
   * `Counts` denotes the number of pods.
   * `PodTemplate ` indicates a patch to the final PodTemplate. 
   



##########
operator/apis/operator/v1alpha1/banyandb_types.go:
##########
@@ -0,0 +1,66 @@
+package v1alpha1
+
+import (
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+type BanyanDBSpec struct {
+
+	// Version of BanyanDB.
+	// +kubebuilder:validation:Required
+	Version string `json:"version"`
+
+	// Image is the BanyanDB Docker image to deploy.
+	// +kubebuilder:validation:Required
+	Image string `json:"image,omitempty"`
+
+	// BanyanDB startup parameters
+	// +kubebuilder:validation:Optional
+	Config map[string]string `json:"config"`
+
+	// BanyanDB Service
+	// +kubebuilder:validation:Optional
+	Service Service `json:"service,omitempty"`
+
+	// BanyanDB Storage
+	// +kubebuilder:validation:Optional
+	Storage corev1.PersistentVolumeClaim `json:"storage,omitempty"`
+}
+
+type BanyanDBState int8
+
+const (
+	Ready BanyanDBState = iota
+	Reconciling
+	Fail
+)
+
+type BanyanDBStatus struct {
+	// BanyanDB current state
+	State BanyanDBState `json:"state,omitempty"`
+	// Represents the latest available observations of the underlying statefulset's current state.
+	// +kubebuilder:validation:Optional
+	Conditions []appsv1.StatefulSetCondition `json:"conditions,omitempty"`

Review Comment:
   `Deployment` is good to me. 



##########
operator/apis/operator/v1alpha1/banyandb_types.go:
##########
@@ -0,0 +1,66 @@
+package v1alpha1
+
+import (
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+type BanyanDBSpec struct {
+
+	// Version of BanyanDB.
+	// +kubebuilder:validation:Required
+	Version string `json:"version"`
+
+	// Image is the BanyanDB Docker image to deploy.
+	// +kubebuilder:validation:Required
+	Image string `json:"image,omitempty"`
+
+	// BanyanDB startup parameters
+	// +kubebuilder:validation:Optional
+	Config map[string]string `json:"config"`
+
+	// BanyanDB Service
+	// +kubebuilder:validation:Optional
+	Service Service `json:"service,omitempty"`
+
+	// BanyanDB Storage
+	// +kubebuilder:validation:Optional
+	Storage corev1.PersistentVolumeClaim `json:"storage,omitempty"`
+}
+
+type BanyanDBState int8
+
+const (
+	Ready BanyanDBState = iota
+	Reconciling

Review Comment:
   Could you elaborate on `Reconciling`? Add more comments to all states.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org