Rework the whole Manila configuration process #125

fmount · 2023-08-26T11:20:26Z

This patch represents an implementation of the proposal [1] and aligns the manila-operator with the work already done in Cinder and the other operators.

There are a few relevant changes in the bootstrap process of Manila, in particular:

It stops using an InitContainer to generate the snippet files that configure each Manila service. The logic that was previously implemented in the InitContainer has been moved to the Controllers, where config files are generated and stored in k8s Secrets.
InitContainers are fully removed from the bootstrap process; It uses to copy additional files (httpd and wsgi config in manila-api) to the target directories, and mount the generated config (0{0,1,2,3}-config.conf to /etc/manila/manila.conf.d, which is used by each service to run;
The relevant content, including scripts, previously stored in a ConfigMap, are now stored in a corresponding k8s Secret, which is mounted to the Service deployment Pods; A total of 4 config snippet files are generated:

00-config.conf contains global settings that are common to every Manila Pod, including ones that are derived from deployment secrets (e.g. database password, etc.)
01-config.conf contains the global customServiceConfig settings that apply to every Manila service.
02-config.conf contains the customServiceConfig settings that are specific to each service.
03-config.conf contains secrets specified by each service's customServiceConfigSecrets.

logging.conf has been removed as it' s no longer required in the switch to a side container approach for logging purposes, which will be part of a follow up PR.
Finally, functional tests are aligned to the use of k8s Secrets instead of the old pattern based on ConfigMaps; kuttl tests are updated and the initContainer has been removed. DBsync now mounts only the required files (a minimal 00-config.conf) and a db-sync-config.json containing the command run through kolla.

[1] openstack-k8s-operators/dev-docs#31

silvacarloss

Looking good @fmount - sorry for the delay on the review.
I have one minor comment and two questions inline, please take a look :)

controllers/manila_controller.go

controllers/manilaapi_controller.go

controllers/manilascheduler_controller.go

fmount · 2023-09-02T11:00:32Z

Looking good @fmount - sorry for the delay on the review. I have one minor comment and two questions inline, please take a look :)

Thanks @silvacarloss for the review! Regarding code deduplication, I think we need to follow up on that: this change is big enough and hard to review because of many changes (but all of them are connected together). We'll talk more about it and
we can follow up!

fmount · 2023-09-04T06:37:14Z

/test manila-operator-build-deploy-kuttl

fmount · 2023-09-04T09:08:14Z

The kuttl error here is pretty clear at this point:

install_yaml switched to the "any-namespace" approach with [1]
there's a pending patch [2] (which is currently failing) to make the manila operator consistent with the behavior introduced in [1]
kuttl here is failing because all the resources are created in the manila-kuttl-test namespace while the Manila CR is applied against openstack namespace. Being instance.Namespace == openstack, it let the rabbitMQ transportURL CR being created in the openstack namespace, and infra operator isn't able to reconcile it and create the secret, required by Manila to unblock the reconcile loop.

[1] openstack-k8s-operators/install_yamls#489
[2] #122

silvacarloss

Looks good, Francesco! Thank you for working on this change

This patch represents an implementation of the proposal [1] and aligns the manila-operator with the work already done in Cinder and the other operators. There are a few relevant changes in the bootstrap process of Manila, in particular: 1. It stops using an InitContainer to generate the snippet files that configure each Manila service. The logic that was previously implemented in the InitContainer has been moved to the Controllers, where config files are generated and stored in k8s Secrets. 2. InitContainers are fully removed from the bootstrap process; It uses to copy additional files (httpd and wsgi config in manila-api) to the target directories, and mount the generated config (0{0,1,2,3}-config.conf to /etc/manila/manila.conf.d, which is used by each service to run; 3. The relevant content, including scripts, previously stored in a ConfigMap, are now stored in a corresponding k8s Secret, which is mounted to the Service deployment Pods; A total of 4 config snippet files are generated: - 00-config.conf contains global settings that are common to every Manila Pod, including ones that are derived from deployment secrets (e.g. database password, etc.) - 01-config.conf contains the global customServiceConfig settings that apply to every Manila service. - 02-config.conf contains the customServiceConfig settings that are specific to each service. - 03-config.conf contains secrets specified by each service's customServiceConfigSecrets. logging.conf has been removed as it' s no longer required in the switch to a side container approach for logging purposes, which will be part of a follow up PR. Finally, functional tests are aligned to the use of k8s Secrets instead of the old pattern based on ConfigMaps; kuttl tests are updated and the initContainer has been removed. DBsync now mounts only the required files (a minimal 00-config.conf) and a db-sync-config.json containing the command run through kolla. [1] openstack-k8s-operators/dev-docs#31 Signed-off-by: Francesco Pantano <[email protected]>

gouthampacha

/lgtm

thanks @fmount

gouthampacha · 2023-09-09T16:00:21Z

api/v1beta1/common_types.go

@@ -79,7 +79,7 @@ type ManilaServiceTemplate struct {
 	// +kubebuilder:default="# add your customization here"
 	// CustomServiceConfig - customize the service config using this parameter to change service defaults,
 	// or overwrite rendered information using raw OpenStack config format. The content gets added to
-	// to /etc/<service>/<service>.conf.d directory as custom.conf file.
+	// to /etc/<service>/<service>.conf.d directory a custom config file.


nit: as a custom config file

openshift-ci · 2023-09-09T16:05:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fmount, gouthampacha, silvacarloss

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [fmount,gouthampacha]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gouthampacha · 2023-09-09T18:16:04Z

/retest-required

silvacarloss · 2023-09-11T19:23:35Z

Tardy LGTM. Thanks, Francesco!

openshift-ci bot requested review from dprince and olliewalsh August 26, 2023 11:20

fmount requested a review from gouthampacha August 26, 2023 11:20

openshift-ci bot added the approved label Aug 26, 2023

fmount requested a review from silvacarloss August 26, 2023 11:20

fmount added the do-not-merge/hold label Aug 26, 2023

fmount force-pushed the init branch 3 times, most recently from 01753cb to 54b3bc5 Compare August 26, 2023 21:09

silvacarloss reviewed Sep 1, 2023

View reviewed changes

controllers/manila_controller.go Outdated Show resolved Hide resolved

controllers/manilaapi_controller.go Outdated Show resolved Hide resolved

controllers/manilascheduler_controller.go Show resolved Hide resolved

fmount force-pushed the init branch from 54b3bc5 to 28363d1 Compare September 2, 2023 10:58

fmount requested a review from silvacarloss September 2, 2023 11:00

fmount mentioned this pull request Sep 4, 2023

Change kuttl tests to run in arbitrary namespace #122

Closed

silvacarloss approved these changes Sep 4, 2023

View reviewed changes

openshift-ci bot assigned silvacarloss Sep 4, 2023

openshift-ci bot added the lgtm label Sep 4, 2023

fmount force-pushed the init branch from 28363d1 to 54812e3 Compare September 7, 2023 05:59

openshift-ci bot removed the lgtm label Sep 7, 2023

fmount removed the do-not-merge/hold label Sep 7, 2023

gouthampacha approved these changes Sep 9, 2023

View reviewed changes

openshift-ci bot assigned gouthampacha Sep 9, 2023

openshift-ci bot added the lgtm label Sep 9, 2023

openshift-merge-robot merged commit 466af12 into openstack-k8s-operators:main Sep 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework the whole Manila configuration process #125

Rework the whole Manila configuration process #125

fmount commented Aug 26, 2023

silvacarloss left a comment

fmount commented Sep 2, 2023

fmount commented Sep 4, 2023

fmount commented Sep 4, 2023

silvacarloss left a comment •

edited

Loading

gouthampacha left a comment

gouthampacha Sep 9, 2023

openshift-ci bot commented Sep 9, 2023

gouthampacha commented Sep 9, 2023

silvacarloss commented Sep 11, 2023

Rework the whole Manila configuration process #125

Rework the whole Manila configuration process #125

Conversation

fmount commented Aug 26, 2023

silvacarloss left a comment

Choose a reason for hiding this comment

fmount commented Sep 2, 2023

fmount commented Sep 4, 2023

fmount commented Sep 4, 2023

silvacarloss left a comment • edited Loading

Choose a reason for hiding this comment

gouthampacha left a comment

Choose a reason for hiding this comment

gouthampacha Sep 9, 2023

Choose a reason for hiding this comment

openshift-ci bot commented Sep 9, 2023

gouthampacha commented Sep 9, 2023

silvacarloss commented Sep 11, 2023

silvacarloss left a comment •

edited

Loading