From 4c06cb8be6e499603a3e63e04a7c3f52ca753eac Mon Sep 17 00:00:00 2001 From: Kamil Sambor Date: Mon, 15 Jan 2024 13:32:53 +0100 Subject: [PATCH 1/5] Reorganize main reconcile flow --- controllers/placementapi_controller.go | 185 ++++++++++++++++--------- 1 file changed, 121 insertions(+), 64 deletions(-) diff --git a/controllers/placementapi_controller.go b/controllers/placementapi_controller.go index 35362f55..dc3f9715 100644 --- a/controllers/placementapi_controller.go +++ b/controllers/placementapi_controller.go @@ -253,6 +253,11 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{}, err } + // initialize status fields + if err = r.initStatus(ctx, h, instance); err != nil { + return ctrl.Result{}, err + } + // Always patch the instance status when exiting this function so we can persist any changes. defer func() { // update the Ready condition based on the sub conditions @@ -275,39 +280,55 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request }() // If we're not deleting this and the service object doesn't have our finalizer, add it. - if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) { + if instance.DeletionTimestamp.IsZero() { + return ctrl.Result{}, r.reconcileDelete(ctx, h, instance) + } + // We create a KeystoneEndpoint CR later and that will automatically get the + // Nova finalizer. So we need a finalizer on the ourselves too so that + // during NovaAPI CR delete we can have a chance to remove the finalizer from + // the our KeystoneEndpoint so that is also deleted. + updated := controllerutil.AddFinalizer(instance, h.GetFinalizer()) + if updated { + Log.Info("Added finalizer to ourselves") + // we intentionally return immediately to force the deferred function + // to persist the Instance with the finalizer. We need to have our own + // finalizer persisted before we try to create the KeystoneEndpoint with + // our finalizer to avoid orphaning the KeystoneEndpoint. return ctrl.Result{}, nil } // - // initialize status + // check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map // - if instance.Status.Conditions == nil { - instance.Status.Conditions = condition.Conditions{} - // initialize conditions used later as Status=Unknown - cl := condition.CreateList( - condition.UnknownCondition(condition.DBReadyCondition, condition.InitReason, condition.DBReadyInitMessage), - condition.UnknownCondition(condition.DBSyncReadyCondition, condition.InitReason, condition.DBSyncReadyInitMessage), - condition.UnknownCondition(condition.ExposeServiceReadyCondition, condition.InitReason, condition.ExposeServiceReadyInitMessage), - condition.UnknownCondition(condition.InputReadyCondition, condition.InitReason, condition.InputReadyInitMessage), - condition.UnknownCondition(condition.ServiceConfigReadyCondition, condition.InitReason, condition.ServiceConfigReadyInitMessage), - condition.UnknownCondition(condition.DeploymentReadyCondition, condition.InitReason, condition.DeploymentReadyInitMessage), - // right now we have no dedicated KeystoneServiceReadyInitMessage and KeystoneEndpointReadyInitMessage - condition.UnknownCondition(condition.KeystoneServiceReadyCondition, condition.InitReason, ""), - condition.UnknownCondition(condition.KeystoneEndpointReadyCondition, condition.InitReason, ""), - condition.UnknownCondition(condition.NetworkAttachmentsReadyCondition, condition.InitReason, condition.NetworkAttachmentsReadyInitMessage), - condition.UnknownCondition(condition.TLSInputReadyCondition, condition.InitReason, condition.InputReadyInitMessage), - // service account, role, rolebinding conditions - condition.UnknownCondition(condition.ServiceAccountReadyCondition, condition.InitReason, condition.ServiceAccountReadyInitMessage), - condition.UnknownCondition(condition.RoleReadyCondition, condition.InitReason, condition.RoleReadyInitMessage), - condition.UnknownCondition(condition.RoleBindingReadyCondition, condition.InitReason, condition.RoleBindingReadyInitMessage), - ) + hash, result, ospSecret, err := ensureSecret( + ctx, + types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, + []string{ + instance.Spec.PasswordSelectors.Service, + instance.Spec.PasswordSelectors.Database, + }, + helper.GetClient(), + &instance.Status.Conditions) + if (err != nil || result != ctrl.Result{}) { + return result, err + } + configMapVars[ospSecret.Name] = env.SetValue(hash) + // all our input checks out so report InputReady + instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage) - instance.Status.Conditions.Init(&cl) + // Handle non-deleted clusters + return r.reconcileNormal(ctx, instance, helper) +} - // Register overall status immediately to have an early feedback e.g. in the cli - return ctrl.Result{}, nil +func (r *PlacementAPIReconciler) initStatus( + ctx context.Context, h *helper.Helper, instance *placementv1.PlacementAPI, +) error { + if err := r.initConditions(ctx, h, instance); err != nil { + return err } + + // NOTE(gibi): initialize the rest of the status fields here + // so that the reconcile loop later can assume they are not nil. if instance.Status.Hash == nil { instance.Status.Hash = map[string]string{} } @@ -315,13 +336,82 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request instance.Status.NetworkAttachments = map[string][]string{} } - // Handle service delete - if !instance.DeletionTimestamp.IsZero() { - return r.reconcileDelete(ctx, instance, helper) - } + return nil +} - // Handle non-deleted clusters - return r.reconcileNormal(ctx, instance, helper) +func (r *PlacementAPIReconciler) initConditions( + ctx context.Context, h *helper.Helper, instance *placementv1.PlacementAPI, +) error { + if instance.Status.Conditions == nil { + instance.Status.Conditions = condition.Conditions{} + // initialize conditions used later as Status=Unknown + cl := condition.CreateList( + condition.UnknownCondition( + condition.DBReadyCondition, + condition.InitReason, + condition.DBReadyInitMessage + ), + condition.UnknownCondition( + condition.DBSyncReadyCondition, + condition.InitReason, + condition.DBSyncReadyInitMessage + ), + condition.UnknownCondition( + condition.ExposeServiceReadyCondition, + condition.InitReason, + condition.ExposeServiceReadyInitMessage + ), + condition.UnknownCondition( + condition.InputReadyCondition, + condition.InitReason, + condition.InputReadyInitMessage + ), + condition.UnknownCondition( + condition.ServiceConfigReadyCondition, + condition.InitReason, + condition.ServiceConfigReadyInitMessage + ), + condition.UnknownCondition( + condition.DeploymentReadyCondition, + condition.InitReason, + condition.DeploymentReadyInitMessage + ), + // right now we have no dedicated KeystoneServiceReadyInitMessage and KeystoneEndpointReadyInitMessage + condition.UnknownCondition( + condition.KeystoneServiceReadyCondition, + condition.InitReason, + "Service registration not started", + ), + condition.UnknownCondition( + condition.KeystoneEndpointReadyCondition, + condition.InitReason, + "KeystoneEndpoint not created", + ), + condition.UnknownCondition( + condition.NetworkAttachmentsReadyCondition, + condition.InitReason, + condition.NetworkAttachmentsReadyInitMessage + ), + // service account, role, rolebinding conditions + condition.UnknownCondition( + condition.ServiceAccountReadyCondition, + condition.InitReason, + condition.ServiceAccountReadyInitMessage + ), + condition.UnknownCondition( + condition.RoleReadyCondition, + condition.InitReason, + condition.RoleReadyInitMessage + ), + condition.UnknownCondition( + condition.RoleBindingReadyCondition, + condition.InitReason, + condition.RoleBindingReadyInitMessage), + ) + + instance.Status.Conditions.Init(&cl) + } + return nil } // fields to index to reconcile when change @@ -828,39 +918,6 @@ func (r *PlacementAPIReconciler) reconcileNormal(ctx context.Context, instance * // ConfigMap configMapVars := make(map[string]env.Setter) - // - // check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map - // - hash, result, ospSecret, err := ensureSecret( - ctx, - types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, - []string{ - instance.Spec.PasswordSelectors.Service, - instance.Spec.PasswordSelectors.Database, - }, - helper.GetClient(), - &instance.Status.Conditions) - if err != nil { - if k8s_errors.IsNotFound(err) { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.InputReadyCondition, - condition.RequestedReason, - condition.SeverityInfo, - condition.InputReadyWaitingMessage)) - return ctrl.Result{RequeueAfter: time.Second * 10}, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret) - } - instance.Status.Conditions.Set(condition.FalseCondition( - condition.InputReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.InputReadyErrorMessage, - err.Error())) - return result, err - } - configMapVars[ospSecret.Name] = env.SetValue(hash) - instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage) - // run check OpenStack secret - end - // // Create ConfigMaps and Secrets required as input for the Service and calculate an overall hash of hashes // From ef36000021aa005933d85ed88a9791d7218dcb3b Mon Sep 17 00:00:00 2001 From: Kamil Sambor Date: Thu, 18 Jan 2024 10:05:01 +0100 Subject: [PATCH 2/5] Remove initcontainer and add more logs --- controllers/placementapi_controller.go | 1007 ++++++++--------- pkg/placement/const.go | 3 + pkg/placement/dbsync.go | 11 +- pkg/placement/deployment.go | 14 +- pkg/placement/initcontainer.go | 96 -- pkg/placement/volumes.go | 52 +- templates/common/common.sh | 36 - templates/placementapi/bin/init.sh | 46 - .../config/placement-api-config.json | 17 +- .../config/placement-dbsync-config.json | 4 +- templates/placementapi/config/placement.conf | 6 +- .../placementapi_controller_test.go | 54 +- .../common/assert_sample_deployment.yaml | 26 - 13 files changed, 537 insertions(+), 835 deletions(-) delete mode 100644 pkg/placement/initcontainer.go delete mode 100755 templates/common/common.sh delete mode 100755 templates/placementapi/bin/init.sh diff --git a/controllers/placementapi_controller.go b/controllers/placementapi_controller.go index dc3f9715..4bcfc610 100644 --- a/controllers/placementapi_controller.go +++ b/controllers/placementapi_controller.go @@ -26,7 +26,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" - "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -42,7 +41,6 @@ import ( common "github.com/openstack-k8s-operators/lib-common/modules/common" condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" - configmap "github.com/openstack-k8s-operators/lib-common/modules/common/configmap" deployment "github.com/openstack-k8s-operators/lib-common/modules/common/deployment" endpoint "github.com/openstack-k8s-operators/lib-common/modules/common/endpoint" env "github.com/openstack-k8s-operators/lib-common/modules/common/env" @@ -51,8 +49,8 @@ import ( labels "github.com/openstack-k8s-operators/lib-common/modules/common/labels" nad "github.com/openstack-k8s-operators/lib-common/modules/common/networkattachment" common_rbac "github.com/openstack-k8s-operators/lib-common/modules/common/rbac" + "github.com/openstack-k8s-operators/lib-common/modules/common/secret" "github.com/openstack-k8s-operators/lib-common/modules/common/service" - "github.com/openstack-k8s-operators/lib-common/modules/common/tls" util "github.com/openstack-k8s-operators/lib-common/modules/common/util" mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" @@ -236,13 +234,15 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request // Request object not found, could have been deleted after reconcile request. // Owned objects are automatically garbage collected. // For additional cleanup logic use finalizers. Return and don't requeue. + Log.Info("Placement instance not found, probably deleted before reconciled. Nothing to do.") return ctrl.Result{}, nil } // Error reading the object - requeue the request. + Log.Error(err, "Failed to read the Placement instance.") return ctrl.Result{}, err } - helper, err := helper.NewHelper( + h, err := helper.NewHelper( instance, r.Client, r.Kclient, @@ -250,11 +250,7 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request Log, ) if err != nil { - return ctrl.Result{}, err - } - - // initialize status fields - if err = r.initStatus(ctx, h, instance); err != nil { + Log.Error(err, "Failed to create lib-common Helper") return ctrl.Result{}, err } @@ -272,7 +268,7 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request instance.Status.Conditions.Set( instance.Status.Conditions.Mirror(condition.ReadyCondition)) } - err := helper.PatchInstance(ctx, instance) + err := h.PatchInstance(ctx, instance) if err != nil { _err = err return @@ -280,44 +276,374 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request }() // If we're not deleting this and the service object doesn't have our finalizer, add it. - if instance.DeletionTimestamp.IsZero() { - return ctrl.Result{}, r.reconcileDelete(ctx, h, instance) - } - // We create a KeystoneEndpoint CR later and that will automatically get the - // Nova finalizer. So we need a finalizer on the ourselves too so that - // during NovaAPI CR delete we can have a chance to remove the finalizer from - // the our KeystoneEndpoint so that is also deleted. - updated := controllerutil.AddFinalizer(instance, h.GetFinalizer()) - if updated { - Log.Info("Added finalizer to ourselves") - // we intentionally return immediately to force the deferred function - // to persist the Instance with the finalizer. We need to have our own - // finalizer persisted before we try to create the KeystoneEndpoint with - // our finalizer to avoid orphaning the KeystoneEndpoint. + if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, h.GetFinalizer()) { return ctrl.Result{}, nil } + // initialize status fields + if err = r.initStatus(ctx, h, instance); err != nil { + return ctrl.Result{}, err + } + + // Handle service delete + if !instance.DeletionTimestamp.IsZero() { + return r.reconcileDelete(ctx, instance, h) + } + // Service account, role, binding + rbacRules := []rbacv1.PolicyRule{ + { + APIGroups: []string{"security.openshift.io"}, + ResourceNames: []string{"anyuid"}, + Resources: []string{"securitycontextconstraints"}, + Verbs: []string{"use"}, + }, + { + APIGroups: []string{""}, + Resources: []string{"pods"}, + Verbs: []string{"create", "get", "list", "watch", "update", "patch", "delete"}, + }, + } + rbacResult, err := common_rbac.ReconcileRbac(ctx, h, instance, rbacRules) + if err != nil { + return rbacResult, err + } else if (rbacResult != ctrl.Result{}) { + return rbacResult, nil + } + + // ConfigMap + configMapVars := make(map[string]env.Setter) // // check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map // - hash, result, ospSecret, err := ensureSecret( + hash, result, _, err := ensureSecret( ctx, types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, []string{ instance.Spec.PasswordSelectors.Service, instance.Spec.PasswordSelectors.Database, }, - helper.GetClient(), + h.GetClient(), &instance.Status.Conditions) - if (err != nil || result != ctrl.Result{}) { + if err != nil { + if k8s_errors.IsNotFound(err) { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.InputReadyCondition, + condition.RequestedReason, + condition.SeverityInfo, + condition.InputReadyWaitingMessage)) + return ctrl.Result{RequeueAfter: time.Second * 10}, fmt.Errorf("OpenStack secret %s not found", instance.Spec.Secret) + } + instance.Status.Conditions.Set(condition.FalseCondition( + condition.InputReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.InputReadyErrorMessage, + err.Error())) return result, err } - configMapVars[ospSecret.Name] = env.SetValue(hash) + configMapVars[instance.Spec.Secret] = env.SetValue(hash) + // all our input checks out so report InputReady instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage) - // Handle non-deleted clusters - return r.reconcileNormal(ctx, instance, helper) + err = r.generateServiceConfigMaps(ctx, h, instance, &configMapVars) + if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.ServiceConfigReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.ServiceConfigReadyErrorMessage, + err.Error())) + return ctrl.Result{}, err + } + + // create hash over all the different input resources to identify if any those changed + // and a restart/recreate is required. + // + inputHash, hashChanged, err := r.createHashOfInputHashes(ctx, instance, configMapVars) + if err != nil { + return ctrl.Result{}, err + } else if hashChanged { + // Hash changed and instance status should be updated (which will be done by main defer func), + // so we need to return and reconcile again + return ctrl.Result{}, nil + } + + instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) + + serviceAnnotations, result, err := r.ensureNetworkAttachments(ctx, h, instance) + if (err != nil || result != ctrl.Result{}) { + return result, err + } + + result, err = r.ensureDB(ctx, h, instance) + if err != nil { + return ctrl.Result{}, err + } + + apiEndpoints, result, err := r.ensureServiceExposed(ctx, h, instance) + + if err != nil { + return ctrl.Result{}, err + } + + err = r.ensureKeystoneServiceUser(ctx, h, instance) + if err != nil { + return ctrl.Result{}, err + } + + result, err = r.ensureKeystoneEndpoint(ctx, h, instance, apiEndpoints) + if err != nil { + return ctrl.Result{}, err + } + + result, err = r.ensureDbSync(ctx, instance, h, serviceAnnotations) + if err != nil { + return result, err + } else if (result != ctrl.Result{}) { + return result, nil + } + + if (err != nil || result != ctrl.Result{}) { + // We can ignore RequeueAfter as we are watching the Service resource + // but we have to return while waiting for the service to be exposed + return ctrl.Result{}, err + } + + result, err = r.ensureDeployment(ctx, h, instance, inputHash, serviceAnnotations) + if (err != nil || result != ctrl.Result{}) { + return result, err + } + + // Only expose the service is the deployment succeeded + if !instance.Status.Conditions.IsTrue(condition.DeploymentReadyCondition) { + Log.Info("Waiting for the Deployment to become Ready before exposing the sevice in Keystone") + return ctrl.Result{}, nil + } + + return ctrl.Result{}, nil +} + +func getServiceLabels() map[string]string { + return map[string]string{ + common.AppSelector: placement.ServiceName, + } +} + +func (r *PlacementAPIReconciler) ensureServiceExposed( + ctx context.Context, + h *helper.Helper, + instance *placementv1.PlacementAPI, +) (map[string]string, ctrl.Result, error) { + var placementEndpoints = map[service.Endpoint]endpoint.Data{ + service.EndpointPublic: {Port: placement.PlacementPublicPort}, + service.EndpointInternal: {Port: placement.PlacementInternalPort}, + } + apiEndpoints := make(map[string]string) + + for endpointType, data := range placementEndpoints { + endpointTypeStr := string(endpointType) + endpointName := placement.ServiceName + "-" + endpointTypeStr + + svcOverride := instance.Spec.Override.Service[endpointType] + if svcOverride.EmbeddedLabelsAnnotations == nil { + svcOverride.EmbeddedLabelsAnnotations = &service.EmbeddedLabelsAnnotations{} + } + + exportLabels := util.MergeStringMaps( + getServiceLabels(), + map[string]string{ + service.AnnotationEndpointKey: endpointTypeStr, + }, + ) + + // Create the service + svc, err := service.NewService( + service.GenericService(&service.GenericServiceDetails{ + Name: endpointName, + Namespace: instance.Namespace, + Labels: exportLabels, + Selector: getServiceLabels(), + Port: service.GenericServicePort{ + Name: endpointName, + Port: data.Port, + Protocol: corev1.ProtocolTCP, + }, + }), + 5, + &svcOverride.OverrideSpec, + ) + if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.ExposeServiceReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.ExposeServiceReadyErrorMessage, + err.Error())) + + return nil, ctrl.Result{}, err + } + + svc.AddAnnotation(map[string]string{ + service.AnnotationEndpointKey: endpointTypeStr, + }) + + // add Annotation to whether creating an ingress is required or not + if endpointType == service.EndpointPublic && svc.GetServiceType() == corev1.ServiceTypeClusterIP { + svc.AddAnnotation(map[string]string{ + service.AnnotationIngressCreateKey: "true", + }) + } else { + svc.AddAnnotation(map[string]string{ + service.AnnotationIngressCreateKey: "false", + }) + if svc.GetServiceType() == corev1.ServiceTypeLoadBalancer { + svc.AddAnnotation(map[string]string{ + service.AnnotationHostnameKey: svc.GetServiceHostname(), // add annotation to register service name in dnsmasq + }) + } + } + + ctrlResult, err := svc.CreateOrPatch(ctx, h) + if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.ExposeServiceReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.ExposeServiceReadyErrorMessage, + err.Error())) + + return nil, ctrlResult, err + } else if (ctrlResult != ctrl.Result{}) { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.ExposeServiceReadyCondition, + condition.RequestedReason, + condition.SeverityInfo, + condition.ExposeServiceReadyRunningMessage)) + return nil, ctrlResult, nil + } + // create service - end + + // TODO: TLS, pass in https as protocol, create TLS cert + apiEndpoints[string(endpointType)], err = svc.GetAPIEndpoint( + svcOverride.EndpointURL, data.Protocol, data.Path) + if err != nil { + return nil, ctrl.Result{}, err + } + } + + instance.Status.Conditions.MarkTrue(condition.ExposeServiceReadyCondition, condition.ExposeServiceReadyMessage) + return apiEndpoints, ctrl.Result{}, nil +} + +func (r *PlacementAPIReconciler) ensureNetworkAttachments( + ctx context.Context, + h *helper.Helper, + instance *placementv1.PlacementAPI, +) (map[string]string, ctrl.Result, error) { + var nadAnnotations map[string]string + var err error + + // networks to attach to + for _, netAtt := range instance.Spec.NetworkAttachments { + _, err := nad.GetNADWithName(ctx, h, netAtt, instance.Namespace) + if err != nil { + if k8s_errors.IsNotFound(err) { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.NetworkAttachmentsReadyCondition, + condition.RequestedReason, + condition.SeverityInfo, + condition.NetworkAttachmentsReadyWaitingMessage, + netAtt)) + return nadAnnotations, ctrl.Result{RequeueAfter: time.Second * 10}, fmt.Errorf("network-attachment-definition %s not found", netAtt) + } + instance.Status.Conditions.Set(condition.FalseCondition( + condition.NetworkAttachmentsReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.NetworkAttachmentsReadyErrorMessage, + err.Error())) + return nadAnnotations, ctrl.Result{}, err + } + } + + nadAnnotations, err = nad.CreateNetworksAnnotation(instance.Namespace, instance.Spec.NetworkAttachments) + if err != nil { + return nadAnnotations, ctrl.Result{}, fmt.Errorf("failed create network annotation from %s: %w", + instance.Spec.NetworkAttachments, err) + } + return nadAnnotations, ctrl.Result{}, nil + +} + +func (r *PlacementAPIReconciler) ensureKeystoneServiceUser( + ctx context.Context, + h *helper.Helper, + instance *placementv1.PlacementAPI, +) error { + // + // create service and user in keystone - https://docs.openstack.org/placement/latest/install/install-rdo.html#configure-user-and-endpoints + // + ksSvcSpec := keystonev1.KeystoneServiceSpec{ + ServiceType: placement.ServiceName, + ServiceName: placement.ServiceName, + ServiceDescription: "Placement Service", + Enabled: true, + ServiceUser: instance.Spec.ServiceUser, + Secret: instance.Spec.Secret, + PasswordSelector: instance.Spec.PasswordSelectors.Service, + } + serviceLabels := getServiceLabels() + ksSvc := keystonev1.NewKeystoneService(ksSvcSpec, instance.Namespace, serviceLabels, time.Duration(10)*time.Second) + _, err := ksSvc.CreateOrPatch(ctx, h) + if err != nil { + return err + } + + // mirror the Status, Reason, Severity and Message of the latest keystoneservice condition + // into a local condition with the type condition.KeystoneServiceReadyCondition + c := ksSvc.GetConditions().Mirror(condition.KeystoneServiceReadyCondition) + if c != nil { + instance.Status.Conditions.Set(c) + } + + return nil +} + +func (r *PlacementAPIReconciler) ensureKeystoneEndpoint( + ctx context.Context, + h *helper.Helper, + instance *placementv1.PlacementAPI, + apiEndpoints map[string]string, +) (ctrl.Result, error) { + + ksEndptSpec := keystonev1.KeystoneEndpointSpec{ + ServiceName: placement.ServiceName, + Endpoints: apiEndpoints, + } + ksEndpt := keystonev1.NewKeystoneEndpoint( + placement.ServiceName, + instance.Namespace, + ksEndptSpec, + getServiceLabels(), + time.Duration(10)*time.Second, + ) + ctrlResult, err := ksEndpt.CreateOrPatch(ctx, h) + if err != nil { + return ctrlResult, err + } + // mirror the Status, Reason, Severity and Message of the latest keystoneendpoint condition + // into a local condition with the type condition.KeystoneEndpointReadyCondition + c := ksEndpt.GetConditions().Mirror(condition.KeystoneEndpointReadyCondition) + if c != nil { + instance.Status.Conditions.Set(c) + } + + if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil + } + + return ctrlResult, nil } func (r *PlacementAPIReconciler) initStatus( @@ -349,32 +675,32 @@ func (r *PlacementAPIReconciler) initConditions( condition.UnknownCondition( condition.DBReadyCondition, condition.InitReason, - condition.DBReadyInitMessage + condition.DBReadyInitMessage, ), condition.UnknownCondition( condition.DBSyncReadyCondition, condition.InitReason, - condition.DBSyncReadyInitMessage + condition.DBSyncReadyInitMessage, ), condition.UnknownCondition( condition.ExposeServiceReadyCondition, condition.InitReason, - condition.ExposeServiceReadyInitMessage + condition.ExposeServiceReadyInitMessage, ), condition.UnknownCondition( condition.InputReadyCondition, condition.InitReason, - condition.InputReadyInitMessage + condition.InputReadyInitMessage, ), condition.UnknownCondition( condition.ServiceConfigReadyCondition, condition.InitReason, - condition.ServiceConfigReadyInitMessage + condition.ServiceConfigReadyInitMessage, ), condition.UnknownCondition( condition.DeploymentReadyCondition, condition.InitReason, - condition.DeploymentReadyInitMessage + condition.DeploymentReadyInitMessage, ), // right now we have no dedicated KeystoneServiceReadyInitMessage and KeystoneEndpointReadyInitMessage condition.UnknownCondition( @@ -390,18 +716,18 @@ func (r *PlacementAPIReconciler) initConditions( condition.UnknownCondition( condition.NetworkAttachmentsReadyCondition, condition.InitReason, - condition.NetworkAttachmentsReadyInitMessage + condition.NetworkAttachmentsReadyInitMessage, ), // service account, role, rolebinding conditions condition.UnknownCondition( condition.ServiceAccountReadyCondition, condition.InitReason, - condition.ServiceAccountReadyInitMessage + condition.ServiceAccountReadyInitMessage, ), condition.UnknownCondition( condition.RoleReadyCondition, condition.InitReason, - condition.RoleReadyInitMessage + condition.RoleReadyInitMessage, ), condition.UnknownCondition( condition.RoleBindingReadyCondition, @@ -567,290 +893,104 @@ func (r *PlacementAPIReconciler) reconcileDelete(ctx context.Context, instance * } } - // Remove the finalizer from our KeystoneService CR - keystoneService, err := keystonev1.GetKeystoneServiceWithName(ctx, helper, placement.ServiceName, instance.Namespace) - if err != nil && !k8s_errors.IsNotFound(err) { - return ctrl.Result{}, err - } - - if err == nil { - if controllerutil.RemoveFinalizer(keystoneService, helper.GetFinalizer()) { - err = r.Update(ctx, keystoneService) - if err != nil && !k8s_errors.IsNotFound(err) { - return ctrl.Result{}, err - } - Log.Info("Removed finalizer from our KeystoneService") - } - } - - // We did all the cleanup on the objects we created so we can remove the - // finalizer from ourselves to allow the deletion - controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) - Log.Info("Reconciled Service delete successfully") - return ctrl.Result{}, nil -} - -func (r *PlacementAPIReconciler) reconcileInit( - ctx context.Context, - instance *placementv1.PlacementAPI, - helper *helper.Helper, - serviceLabels map[string]string, - serviceAnnotations map[string]string, -) (ctrl.Result, error) { - Log := r.GetLogger(ctx) - Log.Info("Reconciling Service init") - // Service account, role, binding - rbacRules := []rbacv1.PolicyRule{ - { - APIGroups: []string{"security.openshift.io"}, - ResourceNames: []string{"anyuid"}, - Resources: []string{"securitycontextconstraints"}, - Verbs: []string{"use"}, - }, - { - APIGroups: []string{""}, - Resources: []string{"pods"}, - Verbs: []string{"create", "get", "list", "watch", "update", "patch", "delete"}, - }, - } - rbacResult, err := common_rbac.ReconcileRbac(ctx, helper, instance, rbacRules) - if err != nil { - return rbacResult, err - } else if (rbacResult != ctrl.Result{}) { - return rbacResult, nil - } - - // - // create service DB instance - // - db := mariadbv1.NewDatabase( - placement.DatabaseName, - instance.Spec.DatabaseUser, - instance.Spec.Secret, - map[string]string{ - "dbName": instance.Spec.DatabaseInstance, - }, - ) - // create or patch the DB - ctrlResult, err := db.CreateOrPatchDB( - ctx, - helper, - ) - if err != nil { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.DBReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.DBReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err - } - if (ctrlResult != ctrl.Result{}) { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.DBReadyCondition, - condition.RequestedReason, - condition.SeverityInfo, - condition.DBReadyRunningMessage)) - return ctrlResult, nil - } - // wait for the DB to be setup - ctrlResult, err = db.WaitForDBCreated(ctx, helper) - if err != nil { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.DBReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.DBReadyErrorMessage, - err.Error())) - return ctrlResult, err - } - if (ctrlResult != ctrl.Result{}) { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.DBReadyCondition, - condition.RequestedReason, - condition.SeverityInfo, - condition.DBReadyRunningMessage)) - return ctrlResult, nil - } - - // update Status.DatabaseHostname, used to config the service - instance.Status.DatabaseHostname = db.GetDatabaseHostname() - instance.Status.Conditions.MarkTrue(condition.DBReadyCondition, condition.DBReadyMessage) - // create service DB - end - - // - // expose the service (create service, route and return the created endpoint URLs) - // - var placementEndpoints = map[service.Endpoint]endpoint.Data{ - service.EndpointPublic: {Port: placement.PlacementPublicPort}, - service.EndpointInternal: {Port: placement.PlacementInternalPort}, - } - apiEndpoints := make(map[string]string) - - for endpointType, data := range placementEndpoints { - endpointTypeStr := string(endpointType) - endpointName := placement.ServiceName + "-" + endpointTypeStr - - svcOverride := instance.Spec.Override.Service[endpointType] - if svcOverride.EmbeddedLabelsAnnotations == nil { - svcOverride.EmbeddedLabelsAnnotations = &service.EmbeddedLabelsAnnotations{} - } - - exportLabels := util.MergeStringMaps( - serviceLabels, - map[string]string{ - service.AnnotationEndpointKey: endpointTypeStr, - }, - ) - - // Create the service - svc, err := service.NewService( - service.GenericService(&service.GenericServiceDetails{ - Name: endpointName, - Namespace: instance.Namespace, - Labels: exportLabels, - Selector: serviceLabels, - Port: service.GenericServicePort{ - Name: endpointName, - Port: data.Port, - Protocol: corev1.ProtocolTCP, - }, - }), - 5, - &svcOverride.OverrideSpec, - ) - if err != nil { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.ExposeServiceReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.ExposeServiceReadyErrorMessage, - err.Error())) - - return ctrl.Result{}, err - } - - svc.AddAnnotation(map[string]string{ - service.AnnotationEndpointKey: endpointTypeStr, - }) - - // add Annotation to whether creating an ingress is required or not - if endpointType == service.EndpointPublic && svc.GetServiceType() == corev1.ServiceTypeClusterIP { - svc.AddAnnotation(map[string]string{ - service.AnnotationIngressCreateKey: "true", - }) - } else { - svc.AddAnnotation(map[string]string{ - service.AnnotationIngressCreateKey: "false", - }) - if svc.GetServiceType() == corev1.ServiceTypeLoadBalancer { - svc.AddAnnotation(map[string]string{ - service.AnnotationHostnameKey: svc.GetServiceHostname(), // add annotation to register service name in dnsmasq - }) - } - } - - ctrlResult, err := svc.CreateOrPatch(ctx, helper) - if err != nil { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.ExposeServiceReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.ExposeServiceReadyErrorMessage, - err.Error())) - - return ctrlResult, err - } else if (ctrlResult != ctrl.Result{}) { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.ExposeServiceReadyCondition, - condition.RequestedReason, - condition.SeverityInfo, - condition.ExposeServiceReadyRunningMessage)) - return ctrlResult, nil - } - // create service - end - - // if TLS is enabled - if instance.Spec.TLS.API.Enabled(endpointType) { - // set endpoint protocol to https - data.Protocol = ptr.To(service.ProtocolHTTPS) - } - - apiEndpoints[string(endpointType)], err = svc.GetAPIEndpoint( - svcOverride.EndpointURL, data.Protocol, data.Path) - if err != nil { - return ctrl.Result{}, err - } - } - - instance.Status.Conditions.MarkTrue(condition.ExposeServiceReadyCondition, condition.ExposeServiceReadyMessage) - // expose service - end + // Remove the finalizer from our KeystoneService CR + keystoneService, err := keystonev1.GetKeystoneServiceWithName(ctx, helper, placement.ServiceName, instance.Namespace) + if err != nil && !k8s_errors.IsNotFound(err) { + return ctrl.Result{}, err + } - // - // create service and user in keystone - https://docs.openstack.org/placement/latest/install/install-rdo.html#configure-user-and-endpoints - // - ksSvcSpec := keystonev1.KeystoneServiceSpec{ - ServiceType: placement.ServiceName, - ServiceName: placement.ServiceName, - ServiceDescription: "Placement Service", - Enabled: true, - ServiceUser: instance.Spec.ServiceUser, - Secret: instance.Spec.Secret, - PasswordSelector: instance.Spec.PasswordSelectors.Service, + if err == nil { + if controllerutil.RemoveFinalizer(keystoneService, helper.GetFinalizer()) { + err = r.Update(ctx, keystoneService) + if err != nil && !k8s_errors.IsNotFound(err) { + return ctrl.Result{}, err + } + Log.Info("Removed finalizer from our KeystoneService") + } } - ksSvc := keystonev1.NewKeystoneService(ksSvcSpec, instance.Namespace, serviceLabels, time.Duration(10)*time.Second) - ctrlResult, err = ksSvc.CreateOrPatch(ctx, helper) + + // We did all the cleanup on the objects we created so we can remove the + // finalizer from ourselves to allow the deletion + controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) + Log.Info("Reconciled Service delete successfully") + return ctrl.Result{}, nil +} + +func (r *PlacementAPIReconciler) ensureDB( + ctx context.Context, + h *helper.Helper, + instance *placementv1.PlacementAPI, +) (ctrl.Result, error) { + // (ksambor) should we use NewDatabaseWithNamespace instead? + db := mariadbv1.NewDatabase( + placement.DatabaseName, + instance.Spec.DatabaseUser, + instance.Spec.Secret, + map[string]string{ + "dbName": instance.Spec.DatabaseInstance, + }, + ) + // create or patch the DB + ctrlResult, err := db.CreateOrPatchDBByName( + ctx, + h, + instance.Spec.DatabaseInstance, + ) if err != nil { - return ctrlResult, err - } - // mirror the Status, Reason, Severity and Message of the latest keystoneservice condition - // into a local condition with the type condition.KeystoneServiceReadyCondition - c := ksSvc.GetConditions().Mirror(condition.KeystoneServiceReadyCondition) - if c != nil { - instance.Status.Conditions.Set(c) + instance.Status.Conditions.Set(condition.FalseCondition( + condition.DBReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.DBReadyErrorMessage, + err.Error())) + return ctrl.Result{}, err } - if (ctrlResult != ctrl.Result{}) { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.DBReadyCondition, + condition.RequestedReason, + condition.SeverityInfo, + condition.DBReadyRunningMessage)) return ctrlResult, nil } - - // - // register endpoints - // - ksEndptSpec := keystonev1.KeystoneEndpointSpec{ - ServiceName: placement.ServiceName, - Endpoints: apiEndpoints, - } - ksEndpt := keystonev1.NewKeystoneEndpoint( - placement.ServiceName, - instance.Namespace, - ksEndptSpec, - serviceLabels, - time.Duration(10)*time.Second, - ) - ctrlResult, err = ksEndpt.CreateOrPatch(ctx, helper) + // wait for the DB to be setup + // (ksambor) should we use WaitForDBCreatedWithTimeout instead? + ctrlResult, err = db.WaitForDBCreated(ctx, h) if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.DBReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.DBReadyErrorMessage, + err.Error())) return ctrlResult, err } - // mirror the Status, Reason, Severity and Message of the latest keystoneendpoint condition - // into a local condition with the type condition.KeystoneEndpointReadyCondition - c = ksEndpt.GetConditions().Mirror(condition.KeystoneEndpointReadyCondition) - if c != nil { - instance.Status.Conditions.Set(c) - } - if (ctrlResult != ctrl.Result{}) { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.DBReadyCondition, + condition.RequestedReason, + condition.SeverityInfo, + condition.DBReadyRunningMessage)) return ctrlResult, nil } - // - // run placement db sync - // + // update Status.DatabaseHostname, used to config the service + instance.Status.DatabaseHostname = db.GetDatabaseHostname() + instance.Status.Conditions.MarkTrue(condition.DBReadyCondition, condition.DBReadyMessage) + return ctrlResult, nil + +} + +func (r *PlacementAPIReconciler) ensureDbSync( + ctx context.Context, + instance *placementv1.PlacementAPI, + helper *helper.Helper, + serviceAnnotations map[string]string, +) (ctrl.Result, error) { + Log := r.GetLogger(ctx) + serviceLabels := getServiceLabels() dbSyncHash := instance.Status.Hash[placementv1.DbSyncHash] jobDef := placement.DbSyncJob(instance, serviceLabels, serviceAnnotations) - if err != nil { - return ctrl.Result{}, err - } dbSyncjob := job.NewJob( jobDef, placementv1.DbSyncHash, @@ -858,7 +998,7 @@ func (r *PlacementAPIReconciler) reconcileInit( time.Duration(5)*time.Second, dbSyncHash, ) - ctrlResult, err = dbSyncjob.DoJob( + ctrlResult, err := dbSyncjob.DoJob( ctx, helper, ) @@ -885,203 +1025,28 @@ func (r *PlacementAPIReconciler) reconcileInit( } instance.Status.Conditions.MarkTrue(condition.DBSyncReadyCondition, condition.DBSyncReadyMessage) - // run placement db sync - end - - Log.Info("Reconciled Service init successfully") - return ctrl.Result{}, nil -} - -func (r *PlacementAPIReconciler) reconcileUpdate(ctx context.Context, instance *placementv1.PlacementAPI, helper *helper.Helper) (ctrl.Result, error) { - Log := r.GetLogger(ctx) - Log.Info("Reconciling Service update") - - // TODO: should have minor update tasks if required - // - delete dbsync hash from status to rerun it? - - Log.Info("Reconciled Service update successfully") - return ctrl.Result{}, nil -} - -func (r *PlacementAPIReconciler) reconcileUpgrade(ctx context.Context, instance *placementv1.PlacementAPI, helper *helper.Helper) (ctrl.Result, error) { - Log := r.GetLogger(ctx) - Log.Info("Reconciling Service update") - // TODO: should have major version upgrade tasks - // -delete dbsync hash from status to rerun it? - - Log.Info("Reconciled Service upgrade successfully") return ctrl.Result{}, nil } -func (r *PlacementAPIReconciler) reconcileNormal(ctx context.Context, instance *placementv1.PlacementAPI, helper *helper.Helper) (ctrl.Result, error) { +func (r *PlacementAPIReconciler) ensureDeployment( + ctx context.Context, + h *helper.Helper, + instance *placementv1.PlacementAPI, + inputHash string, + serviceAnnotations map[string]string) (ctrl.Result, error) { Log := r.GetLogger(ctx) Log.Info("Reconciling Service") - // ConfigMap - configMapVars := make(map[string]env.Setter) - - // - // Create ConfigMaps and Secrets required as input for the Service and calculate an overall hash of hashes - // - - // - // create Configmap required for placement input - // - %-scripts configmap holding scripts to e.g. bootstrap the service - // - %-config configmap holding minimal placement config required to get the service up, user can add additional files to be added to the service - // - parameters which has passwords gets added from the OpenStack secret via the init container - // - err = r.generateServiceConfigMaps(ctx, helper, instance, &configMapVars) - if err != nil { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.ServiceConfigReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.ServiceConfigReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err - } - - // TLS input validation - // - // Validate the CA cert secret if provided - if instance.Spec.TLS.CaBundleSecretName != "" { - hash, ctrlResult, err := tls.ValidateCACertSecret( - ctx, - helper.GetClient(), - types.NamespacedName{ - Name: instance.Spec.TLS.CaBundleSecretName, - Namespace: instance.Namespace, - }, - ) - if err != nil { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.TLSInputReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.TLSInputErrorMessage, - err.Error())) - return ctrlResult, err - } else if (ctrlResult != ctrl.Result{}) { - return ctrlResult, nil - } - - if hash != "" { - configMapVars[tls.CABundleKey] = env.SetValue(hash) - } - } - - // Validate API service certs secrets - certsHash, ctrlResult, err := instance.Spec.TLS.API.ValidateCertSecrets(ctx, helper, instance.Namespace) - if err != nil { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.TLSInputReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.TLSInputErrorMessage, - err.Error())) - return ctrlResult, err - } else if (ctrlResult != ctrl.Result{}) { - return ctrlResult, nil - } - configMapVars[tls.TLSHashName] = env.SetValue(certsHash) - - instance.Status.Conditions.MarkTrue(condition.TLSInputReadyCondition, condition.InputReadyMessage) - // - // create hash over all the different input resources to identify if any those changed - // and a restart/recreate is required. - // - inputHash, hashChanged, err := r.createHashOfInputHashes(ctx, instance, configMapVars) - if err != nil { - return ctrl.Result{}, err - } else if hashChanged { - // Hash changed and instance status should be updated (which will be done by main defer func), - // so we need to return and reconcile again - return ctrl.Result{}, nil - } - instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) - // Create ConfigMaps and Secrets - end - - // - // TODO check when/if Init, Update, or Upgrade should/could be skipped - // - - serviceLabels := map[string]string{ - common.AppSelector: placement.ServiceName, - common.OwnerSelector: instance.Name, - } - - // networks to attach to - for _, netAtt := range instance.Spec.NetworkAttachments { - _, err := nad.GetNADWithName(ctx, helper, netAtt, instance.Namespace) - if err != nil { - if k8s_errors.IsNotFound(err) { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.RequestedReason, - condition.SeverityInfo, - condition.NetworkAttachmentsReadyWaitingMessage, - netAtt)) - return ctrl.Result{RequeueAfter: time.Second * 10}, fmt.Errorf("network-attachment-definition %s not found", netAtt) - } - instance.Status.Conditions.Set(condition.FalseCondition( - condition.NetworkAttachmentsReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err - } - } - - serviceAnnotations, err := nad.CreateNetworksAnnotation(instance.Namespace, instance.Spec.NetworkAttachments) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed create network annotation from %s: %w", - instance.Spec.NetworkAttachments, err) - } - - // Handle service init - ctrlResult, err = r.reconcileInit(ctx, instance, helper, serviceLabels, serviceAnnotations) - if err != nil { - return ctrlResult, err - } else if (ctrlResult != ctrl.Result{}) { - return ctrlResult, nil - } - // Handle service update - ctrlResult, err = r.reconcileUpdate(ctx, instance, helper) - if err != nil { - return ctrlResult, err - } else if (ctrlResult != ctrl.Result{}) { - return ctrlResult, nil - } - - // Handle service upgrade - ctrlResult, err = r.reconcileUpgrade(ctx, instance, helper) - if err != nil { - return ctrlResult, err - } else if (ctrlResult != ctrl.Result{}) { - return ctrlResult, nil - } - - // - // normal reconcile tasks - // + serviceLabels := getServiceLabels() // Define a new Deployment object - deplDef, err := placement.Deployment(ctx, helper, instance, inputHash, serviceLabels, serviceAnnotations) - if err != nil { - instance.Status.Conditions.Set(condition.FalseCondition( - condition.DeploymentReadyCondition, - condition.ErrorReason, - condition.SeverityWarning, - condition.DeploymentReadyErrorMessage, - err.Error())) - return ctrl.Result{}, err - } + deplDef := placement.Deployment(instance, inputHash, serviceLabels, serviceAnnotations) depl := deployment.NewDeployment( deplDef, time.Duration(5)*time.Second, ) - ctrlResult, err = depl.CreateOrPatch(ctx, helper) + ctrlResult, err := depl.CreateOrPatch(ctx, h) if err != nil { instance.Status.Conditions.Set(condition.FalseCondition( condition.DeploymentReadyCondition, @@ -1096,12 +1061,12 @@ func (r *PlacementAPIReconciler) reconcileNormal(ctx context.Context, instance * condition.RequestedReason, condition.SeverityInfo, condition.DeploymentReadyRunningMessage)) - return ctrlResult, nil + return ctrl.Result{}, nil } instance.Status.ReadyCount = depl.GetDeployment().Status.ReadyReplicas // verify if network attachment matches expectations - networkReady, networkAttachmentStatus, err := nad.VerifyNetworkStatusFromAnnotation(ctx, helper, instance.Spec.NetworkAttachments, serviceLabels, instance.Status.ReadyCount) + networkReady, networkAttachmentStatus, err := nad.VerifyNetworkStatusFromAnnotation(ctx, h, instance.Spec.NetworkAttachments, serviceLabels, instance.Status.ReadyCount) if err != nil { return ctrl.Result{}, err } @@ -1149,9 +1114,9 @@ func (r *PlacementAPIReconciler) generateServiceConfigMaps( envVars *map[string]env.Setter, ) error { // - // create Configmap/Secret required for placement input - // - %-scripts configmap holding scripts to e.g. bootstrap the service - // - %-config configmap holding minimal placement config required to get the service up, user can add additional files to be added to the service + // create Secret required for placement input + // - %-scripts secret holding scripts to e.g. bootstrap the service + // - %-config secret holding minimal placement config required to get the service up, user can add additional files to be added to the service // - parameters which has passwords gets added from the ospSecret via the init container // @@ -1182,46 +1147,40 @@ func (r *PlacementAPIReconciler) generateServiceConfigMaps( "ServiceUser": instance.Spec.ServiceUser, "KeystoneInternalURL": keystoneInternalURL, "KeystonePublicURL": keystonePublicURL, + "PlacementPassword": instance.Spec.PasswordSelectors.Service, + "DBUser": instance.Spec.DatabaseUser, + "DBPassword": instance.Spec.PasswordSelectors.Database, + "DBAddress": instance.Status.DatabaseHostname, + "DBName": placement.DatabaseName, "log_file": "/var/log/placement/placement-api.log", } - // create httpd vhost template parameters - httpdVhostConfig := map[string]interface{}{} - for _, endpt := range []service.Endpoint{service.EndpointInternal, service.EndpointPublic} { - endptConfig := map[string]interface{}{} - endptConfig["ServerName"] = fmt.Sprintf("placement-%s.%s.svc", endpt.String(), instance.Namespace) - endptConfig["TLS"] = false // default TLS to false, and set it bellow to true if enabled - if instance.Spec.TLS.API.Enabled(endpt) { - endptConfig["TLS"] = true - endptConfig["SSLCertificateFile"] = fmt.Sprintf("/etc/pki/tls/certs/%s.crt", endpt.String()) - endptConfig["SSLCertificateKeyFile"] = fmt.Sprintf("/etc/pki/tls/private/%s.key", endpt.String()) - } - httpdVhostConfig[endpt.String()] = endptConfig + extraTemplates := map[string]string{ + "placement.conf": "placementapi/config/placement.conf", } - templateParameters["VHosts"] = httpdVhostConfig cms := []util.Template{ // ScriptsConfigMap { - Name: fmt.Sprintf("%s-scripts", instance.Name), - Namespace: instance.Namespace, - Type: util.TemplateTypeScripts, - InstanceType: instance.Kind, - AdditionalTemplate: map[string]string{"common.sh": "/common/common.sh"}, - Labels: cmLabels, + Name: fmt.Sprintf("%s-scripts", instance.Name), + Namespace: instance.Namespace, + Type: util.TemplateTypeScripts, + InstanceType: instance.Kind, + Labels: cmLabels, }, // ConfigMap { - Name: fmt.Sprintf("%s-config-data", instance.Name), - Namespace: instance.Namespace, - Type: util.TemplateTypeConfig, - InstanceType: instance.Kind, - CustomData: customData, - ConfigOptions: templateParameters, - Labels: cmLabels, + Name: fmt.Sprintf("%s-config-data", instance.Name), + Namespace: instance.Namespace, + Type: util.TemplateTypeConfig, + InstanceType: instance.Kind, + CustomData: customData, + ConfigOptions: templateParameters, + Labels: cmLabels, + AdditionalTemplate: extraTemplates, }, } - return configmap.EnsureConfigMaps(ctx, h, instance, cms, envVars) + return secret.EnsureSecrets(ctx, h, instance, cms, envVars) } // createHashOfInputHashes - creates a hash of hashes which gets added to the resources which requires a restart diff --git a/pkg/placement/const.go b/pkg/placement/const.go index 318cf8e1..9cbd8c95 100644 --- a/pkg/placement/const.go +++ b/pkg/placement/const.go @@ -21,6 +21,9 @@ const ( // DatabaseName - DatabaseName = "placement" + //config secret name + ConfigSecretName = "placement-config-data" + // PlacementPublicPort - PlacementPublicPort int32 = 8778 // PlacementInternalPort - diff --git a/pkg/placement/dbsync.go b/pkg/placement/dbsync.go index 13ac6e83..a873b15c 100644 --- a/pkg/placement/dbsync.go +++ b/pkg/placement/dbsync.go @@ -83,16 +83,7 @@ func DbSyncJob( }, } - initContainerDetails := APIDetails{ - ContainerImage: instance.Spec.ContainerImage, - DatabaseHost: instance.Status.DatabaseHostname, - DatabaseUser: instance.Spec.DatabaseUser, - DatabaseName: DatabaseName, - OSPSecret: instance.Spec.Secret, - DBPasswordSelector: instance.Spec.PasswordSelectors.Database, - UserPasswordSelector: instance.Spec.PasswordSelectors.Service, - } - job.Spec.Template.Spec.InitContainers = initContainer(initContainerDetails) + job.Spec.Template.Spec.Volumes = getVolumes(instance.Name) return job } diff --git a/pkg/placement/deployment.go b/pkg/placement/deployment.go index 07312307..75e610b4 100644 --- a/pkg/placement/deployment.go +++ b/pkg/placement/deployment.go @@ -183,17 +183,5 @@ func Deployment( deployment.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector } - initContainerDetails := APIDetails{ - ContainerImage: instance.Spec.ContainerImage, - DatabaseHost: instance.Status.DatabaseHostname, - DatabaseUser: instance.Spec.DatabaseUser, - DatabaseName: DatabaseName, - OSPSecret: instance.Spec.Secret, - DBPasswordSelector: instance.Spec.PasswordSelectors.Database, - UserPasswordSelector: instance.Spec.PasswordSelectors.Service, - VolumeMounts: getInitVolumeMounts(), - } - deployment.Spec.Template.Spec.InitContainers = initContainer(initContainerDetails) - - return deployment, nil + return deployment } diff --git a/pkg/placement/initcontainer.go b/pkg/placement/initcontainer.go deleted file mode 100644 index f62f370b..00000000 --- a/pkg/placement/initcontainer.go +++ /dev/null @@ -1,96 +0,0 @@ -/* - -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 placement - -import ( - env "github.com/openstack-k8s-operators/lib-common/modules/common/env" - - corev1 "k8s.io/api/core/v1" -) - -// APIDetails information -type APIDetails struct { - ContainerImage string - DatabaseHost string - DatabaseUser string - DatabaseName string - OSPSecret string - DBPasswordSelector string - UserPasswordSelector string - VolumeMounts []corev1.VolumeMount -} - -const ( - // InitContainerCommand - - InitContainerCommand = "/usr/local/bin/container-scripts/init.sh" -) - -// initContainer - init container for placement api pods -func initContainer(init APIDetails) []corev1.Container { - runAsUser := int64(0) - - args := []string{ - "-c", - InitContainerCommand, - } - - envVars := map[string]env.Setter{} - envVars["DatabaseHost"] = env.SetValue(init.DatabaseHost) - envVars["DatabaseUser"] = env.SetValue(init.DatabaseUser) - envVars["DatabaseName"] = env.SetValue(init.DatabaseName) - - envs := []corev1.EnvVar{ - { - Name: "DatabasePassword", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: init.OSPSecret, - }, - Key: init.DBPasswordSelector, - }, - }, - }, - { - Name: "PlacementPassword", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: init.OSPSecret, - }, - Key: init.UserPasswordSelector, - }, - }, - }, - } - envs = env.MergeEnvs(envs, envVars) - - return []corev1.Container{ - { - Name: "init", - Image: init.ContainerImage, - SecurityContext: &corev1.SecurityContext{ - RunAsUser: &runAsUser, - }, - Command: []string{ - "/bin/bash", - }, - Args: args, - Env: envs, - VolumeMounts: getInitVolumeMounts(), - }, - } -} diff --git a/pkg/placement/volumes.go b/pkg/placement/volumes.go index 77bded68..fc1d7d73 100644 --- a/pkg/placement/volumes.go +++ b/pkg/placement/volumes.go @@ -22,7 +22,7 @@ import ( // getVolumes - service volumes func getVolumes(name string) []corev1.Volume { var scriptsVolumeDefaultMode int32 = 0755 - var config0640AccessMode int32 = 0640 + var configMode int32 = 0640 return []corev1.Volume{ { @@ -39,20 +39,12 @@ func getVolumes(name string) []corev1.Volume { { Name: "config-data", VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - DefaultMode: &config0640AccessMode, - LocalObjectReference: corev1.LocalObjectReference{ - Name: name + "-config-data", - }, + Secret: &corev1.SecretVolumeSource{ + DefaultMode: &configMode, + SecretName: name + "-config-data", }, }, }, - { - Name: "config-data-merged", - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{Medium: ""}, - }, - }, { Name: "logs", VolumeSource: corev1.VolumeSource{ @@ -63,55 +55,29 @@ func getVolumes(name string) []corev1.Volume { } -// getInitVolumeMounts - general init task VolumeMounts -func getInitVolumeMounts() []corev1.VolumeMount { +// getVolumeMounts - general VolumeMounts +func getVolumeMounts(serviceName string) []corev1.VolumeMount { return []corev1.VolumeMount{ { Name: "scripts", MountPath: "/usr/local/bin/container-scripts", ReadOnly: true, }, - { - Name: "config-data", - MountPath: "/var/lib/config-data/default", - ReadOnly: true, - }, - { - Name: "config-data-merged", - MountPath: "/var/lib/config-data/merged", - ReadOnly: false, - }, { Name: "logs", MountPath: "/var/log/placement", ReadOnly: false, }, - } -} - -// getVolumeMounts - general VolumeMounts -func getVolumeMounts(serviceName string) []corev1.VolumeMount { - return []corev1.VolumeMount{ { - Name: "scripts", - MountPath: "/usr/local/bin/container-scripts", - ReadOnly: true, - }, - { - Name: "config-data-merged", - MountPath: "/var/lib/config-data/merged", + Name: "config-data", + MountPath: "/var/lib/config-data/", ReadOnly: false, }, { - Name: "config-data-merged", + Name: "config-data", MountPath: "/var/lib/kolla/config_files/config.json", SubPath: "placement-" + serviceName + "-config.json", ReadOnly: true, }, - { - Name: "logs", - MountPath: "/var/log/placement", - ReadOnly: false, - }, } } diff --git a/templates/common/common.sh b/templates/common/common.sh deleted file mode 100755 index 35071421..00000000 --- a/templates/common/common.sh +++ /dev/null @@ -1,36 +0,0 @@ -#!/bin//bash -# -# Copyright 2022 Red Hat Inc. -# -# 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. - -set -e - -function merge_config_dir { - echo merge config dir $1 - for conf in $(find $1 -type f) - do - conf_base=$(basename $conf) - - # If CFG already exist in ../merged and is not a json file, - # we expect for now it can be merged using crudini. - # Else, just copy the full file. - if [[ -f /var/lib/config-data/merged/${conf_base} && ${conf_base} != *.json ]]; then - echo merging ${conf} into /var/lib/config-data/merged/${conf_base} - crudini --merge /var/lib/config-data/merged/${conf_base} < ${conf} - else - echo copy ${conf} to /var/lib/config-data/merged/ - cp -f ${conf} /var/lib/config-data/merged/ - fi - done -} diff --git a/templates/placementapi/bin/init.sh b/templates/placementapi/bin/init.sh deleted file mode 100755 index 7c910500..00000000 --- a/templates/placementapi/bin/init.sh +++ /dev/null @@ -1,46 +0,0 @@ -#!/bin//bash -# -# Copyright 2020 Red Hat Inc. -# -# 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. -set -ex - -# This script generates the placement.conf/logging.conf file and -# copies the result to the ephemeral /var/lib/config-data/merged volume. -# -# Secrets are obtained from ENV variables. -export PASSWORD=${PlacementPassword:?"Please specify a PlacementPassword variable."} -export DBHOST=${DatabaseHost:?"Please specify a DatabaseHost variable."} -export DBUSER=${DatabaseUser:?"Please specify a DatabaseUser variable."} -export DBPASSWORD=${DatabasePassword:?"Please specify a DatabasePassword variable."} -export DB=${DatabaseName:-"placement"} - -SVC_CFG=/etc/placement/placement.conf -SVC_CFG_MERGED=/var/lib/config-data/merged/placement.conf - -# expect that the common.sh is in the same dir as the calling script -SCRIPTPATH="$( cd "$(dirname "$0")" >/dev/null 2>&1 ; pwd -P )" -. ${SCRIPTPATH}/common.sh --source-only - -# Copy default service config from container image as base -cp -a ${SVC_CFG} ${SVC_CFG_MERGED} - -# Merge all templates from config CM -for dir in /var/lib/config-data/default -do - merge_config_dir ${dir} -done - -# set secrets -crudini --set ${SVC_CFG_MERGED} placement_database connection mysql+pymysql://${DBUSER}:${DBPASSWORD}@${DBHOST}/${DB} -crudini --set ${SVC_CFG_MERGED} keystone_authtoken password $PASSWORD diff --git a/templates/placementapi/config/placement-api-config.json b/templates/placementapi/config/placement-api-config.json index a0aed17a..2955d08a 100644 --- a/templates/placementapi/config/placement-api-config.json +++ b/templates/placementapi/config/placement-api-config.json @@ -2,23 +2,23 @@ "command": "/usr/sbin/httpd -DFOREGROUND", "config_files": [ { - "source": "/var/lib/config-data/merged/placement.conf", + "source": "/var/lib/config-data/placement.conf", "dest": "/etc/placement/placement.conf", "owner": "placement", "perm": "0600" }, { - "source": "/var/lib/config-data/merged/custom.conf", - "dest": "/etc/placement/placement.conf.d/custom.conf", - "owner": "placement", - "perm": "0600" - }, - { - "source": "/var/lib/config-data/merged/httpd.conf", + "source": "/var/lib/config-data/httpd.conf", "dest": "/etc/httpd/conf/httpd.conf", "owner": "apache", "perm": "0644" }, + { + "source": "/var/lib/config-data/custom.conf", + "dest": "/etc/placement/placement.conf.d/custom.conf", + "owner": "placement", + "perm": "0600" + }, { "source": "/var/lib/config-data/merged/ssl.conf", "dest": "/etc/httpd/conf.d/ssl.conf", @@ -41,7 +41,6 @@ "optional": true, "merge": true } - ], "permissions": [ { diff --git a/templates/placementapi/config/placement-dbsync-config.json b/templates/placementapi/config/placement-dbsync-config.json index 4a7ea0d6..dea1ae00 100644 --- a/templates/placementapi/config/placement-dbsync-config.json +++ b/templates/placementapi/config/placement-dbsync-config.json @@ -2,13 +2,13 @@ "command": "placement-manage db sync", "config_files": [ { - "source": "/var/lib/config-data/merged/placement.conf", + "source": "/var/lib/config-data/placement.conf", "dest": "/etc/placement/placement.conf", "owner": "placement", "perm": "0600" }, { - "source": "/var/lib/config-data/merged/custom.conf", + "source": "/var/lib/config-data/custom.conf", "dest": "/etc/placement/placement.conf.d/custom.conf", "owner": "placement", "perm": "0600" diff --git a/templates/placementapi/config/placement.conf b/templates/placementapi/config/placement.conf index f45c59b0..5ee76710 100644 --- a/templates/placementapi/config/placement.conf +++ b/templates/placementapi/config/placement.conf @@ -8,6 +8,9 @@ log_file = {{ .log_file }} {{end}} debug = true +[placement_database] +connection = mysql+pymysql://{{ .DBUser }}:{{ .DBPassword}}@{{ .DBAddress }}/{{ .DBName }} + [api] auth_strategy = keystone @@ -16,9 +19,8 @@ project_domain_name = Default user_domain_name = Default project_name = service username = {{ .ServiceUser }} +password = {{ .PlacementPassword }} www_authenticate_uri = {{ .KeystonePublicURL }} auth_url = {{ .KeystoneInternalURL }} auth_type = password interface = internal - -[placement_database] diff --git a/tests/functional/placementapi_controller_test.go b/tests/functional/placementapi_controller_test.go index 42ef4d2f..462af74d 100644 --- a/tests/functional/placementapi_controller_test.go +++ b/tests/functional/placementapi_controller_test.go @@ -88,6 +88,24 @@ var _ = Describe("PlacementAPI controller", func() { condition.InputReadyCondition, corev1.ConditionFalse, ) + th.ExpectCondition( + names.PlacementAPIName, + ConditionGetterFunc(PlacementConditionGetter), + condition.RoleBindingReadyCondition, + corev1.ConditionTrue, + ) + th.ExpectCondition( + names.PlacementAPIName, + ConditionGetterFunc(PlacementConditionGetter), + condition.RoleReadyCondition, + corev1.ConditionTrue, + ) + th.ExpectCondition( + names.PlacementAPIName, + ConditionGetterFunc(PlacementConditionGetter), + condition.ServiceAccountReadyCondition, + corev1.ConditionTrue, + ) unknownConditions := []condition.Type{ condition.DBReadyCondition, condition.DBSyncReadyCondition, @@ -97,15 +115,12 @@ var _ = Describe("PlacementAPI controller", func() { condition.KeystoneServiceReadyCondition, condition.KeystoneEndpointReadyCondition, condition.NetworkAttachmentsReadyCondition, - condition.ServiceAccountReadyCondition, - condition.RoleReadyCondition, - condition.RoleBindingReadyCondition, condition.TLSInputReadyCondition, } placement := GetPlacementAPI(names.PlacementAPIName) - // +2 as InputReady and Ready is False asserted above - Expect(placement.Status.Conditions).To(HaveLen(len(unknownConditions) + 2)) + // +5 as InputReady, Ready, Service and Role are ready is False asserted above + Expect(placement.Status.Conditions).To(HaveLen(len(unknownConditions) + 5)) for _, cond := range unknownConditions { th.ExpectCondition( @@ -228,7 +243,7 @@ var _ = Describe("PlacementAPI controller", func() { ) }) It("should create a ConfigMap for placement.conf", func() { - cm := th.GetConfigMap(names.ConfigMapName) + cm := th.GetSecret(names.ConfigMapName) Expect(cm.Data["placement.conf"]).Should( ContainSubstring("auth_url = %s", keystoneAPI.Status.APIEndpoints["internal"])) @@ -236,6 +251,8 @@ var _ = Describe("PlacementAPI controller", func() { ContainSubstring("www_authenticate_uri = %s", keystoneAPI.Status.APIEndpoints["public"])) Expect(cm.Data["placement.conf"]).Should( ContainSubstring("username = placement")) + Expect(cm.Data["placement.conf"]).Should( + ContainSubstring("connection = mysql+pymysql://placement:PlacementDatabasePassword@/placement")) }) It("creates service account, role and rolebindig", func() { @@ -362,28 +379,9 @@ var _ = Describe("PlacementAPI controller", func() { ) job := th.GetJob(names.DBSyncJobName) - Expect(job.Spec.Template.Spec.Volumes).To(HaveLen(4)) - Expect(job.Spec.Template.Spec.InitContainers).To(HaveLen(1)) + Expect(job.Spec.Template.Spec.Volumes).To(HaveLen(3)) Expect(job.Spec.Template.Spec.Containers).To(HaveLen(1)) - init := job.Spec.Template.Spec.InitContainers[0] - Expect(init.VolumeMounts).To(HaveLen(4)) - Expect(init.Args[1]).To(ContainSubstring("init.sh")) - Expect(init.Image).To(Equal("quay.io/podified-antelope-centos9/openstack-placement-api:current-podified")) - env := &corev1.EnvVar{} - Expect(init.Env).To(ContainElement(HaveField("Name", "DatabaseHost"), env)) - Expect(env.Value).To(Equal("hostname-for-openstack")) - Expect(init.Env).To(ContainElement(HaveField("Name", "DatabaseUser"), env)) - Expect(env.Value).To(Equal("placement")) - Expect(init.Env).To(ContainElement(HaveField("Name", "DatabaseName"), env)) - Expect(env.Value).To(Equal("placement")) - Expect(init.Env).To(ContainElement(HaveField("Name", "DatabasePassword"), env)) - Expect(env.ValueFrom.SecretKeyRef.LocalObjectReference.Name).To(Equal(SecretName)) - Expect(env.ValueFrom.SecretKeyRef.Key).To(Equal("PlacementDatabasePassword")) - Expect(init.Env).To(ContainElement(HaveField("Name", "PlacementPassword"), env)) - Expect(env.ValueFrom.SecretKeyRef.LocalObjectReference.Name).To(Equal(SecretName)) - Expect(env.ValueFrom.SecretKeyRef.Key).To(Equal("PlacementPassword")) - container := job.Spec.Template.Spec.Containers[0] Expect(container.VolumeMounts).To(HaveLen(4)) Expect(container.Image).To(Equal("quay.io/podified-antelope-centos9/openstack-placement-api:current-podified")) @@ -671,7 +669,7 @@ var _ = Describe("PlacementAPI controller", func() { deployment := th.GetDeployment(names.DeploymentName) oldConfigHash := GetEnvVarValue(deployment.Spec.Template.Spec.Containers[0].Env, "CONFIG_HASH", "") Expect(oldConfigHash).NotTo(Equal("")) - cm := th.GetConfigMap(names.ConfigMapName) + cm := th.GetSecret(names.ConfigMapName) Expect(cm.Data["custom.conf"]).ShouldNot(ContainSubstring("debug")) Eventually(func(g Gomega) { @@ -687,7 +685,7 @@ var _ = Describe("PlacementAPI controller", func() { g.Expect(newConfigHash).NotTo(Equal("")) g.Expect(newConfigHash).NotTo(Equal(oldConfigHash)) - cm := th.GetConfigMap(names.ConfigMapName) + cm := th.GetSecret(names.ConfigMapName) g.Expect(cm.Data["custom.conf"]).Should(ContainSubstring("debug = true")) }, timeout, interval).Should(Succeed()) }) diff --git a/tests/kuttl/common/assert_sample_deployment.yaml b/tests/kuttl/common/assert_sample_deployment.yaml index 1e16c2e4..9bb6119e 100644 --- a/tests/kuttl/common/assert_sample_deployment.yaml +++ b/tests/kuttl/common/assert_sample_deployment.yaml @@ -178,32 +178,6 @@ spec: successThreshold: 1 timeoutSeconds: 5 resources: {} - initContainers: - - args: - - -c - - /usr/local/bin/container-scripts/init.sh - command: - - /bin/bash - env: - - name: DatabasePassword - valueFrom: - secretKeyRef: - key: PlacementDatabasePassword - name: osp-secret - - name: PlacementPassword - valueFrom: - secretKeyRef: - key: PlacementPassword - name: osp-secret - - name: DatabaseHost - value: openstack - - name: DatabaseName - value: placement - - name: DatabaseUser - value: placement - imagePullPolicy: IfNotPresent - name: init - resources: {} restartPolicy: Always securityContext: {} serviceAccount: placement-placement From 04516ed9e181a6dcdbe721d205909fb418bb5fe4 Mon Sep 17 00:00:00 2001 From: Kamil Sambor Date: Fri, 26 Jan 2024 16:06:25 +0100 Subject: [PATCH 3/5] Sync refactor with tls enable patches --- controllers/placementapi_controller.go | 130 ++++++++++++++---- kuttl-test.yaml | 2 +- pkg/placement/dbsync.go | 2 - pkg/placement/deployment.go | 2 +- .../config/placement-api-config.json | 2 +- templates/placementapi/config/placement.conf | 2 +- .../placementapi_controller_test.go | 6 +- .../common/assert_sample_deployment.yaml | 31 +---- .../common/errors_cleanup_placement.yaml | 23 +--- .../tests/placement_deploy_tls/03-assert.yaml | 32 +---- 10 files changed, 119 insertions(+), 113 deletions(-) diff --git a/controllers/placementapi_controller.go b/controllers/placementapi_controller.go index 4bcfc610..0ef672d4 100644 --- a/controllers/placementapi_controller.go +++ b/controllers/placementapi_controller.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -51,6 +52,7 @@ import ( common_rbac "github.com/openstack-k8s-operators/lib-common/modules/common/rbac" "github.com/openstack-k8s-operators/lib-common/modules/common/secret" "github.com/openstack-k8s-operators/lib-common/modules/common/service" + "github.com/openstack-k8s-operators/lib-common/modules/common/tls" util "github.com/openstack-k8s-operators/lib-common/modules/common/util" mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" @@ -253,6 +255,10 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request Log.Error(err, "Failed to create lib-common Helper") return ctrl.Result{}, err } + // initialize status fields + if err = r.initStatus(ctx, h, instance); err != nil { + return ctrl.Result{}, err + } // Always patch the instance status when exiting this function so we can persist any changes. defer func() { @@ -279,10 +285,6 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, h.GetFinalizer()) { return ctrl.Result{}, nil } - // initialize status fields - if err = r.initStatus(ctx, h, instance); err != nil { - return ctrl.Result{}, err - } // Handle service delete if !instance.DeletionTimestamp.IsZero() { @@ -315,7 +317,7 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request // // check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map // - hash, result, _, err := ensureSecret( + hash, result, secret, err := ensureSecret( ctx, types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, []string{ @@ -346,7 +348,7 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request // all our input checks out so report InputReady instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage) - err = r.generateServiceConfigMaps(ctx, h, instance, &configMapVars) + err = r.generateServiceConfigMaps(ctx, h, instance, secret, &configMapVars) if err != nil { instance.Status.Conditions.Set(condition.FalseCondition( condition.ServiceConfigReadyCondition, @@ -357,6 +359,52 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{}, err } + // TLS input validation + // + // Validate the CA cert secret if provided + if instance.Spec.TLS.CaBundleSecretName != "" { + hash, ctrlResult, err := tls.ValidateCACertSecret( + ctx, + h.GetClient(), + types.NamespacedName{ + Name: instance.Spec.TLS.CaBundleSecretName, + Namespace: instance.Namespace, + }, + ) + if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.TLSInputReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.TLSInputErrorMessage, + err.Error())) + return ctrlResult, err + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil + } + + if hash != "" { + configMapVars[tls.CABundleKey] = env.SetValue(hash) + } + } + + // Validate API service certs secrets + certsHash, ctrlResult, err := instance.Spec.TLS.API.ValidateCertSecrets(ctx, h, instance.Namespace) + if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.TLSInputReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.TLSInputErrorMessage, + err.Error())) + return ctrlResult, err + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil + } + configMapVars[tls.TLSHashName] = env.SetValue(certsHash) + + instance.Status.Conditions.MarkTrue(condition.TLSInputReadyCondition, condition.InputReadyMessage) + // create hash over all the different input resources to identify if any those changed // and a restart/recreate is required. // @@ -398,16 +446,8 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request } result, err = r.ensureDbSync(ctx, instance, h, serviceAnnotations) - if err != nil { - return result, err - } else if (result != ctrl.Result{}) { - return result, nil - } - if (err != nil || result != ctrl.Result{}) { - // We can ignore RequeueAfter as we are watching the Service resource - // but we have to return while waiting for the service to be exposed - return ctrl.Result{}, err + return result, err } result, err = r.ensureDeployment(ctx, h, instance, inputHash, serviceAnnotations) @@ -424,9 +464,10 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{}, nil } -func getServiceLabels() map[string]string { +func getServiceLabels(instance *placementv1.PlacementAPI) map[string]string { return map[string]string{ - common.AppSelector: placement.ServiceName, + common.AppSelector: placement.ServiceName, + common.OwnerSelector: instance.Name, } } @@ -451,7 +492,7 @@ func (r *PlacementAPIReconciler) ensureServiceExposed( } exportLabels := util.MergeStringMaps( - getServiceLabels(), + getServiceLabels(instance), map[string]string{ service.AnnotationEndpointKey: endpointTypeStr, }, @@ -463,7 +504,7 @@ func (r *PlacementAPIReconciler) ensureServiceExposed( Name: endpointName, Namespace: instance.Namespace, Labels: exportLabels, - Selector: getServiceLabels(), + Selector: getServiceLabels(instance), Port: service.GenericServicePort{ Name: endpointName, Port: data.Port, @@ -524,7 +565,12 @@ func (r *PlacementAPIReconciler) ensureServiceExposed( } // create service - end - // TODO: TLS, pass in https as protocol, create TLS cert + // if TLS is enabled + if instance.Spec.TLS.API.Enabled(endpointType) { + // set endpoint protocol to https + data.Protocol = ptr.To(service.ProtocolHTTPS) + } + apiEndpoints[string(endpointType)], err = svc.GetAPIEndpoint( svcOverride.EndpointURL, data.Protocol, data.Path) if err != nil { @@ -593,7 +639,7 @@ func (r *PlacementAPIReconciler) ensureKeystoneServiceUser( Secret: instance.Spec.Secret, PasswordSelector: instance.Spec.PasswordSelectors.Service, } - serviceLabels := getServiceLabels() + serviceLabels := getServiceLabels(instance) ksSvc := keystonev1.NewKeystoneService(ksSvcSpec, instance.Namespace, serviceLabels, time.Duration(10)*time.Second) _, err := ksSvc.CreateOrPatch(ctx, h) if err != nil { @@ -625,7 +671,7 @@ func (r *PlacementAPIReconciler) ensureKeystoneEndpoint( placement.ServiceName, instance.Namespace, ksEndptSpec, - getServiceLabels(), + getServiceLabels(instance), time.Duration(10)*time.Second, ) ctrlResult, err := ksEndpt.CreateOrPatch(ctx, h) @@ -733,6 +779,10 @@ func (r *PlacementAPIReconciler) initConditions( condition.RoleBindingReadyCondition, condition.InitReason, condition.RoleBindingReadyInitMessage), + condition.UnknownCondition( + condition.TLSInputReadyCondition, + condition.InitReason, + condition.InputReadyInitMessage), ) instance.Status.Conditions.Init(&cl) @@ -988,7 +1038,7 @@ func (r *PlacementAPIReconciler) ensureDbSync( serviceAnnotations map[string]string, ) (ctrl.Result, error) { Log := r.GetLogger(ctx) - serviceLabels := getServiceLabels() + serviceLabels := getServiceLabels(instance) dbSyncHash := instance.Status.Hash[placementv1.DbSyncHash] jobDef := placement.DbSyncJob(instance, serviceLabels, serviceAnnotations) dbSyncjob := job.NewJob( @@ -1037,10 +1087,20 @@ func (r *PlacementAPIReconciler) ensureDeployment( Log := r.GetLogger(ctx) Log.Info("Reconciling Service") - serviceLabels := getServiceLabels() + serviceLabels := getServiceLabels(instance) // Define a new Deployment object - deplDef := placement.Deployment(instance, inputHash, serviceLabels, serviceAnnotations) + deplDef, err := placement.Deployment(ctx, h, instance, inputHash, serviceLabels, serviceAnnotations) + + if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.DeploymentReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.DeploymentReadyErrorMessage, + err.Error())) + } + depl := deployment.NewDeployment( deplDef, time.Duration(5)*time.Second, @@ -1111,6 +1171,7 @@ func (r *PlacementAPIReconciler) generateServiceConfigMaps( ctx context.Context, h *helper.Helper, instance *placementv1.PlacementAPI, + ospSecret corev1.Secret, envVars *map[string]env.Setter, ) error { // @@ -1147,14 +1208,29 @@ func (r *PlacementAPIReconciler) generateServiceConfigMaps( "ServiceUser": instance.Spec.ServiceUser, "KeystoneInternalURL": keystoneInternalURL, "KeystonePublicURL": keystonePublicURL, - "PlacementPassword": instance.Spec.PasswordSelectors.Service, + "PlacementPassword": string(ospSecret.Data[instance.Spec.PasswordSelectors.Service]), "DBUser": instance.Spec.DatabaseUser, - "DBPassword": instance.Spec.PasswordSelectors.Database, + "DBPassword": string(ospSecret.Data[instance.Spec.PasswordSelectors.Database]), "DBAddress": instance.Status.DatabaseHostname, "DBName": placement.DatabaseName, "log_file": "/var/log/placement/placement-api.log", } + // create httpd vhost template parameters + httpdVhostConfig := map[string]interface{}{} + for _, endpt := range []service.Endpoint{service.EndpointInternal, service.EndpointPublic} { + endptConfig := map[string]interface{}{} + endptConfig["ServerName"] = fmt.Sprintf("placement-%s.%s.svc", endpt.String(), instance.Namespace) + endptConfig["TLS"] = false // default TLS to false, and set it bellow to true if enabled + if instance.Spec.TLS.API.Enabled(endpt) { + endptConfig["TLS"] = true + endptConfig["SSLCertificateFile"] = fmt.Sprintf("/etc/pki/tls/certs/%s.crt", endpt.String()) + endptConfig["SSLCertificateKeyFile"] = fmt.Sprintf("/etc/pki/tls/private/%s.key", endpt.String()) + } + httpdVhostConfig[endpt.String()] = endptConfig + } + templateParameters["VHosts"] = httpdVhostConfig + extraTemplates := map[string]string{ "placement.conf": "placementapi/config/placement.conf", } diff --git a/kuttl-test.yaml b/kuttl-test.yaml index ef7beaf1..369895d9 100644 --- a/kuttl-test.yaml +++ b/kuttl-test.yaml @@ -34,7 +34,7 @@ kind: TestSuite reportFormat: JSON reportName: kuttl-test-placement namespace: placement-kuttl-tests -timeout: 180 +timeout: 300 parallel: 1 suppress: - events diff --git a/pkg/placement/dbsync.go b/pkg/placement/dbsync.go index a873b15c..dc966487 100644 --- a/pkg/placement/dbsync.go +++ b/pkg/placement/dbsync.go @@ -83,7 +83,5 @@ func DbSyncJob( }, } - job.Spec.Template.Spec.Volumes = getVolumes(instance.Name) - return job } diff --git a/pkg/placement/deployment.go b/pkg/placement/deployment.go index 75e610b4..fd0b3e11 100644 --- a/pkg/placement/deployment.go +++ b/pkg/placement/deployment.go @@ -183,5 +183,5 @@ func Deployment( deployment.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector } - return deployment + return deployment, nil } diff --git a/templates/placementapi/config/placement-api-config.json b/templates/placementapi/config/placement-api-config.json index 2955d08a..36a2fefd 100644 --- a/templates/placementapi/config/placement-api-config.json +++ b/templates/placementapi/config/placement-api-config.json @@ -20,7 +20,7 @@ "perm": "0600" }, { - "source": "/var/lib/config-data/merged/ssl.conf", + "source": "/var/lib/config-data/ssl.conf", "dest": "/etc/httpd/conf.d/ssl.conf", "owner": "apache", "perm": "0644" diff --git a/templates/placementapi/config/placement.conf b/templates/placementapi/config/placement.conf index 5ee76710..aa69f5d0 100644 --- a/templates/placementapi/config/placement.conf +++ b/templates/placementapi/config/placement.conf @@ -9,7 +9,7 @@ log_file = {{ .log_file }} debug = true [placement_database] -connection = mysql+pymysql://{{ .DBUser }}:{{ .DBPassword}}@{{ .DBAddress }}/{{ .DBName }} +connection = mysql+pymysql://{{ .DBUser }}:{{ .DBPassword }}@{{ .DBAddress }}/{{ .DBName }} [api] auth_strategy = keystone diff --git a/tests/functional/placementapi_controller_test.go b/tests/functional/placementapi_controller_test.go index 462af74d..96d73b72 100644 --- a/tests/functional/placementapi_controller_test.go +++ b/tests/functional/placementapi_controller_test.go @@ -252,7 +252,9 @@ var _ = Describe("PlacementAPI controller", func() { Expect(cm.Data["placement.conf"]).Should( ContainSubstring("username = placement")) Expect(cm.Data["placement.conf"]).Should( - ContainSubstring("connection = mysql+pymysql://placement:PlacementDatabasePassword@/placement")) + ContainSubstring("password = 12345678")) + Expect(cm.Data["placement.conf"]).Should( + ContainSubstring("connection = mysql+pymysql://placement:12345678@/placement")) }) It("creates service account, role and rolebindig", func() { @@ -757,7 +759,7 @@ var _ = Describe("PlacementAPI controller", func() { Expect(container.ReadinessProbe.HTTPGet.Scheme).To(Equal(corev1.URISchemeHTTPS)) Expect(container.LivenessProbe.HTTPGet.Scheme).To(Equal(corev1.URISchemeHTTPS)) - configDataMap := th.GetConfigMap(names.ConfigMapName) + configDataMap := th.GetSecret(names.ConfigMapName) Expect(configDataMap).ShouldNot(BeNil()) Expect(configDataMap.Data).Should(HaveKey("httpd.conf")) Expect(configDataMap.Data).Should(HaveKey("ssl.conf")) diff --git a/tests/kuttl/common/assert_sample_deployment.yaml b/tests/kuttl/common/assert_sample_deployment.yaml index 9bb6119e..a4fd5dc1 100644 --- a/tests/kuttl/common/assert_sample_deployment.yaml +++ b/tests/kuttl/common/assert_sample_deployment.yaml @@ -142,10 +142,10 @@ spec: - mountPath: /usr/local/bin/container-scripts name: scripts readOnly: true - - mountPath: /var/lib/config-data/merged - name: config-data-merged + - mountPath: /var/lib/config-data/ + name: config-data - mountPath: /var/lib/kolla/config_files/config.json - name: config-data-merged + name: config-data readOnly: true subPath: placement-api-config.json - mountPath: /var/log/placement @@ -186,17 +186,6 @@ status: availableReplicas: 1 replicas: 1 --- -# the openshift annotations can't be checked through the deployment above -apiVersion: v1 -kind: Pod -metadata: - annotations: - openshift.io/scc: anyuid - labels: - service: placement -status: - phase: Running ---- apiVersion: v1 kind: Service metadata: @@ -226,19 +215,7 @@ spec: type: ClusterIP --- apiVersion: v1 -kind: ConfigMap -metadata: - labels: - placement.openstack.org/name: placement - name: placement-scripts - ownerReferences: - - blockOwnerDeletion: true - controller: true - kind: PlacementAPI - name: placement ---- -apiVersion: v1 -kind: ConfigMap +kind: Secret metadata: labels: placement.openstack.org/name: placement diff --git a/tests/kuttl/common/errors_cleanup_placement.yaml b/tests/kuttl/common/errors_cleanup_placement.yaml index f886461a..1a854f53 100644 --- a/tests/kuttl/common/errors_cleanup_placement.yaml +++ b/tests/kuttl/common/errors_cleanup_placement.yaml @@ -18,15 +18,6 @@ kind: Deployment metadata: name: placement --- -# the openshift annotations can't be checked through the deployment above -apiVersion: v1 -kind: Pod -metadata: - annotations: - openshift.io/scc: anyuid - labels: - service: placement ---- apiVersion: v1 kind: Service metadata: @@ -70,19 +61,7 @@ spec: type: ClusterIP --- apiVersion: v1 -kind: ConfigMap -metadata: - labels: - placement.openstack.org/name: placement - name: placement-scripts - ownerReferences: - - blockOwnerDeletion: true - controller: true - kind: PlacementAPI - name: placement ---- -apiVersion: v1 -kind: ConfigMap +kind: Secret metadata: labels: placement.openstack.org/name: placement diff --git a/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml b/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml index 01460e72..e1787c5d 100644 --- a/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml +++ b/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml @@ -149,10 +149,10 @@ spec: - mountPath: /usr/local/bin/container-scripts name: scripts readOnly: true - - mountPath: /var/lib/config-data/merged - name: config-data-merged + - mountPath: /var/lib/config-data + name: config-data - mountPath: /var/lib/kolla/config_files/config.json - name: config-data-merged + name: config-data readOnly: true subPath: placement-api-config.json - mountPath: /var/log/placement @@ -205,32 +205,6 @@ spec: successThreshold: 1 timeoutSeconds: 5 resources: {} - initContainers: - - args: - - -c - - /usr/local/bin/container-scripts/init.sh - command: - - /bin/bash - env: - - name: DatabasePassword - valueFrom: - secretKeyRef: - key: PlacementDatabasePassword - name: osp-secret - - name: PlacementPassword - valueFrom: - secretKeyRef: - key: PlacementPassword - name: osp-secret - - name: DatabaseHost - value: openstack - - name: DatabaseName - value: placement - - name: DatabaseUser - value: placement - imagePullPolicy: IfNotPresent - name: init - resources: {} restartPolicy: Always securityContext: {} serviceAccount: placement-placement From fa32909d9a9951f0dddec2cda92686d022774425 Mon Sep 17 00:00:00 2001 From: Kamil Sambor Date: Fri, 2 Feb 2024 12:35:33 +0100 Subject: [PATCH 4/5] Fix scripts volum issue --- controllers/placementapi_controller.go | 21 +++++++++++-------- pkg/placement/volumes.go | 8 +++---- .../config/placement-api-config.json | 8 +++---- .../config/placement-dbsync-config.json | 4 ++-- .../common/assert_sample_deployment.yaml | 6 +++--- .../common/errors_cleanup_placement.yaml | 21 +++++++++++++++++++ .../tests/placement_deploy_tls/03-assert.yaml | 6 +++--- 7 files changed, 48 insertions(+), 26 deletions(-) diff --git a/controllers/placementapi_controller.go b/controllers/placementapi_controller.go index 0ef672d4..a249c9f5 100644 --- a/controllers/placementapi_controller.go +++ b/controllers/placementapi_controller.go @@ -431,7 +431,9 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request apiEndpoints, result, err := r.ensureServiceExposed(ctx, h, instance) - if err != nil { + if (err != nil || result != ctrl.Result{}) { + // We can ignore RequeueAfter as we are watching the Service resource + // but we have to return while waiting for the service to be exposed return ctrl.Result{}, err } @@ -441,10 +443,10 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request } result, err = r.ensureKeystoneEndpoint(ctx, h, instance, apiEndpoints) - if err != nil { + if (err != nil || result != ctrl.Result{}) { + // We can ignore RequeueAfter as we are watching the KeystoneEndpoint resource return ctrl.Result{}, err } - result, err = r.ensureDbSync(ctx, instance, h, serviceAnnotations) if (err != nil || result != ctrl.Result{}) { return result, err @@ -482,6 +484,7 @@ func (r *PlacementAPIReconciler) ensureServiceExposed( } apiEndpoints := make(map[string]string) + serviceLabels := getServiceLabels(instance) for endpointType, data := range placementEndpoints { endpointTypeStr := string(endpointType) endpointName := placement.ServiceName + "-" + endpointTypeStr @@ -492,7 +495,7 @@ func (r *PlacementAPIReconciler) ensureServiceExposed( } exportLabels := util.MergeStringMaps( - getServiceLabels(instance), + serviceLabels, map[string]string{ service.AnnotationEndpointKey: endpointTypeStr, }, @@ -504,7 +507,7 @@ func (r *PlacementAPIReconciler) ensureServiceExposed( Name: endpointName, Namespace: instance.Namespace, Labels: exportLabels, - Selector: getServiceLabels(instance), + Selector: serviceLabels, Port: service.GenericServicePort{ Name: endpointName, Port: data.Port, @@ -522,7 +525,7 @@ func (r *PlacementAPIReconciler) ensureServiceExposed( condition.ExposeServiceReadyErrorMessage, err.Error())) - return nil, ctrl.Result{}, err + return apiEndpoints, ctrl.Result{}, err } svc.AddAnnotation(map[string]string{ @@ -554,14 +557,14 @@ func (r *PlacementAPIReconciler) ensureServiceExposed( condition.ExposeServiceReadyErrorMessage, err.Error())) - return nil, ctrlResult, err + return apiEndpoints, ctrlResult, err } else if (ctrlResult != ctrl.Result{}) { instance.Status.Conditions.Set(condition.FalseCondition( condition.ExposeServiceReadyCondition, condition.RequestedReason, condition.SeverityInfo, condition.ExposeServiceReadyRunningMessage)) - return nil, ctrlResult, nil + return apiEndpoints, ctrlResult, nil } // create service - end @@ -574,7 +577,7 @@ func (r *PlacementAPIReconciler) ensureServiceExposed( apiEndpoints[string(endpointType)], err = svc.GetAPIEndpoint( svcOverride.EndpointURL, data.Protocol, data.Path) if err != nil { - return nil, ctrl.Result{}, err + return apiEndpoints, ctrl.Result{}, err } } diff --git a/pkg/placement/volumes.go b/pkg/placement/volumes.go index fc1d7d73..578186ca 100644 --- a/pkg/placement/volumes.go +++ b/pkg/placement/volumes.go @@ -28,11 +28,9 @@ func getVolumes(name string) []corev1.Volume { { Name: "scripts", VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ + Secret: &corev1.SecretVolumeSource{ DefaultMode: &scriptsVolumeDefaultMode, - LocalObjectReference: corev1.LocalObjectReference{ - Name: name + "-scripts", - }, + SecretName: name + "-scripts", }, }, }, @@ -70,7 +68,7 @@ func getVolumeMounts(serviceName string) []corev1.VolumeMount { }, { Name: "config-data", - MountPath: "/var/lib/config-data/", + MountPath: "/var/lib/config-data/merged", ReadOnly: false, }, { diff --git a/templates/placementapi/config/placement-api-config.json b/templates/placementapi/config/placement-api-config.json index 36a2fefd..d9bfe9b7 100644 --- a/templates/placementapi/config/placement-api-config.json +++ b/templates/placementapi/config/placement-api-config.json @@ -2,25 +2,25 @@ "command": "/usr/sbin/httpd -DFOREGROUND", "config_files": [ { - "source": "/var/lib/config-data/placement.conf", + "source": "/var/lib/config-data/merged/placement.conf", "dest": "/etc/placement/placement.conf", "owner": "placement", "perm": "0600" }, { - "source": "/var/lib/config-data/httpd.conf", + "source": "/var/lib/config-data/merged/httpd.conf", "dest": "/etc/httpd/conf/httpd.conf", "owner": "apache", "perm": "0644" }, { - "source": "/var/lib/config-data/custom.conf", + "source": "/var/lib/config-data/merged/custom.conf", "dest": "/etc/placement/placement.conf.d/custom.conf", "owner": "placement", "perm": "0600" }, { - "source": "/var/lib/config-data/ssl.conf", + "source": "/var/lib/config-data/merged/ssl.conf", "dest": "/etc/httpd/conf.d/ssl.conf", "owner": "apache", "perm": "0644" diff --git a/templates/placementapi/config/placement-dbsync-config.json b/templates/placementapi/config/placement-dbsync-config.json index dea1ae00..4a7ea0d6 100644 --- a/templates/placementapi/config/placement-dbsync-config.json +++ b/templates/placementapi/config/placement-dbsync-config.json @@ -2,13 +2,13 @@ "command": "placement-manage db sync", "config_files": [ { - "source": "/var/lib/config-data/placement.conf", + "source": "/var/lib/config-data/merged/placement.conf", "dest": "/etc/placement/placement.conf", "owner": "placement", "perm": "0600" }, { - "source": "/var/lib/config-data/custom.conf", + "source": "/var/lib/config-data/merged/custom.conf", "dest": "/etc/placement/placement.conf.d/custom.conf", "owner": "placement", "perm": "0600" diff --git a/tests/kuttl/common/assert_sample_deployment.yaml b/tests/kuttl/common/assert_sample_deployment.yaml index a4fd5dc1..5824c140 100644 --- a/tests/kuttl/common/assert_sample_deployment.yaml +++ b/tests/kuttl/common/assert_sample_deployment.yaml @@ -142,14 +142,14 @@ spec: - mountPath: /usr/local/bin/container-scripts name: scripts readOnly: true - - mountPath: /var/lib/config-data/ + - mountPath: /var/log/placement + name: logs + - mountPath: /var/lib/config-data/merged name: config-data - mountPath: /var/lib/kolla/config_files/config.json name: config-data readOnly: true subPath: placement-api-config.json - - mountPath: /var/log/placement - name: logs - args: - -c - /usr/local/bin/kolla_start diff --git a/tests/kuttl/common/errors_cleanup_placement.yaml b/tests/kuttl/common/errors_cleanup_placement.yaml index 1a854f53..7679e247 100644 --- a/tests/kuttl/common/errors_cleanup_placement.yaml +++ b/tests/kuttl/common/errors_cleanup_placement.yaml @@ -18,6 +18,15 @@ kind: Deployment metadata: name: placement --- +# the openshift annotations can't be checked through the deployment above +apiVersion: v1 +kind: Pod +metadata: + annotations: + openshift.io/scc: anyuid + labels: + service: placement +--- apiVersion: v1 kind: Service metadata: @@ -62,6 +71,18 @@ spec: --- apiVersion: v1 kind: Secret +metadata: + labels: + placement.openstack.org/name: placement + name: placement-scripts + ownerReferences: + - blockOwnerDeletion: true + controller: true + kind: PlacementAPI + name: placement +--- +apiVersion: v1 +kind: Secret metadata: labels: placement.openstack.org/name: placement diff --git a/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml b/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml index e1787c5d..9c9b9b3f 100644 --- a/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml +++ b/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml @@ -149,14 +149,14 @@ spec: - mountPath: /usr/local/bin/container-scripts name: scripts readOnly: true - - mountPath: /var/lib/config-data + - mountPath: /var/log/placement + name: logs + - mountPath: /var/lib/config-data/merged name: config-data - mountPath: /var/lib/kolla/config_files/config.json name: config-data readOnly: true subPath: placement-api-config.json - - mountPath: /var/log/placement - name: logs - mountPath: /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem name: combined-ca-bundle readOnly: true From 826dafd93ed358771fe1cf8fbab2006c75e2c89e Mon Sep 17 00:00:00 2001 From: Kamil Sambor Date: Tue, 6 Feb 2024 11:02:23 +0100 Subject: [PATCH 5/5] Change secret mounting path to allign with nova-operator --- pkg/placement/volumes.go | 2 +- .../placementapi/config/placement-api-config.json | 8 ++++---- .../config/placement-dbsync-config.json | 4 ++-- tests/kuttl/common/assert_sample_deployment.yaml | 13 ++++++++++++- .../kuttl/tests/placement_deploy_tls/03-assert.yaml | 2 +- 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/pkg/placement/volumes.go b/pkg/placement/volumes.go index 578186ca..fa60ef15 100644 --- a/pkg/placement/volumes.go +++ b/pkg/placement/volumes.go @@ -68,7 +68,7 @@ func getVolumeMounts(serviceName string) []corev1.VolumeMount { }, { Name: "config-data", - MountPath: "/var/lib/config-data/merged", + MountPath: "/var/lib/openstack/config", ReadOnly: false, }, { diff --git a/templates/placementapi/config/placement-api-config.json b/templates/placementapi/config/placement-api-config.json index d9bfe9b7..210cc117 100644 --- a/templates/placementapi/config/placement-api-config.json +++ b/templates/placementapi/config/placement-api-config.json @@ -2,25 +2,25 @@ "command": "/usr/sbin/httpd -DFOREGROUND", "config_files": [ { - "source": "/var/lib/config-data/merged/placement.conf", + "source": "/var/lib/openstack/config/placement.conf", "dest": "/etc/placement/placement.conf", "owner": "placement", "perm": "0600" }, { - "source": "/var/lib/config-data/merged/httpd.conf", + "source": "/var/lib/openstack/config/httpd.conf", "dest": "/etc/httpd/conf/httpd.conf", "owner": "apache", "perm": "0644" }, { - "source": "/var/lib/config-data/merged/custom.conf", + "source": "/var/lib/openstack/config/custom.conf", "dest": "/etc/placement/placement.conf.d/custom.conf", "owner": "placement", "perm": "0600" }, { - "source": "/var/lib/config-data/merged/ssl.conf", + "source": "/var/lib/openstack/config/ssl.conf", "dest": "/etc/httpd/conf.d/ssl.conf", "owner": "apache", "perm": "0644" diff --git a/templates/placementapi/config/placement-dbsync-config.json b/templates/placementapi/config/placement-dbsync-config.json index 4a7ea0d6..fd7a407b 100644 --- a/templates/placementapi/config/placement-dbsync-config.json +++ b/templates/placementapi/config/placement-dbsync-config.json @@ -2,13 +2,13 @@ "command": "placement-manage db sync", "config_files": [ { - "source": "/var/lib/config-data/merged/placement.conf", + "source": "/var/lib/openstack/config/placement.conf", "dest": "/etc/placement/placement.conf", "owner": "placement", "perm": "0600" }, { - "source": "/var/lib/config-data/merged/custom.conf", + "source": "/var/lib/openstack/config/custom.conf", "dest": "/etc/placement/placement.conf.d/custom.conf", "owner": "placement", "perm": "0600" diff --git a/tests/kuttl/common/assert_sample_deployment.yaml b/tests/kuttl/common/assert_sample_deployment.yaml index 5824c140..cc80074f 100644 --- a/tests/kuttl/common/assert_sample_deployment.yaml +++ b/tests/kuttl/common/assert_sample_deployment.yaml @@ -144,7 +144,7 @@ spec: readOnly: true - mountPath: /var/log/placement name: logs - - mountPath: /var/lib/config-data/merged + - mountPath: /var/lib/openstack/config name: config-data - mountPath: /var/lib/kolla/config_files/config.json name: config-data @@ -186,6 +186,17 @@ status: availableReplicas: 1 replicas: 1 --- +# the openshift annotations can't be checked through the deployment above +apiVersion: v1 +kind: Pod +metadata: + annotations: + openshift.io/scc: anyuid + labels: + service: placement +status: + phase: Running +--- apiVersion: v1 kind: Service metadata: diff --git a/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml b/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml index 9c9b9b3f..0935f8be 100644 --- a/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml +++ b/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml @@ -151,7 +151,7 @@ spec: readOnly: true - mountPath: /var/log/placement name: logs - - mountPath: /var/lib/config-data/merged + - mountPath: /var/lib/openstack/config name: config-data - mountPath: /var/lib/kolla/config_files/config.json name: config-data