Updates and mariadb related fixes #245

weinimo · 2024-02-19T09:28:34Z

Used openstack-k8s-operators/designate-operator#147 as a template.

Depends-On: install_yamls#742

openshift-ci · 2024-02-19T09:28:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: weinimo

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~~ [weinimo]

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

weinimo · 2024-02-19T09:58:41Z

/retest

go.mod

controllers/octavia_controller.go

go.mod

zzzeek · 2024-02-21T14:19:42Z

pkg/octavia/dbsync.go

@@ -83,7 +83,7 @@ func DbSyncJob(
 	initContainerDetails := APIDetails{
 		ContainerImage:       instance.Spec.OctaviaAPI.ContainerImage,
 		DatabaseHost:         instance.Status.DatabaseHostname,
-		DatabaseUser:         instance.Spec.DatabaseUser,
+		DatabaseAccount:      instance.Spec.DatabaseAccount,


this part is going to have to change more. The prior use of DatabaseUser was also not appropriate here as this forms part of the job hash. it means as the username changes, the hash of the dbsync job changes and it will keep running again. This hash should only be based on the database name, not the username.

Yesterday I figured all this out and made these changes for designate and heat; the database account / secret has to be put into the configmaps, which also ideally needs to be a secret, not a configmap.

see the designate / heat PRs for how I changed the job hash for dbsync and others to not use account:
openstack-k8s-operators/designate-operator#156 openstack-k8s-operators/heat-operator#322

these also change from configmap to secret, based on guidance in this doc: https://github.com/openstack-k8s-operators/docs/blob/main/service_config.md#proposed-approach

I took a look at the keystone-operator to see what APIs they use to use a Secret for the config

thread: https://redhat-internal.slack.com/archives/CQXJFGMK6/p1708453474221679

I also replaced ConfigMapVolumeSource with SecretVolumeSource. I see in the Designate PR that you removed DBPasswordSelector as well, so I removed it here too.

weinimo · 2024-02-21T15:47:35Z

Current error:
E0221 14:33:23.689026 1 reflector.go:147] go/pkg/mod/k8s.io/[email protected]/tools/cache/reflector.go:229: Failed to watch *v1beta1.MariaDBAccount: failed to list *v1beta1.MariaDBAccount: mariadbaccounts.mariadb.openstack.org is forbidden: User "system:serviceaccount:openstack-operators:octavia-operator-controller-manager" cannot list resource "mariadbaccounts" in API group "mariadb.openstack.org" at the cluster scope

See https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openstack-k8s-operators_octavia-operator/245/pull-ci-openstack-k8s-operators-octavia-operator-main-octavia-operator-build-deploy-kuttl/1760298908078051328/artifacts/octavia-operator-build-deploy-kuttl/openstack-k8s-operators-gather/artifacts/must-gather/quay-io-openstack-k8s-operators-openstack-must-gather-sha256-f745587ef185f9023b8c83c4d40a4e1c7478de462a91dc6280e5277545650bbe/namespaces/openstack-operators/pods/octavia-operator-controller-manager-d789d9578-kt2wv/logs/manager.log

zzzeek · 2024-02-21T15:50:57Z

Current error: E0221 14:33:23.689026 1 reflector.go:147] go/pkg/mod/k8s.io/[email protected]/tools/cache/reflector.go:229: Failed to watch *v1beta1.MariaDBAccount: failed to list *v1beta1.MariaDBAccount: mariadbaccounts.mariadb.openstack.org is forbidden: User "system:serviceaccount:openstack-operators:octavia-operator-controller-manager" cannot list resource "mariadbaccounts" in API group "mariadb.openstack.org" at the cluster scope

See https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openstack-k8s-operators_octavia-operator/245/pull-ci-openstack-k8s-operators-octavia-operator-main-octavia-operator-build-deploy-kuttl/1760298908078051328/artifacts/octavia-operator-build-deploy-kuttl/openstack-k8s-operators-gather/artifacts/must-gather/quay-io-openstack-k8s-operators-openstack-must-gather-sha256-f745587ef185f9023b8c83c4d40a4e1c7478de462a91dc6280e5277545650bbe/namespaces/openstack-operators/pods/octavia-operator-controller-manager-d789d9578-kt2wv/logs/manager.log

add a role directive on top of the controller, near the mariadbdatabase one at

octavia-operator/controllers/octavia_controller.go

Line 80 in ab4ee64

    
           // +kubebuilder:rbac:groups=mariadb.openstack.org,resources=mariadbdatabases,verbs=get;list;watch;create;update;patch;delete;

// +kubebuilder:rbac:groups=mariadb.openstack.org,resources=mariadbaccounts,verbs=get;list;watch;create;update;patch;delete

then do a make manifests and you should see new entries in config/rbac/role.yaml

the controller should already have some of these roles...

weinimo · 2024-03-05T17:07:38Z

/test all

weinimo · 2024-03-05T17:28:03Z

/test all

weinimo · 2024-03-06T16:42:36Z

/test octavia-operator-build-deploy-kuttl

weinimo · 2024-03-06T16:48:10Z

/test octavia-operator-build-deploy-kuttl

zzzeek

I've gone through everything we should need to get this ready. There are a few additional steps that could happen here, including reorg for my.cnf file and switch EnsureConfigMaps to EnsureSecret, however I can do these in a followup

api/v1beta1/amphoracontroller_types.go

api/v1beta1/octavia_types.go

api/v1beta1/octaviaapi_types.go

controllers/amphoracontroller_controller.go

pkg/octaviaapi/deployment.go

templates/octavia/bin/init.sh

templates/octavia/config/octavia.conf

templates/octaviaamphoracontroller/bin/init.sh

templates/octaviaapi/bin/init.sh

zzzeek · 2024-03-11T22:15:27Z

/retest

weinimo · 2024-03-12T09:08:07Z

kuttl test are currently broken. openstack-k8s-operators/install_yamls#742 should fix it.

weinimo · 2024-03-13T10:49:20Z

/retest

migrate von databaseUsername to databaseAccount and fully use MariaDBAccount Use EnsureMariaDBAccount fix database.connection and more PR review comments Remove DBPasswordSelector attribute Replace ConfigMapVolumeSource ... with SecretVolumeSource

openshift-ci · 2024-03-13T15:35:11Z

@weinimo: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/octavia-operator-build-deploy-kuttl	`ff1f0df`	link	true	`/test octavia-operator-build-deploy-kuttl`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

beagles · 2024-03-13T16:04:32Z

looks like db-sync job is failing

zzzeek · 2024-03-13T16:08:18Z

that usually happens on these PRs because the DB URL is not correct in the conf file.

i can take a look if you want

weinimo · 2024-03-13T17:03:59Z

I will be out till 25th of March, so I would appreciate if you could have a look in the meantime. I pushed the changes of this PR to https://github.com/openstack-k8s-operators/octavia-operator/tree/mariadb-bump so that you could push commits there and create a new PR if needed. Thanks.

zzzeek · 2024-03-13T18:00:43Z

controllers/octavia_controller.go

+		instance.Spec.DatabaseInstance,  // mariadb/galera service to target
+		octavia.PersistenceDatabaseName, // name used in CREATE DATABASE in mariadb
+		"octavia-persistence",           // CR name for MariaDBDatabase
+		instance.Spec.DatabaseAccount,   // CR name for MariaDBAccount


OK, this is the problem, or at least one of them, a MariaDBAccount is only for one MariaDBDatabase. If octavia has two databases, we need two MariaDBAccounts

weinimo · 2024-03-14T12:11:03Z

Superseded by #269

weinimo requested review from gibizer, beagles and gthiemonge February 19, 2024 09:28

openshift-ci bot requested review from dprince and frenzyfriday February 19, 2024 09:28

openshift-merge-robot added the needs-rebase label Feb 19, 2024

openshift-ci bot added the approved label Feb 19, 2024

weinimo force-pushed the mariadb-bump branch from 50851fe to f3eed9b Compare February 19, 2024 09:37

openshift-merge-robot removed the needs-rebase label Feb 19, 2024

weinimo commented Feb 19, 2024

View reviewed changes

go.mod Outdated Show resolved Hide resolved

weinimo force-pushed the mariadb-bump branch 6 times, most recently from 52f6471 to 718da2e Compare February 20, 2024 13:59

weinimo mentioned this pull request Feb 20, 2024

Update kuttl test #244

Closed

weinimo commented Feb 21, 2024

View reviewed changes

controllers/octavia_controller.go Show resolved Hide resolved

go.mod Outdated Show resolved Hide resolved

weinimo changed the title ~~Bump mariadb-operator version~~ Updates and mariadb related fixes Feb 21, 2024

weinimo assigned zzzeek Feb 21, 2024

weinimo force-pushed the mariadb-bump branch from d70c69f to f13bf1b Compare February 21, 2024 13:10

zzzeek reviewed Feb 21, 2024

View reviewed changes

openshift-merge-robot added the needs-rebase label Feb 22, 2024

weinimo force-pushed the mariadb-bump branch from 65c10a9 to 203aae9 Compare March 5, 2024 13:44

openshift-merge-robot removed the needs-rebase label Mar 5, 2024

weinimo force-pushed the mariadb-bump branch from 203aae9 to 66c7fba Compare March 5, 2024 17:03

openshift-ci bot added the do-not-merge/work-in-progress label Mar 5, 2024

weinimo force-pushed the mariadb-bump branch from 3a6fa0c to 6f3b39f Compare March 5, 2024 17:27

weinimo force-pushed the mariadb-bump branch from 6f3b39f to 8f8b4a4 Compare March 6, 2024 10:58

zzzeek requested changes Mar 9, 2024

View reviewed changes

weinimo marked this pull request as ready for review March 11, 2024 11:16

openshift-ci bot removed the do-not-merge/work-in-progress label Mar 11, 2024

openshift-ci bot requested review from slawqo and viroel March 11, 2024 11:16

weinimo force-pushed the mariadb-bump branch from 9c3a4d7 to c99979c Compare March 11, 2024 11:20

weinimo requested a review from zzzeek March 12, 2024 09:06

weinimo force-pushed the mariadb-bump branch from 5eefbb0 to 2f9720a Compare March 13, 2024 13:09

weinimo added 2 commits March 13, 2024 09:56

update kuttl test

73769c4

migrate von databaseUsername to databaseAccount and fully use MariaDBAccount Use EnsureMariaDBAccount fix database.connection and more PR review comments Remove DBPasswordSelector attribute Replace ConfigMapVolumeSource ... with SecretVolumeSource

Fixed build errors

215efdb

weinimo force-pushed the mariadb-bump branch from 849ce97 to 215efdb Compare March 13, 2024 13:57

Undo modification of metadata annotation in kuttl

ff1f0df

weinimo force-pushed the mariadb-bump branch from c5752a5 to ff1f0df Compare March 13, 2024 14:35

zzzeek reviewed Mar 13, 2024

View reviewed changes

weinimo closed this Mar 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates and mariadb related fixes #245

Updates and mariadb related fixes #245

weinimo commented Feb 19, 2024 •

edited

Loading

openshift-ci bot commented Feb 19, 2024

weinimo commented Feb 19, 2024

zzzeek Feb 21, 2024

zzzeek Feb 21, 2024

This comment was marked as resolved.

weinimo Mar 12, 2024

weinimo Mar 12, 2024

weinimo commented Feb 21, 2024

zzzeek commented Feb 21, 2024

weinimo commented Mar 5, 2024

weinimo commented Mar 5, 2024

weinimo commented Mar 6, 2024

weinimo commented Mar 6, 2024

zzzeek left a comment

zzzeek commented Mar 11, 2024

weinimo commented Mar 12, 2024

weinimo commented Mar 13, 2024

openshift-ci bot commented Mar 13, 2024

beagles commented Mar 13, 2024

zzzeek commented Mar 13, 2024

weinimo commented Mar 13, 2024

zzzeek Mar 13, 2024

weinimo commented Mar 14, 2024

Updates and mariadb related fixes #245

Updates and mariadb related fixes #245

Conversation

weinimo commented Feb 19, 2024 • edited Loading

openshift-ci bot commented Feb 19, 2024

weinimo commented Feb 19, 2024

zzzeek Feb 21, 2024

Choose a reason for hiding this comment

zzzeek Feb 21, 2024

Choose a reason for hiding this comment

This comment was marked as resolved.

weinimo Mar 12, 2024

Choose a reason for hiding this comment

weinimo Mar 12, 2024

Choose a reason for hiding this comment

weinimo commented Feb 21, 2024

zzzeek commented Feb 21, 2024

weinimo commented Mar 5, 2024

weinimo commented Mar 5, 2024

weinimo commented Mar 6, 2024

weinimo commented Mar 6, 2024

zzzeek left a comment

Choose a reason for hiding this comment

zzzeek commented Mar 11, 2024

weinimo commented Mar 12, 2024

weinimo commented Mar 13, 2024

openshift-ci bot commented Mar 13, 2024

beagles commented Mar 13, 2024

zzzeek commented Mar 13, 2024

weinimo commented Mar 13, 2024

zzzeek Mar 13, 2024

Choose a reason for hiding this comment

weinimo commented Mar 14, 2024

weinimo commented Feb 19, 2024 •

edited

Loading