From 55512497ddd3f8d9be4e34eaa3bd1a8d8cf40c39 Mon Sep 17 00:00:00 2001 From: yandongxiao Date: Mon, 11 Mar 2024 20:52:41 +0800 Subject: [PATCH] [Enhancement] Check volume name and mount path when default volume is added Signed-off-by: yandongxiao --- .../charts/starrocks/values.yaml | 11 ++++++++-- helm-charts/charts/kube-starrocks/values.yaml | 11 ++++++++-- pkg/apis/starrocks/v1/load_type.go | 22 ++++++++++++++++++- pkg/k8sutils/k8sutils.go | 18 +++++++++++++++ pkg/k8sutils/templates/pod/mount.go | 6 ++--- pkg/k8sutils/templates/pod/mount_test.go | 12 +++------- pkg/subcontrollers/be/be_pod.go | 14 +++++++----- pkg/subcontrollers/cn/cn_pod.go | 6 ++--- pkg/subcontrollers/fe/fe_pod.go | 6 ++--- .../feproxy/feproxy_controller.go | 2 +- 10 files changed, 77 insertions(+), 31 deletions(-) diff --git a/helm-charts/charts/kube-starrocks/charts/starrocks/values.yaml b/helm-charts/charts/kube-starrocks/charts/starrocks/values.yaml index f94eb2fe..d7a94f0d 100644 --- a/helm-charts/charts/kube-starrocks/charts/starrocks/values.yaml +++ b/helm-charts/charts/kube-starrocks/charts/starrocks/values.yaml @@ -212,6 +212,9 @@ starrocksFESpec: # fe storageSpec for persistent metadata. # Note: Once set, the following fields will not be allowed to be modified. storageSpec: + # Specifies the name of the volume to mount. If left unspecified, an `emptyDir` volume will be used by default, which + # is ephemeral and data will be lost on pod restart. For persistent storage, it is recommended to specify a volume name. + # For example, using `fe` as the name would be appropriate. name: "" # the storageClassName represent the used storageclass name. if not set will use k8s cluster default storageclass. # you must set name when you set storageClassName @@ -433,7 +436,9 @@ starrocksCnSpec: # specify storageclass name and request size. # Note: Once set, the following fields will not be allowed to be modified. storageSpec: - # the name of volume for mount. if not will use emptyDir. + # Specifies the name of the volume to mount. If left unspecified, an `emptyDir` volume will be used by default, which + # is ephemeral and data will be lost on pod restart. For persistent storage, it is recommended to specify a volume name. + # For example, using `cn` as the name would be appropriate. name: "" # the storageClassName represent the used storageclass name. if not set will use k8s cluster default storageclass. # you must set name when you set storageClassName @@ -617,7 +622,9 @@ starrocksBeSpec: # be storageSpec for persistent storage. # Note: Once set, the following fields will not be allowed to be modified. storageSpec: - # the name of volume for mount. if not will use emptyDir. + # Specifies the name of the volume to mount. If left unspecified, an `emptyDir` volume will be used by default, which + # is ephemeral and data will be lost on pod restart. For persistent storage, it is recommended to specify a volume name. + # For example, using `be` as the name would be appropriate. name: "" # the storageClassName represent the used storageclass name. if not set will use k8s cluster default storageclass. # you must set name when you set storageClassName diff --git a/helm-charts/charts/kube-starrocks/values.yaml b/helm-charts/charts/kube-starrocks/values.yaml index bbd1f282..e6536f3d 100644 --- a/helm-charts/charts/kube-starrocks/values.yaml +++ b/helm-charts/charts/kube-starrocks/values.yaml @@ -309,6 +309,9 @@ starrocks: # fe storageSpec for persistent metadata. # Note: Once set, the following fields will not be allowed to be modified. storageSpec: + # Specifies the name of the volume to mount. If left unspecified, an `emptyDir` volume will be used by default, which + # is ephemeral and data will be lost on pod restart. For persistent storage, it is recommended to specify a volume name. + # For example, using `fe` as the name would be appropriate. name: "" # the storageClassName represent the used storageclass name. if not set will use k8s cluster default storageclass. # you must set name when you set storageClassName @@ -530,7 +533,9 @@ starrocks: # specify storageclass name and request size. # Note: Once set, the following fields will not be allowed to be modified. storageSpec: - # the name of volume for mount. if not will use emptyDir. + # Specifies the name of the volume to mount. If left unspecified, an `emptyDir` volume will be used by default, which + # is ephemeral and data will be lost on pod restart. For persistent storage, it is recommended to specify a volume name. + # For example, using `cn` as the name would be appropriate. name: "" # the storageClassName represent the used storageclass name. if not set will use k8s cluster default storageclass. # you must set name when you set storageClassName @@ -714,7 +719,9 @@ starrocks: # be storageSpec for persistent storage. # Note: Once set, the following fields will not be allowed to be modified. storageSpec: - # the name of volume for mount. if not will use emptyDir. + # Specifies the name of the volume to mount. If left unspecified, an `emptyDir` volume will be used by default, which + # is ephemeral and data will be lost on pod restart. For persistent storage, it is recommended to specify a volume name. + # For example, using `be` as the name would be appropriate. name: "" # the storageClassName represent the used storageclass name. if not set will use k8s cluster default storageclass. # you must set name when you set storageClassName diff --git a/pkg/apis/starrocks/v1/load_type.go b/pkg/apis/starrocks/v1/load_type.go index 2f1d3c86..eb9f34ce 100644 --- a/pkg/apis/starrocks/v1/load_type.go +++ b/pkg/apis/starrocks/v1/load_type.go @@ -77,7 +77,27 @@ type StarRocksLoadSpec struct { // +optional Service *StarRocksService `json:"service,omitempty"` - // StorageVolumes defines the additional storage for meta storage. + // StorageVolumes defines the additional storage for meta or storage. + // + // For FE components + // If the storage volume name is fe-meta or the volume mount path is /opt/starrocks/fe/meta, + // then it will be recognized as storing the configuration of fe meta. + // If the storage volume name is fe-log or the volume mount path is /opt/starrocks/fe/log, + // then it will be recognized as storing the log of fe. + // + // For BE components + // If the storage volume name is be-storage(or be-data) or the volume mount path is /opt/starrocks/be/storage, + // then it will be recognized as storing the data of be. + // If the storage volume name is be-log or the volume mount path is /opt/starrocks/be/log, + // then it will be recognized as storing the log of be. + // + // For CN components + // If the storage volume name is cn-log or the volume mount path is /opt/starrocks/cn/log, + // then it will be recognized as storing the log of cn. + // + // If operator can't find the storage volume name or the volume mount path in the storage volume, it will create + // a default storage volume by using emptyDir. + // // +optional StorageVolumes []StorageVolume `json:"storageVolumes,omitempty"` diff --git a/pkg/k8sutils/k8sutils.go b/pkg/k8sutils/k8sutils.go index 55fcb25d..82044a1b 100644 --- a/pkg/k8sutils/k8sutils.go +++ b/pkg/k8sutils/k8sutils.go @@ -394,6 +394,24 @@ func GetValueFromSecret(ctx context.Context, k8sClient client.Client, namespace return string(value), nil } +func HasVolume(volumes []corev1.Volume, newVolumeName string) bool { + for _, v := range volumes { + if v.Name == newVolumeName { + return true + } + } + return false +} + +func HasMountPath(mounts []corev1.VolumeMount, newMountPath string) bool { + for _, v := range mounts { + if v.Name == newMountPath { + return true + } + } + return false +} + func CheckVolumes(volumes []corev1.Volume, mounts []corev1.VolumeMount) error { // check mount path first mountPaths := make(map[string]bool) diff --git a/pkg/k8sutils/templates/pod/mount.go b/pkg/k8sutils/templates/pod/mount.go index f55bc06d..ddf181f0 100644 --- a/pkg/k8sutils/templates/pod/mount.go +++ b/pkg/k8sutils/templates/pod/mount.go @@ -17,15 +17,13 @@ func IsSpecialStorageClass(storageClassName *string) bool { // MountStorageVolumes parse StorageVolumes from spec and mount them to pod. // If StorageClassName is EmptyDir, mount an emptyDir volume to pod. -func MountStorageVolumes(spec v1.SpecInterface) ([]corev1.Volume, []corev1.VolumeMount, map[string]bool) { +func MountStorageVolumes(spec v1.SpecInterface) ([]corev1.Volume, []corev1.VolumeMount) { var volumes []corev1.Volume var volumeMounts []corev1.VolumeMount - vexist := make(map[string]bool) for _, sv := range spec.GetStorageVolumes() { if strings.HasPrefix(sv.StorageSize, "0") { continue } - vexist[sv.MountPath] = true if IsSpecialStorageClass(sv.StorageClassName) { switch *sv.StorageClassName { case v1.EmptyDir: @@ -37,7 +35,7 @@ func MountStorageVolumes(spec v1.SpecInterface) ([]corev1.Volume, []corev1.Volum volumes, volumeMounts = MountPersistentVolumeClaim(volumes, volumeMounts, sv.Name, sv.MountPath, sv.SubPath) } } - return volumes, volumeMounts, vexist + return volumes, volumeMounts } func MountPersistentVolumeClaim(volumes []corev1.Volume, volumeMounts []corev1.VolumeMount, diff --git a/pkg/k8sutils/templates/pod/mount_test.go b/pkg/k8sutils/templates/pod/mount_test.go index cff4621d..3a71a493 100644 --- a/pkg/k8sutils/templates/pod/mount_test.go +++ b/pkg/k8sutils/templates/pod/mount_test.go @@ -4,8 +4,9 @@ import ( "reflect" "testing" - v1 "github.com/StarRocks/starrocks-kubernetes-operator/pkg/apis/starrocks/v1" corev1 "k8s.io/api/core/v1" + + v1 "github.com/StarRocks/starrocks-kubernetes-operator/pkg/apis/starrocks/v1" ) func TestMountConfigMapInfo(t *testing.T) { @@ -211,7 +212,6 @@ func TestMountStorageVolumes(t *testing.T) { args args want []corev1.Volume want1 []corev1.VolumeMount - want2 map[string]bool }{ { name: "test mount storage volumes", @@ -247,23 +247,17 @@ func TestMountStorageVolumes(t *testing.T) { MountPath: "/pkg/mounts/volumes1", }, }, - want2: map[string]bool{ - "/pkg/mounts/volumes1": true, - }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, got1, got2 := MountStorageVolumes(tt.args.spec) + got, got1 := MountStorageVolumes(tt.args.spec) if !reflect.DeepEqual(got, tt.want) { t.Errorf("MountStorageVolumes() got = %v, want %v", got, tt.want) } if !reflect.DeepEqual(got1, tt.want1) { t.Errorf("MountStorageVolumes() got1 = %v, want %v", got1, tt.want1) } - if !reflect.DeepEqual(got2, tt.want2) { - t.Errorf("MountStorageVolumes() got2 = %v, want %v", got2, tt.want2) - } }) } } diff --git a/pkg/subcontrollers/be/be_pod.go b/pkg/subcontrollers/be/be_pod.go index cd7300cf..217aeb3d 100644 --- a/pkg/subcontrollers/be/be_pod.go +++ b/pkg/subcontrollers/be/be_pod.go @@ -32,6 +32,7 @@ const ( _logName = "be-log" _beConfigPath = "/etc/starrocks/be/conf" _storageName = "be-storage" + _storageName2 = "be-data" // helm chart use this format _storagePath = "/opt/starrocks/be/storage" _envBeConfigPath = "CONFIGMAP_MOUNT_PATH" ) @@ -41,14 +42,15 @@ func (be *BeController) buildPodTemplate(src *srapi.StarRocksCluster, config map metaName := src.Name + "-" + srapi.DEFAULT_BE beSpec := src.Spec.StarRocksBeSpec - vols, volumeMounts, vexist := pod.MountStorageVolumes(beSpec) - // add default volume about log, if meta not configure. - if _, ok := vexist[_logPath]; !ok { - vols, volumeMounts = pod.MountEmptyDirVolume(vols, volumeMounts, _logName, _logPath, "") - } - if _, ok := vexist[_storagePath]; !ok { + vols, volumeMounts := pod.MountStorageVolumes(beSpec) + + if !k8sutils.HasVolume(vols, _storageName) && !k8sutils.HasVolume(vols, _storageName2) && + !k8sutils.HasMountPath(volumeMounts, _storagePath) { vols, volumeMounts = pod.MountEmptyDirVolume(vols, volumeMounts, _storageName, _storagePath, "") } + if !k8sutils.HasVolume(vols, _logName) && !k8sutils.HasMountPath(volumeMounts, _logPath) { + vols, volumeMounts = pod.MountEmptyDirVolume(vols, volumeMounts, _logName, _logPath, "") + } // mount configmap, secrets to pod if needed vols, volumeMounts = pod.MountConfigMapInfo(vols, volumeMounts, beSpec.ConfigMapInfo, _beConfigPath) diff --git a/pkg/subcontrollers/cn/cn_pod.go b/pkg/subcontrollers/cn/cn_pod.go index df0bed99..4330fc4d 100644 --- a/pkg/subcontrollers/cn/cn_pod.go +++ b/pkg/subcontrollers/cn/cn_pod.go @@ -46,9 +46,9 @@ const ( // buildPodTemplate construct the podTemplate for deploy cn. func (cc *CnController) buildPodTemplate(ctx context.Context, object srobject.StarRocksObject, cnSpec *srapi.StarRocksCnSpec, config map[string]interface{}) (*corev1.PodTemplateSpec, error) { - vols, volumeMounts, vexist := pod.MountStorageVolumes(cnSpec) - // add default volume about log - if _, ok := vexist[_logPath]; !ok { + vols, volumeMounts := pod.MountStorageVolumes(cnSpec) + + if !k8sutils.HasVolume(vols, _logName) && !k8sutils.HasMountPath(volumeMounts, _logPath) { vols, volumeMounts = pod.MountEmptyDirVolume(vols, volumeMounts, _logName, _logPath, "") } diff --git a/pkg/subcontrollers/fe/fe_pod.go b/pkg/subcontrollers/fe/fe_pod.go index 3af63b27..2d2ef2c1 100644 --- a/pkg/subcontrollers/fe/fe_pod.go +++ b/pkg/subcontrollers/fe/fe_pod.go @@ -41,12 +41,12 @@ func (fc *FeController) buildPodTemplate(src *srapi.StarRocksCluster, config map metaName := src.Name + "-" + srapi.DEFAULT_FE feSpec := src.Spec.StarRocksFeSpec - vols, volMounts, vexist := pod.MountStorageVolumes(feSpec) + vols, volMounts := pod.MountStorageVolumes(feSpec) // add default volume about log, meta if not configure. - if _, ok := vexist[_metaPath]; !ok { + if !k8sutils.HasVolume(vols, _metaName) && !k8sutils.HasMountPath(volMounts, _metaPath) { vols, volMounts = pod.MountEmptyDirVolume(vols, volMounts, _metaName, _metaPath, "") } - if _, ok := vexist[_logPath]; !ok { + if !k8sutils.HasVolume(vols, _logName) && !k8sutils.HasMountPath(volMounts, _logPath) { vols, volMounts = pod.MountEmptyDirVolume(vols, volMounts, _logName, _logPath, "") } diff --git a/pkg/subcontrollers/feproxy/feproxy_controller.go b/pkg/subcontrollers/feproxy/feproxy_controller.go index 8d048814..f08d1a76 100644 --- a/pkg/subcontrollers/feproxy/feproxy_controller.go +++ b/pkg/subcontrollers/feproxy/feproxy_controller.go @@ -195,7 +195,7 @@ func (controller *FeProxyController) ClearResources(ctx context.Context, src *sr func (controller *FeProxyController) buildPodTemplate(src *srapi.StarRocksCluster) corev1.PodTemplateSpec { feProxySpec := src.Spec.StarRocksFeProxySpec - vols, volumeMounts, _ := pod.MountStorageVolumes(feProxySpec) + vols, volumeMounts := pod.MountStorageVolumes(feProxySpec) vols, volumeMounts = pod.MountConfigMaps(vols, volumeMounts, []srapi.ConfigMapReference{ {