From a7af3148c74f4f279ab2b03757ee1d7163026969 Mon Sep 17 00:00:00 2001 From: Abdul Hameed Date: Mon, 2 Dec 2024 18:29:33 -0500 Subject: [PATCH 1/7] fix: Feast Operator updated the kustomize version to v5.5.0 Signed-off-by: Abdul Hameed --- infra/feast-operator/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infra/feast-operator/Makefile b/infra/feast-operator/Makefile index 310d64afaaa..d114ada9aa3 100644 --- a/infra/feast-operator/Makefile +++ b/infra/feast-operator/Makefile @@ -215,7 +215,7 @@ ENVTEST ?= $(LOCALBIN)/setup-envtest-$(ENVTEST_VERSION) GOLANGCI_LINT = $(LOCALBIN)/golangci-lint-$(GOLANGCI_LINT_VERSION) ## Tool Versions -KUSTOMIZE_VERSION ?= v5.3.0 +KUSTOMIZE_VERSION ?= v5.5.0 CONTROLLER_TOOLS_VERSION ?= v0.14.0 ENVTEST_VERSION ?= release-0.17 GOLANGCI_LINT_VERSION ?= v1.57.2 From 609e9a853b526fa1af2a1b057bf46b7213c22cb2 Mon Sep 17 00:00:00 2001 From: Abdul Hameed Date: Tue, 3 Dec 2024 16:27:33 -0500 Subject: [PATCH 2/7] feat : Feast Operator support log level configuration for services Signed-off-by: Abdul Hameed --- .../api/v1alpha1/featurestore_types.go | 12 ++++ .../crd/bases/feast.dev_featurestores.yaml | 66 +++++++++++++++++++ infra/feast-operator/dist/install.yaml | 66 +++++++++++++++++++ .../internal/controller/services/services.go | 24 +++++++ 4 files changed, 168 insertions(+) diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index 17a029c02ea..f256435d7e4 100644 --- a/infra/feast-operator/api/v1alpha1/featurestore_types.go +++ b/infra/feast-operator/api/v1alpha1/featurestore_types.go @@ -77,6 +77,10 @@ type OfflineStore struct { ServiceConfigs `json:",inline"` Persistence *OfflineStorePersistence `json:"persistence,omitempty"` TLS *OfflineTlsConfigs `json:"tls,omitempty"` + // LogLevel sets the logging level for the offline store service + // Allowed values: "debug", "info", "warning", "error", "critical". + // +kubebuilder:validation:Enum=debug;info;warning;error;critical + LogLevel string `json:"logLevel,omitempty"` } // OfflineTlsConfigs configures server TLS for the offline feast service. in an openshift cluster, this is configured by default using service serving certificates. @@ -130,6 +134,10 @@ type OnlineStore struct { ServiceConfigs `json:",inline"` Persistence *OnlineStorePersistence `json:"persistence,omitempty"` TLS *TlsConfigs `json:"tls,omitempty"` + // LogLevel sets the logging level for the online store service + // Allowed values: "debug", "info", "warning", "error", "critical". + // +kubebuilder:validation:Enum=debug;info;warning;error;critical + LogLevel string `json:"logLevel,omitempty"` } // OnlineStorePersistence configures the persistence settings for the online store service @@ -246,6 +254,10 @@ type PvcCreate struct { type Registry struct { Local *LocalRegistryConfig `json:"local,omitempty"` Remote *RemoteRegistryConfig `json:"remote,omitempty"` + // LogLevel sets the logging level for the registry service + // Allowed values: "debug", "info", "warning", "error", "critical". + // +kubebuilder:validation:Enum=debug;info;warning;error;critical + LogLevel string `json:"logLevel,omitempty"` } // RemoteRegistryConfig points to a remote feast registry server. When set, the operator will not deploy a registry for this FeatureStore CR. diff --git a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml index 1402a64056c..d8fc63c6697 100644 --- a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml +++ b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml @@ -227,6 +227,17 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + logLevel: + description: |- + LogLevel sets the logging level for the offline store service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string persistence: description: OfflineStorePersistence configures the persistence settings for the offline store service @@ -576,6 +587,17 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + logLevel: + description: |- + LogLevel sets the logging level for the online store service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string persistence: description: OnlineStorePersistence configures the persistence settings for the online store service @@ -1180,6 +1202,17 @@ spec: rule: '(!has(self.disable) || !self.disable) ? has(self.secretRef) : true' type: object + logLevel: + description: |- + LogLevel sets the logging level for the registry service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string remote: description: |- RemoteRegistryConfig points to a remote feast registry server. When set, the operator will not deploy a registry for this FeatureStore CR. @@ -1429,6 +1462,17 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + logLevel: + description: |- + LogLevel sets the logging level for the offline store service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string persistence: description: OfflineStorePersistence configures the persistence settings for the offline store service @@ -1784,6 +1828,17 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + logLevel: + description: |- + LogLevel sets the logging level for the online store service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string persistence: description: OnlineStorePersistence configures the persistence settings for the online store service @@ -2400,6 +2455,17 @@ spec: rule: '(!has(self.disable) || !self.disable) ? has(self.secretRef) : true' type: object + logLevel: + description: |- + LogLevel sets the logging level for the registry service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string remote: description: |- RemoteRegistryConfig points to a remote feast registry server. When set, the operator will not deploy a registry for this FeatureStore CR. diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index 18ab82e9ca2..f07090cbaca 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -235,6 +235,17 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + logLevel: + description: |- + LogLevel sets the logging level for the offline store service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string persistence: description: OfflineStorePersistence configures the persistence settings for the offline store service @@ -584,6 +595,17 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + logLevel: + description: |- + LogLevel sets the logging level for the online store service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string persistence: description: OnlineStorePersistence configures the persistence settings for the online store service @@ -1188,6 +1210,17 @@ spec: rule: '(!has(self.disable) || !self.disable) ? has(self.secretRef) : true' type: object + logLevel: + description: |- + LogLevel sets the logging level for the registry service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string remote: description: |- RemoteRegistryConfig points to a remote feast registry server. When set, the operator will not deploy a registry for this FeatureStore CR. @@ -1437,6 +1470,17 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + logLevel: + description: |- + LogLevel sets the logging level for the offline store service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string persistence: description: OfflineStorePersistence configures the persistence settings for the offline store service @@ -1792,6 +1836,17 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + logLevel: + description: |- + LogLevel sets the logging level for the online store service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string persistence: description: OnlineStorePersistence configures the persistence settings for the online store service @@ -2408,6 +2463,17 @@ spec: rule: '(!has(self.disable) || !self.disable) ? has(self.secretRef) : true' type: object + logLevel: + description: |- + LogLevel sets the logging level for the registry service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string remote: description: |- RemoteRegistryConfig points to a remote feast registry server. When set, the operator will not deploy a registry for this FeatureStore CR. diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index b1878ee00ae..6e4d48f3698 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -358,6 +358,11 @@ func (feast *FeastServices) getContainerCommand(feastType FeastServiceType) []st deploySettings := FeastServiceConstants[feastType] targetPort := deploySettings.TargetHttpPort tls := feast.getTlsConfigs(feastType) + logLevel := feast.getLogLevelForType(feastType) + if logLevel != nil { + deploySettings.Command = append(deploySettings.Command, "--log-level="+strings.ToUpper(*logLevel)) + } + if tls.IsTLS() { targetPort = deploySettings.TargetHttpsPort feastTlsPath := GetTlsPath(feastType) @@ -474,6 +479,25 @@ func (feast *FeastServices) getServiceConfigs(feastType FeastServiceType) feastd return feastdevv1alpha1.ServiceConfigs{} } +func (feast *FeastServices) getLogLevelForType(feastType FeastServiceType) *string { + services := feast.Handler.FeatureStore.Status.Applied.Services + switch feastType { + case OfflineFeastType: + if services.OfflineStore != nil && services.OfflineStore.LogLevel != "" { + return &services.OfflineStore.LogLevel + } + case OnlineFeastType: + if services.OnlineStore != nil && services.OnlineStore.LogLevel != "" { + return &services.OnlineStore.LogLevel + } + case RegistryFeastType: + if services.Registry != nil && services.Registry.LogLevel != "" { + return &services.Registry.LogLevel + } + } + return nil +} + // GetObjectMeta returns the feast k8s object metadata func (feast *FeastServices) GetObjectMeta(feastType FeastServiceType) metav1.ObjectMeta { return metav1.ObjectMeta{Name: feast.GetFeastServiceName(feastType), Namespace: feast.Handler.FeatureStore.Namespace} From 94acc49a2529de717a0ead4c9194e47ad36f563b Mon Sep 17 00:00:00 2001 From: Abdul Hameed Date: Wed, 4 Dec 2024 11:01:19 -0500 Subject: [PATCH 3/7] Update infra/feast-operator/internal/controller/services/services.go Co-authored-by: Tommy Hughes IV Signed-off-by: Abdul Hameed --- infra/feast-operator/internal/controller/services/services.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 6e4d48f3698..c2e04f5f1e7 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -357,12 +357,11 @@ func (feast *FeastServices) setDeployment(deploy *appsv1.Deployment, feastType F func (feast *FeastServices) getContainerCommand(feastType FeastServiceType) []string { deploySettings := FeastServiceConstants[feastType] targetPort := deploySettings.TargetHttpPort - tls := feast.getTlsConfigs(feastType) logLevel := feast.getLogLevelForType(feastType) if logLevel != nil { deploySettings.Command = append(deploySettings.Command, "--log-level="+strings.ToUpper(*logLevel)) } - + tls := feast.getTlsConfigs(feastType) if tls.IsTLS() { targetPort = deploySettings.TargetHttpsPort feastTlsPath := GetTlsPath(feastType) From 7ea29c54e7764e2adf8620f12935b7a711d7ebc3 Mon Sep 17 00:00:00 2001 From: Abdul Hameed Date: Wed, 4 Dec 2024 11:08:58 -0500 Subject: [PATCH 4/7] Update infra/feast-operator/internal/controller/services/services.go Co-authored-by: Tommy Hughes IV Signed-off-by: Abdul Hameed --- infra/feast-operator/internal/controller/services/services.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index c2e04f5f1e7..2b55e581645 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -359,7 +359,7 @@ func (feast *FeastServices) getContainerCommand(feastType FeastServiceType) []st targetPort := deploySettings.TargetHttpPort logLevel := feast.getLogLevelForType(feastType) if logLevel != nil { - deploySettings.Command = append(deploySettings.Command, "--log-level="+strings.ToUpper(*logLevel)) + deploySettings.Command = append(deploySettings.Command, []string{"--log-level", strings.ToUpper(*logLevel)}...) } tls := feast.getTlsConfigs(feastType) if tls.IsTLS() { From 9fb533930c51dcc471b2df3a1ba6db811a4c4fe4 Mon Sep 17 00:00:00 2001 From: Abdul Hameed Date: Wed, 4 Dec 2024 15:41:54 -0500 Subject: [PATCH 5/7] added unit test for loglevel Signed-off-by: Abdul Hameed --- infra/feast-operator/Makefile | 2 +- .../featurestore_controller_loglevel_test.go | 196 ++++++++++++++++++ 2 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 infra/feast-operator/internal/controller/featurestore_controller_loglevel_test.go diff --git a/infra/feast-operator/Makefile b/infra/feast-operator/Makefile index d114ada9aa3..310d64afaaa 100644 --- a/infra/feast-operator/Makefile +++ b/infra/feast-operator/Makefile @@ -215,7 +215,7 @@ ENVTEST ?= $(LOCALBIN)/setup-envtest-$(ENVTEST_VERSION) GOLANGCI_LINT = $(LOCALBIN)/golangci-lint-$(GOLANGCI_LINT_VERSION) ## Tool Versions -KUSTOMIZE_VERSION ?= v5.5.0 +KUSTOMIZE_VERSION ?= v5.3.0 CONTROLLER_TOOLS_VERSION ?= v0.14.0 ENVTEST_VERSION ?= release-0.17 GOLANGCI_LINT_VERSION ?= v1.57.2 diff --git a/infra/feast-operator/internal/controller/featurestore_controller_loglevel_test.go b/infra/feast-operator/internal/controller/featurestore_controller_loglevel_test.go new file mode 100644 index 00000000000..681d8254fb0 --- /dev/null +++ b/infra/feast-operator/internal/controller/featurestore_controller_loglevel_test.go @@ -0,0 +1,196 @@ +/* +Copyright 2024 Feast Community. + +Licensed 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 controller + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" + "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/feast-dev/feast/infra/feast-operator/api/feastversion" + feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1" + "github.com/feast-dev/feast/infra/feast-operator/internal/controller/handler" + "github.com/feast-dev/feast/infra/feast-operator/internal/controller/services" +) + +var _ = Describe("FeatureStore Controller - Feast service LogLevel", func() { + Context("When reconciling a FeatureStore resource", func() { + const resourceName = "test-loglevel" + + ctx := context.Background() + + typeNamespacedName := types.NamespacedName{ + Name: resourceName, + Namespace: "default", + } + featurestore := &feastdevv1alpha1.FeatureStore{} + + BeforeEach(func() { + By("creating the custom resource for the Kind FeatureStore") + err := k8sClient.Get(ctx, typeNamespacedName, featurestore) + if err != nil && errors.IsNotFound(err) { + resource := &feastdevv1alpha1.FeatureStore{ + ObjectMeta: metav1.ObjectMeta{ + Name: resourceName, + Namespace: "default", + }, + Spec: feastdevv1alpha1.FeatureStoreSpec{ + FeastProject: feastProject, + Services: &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{}, + }, + OnlineStore: &feastdevv1alpha1.OnlineStore{ + LogLevel: "debug", + }, + OfflineStore: &feastdevv1alpha1.OfflineStore{ + LogLevel: "info", + }, + }, + }, + } + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + } + }) + AfterEach(func() { + resource := &feastdevv1alpha1.FeatureStore{} + err := k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + + By("Cleanup the specific resource instance FeatureStore") + Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + }) + + It("should successfully reconcile the resource", func() { + By("Reconciling the created resource") + controllerReconciler := &FeatureStoreReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + resource := &feastdevv1alpha1.FeatureStore{} + err = k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + feast := services.FeastServices{ + Handler: handler.FeastHandler{ + Client: controllerReconciler.Client, + Context: ctx, + Scheme: controllerReconciler.Scheme, + FeatureStore: resource, + }, + } + + Expect(resource.Status).NotTo(BeNil()) + Expect(resource.Status.FeastVersion).To(Equal(feastversion.FeastVersion)) + Expect(resource.Status.Applied.FeastProject).To(Equal(resource.Spec.FeastProject)) + Expect(resource.Status.Applied.Services).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OnlineStore).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.Registry).NotTo(BeNil()) + + Expect(resource.Status.Conditions).NotTo(BeEmpty()) + cond := apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.ReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.ReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.ReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.RegistryReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.RegistryReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.RegistryReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.ClientReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.ClientReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.ClientReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.OfflineStoreReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.OfflineStoreReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.OfflineStoreReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.OnlineStoreReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.OnlineStoreReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.OnlineStoreReadyMessage)) + Expect(resource.Status.Phase).To(Equal(feastdevv1alpha1.ReadyPhase)) + + deploy := &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.RegistryFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Replicas).To(Equal(&services.DefaultReplicas)) + Expect(controllerutil.HasControllerReference(deploy)).To(BeTrue()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + command := deploy.Spec.Template.Spec.Containers[0].Command + Expect(command).NotTo(ContainElement("--log-level")) + + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OfflineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Replicas).To(Equal(&services.DefaultReplicas)) + Expect(controllerutil.HasControllerReference(deploy)).To(BeTrue()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + command = deploy.Spec.Template.Spec.Containers[0].Command + Expect(command).To(ContainElement("--log-level")) + Expect(command).To(ContainElement("INFO")) + + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OnlineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Replicas).To(Equal(&services.DefaultReplicas)) + Expect(controllerutil.HasControllerReference(deploy)).To(BeTrue()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + command = deploy.Spec.Template.Spec.Containers[0].Command + Expect(command).To(ContainElement("--log-level")) + Expect(command).To(ContainElement("DEBUG")) + }) + + }) +}) From fa79e952097cd6467bb1820c6ae3f3f78ce871dc Mon Sep 17 00:00:00 2001 From: Abdul Hameed Date: Thu, 5 Dec 2024 13:56:28 -0500 Subject: [PATCH 6/7] fix the loglevel command Signed-off-by: Abdul Hameed --- ...alpha1_featurestore_services_loglevel.yaml | 17 +++++++++++++++ .../internal/controller/services/services.go | 21 ++++++++++++------- .../controller/services/services_types.go | 8 +++---- 3 files changed, 35 insertions(+), 11 deletions(-) create mode 100644 infra/feast-operator/config/samples/v1alpha1_featurestore_services_loglevel.yaml diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_services_loglevel.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_services_loglevel.yaml new file mode 100644 index 00000000000..cf8211b3f7b --- /dev/null +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_services_loglevel.yaml @@ -0,0 +1,17 @@ +apiVersion: feast.dev/v1alpha1 +kind: FeatureStore +metadata: + name: sample-services-loglevel +spec: + feastProject: my_project + services: + onlineStore: + logLevel: debug + offlineStore: + logLevel: debug + registry: + logLevel: info + local: + persistence: + file: + path: /data/registry.db \ No newline at end of file diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 2b55e581645..a6fc1c07de2 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -355,29 +355,36 @@ func (feast *FeastServices) setDeployment(deploy *appsv1.Deployment, feastType F } func (feast *FeastServices) getContainerCommand(feastType FeastServiceType) []string { - deploySettings := FeastServiceConstants[feastType] - targetPort := deploySettings.TargetHttpPort + baseCommand := "feast" + options := []string{} logLevel := feast.getLogLevelForType(feastType) if logLevel != nil { - deploySettings.Command = append(deploySettings.Command, []string{"--log-level", strings.ToUpper(*logLevel)}...) + options = append(options, "--log-level", strings.ToUpper(*logLevel)) } + + deploySettings := FeastServiceConstants[feastType] + targetPort := deploySettings.TargetHttpPort tls := feast.getTlsConfigs(feastType) if tls.IsTLS() { targetPort = deploySettings.TargetHttpsPort feastTlsPath := GetTlsPath(feastType) - deploySettings.Command = append(deploySettings.Command, []string{"--key", feastTlsPath + tls.SecretKeyNames.TlsKey, + deploySettings.Args = append(deploySettings.Args, []string{"--key", feastTlsPath + tls.SecretKeyNames.TlsKey, "--cert", feastTlsPath + tls.SecretKeyNames.TlsCrt}...) } - deploySettings.Command = append(deploySettings.Command, []string{"-p", strconv.Itoa(int(targetPort))}...) + deploySettings.Args = append(deploySettings.Args, []string{"-p", strconv.Itoa(int(targetPort))}...) if feastType == OfflineFeastType { if tls.IsTLS() && feast.Handler.FeatureStore.Status.Applied.Services.OfflineStore.TLS.VerifyClient != nil { - deploySettings.Command = append(deploySettings.Command, + deploySettings.Args = append(deploySettings.Args, []string{"--verify_client", strconv.FormatBool(*feast.Handler.FeatureStore.Status.Applied.Services.OfflineStore.TLS.VerifyClient)}...) } } - return deploySettings.Command + // Combine base command, options, and arguments + feastCommand := append([]string{baseCommand}, options...) + feastCommand = append(feastCommand, deploySettings.Args...) + + return feastCommand } func (feast *FeastServices) offlineClientPodConfigs(podSpec *corev1.PodSpec) { diff --git a/infra/feast-operator/internal/controller/services/services_types.go b/infra/feast-operator/internal/controller/services/services_types.go index 2c454459d88..b7c0f5f048b 100644 --- a/infra/feast-operator/internal/controller/services/services_types.go +++ b/infra/feast-operator/internal/controller/services/services_types.go @@ -87,17 +87,17 @@ var ( FeastServiceConstants = map[FeastServiceType]deploymentSettings{ OfflineFeastType: { - Command: []string{"feast", "serve_offline", "-h", "0.0.0.0"}, + Args: []string{"serve_offline", "-h", "0.0.0.0"}, TargetHttpPort: 8815, TargetHttpsPort: 8816, }, OnlineFeastType: { - Command: []string{"feast", "serve", "-h", "0.0.0.0"}, + Args: []string{"serve", "-h", "0.0.0.0"}, TargetHttpPort: 6566, TargetHttpsPort: 6567, }, RegistryFeastType: { - Command: []string{"feast", "serve_registry"}, + Args: []string{"serve_registry"}, TargetHttpPort: 6570, TargetHttpsPort: 6571, }, @@ -234,7 +234,7 @@ type AuthzConfig struct { } type deploymentSettings struct { - Command []string + Args []string TargetHttpPort int32 TargetHttpsPort int32 } From fc4fc791bb8dc06bbf755831a769ea2cea14d5c4 Mon Sep 17 00:00:00 2001 From: Abdul Hameed Date: Thu, 5 Dec 2024 16:50:06 -0500 Subject: [PATCH 7/7] moved the logLevel under LocalRegistryConfig and updated testcase to validate it Signed-off-by: Abdul Hameed --- .../api/v1alpha1/featurestore_types.go | 8 +-- .../crd/bases/feast.dev_featurestores.yaml | 44 ++++++------ ...alpha1_featurestore_services_loglevel.yaml | 6 +- infra/feast-operator/dist/install.yaml | 46 ++++++------ .../featurestore_controller_loglevel_test.go | 72 ++++++++++++++++++- .../internal/controller/services/services.go | 4 +- 6 files changed, 122 insertions(+), 58 deletions(-) diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index f256435d7e4..0b516630cd2 100644 --- a/infra/feast-operator/api/v1alpha1/featurestore_types.go +++ b/infra/feast-operator/api/v1alpha1/featurestore_types.go @@ -185,6 +185,10 @@ type LocalRegistryConfig struct { ServiceConfigs `json:",inline"` Persistence *RegistryPersistence `json:"persistence,omitempty"` TLS *TlsConfigs `json:"tls,omitempty"` + // LogLevel sets the logging level for the registry service + // Allowed values: "debug", "info", "warning", "error", "critical". + // +kubebuilder:validation:Enum=debug;info;warning;error;critical + LogLevel string `json:"logLevel,omitempty"` } // RegistryPersistence configures the persistence settings for the registry service @@ -254,10 +258,6 @@ type PvcCreate struct { type Registry struct { Local *LocalRegistryConfig `json:"local,omitempty"` Remote *RemoteRegistryConfig `json:"remote,omitempty"` - // LogLevel sets the logging level for the registry service - // Allowed values: "debug", "info", "warning", "error", "critical". - // +kubebuilder:validation:Enum=debug;info;warning;error;critical - LogLevel string `json:"logLevel,omitempty"` } // RemoteRegistryConfig points to a remote feast registry server. When set, the operator will not deploy a registry for this FeatureStore CR. diff --git a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml index d8fc63c6697..6929796d34f 100644 --- a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml +++ b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml @@ -961,6 +961,17 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + logLevel: + description: |- + LogLevel sets the logging level for the registry service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string persistence: description: RegistryPersistence configures the persistence settings for the registry service @@ -1202,17 +1213,6 @@ spec: rule: '(!has(self.disable) || !self.disable) ? has(self.secretRef) : true' type: object - logLevel: - description: |- - LogLevel sets the logging level for the registry service - Allowed values: "debug", "info", "warning", "error", "critical". - enum: - - debug - - info - - warning - - error - - critical - type: string remote: description: |- RemoteRegistryConfig points to a remote feast registry server. When set, the operator will not deploy a registry for this FeatureStore CR. @@ -2210,6 +2210,17 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + logLevel: + description: |- + LogLevel sets the logging level for the registry service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string persistence: description: RegistryPersistence configures the persistence settings for the registry service @@ -2455,17 +2466,6 @@ spec: rule: '(!has(self.disable) || !self.disable) ? has(self.secretRef) : true' type: object - logLevel: - description: |- - LogLevel sets the logging level for the registry service - Allowed values: "debug", "info", "warning", "error", "critical". - enum: - - debug - - info - - warning - - error - - critical - type: string remote: description: |- RemoteRegistryConfig points to a remote feast registry server. When set, the operator will not deploy a registry for this FeatureStore CR. diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_services_loglevel.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_services_loglevel.yaml index cf8211b3f7b..7ae96d44f01 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_services_loglevel.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_services_loglevel.yaml @@ -10,8 +10,6 @@ spec: offlineStore: logLevel: debug registry: - logLevel: info local: - persistence: - file: - path: /data/registry.db \ No newline at end of file + logLevel: info + diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index f07090cbaca..9e213994eba 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -969,6 +969,17 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + logLevel: + description: |- + LogLevel sets the logging level for the registry service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string persistence: description: RegistryPersistence configures the persistence settings for the registry service @@ -1210,17 +1221,6 @@ spec: rule: '(!has(self.disable) || !self.disable) ? has(self.secretRef) : true' type: object - logLevel: - description: |- - LogLevel sets the logging level for the registry service - Allowed values: "debug", "info", "warning", "error", "critical". - enum: - - debug - - info - - warning - - error - - critical - type: string remote: description: |- RemoteRegistryConfig points to a remote feast registry server. When set, the operator will not deploy a registry for this FeatureStore CR. @@ -2218,6 +2218,17 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + logLevel: + description: |- + LogLevel sets the logging level for the registry service + Allowed values: "debug", "info", "warning", "error", "critical". + enum: + - debug + - info + - warning + - error + - critical + type: string persistence: description: RegistryPersistence configures the persistence settings for the registry service @@ -2463,17 +2474,6 @@ spec: rule: '(!has(self.disable) || !self.disable) ? has(self.secretRef) : true' type: object - logLevel: - description: |- - LogLevel sets the logging level for the registry service - Allowed values: "debug", "info", "warning", "error", "critical". - enum: - - debug - - info - - warning - - error - - critical - type: string remote: description: |- RemoteRegistryConfig points to a remote feast registry server. When set, the operator will not deploy a registry for this FeatureStore CR. @@ -2960,7 +2960,7 @@ spec: - --leader-elect command: - /manager - image: feastdev/feast-operator:0.41.0 + image: feastdev/feast-operator:0.42.0 livenessProbe: httpGet: path: /healthz diff --git a/infra/feast-operator/internal/controller/featurestore_controller_loglevel_test.go b/infra/feast-operator/internal/controller/featurestore_controller_loglevel_test.go index 681d8254fb0..70f33486fce 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_loglevel_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_loglevel_test.go @@ -60,7 +60,9 @@ var _ = Describe("FeatureStore Controller - Feast service LogLevel", func() { FeastProject: feastProject, Services: &feastdevv1alpha1.FeatureStoreServices{ Registry: &feastdevv1alpha1.Registry{ - Local: &feastdevv1alpha1.LocalRegistryConfig{}, + Local: &feastdevv1alpha1.LocalRegistryConfig{ + LogLevel: "error", + }, }, OnlineStore: &feastdevv1alpha1.OnlineStore{ LogLevel: "debug", @@ -83,7 +85,7 @@ var _ = Describe("FeatureStore Controller - Feast service LogLevel", func() { Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) }) - It("should successfully reconcile the resource", func() { + It("should successfully reconcile the resource with logLevel", func() { By("Reconciling the created resource") controllerReconciler := &FeatureStoreReconciler{ Client: k8sClient, @@ -163,7 +165,8 @@ var _ = Describe("FeatureStore Controller - Feast service LogLevel", func() { Expect(controllerutil.HasControllerReference(deploy)).To(BeTrue()) Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) command := deploy.Spec.Template.Spec.Containers[0].Command - Expect(command).NotTo(ContainElement("--log-level")) + Expect(command).To(ContainElement("--log-level")) + Expect(command).To(ContainElement("ERROR")) err = k8sClient.Get(ctx, types.NamespacedName{ Name: feast.GetFeastServiceName(services.OfflineFeastType), @@ -192,5 +195,68 @@ var _ = Describe("FeatureStore Controller - Feast service LogLevel", func() { Expect(command).To(ContainElement("DEBUG")) }) + It("should not include --log-level parameter when logLevel is not specified for any service", func() { + By("Updating the FeatureStore resource without specifying logLevel for any service") + resource := &feastdevv1alpha1.FeatureStore{} + err := k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + + resource.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{}, + }, + OnlineStore: &feastdevv1alpha1.OnlineStore{}, + OfflineStore: &feastdevv1alpha1.OfflineStore{}, + } + Expect(k8sClient.Update(ctx, resource)).To(Succeed()) + + controllerReconciler := &FeatureStoreReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + feast := services.FeastServices{ + Handler: handler.FeastHandler{ + Client: controllerReconciler.Client, + Context: ctx, + Scheme: controllerReconciler.Scheme, + FeatureStore: resource, + }, + } + + deploy := &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.RegistryFeastType), + Namespace: resource.Namespace, + }, deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + command := deploy.Spec.Template.Spec.Containers[0].Command + Expect(command).NotTo(ContainElement("--log-level")) + + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OfflineFeastType), + Namespace: resource.Namespace, + }, deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + command = deploy.Spec.Template.Spec.Containers[0].Command + Expect(command).NotTo(ContainElement("--log-level")) + + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OnlineFeastType), + Namespace: resource.Namespace, + }, deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + command = deploy.Spec.Template.Spec.Containers[0].Command + Expect(command).NotTo(ContainElement("--log-level")) + }) + }) }) diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index a6fc1c07de2..60aabebe024 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -497,8 +497,8 @@ func (feast *FeastServices) getLogLevelForType(feastType FeastServiceType) *stri return &services.OnlineStore.LogLevel } case RegistryFeastType: - if services.Registry != nil && services.Registry.LogLevel != "" { - return &services.Registry.LogLevel + if services.Registry != nil && services.Registry.Local.LogLevel != "" { + return &services.Registry.Local.LogLevel } } return nil