You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/07/29 03:34:26 UTC

[GitHub] [skywalking-swck] SzyWilliam opened a new pull request, #70: [Feature] Add BanyanDB CRD & Controllers

SzyWilliam opened a new pull request, #70:
URL: https://github.com/apache/skywalking-swck/pull/70

   # Original Issue
   https://github.com/apache/skywalking/issues/9396
   
   # What changes brought in this PR
   In this PR, I add CRD for BanyanDB.
   1. Use StatefulSet as BanyanDB's underlying controller, so that we can benefit from its stable network identifiers or storage, plus future extensions.
   2. Use Service as BanyanDB's networking identifier for internal access & communication.
   3. Use LocalPV as BanyanDB's default storage choice.
   
   


-- 
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


[GitHub] [skywalking-swck] SzyWilliam commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1238952891

   @dashanji @hanahmily @lujiajing1126 Hi, I've completed a minimal working BanyanDB CRD&Controller for SWCK. It passes e2e tests. Please take a look at this, thanks! 
   There are still some advanced features to be added, like TLS config for banyandb. I plan to add these features in future PRs. 


-- 
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


[GitHub] [skywalking-swck] SzyWilliam commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1253238595

   @hanahmily Thanks for the reviews! I've exposed HTTP port and added an ingress resource to route HTTP requests, PTAL. I have a question: according to [banyandb github page](https://github.com/apache/skywalking-banyandb), we do not support http server for now. Does banyandb support HTTP server now? If so, how I can query data using HTTP?


-- 
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


[GitHub] [skywalking-swck] wu-sheng commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1198896239

   Then this PR should be labeled aa draft or use issue or discussion panel to talk.
   PR could trigger mail notifications for reviewers.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r933180459


##########
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:
   Affinity is missing



-- 
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


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

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r964424471


##########
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

Review Comment:
   I've deleted this `BanyanDBState`



##########
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:
   done



-- 
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


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

Posted by GitBox <gi...@apache.org>.
dashanji commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r967950902


##########
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:
   The key to exposing the template is whether the user needs it. We should support the [Affinity](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/) here.



-- 
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


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

Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r932853017


##########
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"`
+}
+
+// +kubebuilder:object:root=true
+
+// BanyanDB is the Schema for the BanyanDB API
+type BanyanDB struct {
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	Spec   BanyanDBSpec   `json:"spec,omitempty"`

Review Comment:
   Spec and Status should not be `omitempty` as I understand



##########
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

Review Comment:
   The marker here is in conflict with the `omitempty` option below. 



-- 
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


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

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r964424016


##########
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:
   done



-- 
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


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

Posted by GitBox <gi...@apache.org>.
hanahmily commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r976087030


##########
operator/pkg/operator/manifests/banyandb/templates/ingress.yaml:
##########
@@ -0,0 +1,53 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you 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.
+
+{{- $ingress := .Spec.Service.Ingress }}
+{{ if $ingress.Host }}
+apiVersion: networking.k8s.io/v1
+kind: Ingress
+metadata:
+  name: {{ .Name }}-banyandb
+  namespace: {{ .Namespace }}
+  labels:
+    app: ui
+    operator.skywalking.apache.org/ui-name: {{ .Name }}
+    operator.skywalking.apache.org/application: ui
+    operator.skywalking.apache.org/component: deployment
+  annotations:
+    {{- range $key, $value := $ingress.Annotations }}
+      {{ $key }}: {{ $value | quote }}
+      {{- end }}
+spec:
+  rules:
+    - host: {{ $ingress.Host }}
+      http:
+        paths:
+          - backend:
+              service:
+                name: {{ .Name }}-banyandb

Review Comment:
   Route traffics to the service which is serving 17913



##########
operator/apis/operator/v1alpha1/banyandb_types.go:
##########
@@ -0,0 +1,103 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 v1alpha1
+
+import (
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// BanyanDBSpec defines the desired state of BanyanDB
+type BanyanDBSpec struct {
+	// Version of BanyanDB.
+	// +kubebuilder:validation:Required
+	Version string `json:"version"`
+
+	// Number of replicas
+	// +kubebuilder:validation:Required
+	Counts int `json:"counts"`
+
+	// Pod template of each BanyanDB instance
+	// +kubebuilder:validation:Required
+	Image string `json:"image"`
+
+	// Pod affinity
+	// +kubebuilder:validation:Optional
+	Affinity corev1.Affinity `json:"affinity"`
+
+	// BanyanDB startup parameters
+	// +kubebuilder:validation:Optional
+	Config []string `json:"config"`
+
+	// TODO support TLS settings
+	// BanyanDB Service
+	// +kubebuilder:validation:Optional
+	Service Service `json:"service,omitempty"`

Review Comment:
   I tend to add a new service named `HTTPSvc`. As a result , there are two service fields in this spec as below
   
   ```suggestion
   	GRPCSvc Service `json:"service,omitempty"`
   	HTTPSvc Service `json:"service,omitempty"`
   ```
   



##########
operator/pkg/operator/manifests/banyandb/templates/service.yaml:
##########
@@ -0,0 +1,60 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you 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.
+
+{{- $svc := .Spec.Service.Template }}
+apiVersion: v1
+kind: Service

Review Comment:
   Create a new service to serve 17913.



##########
operator/pkg/operator/manifests/banyandb/templates/service.yaml:
##########
@@ -0,0 +1,60 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you 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.
+
+{{- $svc := .Spec.Service.Template }}
+apiVersion: v1
+kind: Service
+metadata:
+  name: {{ .Name }}-banyandb
+  namespace: {{ .Namespace }}
+  labels:
+    app: banyandb
+    operator.skywalking.apache.org/banyandb-name: {{ .Name }}
+    operator.skywalking.apache.org/application: banyandb
+    operator.skywalking.apache.org/component: service
+spec:
+  type: {{ $svc.Type }}
+  selector:
+    app: banyandb
+    operator.skywalking.apache.org/banyandb-name: {{ .Name }}
+  ports:
+    - port: 17912
+      name: grpc
+    - port: 2121
+      name: observability
+    - port: 6060
+      name: pprof

Review Comment:
   Please remove these ports. We don't tend to expose them through a service.



-- 
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


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

Posted by GitBox <gi...@apache.org>.
hanahmily commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1244919854

   > @dashanji Thanks for the reviews. @hanahmily I am trying to write e2e tests for swck banyandb. Could you please tell me how I can query banyandb pod using client tools like [cli](https://github.com/apache/skywalking-banyandb/tree/main/bydbctl)?
   
   bydbctl is WIP. you could query data from OAP instead of the banyandb server as https://github.com/apache/skywalking-banyandb/tree/main/test/e2e-v2 .


-- 
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


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

Posted by 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


[GitHub] [skywalking-swck] dashanji commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
dashanji commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1198876197

   @wu-sheng  @lujiajing1126 This is a draft pr for the osapp project.  The student @WillemJiang should still be coding, but I'm not familiar with some features of Banyandb, so could you please confirm whether the Spec field can meet all the current banyandb requirements? 


-- 
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


[GitHub] [skywalking-swck] wu-sheng commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1238967301

   What is the storage volume mount policy? 
   
   @hanahmily When we use this on the demo deployment, we need to make sure storage volume could mounted back when pod reboot(this happens time to time on our demo GCP.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r964425919


##########
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:
   @hanahmily should we expose total `PodTemplate` to user?  I found that in OapServer CRD we only expose some key fields like`Image`, while providing default value for other fields. Should we follow this practice?



-- 
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


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

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r976058763


##########
operator/pkg/operator/manifests/banyandb/templates/cluster_role.yaml:
##########
@@ -0,0 +1,31 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you 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.
+
+kind: ClusterRole

Review Comment:
   Done. I have removed ClusterRole & ClusterRoleBinding



-- 
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


[GitHub] [skywalking-swck] dashanji commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
dashanji commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1242644804

   > What is the storage volume mount policy?
   > 
   > @hanahmily When we use this on the demo deployment, we need to make sure storage volume could mounted back when pod reboot(this happens time to time on our demo GCP.
   
   At present, we don't have the storage volume,  the data is stored in the banyandb's memory and will be cleared when rebooting. Do you mean we need to create a [persistent volume](https://kubernetes.io/docs/concepts/storage/persistent-volumes/) for banyandb?


-- 
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


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

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r964423590


##########
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"`
+}
+
+// +kubebuilder:object:root=true
+
+// BanyanDB is the Schema for the BanyanDB API
+type BanyanDB struct {
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	Spec   BanyanDBSpec   `json:"spec,omitempty"`

Review Comment:
   The kubebuilder automatically generates `omitempty`, as well as in other resource types. Should I delete it manually?



-- 
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


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

Posted by GitBox <gi...@apache.org>.
dashanji commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r978609872


##########
operator/controllers/operator/banyandb_controller.go:
##########
@@ -0,0 +1,134 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 operator
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/go-logr/logr"
+	apps "k8s.io/api/apps/v1"
+	apiequal "k8s.io/apimachinery/pkg/api/equality"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/tools/record"
+	"k8s.io/client-go/util/retry"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	runtimelog "sigs.k8s.io/controller-runtime/pkg/log"
+
+	"github.com/apache/skywalking-swck/operator/apis/operator/v1alpha1"
+	operatorv1alpha1 "github.com/apache/skywalking-swck/operator/apis/operator/v1alpha1"
+	"github.com/apache/skywalking-swck/operator/pkg/kubernetes"
+)
+
+// BanyanDBReconciler reconciles a BanyanDB object
+type BanyanDBReconciler struct {
+	client.Client
+	Scheme   *runtime.Scheme
+	FileRepo kubernetes.Repo
+	Recorder record.EventRecorder
+}
+
+//+kubebuilder:rbac:groups=operator.skywalking.apache.org,resources=banyandbs,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups=operator.skywalking.apache.org,resources=banyandbs/status,verbs=get;update;patch
+//+kubebuilder:rbac:groups=operator.skywalking.apache.org,resources=banyandbs/finalizers,verbs=update
+//+kubebuilder:rbac:groups="",resources=services;serviceaccounts;persistentvolumeclaims,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles;clusterrolebindings,verbs=*
+
+func (r *BanyanDBReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+	log := runtimelog.FromContext(ctx)
+	log.Info("=====================reconcile started================================")
+
+	var banyanDB v1alpha1.BanyanDB
+	if err := r.Get(ctx, req.NamespacedName, &banyanDB); err != nil {
+		return ctrl.Result{}, client.IgnoreNotFound(err)
+	}
+
+	ff, err := r.FileRepo.GetFilesRecursive("templates")
+	if err != nil {
+		log.Error(err, "failed to load resource templates")
+		return ctrl.Result{}, err
+	}
+	app := kubernetes.Application{
+		Client:   r.Client,
+		CR:       &banyanDB,
+		FileRepo: r.FileRepo,
+		GVK:      operatorv1alpha1.GroupVersion.WithKind("BanyanDB"),
+		Recorder: r.Recorder,
+	}
+
+	if err := app.ApplyAll(ctx, ff, log); err != nil {
+		return ctrl.Result{}, err
+	}
+
+	if err := r.checkState(ctx, log, &banyanDB); err != nil {
+		log.Error(err, "failed to check sub resources state")
+		return ctrl.Result{}, err
+	}
+
+	return ctrl.Result{RequeueAfter: schedDuration}, nil
+}
+
+func (r *BanyanDBReconciler) checkState(ctx context.Context, log logr.Logger, banyanDB *operatorv1alpha1.BanyanDB) error {
+	overlay := operatorv1alpha1.BanyanDBStatus{}
+	deployment := apps.Deployment{}
+	errCol := new(kubernetes.ErrorCollector)
+	if err := r.Client.Get(ctx, client.ObjectKey{Namespace: banyanDB.Namespace, Name: banyanDB.Name + "-banyandb"}, &deployment); err != nil && !apierrors.IsNotFound(err) {
+		errCol.Collect(fmt.Errorf("failed to get deployment: %w", err))
+	} else {
+		overlay.Conditions = deployment.Status.Conditions
+		overlay.AvailableReplicas = deployment.Status.AvailableReplicas
+	}
+	if apiequal.Semantic.DeepDerivative(overlay, banyanDB.Status) {
+		log.Info("Status keeps the same as before")

Review Comment:
   Make sense. We could return here to avoid update the same resources. Thanks for the catching, and the other controllers have the same questions. Could you please update them together ? @SzyWilliam 



-- 
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


[GitHub] [skywalking-swck] wu-sheng commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1255705657

   It is great to see this PR is getting ready after months of work. Congrats.


-- 
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


[GitHub] [skywalking-swck] wu-sheng commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1198859794

   I can't see an e2e to verify this. 
   SWCK should be verified whether it really could deploy.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
hanahmily commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r933252975


##########
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:
   Have to support multi-storages, the type could be `[]corev1.PersistentVolumeClaim`



-- 
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


[GitHub] [skywalking-swck] wu-sheng commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1198896434

   BTW, I think you ping a wrong people.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r978280770


##########
operator/controllers/operator/banyandb_controller.go:
##########
@@ -0,0 +1,134 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 operator
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/go-logr/logr"
+	apps "k8s.io/api/apps/v1"
+	apiequal "k8s.io/apimachinery/pkg/api/equality"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/tools/record"
+	"k8s.io/client-go/util/retry"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	runtimelog "sigs.k8s.io/controller-runtime/pkg/log"
+
+	"github.com/apache/skywalking-swck/operator/apis/operator/v1alpha1"
+	operatorv1alpha1 "github.com/apache/skywalking-swck/operator/apis/operator/v1alpha1"
+	"github.com/apache/skywalking-swck/operator/pkg/kubernetes"
+)
+
+// BanyanDBReconciler reconciles a BanyanDB object
+type BanyanDBReconciler struct {
+	client.Client
+	Scheme   *runtime.Scheme
+	FileRepo kubernetes.Repo
+	Recorder record.EventRecorder
+}
+
+//+kubebuilder:rbac:groups=operator.skywalking.apache.org,resources=banyandbs,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups=operator.skywalking.apache.org,resources=banyandbs/status,verbs=get;update;patch
+//+kubebuilder:rbac:groups=operator.skywalking.apache.org,resources=banyandbs/finalizers,verbs=update
+//+kubebuilder:rbac:groups="",resources=services;serviceaccounts;persistentvolumeclaims,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles;clusterrolebindings,verbs=*
+
+func (r *BanyanDBReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+	log := runtimelog.FromContext(ctx)
+	log.Info("=====================reconcile started================================")
+
+	var banyanDB v1alpha1.BanyanDB
+	if err := r.Get(ctx, req.NamespacedName, &banyanDB); err != nil {
+		return ctrl.Result{}, client.IgnoreNotFound(err)
+	}
+
+	ff, err := r.FileRepo.GetFilesRecursive("templates")
+	if err != nil {
+		log.Error(err, "failed to load resource templates")
+		return ctrl.Result{}, err
+	}
+	app := kubernetes.Application{
+		Client:   r.Client,
+		CR:       &banyanDB,
+		FileRepo: r.FileRepo,
+		GVK:      operatorv1alpha1.GroupVersion.WithKind("BanyanDB"),
+		Recorder: r.Recorder,
+	}
+
+	if err := app.ApplyAll(ctx, ff, log); err != nil {
+		return ctrl.Result{}, err
+	}
+
+	if err := r.checkState(ctx, log, &banyanDB); err != nil {
+		log.Error(err, "failed to check sub resources state")
+		return ctrl.Result{}, err
+	}
+
+	return ctrl.Result{RequeueAfter: schedDuration}, nil
+}
+
+func (r *BanyanDBReconciler) checkState(ctx context.Context, log logr.Logger, banyanDB *operatorv1alpha1.BanyanDB) error {
+	overlay := operatorv1alpha1.BanyanDBStatus{}
+	deployment := apps.Deployment{}
+	errCol := new(kubernetes.ErrorCollector)
+	if err := r.Client.Get(ctx, client.ObjectKey{Namespace: banyanDB.Namespace, Name: banyanDB.Name + "-banyandb"}, &deployment); err != nil && !apierrors.IsNotFound(err) {
+		errCol.Collect(fmt.Errorf("failed to get deployment: %w", err))
+	} else {
+		overlay.Conditions = deployment.Status.Conditions
+		overlay.AvailableReplicas = deployment.Status.AvailableReplicas
+	}
+	if apiequal.Semantic.DeepDerivative(overlay, banyanDB.Status) {
+		log.Info("Status keeps the same as before")

Review Comment:
   I think we have return here to avoid unexpected reconciliation? @dashanji 



-- 
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


[GitHub] [skywalking-swck] SzyWilliam commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1256832153

   @lujiajing1126 Thanks for the careful review! I've made changes on code, please take a look again.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r977429430


##########
operator/pkg/operator/manifests/banyandb/templates/ingress.yaml:
##########
@@ -0,0 +1,53 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you 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.
+
+{{- $ingress := .Spec.Service.Ingress }}
+{{ if $ingress.Host }}
+apiVersion: networking.k8s.io/v1
+kind: Ingress
+metadata:
+  name: {{ .Name }}-banyandb
+  namespace: {{ .Namespace }}
+  labels:
+    app: ui
+    operator.skywalking.apache.org/ui-name: {{ .Name }}
+    operator.skywalking.apache.org/application: ui
+    operator.skywalking.apache.org/component: deployment
+  annotations:
+    {{- range $key, $value := $ingress.Annotations }}
+      {{ $key }}: {{ $value | quote }}
+      {{- end }}
+spec:
+  rules:
+    - host: {{ $ingress.Host }}
+      http:
+        paths:
+          - backend:
+              service:
+                name: {{ .Name }}-banyandb

Review Comment:
   done



##########
operator/pkg/operator/manifests/banyandb/templates/service.yaml:
##########
@@ -0,0 +1,60 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you 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.
+
+{{- $svc := .Spec.Service.Template }}
+apiVersion: v1
+kind: Service
+metadata:
+  name: {{ .Name }}-banyandb
+  namespace: {{ .Namespace }}
+  labels:
+    app: banyandb
+    operator.skywalking.apache.org/banyandb-name: {{ .Name }}
+    operator.skywalking.apache.org/application: banyandb
+    operator.skywalking.apache.org/component: service
+spec:
+  type: {{ $svc.Type }}
+  selector:
+    app: banyandb
+    operator.skywalking.apache.org/banyandb-name: {{ .Name }}
+  ports:
+    - port: 17912
+      name: grpc
+    - port: 2121
+      name: observability
+    - port: 6060
+      name: pprof

Review Comment:
   done



##########
operator/pkg/operator/manifests/banyandb/templates/service.yaml:
##########
@@ -0,0 +1,60 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you 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.
+
+{{- $svc := .Spec.Service.Template }}
+apiVersion: v1
+kind: Service

Review Comment:
   done



-- 
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


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

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r976058891


##########
operator/apis/operator/v1alpha1/banyandb_types.go:
##########
@@ -0,0 +1,103 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 v1alpha1
+
+import (
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// BanyanDBSpec defines the desired state of BanyanDB
+type BanyanDBSpec struct {
+	// Version of BanyanDB.
+	// +kubebuilder:validation:Required
+	Version string `json:"version"`
+
+	// Number of replicas
+	// +kubebuilder:validation:Required
+	Counts int `json:"counts"`
+
+	// Pod template of each BanyanDB instance
+	// +kubebuilder:validation:Required
+	Image string `json:"image"`
+
+	// Pod affinity
+	// +kubebuilder:validation:Optional
+	Affinity corev1.Affinity `json:"affinity"`
+
+	// BanyanDB startup parameters
+	// +kubebuilder:validation:Optional
+	Config []string `json:"config"`
+
+	// TODO support TLS settings
+	// BanyanDB Service
+	// +kubebuilder:validation:Optional
+	Service Service `json:"service,omitempty"`

Review Comment:
   Done



-- 
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


[GitHub] [skywalking-swck] wu-sheng commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1255790613

   @lujiajing1126 We need your confirmation here. Please recheck.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r976040130


##########
operator/pkg/operator/manifests/banyandb/templates/deployment.yaml:
##########
@@ -0,0 +1,116 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you 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.
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {{ .Name }}-banyandb
+  namespace: {{ .Namespace }}
+  labels:
+    app: banyandb
+    operator.skywalking.apache.org/banyandb-name: {{ .Name }}
+    operator.skywalking.apache.org/application: banyandb
+    operator.skywalking.apache.org/component: deployment
+spec:
+  replicas: {{ .Spec.Counts }}
+  selector:
+    matchLabels:
+      app: banyandb
+      operator.skywalking.apache.org/banyandb-name: {{ .Name }}
+  template:
+    metadata:
+      labels:
+        app: banyandb
+        operator.skywalking.apache.org/banyandb-name: {{ .Name }}
+        operator.skywalking.apache.org/application: banyandb
+        operator.skywalking.apache.org/component: pod
+    spec:
+      serviceAccountName: {{ .Name }}-banyandb
+
+      {{- if .Spec.Storages }}
+      volumes:
+        {{- range $storage := .Spec.Storages }}
+        - name: {{ $storage.Name }}
+          persistentVolumeClaim:
+            claimName: {{ $storage.Name }}-banyandb
+        {{- end }}
+      {{- end}}
+      containers:
+        - name: banyandb-container
+          image: {{ .Spec.Image }}
+          imagePullPolicy: IfNotPresent
+          args:
+            {{- range $value := .Spec.Config }}
+            - {{ $value }}
+            {{- end }}
+          ports:
+            - containerPort: 17912

Review Comment:
   done



-- 
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


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

Posted by GitBox <gi...@apache.org>.
hanahmily commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1253261943

   >  we do not support http server for now. Does banyandb support HTTP server now? 
   
   We supported the HTTP server in the main branch. It will be released in v0.2.0.
   
   > If so, how I can query data using HTTP?
   
   bydbctl will take it. The primitive commands will be released in v0.2.0 with the HTTP server. 
   
   And you could access the HTTP endpoint directly. The API specifications are at https://github.com/apache/skywalking-banyandb/blob/main/api/proto/openapi
   
   


-- 
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


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

Posted by GitBox <gi...@apache.org>.
dashanji commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r967952159


##########
test/e2e/banyandb/e2e.yaml:
##########
@@ -0,0 +1,100 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You 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.
+
+setup:
+  env: kind
+  file: ../kind.yaml
+  steps:
+    - name: prepare e2e.yaml
+      command: bash hack/prepare-e2e.sh
+    - name: install cert-manager
+      command: |
+        # kind k8s cluster is in $TMPDIR
+        export KUBECONFIG=$TMPDIR/e2e-k8s.config
+        kubectl apply -f https://github.com/cert-manager/cert-manager/releases/download/v1.8.0/cert-manager.yaml
+      wait:
+        - namespace: cert-manager
+          resource: pod
+          for: condition=Ready
+    - name: install operator
+      command: |
+        export OPERATOR_IMG=controller
+        make -C operator docker-build   
+        kind load docker-image controller
+        make -C operator install
+        make -C operator deploy
+      wait:
+        - namespace: skywalking-swck-system
+          resource: pod
+          for: condition=Ready
+    - name: setup banyandb
+      command: |
+        kubectl apply -f test/e2e/deploy-banyandb.yaml
+    - name: setup oapserver and ui
+      command: |
+        kubectl create namespace skywalking-system
+        kubectl apply -f test/e2e/skywalking-components-with-banyandb.yaml
+      wait:
+        - namespace: default
+          resource: OAPServer/default
+          for: condition=Available
+        - namespace: skywalking-system
+          resource: UI/skywalking-system
+          for: condition=Available
+    - name: setup java agent demo
+      command: |
+        kubectl label namespace skywalking-system swck-injection=enabled
+        sed 's/oap-service/default-oap.default/' test/e2e/demo.yaml | kubectl create -f -
+      wait:
+        - namespace: skywalking-system
+          resource: deployment/demo
+          for: condition=Available
+  kind:
+    expose-ports:
+      - namespace: skywalking-system
+        resource: service/demo
+        port: 8085
+      - namespace: default
+        resource: service/default-oap
+        port: 12800
+      - namespace: default
+        resource: service/banyandb-test-banyandb
+        port: 17912
+      - namespace: skywalking-system
+        resource: service/skywalking-system-ui
+        port: 80
+  timeout: 20m
+
+cleanup:
+  on: never
+
+trigger:
+  action: http
+  interval: 10s
+  times: 5
+  url: http://${service_demo_host}:${service_demo_8085}/hello
+  method: GET
+
+verify:
+  # verify with retry strategy
+  retry:
+    # max retry count
+    count: 10
+    # the interval between two attempts, e.g. 10s, 1m.
+    interval: 10s
+  cases:
+    - includes:
+        - ../oapserver-cases.yaml
+        - ../ui-cases.yaml

Review Comment:
   As we discussed before, Is there a way to verify the data is stored in banyandb? How about using the [cli](https://github.com/apache/skywalking-banyandb/tree/main/bydbctl)? 



-- 
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


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

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r933150086


##########
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:
   Got it. 



-- 
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


[GitHub] [skywalking-swck] lujiajing1126 commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1199250309

   > @lujiajing1126 Thanks for your reply! If we consider future extensions, a StatefulSet will be great for a BanyanDB cluster since it provides bindings on pv & network identifier. Why don't we use StatefulSet now?
   
   Actually, I'm neutral about the choice. I would just say `Deployment` is definitely much simpler and thus has lower cost. The cluster version is on our roadmap but with very few details. It is not possible for us now to cover all what we need in the future.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r969162317


##########
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:
   Got it



-- 
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


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

Posted by GitBox <gi...@apache.org>.
hanahmily commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r974971788


##########
operator/pkg/operator/manifests/banyandb/templates/deployment.yaml:
##########
@@ -0,0 +1,116 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you 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.
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {{ .Name }}-banyandb
+  namespace: {{ .Namespace }}
+  labels:
+    app: banyandb
+    operator.skywalking.apache.org/banyandb-name: {{ .Name }}
+    operator.skywalking.apache.org/application: banyandb
+    operator.skywalking.apache.org/component: deployment
+spec:
+  replicas: {{ .Spec.Counts }}
+  selector:
+    matchLabels:
+      app: banyandb
+      operator.skywalking.apache.org/banyandb-name: {{ .Name }}
+  template:
+    metadata:
+      labels:
+        app: banyandb
+        operator.skywalking.apache.org/banyandb-name: {{ .Name }}
+        operator.skywalking.apache.org/application: banyandb
+        operator.skywalking.apache.org/component: pod
+    spec:
+      serviceAccountName: {{ .Name }}-banyandb
+
+      {{- if .Spec.Storages }}
+      volumes:
+        {{- range $storage := .Spec.Storages }}
+        - name: {{ $storage.Name }}
+          persistentVolumeClaim:
+            claimName: {{ $storage.Name }}-banyandb
+        {{- end }}
+      {{- end}}
+      containers:
+        - name: banyandb-container
+          image: {{ .Spec.Image }}
+          imagePullPolicy: IfNotPresent
+          args:
+            {{- range $value := .Spec.Config }}
+            - {{ $value }}
+            {{- end }}
+          ports:
+            - containerPort: 17912

Review Comment:
   we need to expose 17913 to serve the HTTP endpoint.



##########
operator/apis/operator/v1alpha1/banyandb_types.go:
##########
@@ -0,0 +1,103 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 v1alpha1
+
+import (
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// BanyanDBSpec defines the desired state of BanyanDB
+type BanyanDBSpec struct {
+	// Version of BanyanDB.
+	// +kubebuilder:validation:Required
+	Version string `json:"version"`
+
+	// Number of replicas
+	// +kubebuilder:validation:Required
+	Counts int `json:"counts"`
+
+	// Pod template of each BanyanDB instance
+	// +kubebuilder:validation:Required
+	Image string `json:"image"`
+
+	// Pod affinity
+	// +kubebuilder:validation:Optional
+	Affinity corev1.Affinity `json:"affinity"`
+
+	// BanyanDB startup parameters
+	// +kubebuilder:validation:Optional
+	Config []string `json:"config"`
+
+	// TODO support TLS settings
+	// BanyanDB Service
+	// +kubebuilder:validation:Optional
+	Service Service `json:"service,omitempty"`

Review Comment:
   Why the HTTP service as I mentioned in the previous round?



##########
operator/pkg/operator/manifests/banyandb/templates/cluster_role.yaml:
##########
@@ -0,0 +1,31 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you 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.
+
+kind: ClusterRole

Review Comment:
   Why do we need this role since the server doesn't touch the API server?



-- 
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


[GitHub] [skywalking-swck] SzyWilliam commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1253284919

   @hanahmily Thanks for the reply and providing framework on Service. I'll use separate gRPC Service and HTTP service. I plan to use bydbctl to verify banyandb data storage in E2E Tests after 0.2.0 release, so I'll just add a todo for now. 


-- 
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


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

Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r932889754


##########
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

Review Comment:
   `string` is much better for readability and inspection



##########
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:
   We only need `*corev1.PersistentVolumeClaimSpec` here?



-- 
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


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

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r979138799


##########
operator/controllers/operator/banyandb_controller.go:
##########
@@ -0,0 +1,134 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 operator
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/go-logr/logr"
+	apps "k8s.io/api/apps/v1"
+	apiequal "k8s.io/apimachinery/pkg/api/equality"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/tools/record"
+	"k8s.io/client-go/util/retry"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	runtimelog "sigs.k8s.io/controller-runtime/pkg/log"
+
+	"github.com/apache/skywalking-swck/operator/apis/operator/v1alpha1"
+	operatorv1alpha1 "github.com/apache/skywalking-swck/operator/apis/operator/v1alpha1"
+	"github.com/apache/skywalking-swck/operator/pkg/kubernetes"
+)
+
+// BanyanDBReconciler reconciles a BanyanDB object
+type BanyanDBReconciler struct {
+	client.Client
+	Scheme   *runtime.Scheme
+	FileRepo kubernetes.Repo
+	Recorder record.EventRecorder
+}
+
+//+kubebuilder:rbac:groups=operator.skywalking.apache.org,resources=banyandbs,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups=operator.skywalking.apache.org,resources=banyandbs/status,verbs=get;update;patch
+//+kubebuilder:rbac:groups=operator.skywalking.apache.org,resources=banyandbs/finalizers,verbs=update
+//+kubebuilder:rbac:groups="",resources=services;serviceaccounts;persistentvolumeclaims,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles;clusterrolebindings,verbs=*
+
+func (r *BanyanDBReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+	log := runtimelog.FromContext(ctx)
+	log.Info("=====================reconcile started================================")
+
+	var banyanDB v1alpha1.BanyanDB
+	if err := r.Get(ctx, req.NamespacedName, &banyanDB); err != nil {
+		return ctrl.Result{}, client.IgnoreNotFound(err)
+	}
+
+	ff, err := r.FileRepo.GetFilesRecursive("templates")
+	if err != nil {
+		log.Error(err, "failed to load resource templates")
+		return ctrl.Result{}, err
+	}
+	app := kubernetes.Application{
+		Client:   r.Client,
+		CR:       &banyanDB,
+		FileRepo: r.FileRepo,
+		GVK:      operatorv1alpha1.GroupVersion.WithKind("BanyanDB"),
+		Recorder: r.Recorder,
+	}
+
+	if err := app.ApplyAll(ctx, ff, log); err != nil {
+		return ctrl.Result{}, err
+	}
+
+	if err := r.checkState(ctx, log, &banyanDB); err != nil {
+		log.Error(err, "failed to check sub resources state")
+		return ctrl.Result{}, err
+	}
+
+	return ctrl.Result{RequeueAfter: schedDuration}, nil
+}
+
+func (r *BanyanDBReconciler) checkState(ctx context.Context, log logr.Logger, banyanDB *operatorv1alpha1.BanyanDB) error {
+	overlay := operatorv1alpha1.BanyanDBStatus{}
+	deployment := apps.Deployment{}
+	errCol := new(kubernetes.ErrorCollector)
+	if err := r.Client.Get(ctx, client.ObjectKey{Namespace: banyanDB.Namespace, Name: banyanDB.Name + "-banyandb"}, &deployment); err != nil && !apierrors.IsNotFound(err) {
+		errCol.Collect(fmt.Errorf("failed to get deployment: %w", err))
+	} else {
+		overlay.Conditions = deployment.Status.Conditions
+		overlay.AvailableReplicas = deployment.Status.AvailableReplicas
+	}
+	if apiequal.Semantic.DeepDerivative(overlay, banyanDB.Status) {
+		log.Info("Status keeps the same as before")

Review Comment:
   done



-- 
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


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

Posted by GitBox <gi...@apache.org>.
hanahmily commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r933138920


##########
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:
   This operator is not only for a standalone server but should support a banyandb cluster in our roadmap. Users who set up more than one pod will mess up the data. It would help if you left a caveat in the document to mention that. But I don't want a checker to forbid this behavior. We need more flexibility in the current phase of BanyanDB.



-- 
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


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

Posted by GitBox <gi...@apache.org>.
hanahmily commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r933257317


##########
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 will introduce a `PodTemplate` field to support `Affinity`. 



-- 
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


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

Posted by GitBox <gi...@apache.org>.
dashanji commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r967946254


##########
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"`
+}
+
+// +kubebuilder:object:root=true
+
+// BanyanDB is the Schema for the BanyanDB API
+type BanyanDB struct {
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	Spec   BanyanDBSpec   `json:"spec,omitempty"`

Review Comment:
   It doesn't matter.



-- 
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


[GitHub] [skywalking-swck] SzyWilliam commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1254778560

   I've made changes on code:
   1.  separate grpc & http into two service resources, expose 17912/17913 respectively.
   2.  remove unused ports 2121 6060.
   @hanahmily Please take a look at it again, thanks!


-- 
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


[GitHub] [skywalking-swck] dashanji commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
dashanji commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1251151214

   @SzyWilliam  LGTM, thanks for your patience and hard work! Hope you will continue to improve it in the next PRs. 


-- 
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


[GitHub] [skywalking-swck] SzyWilliam commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1251941890

   @dashanji Thanks for your reviews! I've completed the full banyandb crd & controller implementation with corresponding e2e tests. @wu-sheng @hanahmily @lujiajing1126 Please take a look at it~🫰


-- 
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


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

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r964423826


##########
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:
   Got it, I'll add it in the future.



##########
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:
   done



##########
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:
   done



-- 
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


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

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r933131000


##########
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:
   @hanahmily Thanks for the reviews! May I ask why we should deploy multiple BanyanDB pods? Under multiple BanyanDB instances, can write or query requests be routed to any random BanyanDB Pod? 



-- 
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


[GitHub] [skywalking-swck] lujiajing1126 commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1199206832

   > @wu-sheng @lujiajing1126 This is a draft pr for the osapp project. The student @SzyWilliam should still be coding, but I'm not familiar with some features of Banyandb, so could you please confirm whether the Spec field can meet all the current banyandb requirements?
   
   As I understand, since the current version of BanyanDB only has a single node, Deployment can meet our requirement.
   
   The [operator](https://github.com/VictoriaMetrics/operator) for `VictoriaMetrics` and its `VMSingle` CRD can be a good reference for us.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
hanahmily commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1199278097

   `Deployment` is better here. The cluster mode will know each node's roles, we don't leverage statefulset to get stable and ordered network endpoints and pv. 


-- 
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


[GitHub] [skywalking-swck] wu-sheng merged pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
wu-sheng merged PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70


-- 
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


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

Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#discussion_r978280770


##########
operator/controllers/operator/banyandb_controller.go:
##########
@@ -0,0 +1,134 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 operator
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/go-logr/logr"
+	apps "k8s.io/api/apps/v1"
+	apiequal "k8s.io/apimachinery/pkg/api/equality"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/tools/record"
+	"k8s.io/client-go/util/retry"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	runtimelog "sigs.k8s.io/controller-runtime/pkg/log"
+
+	"github.com/apache/skywalking-swck/operator/apis/operator/v1alpha1"
+	operatorv1alpha1 "github.com/apache/skywalking-swck/operator/apis/operator/v1alpha1"
+	"github.com/apache/skywalking-swck/operator/pkg/kubernetes"
+)
+
+// BanyanDBReconciler reconciles a BanyanDB object
+type BanyanDBReconciler struct {
+	client.Client
+	Scheme   *runtime.Scheme
+	FileRepo kubernetes.Repo
+	Recorder record.EventRecorder
+}
+
+//+kubebuilder:rbac:groups=operator.skywalking.apache.org,resources=banyandbs,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups=operator.skywalking.apache.org,resources=banyandbs/status,verbs=get;update;patch
+//+kubebuilder:rbac:groups=operator.skywalking.apache.org,resources=banyandbs/finalizers,verbs=update
+//+kubebuilder:rbac:groups="",resources=services;serviceaccounts;persistentvolumeclaims,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles;clusterrolebindings,verbs=*
+
+func (r *BanyanDBReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+	log := runtimelog.FromContext(ctx)
+	log.Info("=====================reconcile started================================")
+
+	var banyanDB v1alpha1.BanyanDB
+	if err := r.Get(ctx, req.NamespacedName, &banyanDB); err != nil {
+		return ctrl.Result{}, client.IgnoreNotFound(err)
+	}
+
+	ff, err := r.FileRepo.GetFilesRecursive("templates")
+	if err != nil {
+		log.Error(err, "failed to load resource templates")
+		return ctrl.Result{}, err
+	}
+	app := kubernetes.Application{
+		Client:   r.Client,
+		CR:       &banyanDB,
+		FileRepo: r.FileRepo,
+		GVK:      operatorv1alpha1.GroupVersion.WithKind("BanyanDB"),
+		Recorder: r.Recorder,
+	}
+
+	if err := app.ApplyAll(ctx, ff, log); err != nil {
+		return ctrl.Result{}, err
+	}
+
+	if err := r.checkState(ctx, log, &banyanDB); err != nil {
+		log.Error(err, "failed to check sub resources state")
+		return ctrl.Result{}, err
+	}
+
+	return ctrl.Result{RequeueAfter: schedDuration}, nil
+}
+
+func (r *BanyanDBReconciler) checkState(ctx context.Context, log logr.Logger, banyanDB *operatorv1alpha1.BanyanDB) error {
+	overlay := operatorv1alpha1.BanyanDBStatus{}
+	deployment := apps.Deployment{}
+	errCol := new(kubernetes.ErrorCollector)
+	if err := r.Client.Get(ctx, client.ObjectKey{Namespace: banyanDB.Namespace, Name: banyanDB.Name + "-banyandb"}, &deployment); err != nil && !apierrors.IsNotFound(err) {
+		errCol.Collect(fmt.Errorf("failed to get deployment: %w", err))
+	} else {
+		overlay.Conditions = deployment.Status.Conditions
+		overlay.AvailableReplicas = deployment.Status.AvailableReplicas
+	}
+	if apiequal.Semantic.DeepDerivative(overlay, banyanDB.Status) {
+		log.Info("Status keeps the same as before")

Review Comment:
   I think we have to return here to avoid unexpected reconciliation? @dashanji 



-- 
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


[GitHub] [skywalking-swck] SzyWilliam commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1199214846

   > As I understand, since the current version of BanyanDB only has a single node, Deployment can meet our requirement.
   @lujiajing1126 Thanks for your reply! If we consider future extensions, a StatefulSet will be great for a BanyanDB cluster since it provides bindings on pv & network identifier. Why don't we use StatefulSet now?
   


-- 
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


[GitHub] [skywalking-swck] SzyWilliam commented on pull request #70: [Feature] Add BanyanDB CRD & Controllers

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #70:
URL: https://github.com/apache/skywalking-swck/pull/70#issuecomment-1244913144

   @dashanji Thanks for the reviews. @hanahmily I am trying to write e2e tests for swck banyandb. Could you please tell me how I can query banyandb pod using client tools like [cli](https://github.com/apache/skywalking-banyandb/tree/main/bydbctl)?


-- 
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