diff --git a/internal/controller/config/defaults.go b/internal/controller/config/defaults.go index 8a6fd1f..00c5dfe 100644 --- a/internal/controller/config/defaults.go +++ b/internal/controller/config/defaults.go @@ -22,112 +22,18 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "text/template" - "time" ) //go:embed templates/*.yaml.tmpl var templateFS embed.FS const ( - DefaultImageValue = "MustSetInConfig" - - MLPipelineUIConfigMapPrefix = "ds-pipeline-ui-configmap-" - ArtifactScriptConfigMapNamePrefix = "ds-pipeline-artifact-script-" - ArtifactScriptConfigMapKey = "artifact_script" - DSPServicePrefix = "ds-pipeline" - - DBSecretNamePrefix = "ds-pipeline-db-" - DBSecretKey = "password" - - MariaDBName = "mlpipeline" - MariaDBHostPrefix = "mariadb" - MariaDBHostPort = "3306" - MariaDBUser = "mlpipeline" - MariaDBNamePVCSize = "10Gi" - - MinioHostPrefix = "minio" - MinioPort = "9000" - MinioScheme = "http" - MinioDefaultBucket = "mlpipeline" - MinioPVCSize = "10Gi" - - ObjectStorageSecretName = "mlpipeline-minio-artifact" // hardcoded in kfp-tekton - ObjectStorageAccessKey = "accesskey" - ObjectStorageSecretKey = "secretkey" - GrpcImage = "GRPC_IMAGE" RestImage = "REST_IMAGE" DefaultGrpcImage = "gcr.io/tfx-oss-public/ml_metadata_store_server:1.14.0" DefaultRestImage = "quay.io/opendatahub/model-registry:latest" ) -// DSPO Config File Paths -const ( - APIServerImagePath = "Images.ApiServer" - APIServerArtifactImagePath = "Images.Artifact" - PersistenceAgentImagePath = "Images.PersistentAgent" - ScheduledWorkflowImagePath = "Images.ScheduledWorkflow" - APIServerCacheImagePath = "Images.Cache" - APIServerMoveResultsImagePath = "Images.MoveResultsImage" - MariaDBImagePath = "Images.MariaDB" - OAuthProxyImagePath = "Images.OAuthProxy" - MlmdEnvoyImagePath = "Images.MlmdEnvoy" - MlmdGRPCImagePath = "Images.MlmdGRPC" - MlmdWriterImagePath = "Images.MlmdWriter" -) - -// ModelRegistry Status Condition Types -const ( - DatabaseAvailable = "DatabaseAvailable" - ObjectStoreAvailable = "ObjectStoreAvailable" - APIServerReady = "APIServerReady" - PersistenceAgentReady = "PersistenceAgentReady" - ScheduledWorkflowReady = "ScheduledWorkflowReady" - CrReady = "Ready" -) - -// ModelRegistry Ready Status Condition Reasons -// As per k8s api convention: Reason is intended -// to be used in concise output, such as one-line -// kubectl get output, and in summarizing -// occurrences of causes -const ( - MinimumReplicasAvailable = "MinimumReplicasAvailable" - FailingToDeploy = "FailingToDeploy" - Deploying = "Deploying" - ComponentDeploymentNotFound = "ComponentDeploymentNotFound" -) - -// Any required Configmap paths can be added here, -// they will be automatically included for required -// validation check -var requiredFields = []string{ - APIServerImagePath, - APIServerArtifactImagePath, - PersistenceAgentImagePath, - ScheduledWorkflowImagePath, - APIServerCacheImagePath, - APIServerMoveResultsImagePath, - MariaDBImagePath, - OAuthProxyImagePath, -} - -// DefaultDBConnectionTimeout is the default DB storage healthcheck timeout -const DefaultDBConnectionTimeout = time.Second * 15 - -// DefaultObjStoreConnectionTimeout is the default Object storage healthcheck timeout -const DefaultObjStoreConnectionTimeout = time.Second * 15 - -const DefaultMaxConcurrentReconciles = 10 - -const MlmdGrpcPort = 8080 - -const MlmdRestPort = 9090 - -func GetConfigRequiredFields() []string { - return requiredFields -} - // Default ResourceRequirements var ( MlmdRestResourceRequirements = createResourceRequirement(resource.MustParse("100m"), resource.MustParse("256Mi"), resource.MustParse("100m"), resource.MustParse("256Mi")) diff --git a/internal/controller/config/templates/deployment.yaml.tmpl b/internal/controller/config/templates/deployment.yaml.tmpl index 0d32e4b..92bd439 100644 --- a/internal/controller/config/templates/deployment.yaml.tmpl +++ b/internal/controller/config/templates/deployment.yaml.tmpl @@ -6,7 +6,6 @@ metadata: labels: app: {{.Name}} component: model-registry - registry: {{.Name}} spec: replicas: 1 revisionHistoryLimit: 0 @@ -14,13 +13,11 @@ spec: matchLabels: app: {{.Name}} component: model-registry - registry: {{.Name}} template: metadata: labels: app: {{.Name}} component: model-registry - registry: {{.Name}} spec: containers: - args: diff --git a/internal/controller/config/templates/service.yaml.tmpl b/internal/controller/config/templates/service.yaml.tmpl index 23e6093..8cdf133 100644 --- a/internal/controller/config/templates/service.yaml.tmpl +++ b/internal/controller/config/templates/service.yaml.tmpl @@ -6,7 +6,6 @@ metadata: labels: app: {{.Name}} component: model-registry - registry: {{.Name}} spec: ports: - name: grpc-api @@ -18,5 +17,4 @@ spec: selector: app: {{.Name}} component: model-registry - registry: {{.Name}} type: ClusterIP diff --git a/internal/controller/config/templates/serviceaccount.yaml.tmpl b/internal/controller/config/templates/serviceaccount.yaml.tmpl index 41028cf..32cd1f4 100644 --- a/internal/controller/config/templates/serviceaccount.yaml.tmpl +++ b/internal/controller/config/templates/serviceaccount.yaml.tmpl @@ -6,4 +6,3 @@ metadata: labels: app: {{.Name}} component: model-registry - registry: {{.Name}} diff --git a/internal/controller/modelregistry_controller.go b/internal/controller/modelregistry_controller.go index 55b6049..f63c94d 100644 --- a/internal/controller/modelregistry_controller.go +++ b/internal/controller/modelregistry_controller.go @@ -259,6 +259,70 @@ func (r *ModelRegistryReconciler) updateRegistryResources(ctx context.Context, p return result, nil } +func (r *ModelRegistryReconciler) setRegistryStatus(ctx context.Context, req ctrl.Request, operationResult OperationResult) error { + log := klog.FromContext(ctx) + + modelRegistry := &modelregistryv1alpha1.ModelRegistry{} + if err := r.Get(ctx, req.NamespacedName, modelRegistry); err != nil { + log.Error(err, "Failed to re-fetch modelRegistry") + return err + } + + status := metav1.ConditionTrue + reason := ReasonCreated + message := "Deployment for custom resource %s was successfully created" + switch operationResult { + case ResourceCreated: + status = metav1.ConditionFalse + reason = ReasonCreating + message = "Creating deployment for custom resource %s" + case ResourceUpdated: + status = metav1.ConditionFalse + reason = ReasonUpdating + message = "Updating deployment for custom resource %s" + } + + meta.SetStatusCondition(&modelRegistry.Status.Conditions, metav1.Condition{Type: ConditionTypeProgressing, + Status: status, Reason: reason, + Message: fmt.Sprintf(message, modelRegistry.Name)}) + + // determine registry available condition + deployment := &appsv1.Deployment{} + if err := r.Get(ctx, req.NamespacedName, deployment); err != nil { + log.Error(err, "Failed to get modelRegistry deployment", "name", req.NamespacedName) + return err + } + log.V(10).Info("Found service deployment", "name", len(deployment.Name)) + + // check deployment availability + available := false + for _, c := range deployment.Status.Conditions { + if c.Type == appsv1.DeploymentAvailable { + available = c.Status == corev1.ConditionTrue + break + } + } + + if available { + status = metav1.ConditionTrue + reason = ReasonAvailable + message = "Deployment for custom resource %s is available" + } else { + status = metav1.ConditionFalse + reason = ReasonUnavailable + message = "Deployment for custom resource %s is not available" + } + meta.SetStatusCondition(&modelRegistry.Status.Conditions, metav1.Condition{Type: ConditionTypeAvailable, + Status: status, Reason: reason, + Message: fmt.Sprintf(message, modelRegistry.Name)}) + + if err := r.Status().Update(ctx, modelRegistry); err != nil { + log.Error(err, "Failed to update modelRegistry status") + return err + } + return nil +} + func (r *ModelRegistryReconciler) createOrUpdateDeployment(ctx context.Context, params *ModelRegistryParams, registry *modelregistryv1alpha1.ModelRegistry, templateName string) (result OperationResult, err error) { result = ResourceUnchanged @@ -311,7 +375,6 @@ func (r *ModelRegistryReconciler) createOrUpdateServiceAccount(ctx context.Conte } //go:generate go-enum -type=OperationResult - type OperationResult int const ( @@ -346,8 +409,10 @@ func (r *ModelRegistryReconciler) createOrUpdate(ctx context.Context, currObj cl return result, err } + // hack: envtest is missing typemeta for some reason, hence the ignores for apiVersion and kind!!! // create a patch by comparing objects - patchResult, err := patch.DefaultPatchMaker.Calculate(currObj, newObj, patch.IgnoreStatusFields()) + patchResult, err := patch.DefaultPatchMaker.Calculate(currObj, newObj, patch.IgnoreStatusFields(), + patch.IgnoreField("apiVersion"), patch.IgnoreField("kind")) if err != nil { return result, err } @@ -420,68 +485,3 @@ func (r *ModelRegistryReconciler) logResultAsEvent(registry *modelregistryv1alph registry.Namespace)) } } - -func (r *ModelRegistryReconciler) setRegistryStatus(ctx context.Context, req ctrl.Request, operationResult OperationResult) error { - log := klog.FromContext(ctx) - - modelRegistry := &modelregistryv1alpha1.ModelRegistry{} - if err := r.Get(ctx, req.NamespacedName, modelRegistry); err != nil { - log.Error(err, "Failed to re-fetch modelRegistry") - return err - } - - status := metav1.ConditionTrue - reason := ReasonCreated - message := "Deployment for custom resource %s was successfully created" - if operationResult != ResourceUnchanged { - status = metav1.ConditionFalse - } - if operationResult == ResourceCreated { - reason = ReasonCreating - message = "Creating deployment for custom resource %s" - } - if operationResult == ResourceUpdated { - reason = ReasonUpdating - message = "Updating deployment for custom resource %s" - } - - meta.SetStatusCondition(&modelRegistry.Status.Conditions, metav1.Condition{Type: ConditionTypeProgressing, - Status: status, Reason: reason, - Message: fmt.Sprintf(message, modelRegistry.Name)}) - - // determine registry available condition - deployment := &appsv1.Deployment{} - if err := r.Get(ctx, req.NamespacedName, deployment); err != nil { - log.Error(err, "Failed to get modelRegistry deployment", "name", req.NamespacedName) - return err - } - log.V(10).Info("Found service deployment", "name", len(deployment.Name)) - - // check deployment availability - available := false - for _, c := range deployment.Status.Conditions { - if c.Type == appsv1.DeploymentAvailable { - available = c.Status == corev1.ConditionTrue - break - } - } - - if available { - status = metav1.ConditionTrue - reason = ReasonAvailable - message = "Deployment for custom resource %s is available" - } else { - status = metav1.ConditionFalse - reason = ReasonUnavailable - message = "Deployment for custom resource %s is not available" - } - meta.SetStatusCondition(&modelRegistry.Status.Conditions, metav1.Condition{Type: ConditionTypeAvailable, - Status: status, Reason: reason, - Message: fmt.Sprintf(message, modelRegistry.Name)}) - - if err := r.Status().Update(ctx, modelRegistry); err != nil { - log.Error(err, "Failed to update modelRegistry status") - return err - } - return nil -} diff --git a/internal/controller/modelregistry_controller_test.go b/internal/controller/modelregistry_controller_test.go new file mode 100644 index 0000000..31eabac --- /dev/null +++ b/internal/controller/modelregistry_controller_test.go @@ -0,0 +1,182 @@ +/* +Copyright 2023. + +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" + "fmt" + "github.com/opendatahub-io/model-registry-operator/internal/controller/config" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/client-go/tools/record" + "os" + ctrl "sigs.k8s.io/controller-runtime" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/opendatahub-io/model-registry-operator/api/v1alpha1" +) + +var _ = Describe("ModelRegistry controller", func() { + Context("ModelRegistry controller test", func() { + + const ModelRegistryName = "test-modelregistry" + + ctx := context.Background() + + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: ModelRegistryName, + Namespace: ModelRegistryName, + }, + } + + typeNamespaceName := types.NamespacedName{Name: ModelRegistryName, Namespace: ModelRegistryName} + modelRegistry := &v1alpha1.ModelRegistry{} + + // load templates + template, err := config.ParseTemplates() + Expect(err).To(Not(HaveOccurred())) + + BeforeEach(func() { + By("Creating the Namespace to perform the tests") + err := k8sClient.Create(ctx, namespace) + Expect(err).To(Not(HaveOccurred())) + + By("Setting the Image ENV VARs which stores the Server images") + err = os.Setenv("GRPC_IMAGE", "gcr.io/tfx-oss-public/ml_metadata_store_server:1.14.0") + Expect(err).To(Not(HaveOccurred())) + err = os.Setenv("REST_IMAGE", "quay.io/opendatahub/model-registry:latest") + Expect(err).To(Not(HaveOccurred())) + + By("creating the custom resource for the Kind ModelRegistry") + err = k8sClient.Get(ctx, typeNamespaceName, modelRegistry) + if err != nil && errors.IsNotFound(err) { + // Let's mock our custom resource at the same way that we would + // apply on the cluster the manifest under config/samples + var gRPCPort int32 = 9090 + var restPort int32 = 8080 + var postgresPort int32 = 5432 + modelRegistry = &v1alpha1.ModelRegistry{ + ObjectMeta: metav1.ObjectMeta{ + Name: ModelRegistryName, + Namespace: namespace.Name, + }, + Spec: v1alpha1.ModelRegistrySpec{ + Grpc: v1alpha1.GrpcSpec{ + Port: &gRPCPort, + }, + Rest: v1alpha1.RestSpec{ + Port: &restPort, + }, + Postgres: v1alpha1.PostgresConfig{ + Host: "model-registry-db", + Port: &postgresPort, + Database: "model-registry", + Username: "mlmduser", + PasswordSecret: &v1alpha1.SecretKeyValue{ + Name: "model-registry-db", + Key: "database-password", + }, + }, + }, + } + err = k8sClient.Create(ctx, modelRegistry) + Expect(err).To(Not(HaveOccurred())) + } + }) + + AfterEach(func() { + By("removing the custom resource for the Kind ModelRegistry") + found := &v1alpha1.ModelRegistry{} + err := k8sClient.Get(ctx, typeNamespaceName, found) + Expect(err).To(Not(HaveOccurred())) + + Eventually(func() error { + return k8sClient.Delete(context.TODO(), found) + }, 2*time.Minute, time.Second).Should(Succeed()) + + // TODO(user): Attention if you improve this code by adding other context test you MUST + // be aware of the current delete namespace limitations. + // More info: https://book.kubebuilder.io/reference/envtest.html#testing-considerations + By("Deleting the Namespace to perform the tests") + _ = k8sClient.Delete(ctx, namespace) + + By("Removing the Image ENV VARs which stores the Server images") + _ = os.Unsetenv("GRPC_IMAGE") + _ = os.Unsetenv("REST_IMAGE") + }) + + It("should successfully reconcile a custom resource for ModelRegistry", func() { + By("Checking if the custom resource was successfully created") + Eventually(func() error { + found := &v1alpha1.ModelRegistry{} + return k8sClient.Get(ctx, typeNamespaceName, found) + }, time.Minute, time.Second).Should(Succeed()) + + modelRegistryReconciler := &ModelRegistryReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + Recorder: &record.FakeRecorder{}, + Log: ctrl.Log.WithName("controller"), + Template: template, + } + + By("Reconciling the custom resource created") + Eventually(func() error { + result, err := modelRegistryReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespaceName, + }) + if err != nil { + return err + } + if !result.IsZero() { + return fmt.Errorf("non-empty reconcile result") + } + // reconcile done! + return nil + }, time.Minute, time.Second).Should(Succeed()) + //Expect(err).To(Not(HaveOccurred())) + + By("Checking if Deployment was successfully created in the reconciliation") + Eventually(func() error { + found := &appsv1.Deployment{} + return k8sClient.Get(ctx, typeNamespaceName, found) + }, time.Minute, time.Second).Should(Succeed()) + + By("Checking the latest Status Condition added to the ModelRegistry instance") + Eventually(func() error { + err := k8sClient.Get(ctx, typeNamespaceName, modelRegistry) + Expect(err).To(Not(HaveOccurred())) + if !meta.IsStatusConditionTrue(modelRegistry.Status.Conditions, ConditionTypeProgressing) { + return fmt.Errorf("Condition %s is not true", ConditionTypeProgressing) + } + //if !meta.IsStatusConditionTrue(modelRegistry.Status.Conditions, ConditionTypeAvailable) { + // return fmt.Errorf("Condition %s is not true", ConditionTypeAvailable) + //} + return nil + }, time.Minute, time.Second).Should(Succeed()) + }) + }) +}) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index a5d34ee..884d388 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -53,6 +53,7 @@ var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) By("bootstrapping test environment") + useExistingCluster := false testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, ErrorIfCRDPathMissing: true, @@ -64,6 +65,7 @@ var _ = BeforeSuite(func() { // the tests directly. When we run make test it will be setup and used automatically. BinaryAssetsDirectory: filepath.Join("..", "..", "bin", "k8s", fmt.Sprintf("1.28.0-%s-%s", runtime.GOOS, runtime.GOARCH)), + UseExistingCluster: &useExistingCluster, } var err error