Remove MariaDB CRD #173

zzzeek · 2023-12-06T17:36:09Z

openstack-k8s-operators now uses Galera exclusively for MySQL/MariaDB access, so the MariaDB CRD can be fully removed.

config/rbac/role.yaml

config/samples/mariadb_v1beta1_mariadb.yaml

zzzeek · 2023-12-06T20:01:00Z

/retest

zzzeek · 2023-12-06T20:39:04Z

OK it looks like there are references to this from both install_yamls as well as openstack-operator, so I can't quite trace out how the build is failing but we would assume the openstack-operator build is somehow failing because things it expects to see have been removed from here.

To support upstream mariadb-operator removal of the "MariaDB" resource in [1], openstack-operator should refer only to Galera as the relational database deployment [1] openstack-k8s-operators/mariadb-operator#173

openstack-k8s-operators now uses Galera exclusively for MySQL/MariaDB access, so the MariaDB CRD can be fully removed.

zzzeek · 2023-12-15T13:56:42Z

/retest

zzzeek · 2023-12-15T15:37:08Z

this should be all set now

gibizer

I'm wondering if this test helper needs to be change as well:
https://github.com/openstack-k8s-operators/mariadb-operator/blob/4cd7b3f5eedec8fef56396cfb88f3cbd795dbb08/api/test/helpers/crd.go#L103C11-L152
as it refers to the code that looks for MariaDB CR by name

mariadb-operator/api/v1beta1/mariadbdatabase_funcs.go

Lines 79 to 87 in 4cd7b3f

    
           // When the MariaDB CR provides the Service it sets the "cr" label of the 
        
           // Service to "mariadb-<name of the MariaDB CR>". So we use this label 
        
           // to select the right Service. See: 
        
           // https://github.com/openstack-k8s-operators/mariadb-operator/blob/5781b0cf1087d7d28fa285bd5c44689acba92183/pkg/service.go#L17 
        
           // https://github.com/openstack-k8s-operators/mariadb-operator/blob/590ffdc5ad86fe653f9cd8a7102bb76dfe2e36d1/pkg/utils.go#L4 
        
           selector := map[string]string{ 
        
           	"app": "mariadb", 
        
           	"cr":  fmt.Sprintf("mariadb-%s", name), 
        
           }

api/v1beta1/galera_types.go

zzzeek · 2023-12-18T15:58:38Z

I'm wondering if this test helper needs to be change as well: https://github.com/openstack-k8s-operators/mariadb-operator/blob/4cd7b3f5eedec8fef56396cfb88f3cbd795dbb08/api/test/helpers/crd.go#L103C11-L152 as it refers to the code that looks for MariaDB CR by name

mariadb-operator/api/v1beta1/mariadbdatabase_funcs.go

Lines 79 to 87 in 4cd7b3f

// When the MariaDB CR provides the Service it sets the "cr" label of the

// Service to "mariadb-<name of the MariaDB CR>". So we use this label

// to select the right Service. See:

// https://github.com/openstack-k8s-operators/mariadb-operator/blob/5781b0cf1087d7d28fa285bd5c44689acba92183/pkg/service.go#L17

// https://github.com/openstack-k8s-operators/mariadb-operator/blob/590ffdc5ad86fe653f9cd8a7102bb76dfe2e36d1/pkg/utils.go#L4

selector := map[string]string{

"app": "mariadb",

"cr": fmt.Sprintf("mariadb-%s", name),

}

cc @dciabrin does this comment also apply to the Galera CR ?

dprince · 2024-01-04T14:51:44Z

I'm wondering if this test helper needs to be change as well: https://github.com/openstack-k8s-operators/mariadb-operator/blob/4cd7b3f5eedec8fef56396cfb88f3cbd795dbb08/api/test/helpers/crd.go#L103C11-L152 as it refers to the code that looks for MariaDB CR by name

mariadb-operator/api/v1beta1/mariadbdatabase_funcs.go

Lines 79 to 87 in 4cd7b3f

// When the MariaDB CR provides the Service it sets the "cr" label of the

// Service to "mariadb-<name of the MariaDB CR>". So we use this label

// to select the right Service. See:

// https://github.com/openstack-k8s-operators/mariadb-operator/blob/5781b0cf1087d7d28fa285bd5c44689acba92183/pkg/service.go#L17

// https://github.com/openstack-k8s-operators/mariadb-operator/blob/590ffdc5ad86fe653f9cd8a7102bb76dfe2e36d1/pkg/utils.go#L4

selector := map[string]string{

"app": "mariadb",

"cr": fmt.Sprintf("mariadb-%s", name),

}

cc @dciabrin does this comment also apply to the Galera CR ?

I think you are correct. The test helper will need to be updated. But the breakage wouldn't occur until renovate bumps up the mariadb-operator/api dependency right? So we could land this and gradually update the operators to use an equivalent Galera test helper gradually

zzzeek · 2024-01-04T17:20:13Z

I'm wondering if this test helper needs to be change as well: https://github.com/openstack-k8s-operators/mariadb-operator/blob/4cd7b3f5eedec8fef56396cfb88f3cbd795dbb08/api/test/helpers/crd.go#L103C11-L152 as it refers to the code that looks for MariaDB CR by name

mariadb-operator/api/v1beta1/mariadbdatabase_funcs.go

Lines 79 to 87 in 4cd7b3f

// When the MariaDB CR provides the Service it sets the "cr" label of the

// Service to "mariadb-<name of the MariaDB CR>". So we use this label

// to select the right Service. See:

// https://github.com/openstack-k8s-operators/mariadb-operator/blob/5781b0cf1087d7d28fa285bd5c44689acba92183/pkg/service.go#L17

// https://github.com/openstack-k8s-operators/mariadb-operator/blob/590ffdc5ad86fe653f9cd8a7102bb76dfe2e36d1/pkg/utils.go#L4

selector := map[string]string{

"app": "mariadb",

"cr": fmt.Sprintf("mariadb-%s", name),

}

cc @dciabrin does this comment also apply to the Galera CR ?

I think you are correct. The test helper will need to be updated. But the breakage wouldn't occur until renovate bumps up the mariadb-operator/api dependency right? So we could land this and gradually update the operators to use an equivalent Galera test helper gradually

still trying to understand the complete flow of this helper, I'm not yet seeing the "breakage" here since this helper seems to be in a closed loop against the local mariadb API in any case (comments are out of date since openstack-k8s-operators/lib-common#352 occurred) but this seems to raise a bigger issue which is that the mariadbv1.Database API is what all the services are using to create databases, and that's not yet integrated with the new MariaDBAccount thing I just merged, seems like it should be.

dciabrin · 2024-01-08T15:00:39Z

I'm wondering if this test helper needs to be change as well: https://github.com/openstack-k8s-operators/mariadb-operator/blob/4cd7b3f5eedec8fef56396cfb88f3cbd795dbb08/api/test/helpers/crd.go#L103C11-L152 as it refers to the code that looks for MariaDB CR by name

cc @dciabrin does this comment also apply to the Galera CR ?

Labeling shouldn't change, the Galera CR and the MariaDB CR both use the same labels so clients looking for the database service don't need to treat MariaDB and Galera separately. Back when this was introduced, the only component that would distinguish those two CR was the MariaDBDatabase controller.
So I would say this labeling should all be internal, and no change is needed. If we want to revisit later and replace references to mariadb by galera, this can be done in another PR.

zzzeek · 2024-01-08T15:02:36Z

OK so this is ready

To support upstream mariadb-operator removal of the "MariaDB" resource in [1], openstack-operator should refer only to Galera as the relational database deployment [1] openstack-k8s-operators/mariadb-operator#173

zzzeek · 2024-01-11T16:52:32Z

I've deployed openstack locally using this version of the operator, there's no MariaDB crd in the cluster at all and everything is running fine

$ oc get crds | grep mariadb
galeras.mariadb.openstack.org                                     2024-01-11T16:21:22Z
mariadbaccounts.mariadb.openstack.org                             2024-01-11T16:21:22Z
mariadbdatabases.mariadb.openstack.org                            2024-01-11T16:21:22Z

[classic@hab-11 install_yamls]$ oc get pods
NAME                           READY   STATUS    RESTARTS      AGE
ceilometer-0                   3/3     Running   0             24m
cinder-api-0                   2/2     Running   0             20m
cinder-scheduler-0             2/2     Running   0             20m
dnsmasq-dns-69cb44859d-gcjgb   1/1     Running   0             16m
glance-default-single-0        3/3     Running   0             24m
keystone-6d94d9cd4-794x5       1/1     Running   0             23m
memcached-0                    1/1     Running   0             30m
neutron-84f6b4848c-gcjwz       2/2     Running   0             17m
nova-api-0                     2/2     Running   0             15m
nova-cell0-conductor-0         1/1     Running   0             17m
nova-cell1-conductor-0         1/1     Running   0             15m
nova-cell1-novncproxy-0        1/1     Running   0             17m
nova-metadata-0                2/2     Running   0             15m
nova-scheduler-0               1/1     Running   0             15m
openstack-cell1-galera-0       1/1     Running   0             30m
openstack-galera-0             1/1     Running   0             30m
openstackclient                1/1     Running   0             21m
ovn-controller-5gzth           3/3     Running   0             30m
ovn-northd-79cf7987f5-54jjw    1/1     Running   0             27m
ovsdbserver-nb-0               1/1     Running   0             30m
ovsdbserver-sb-0               1/1     Running   0             30m
placement-7444b6685d-nhf8k     2/2     Running   0             21m
rabbitmq-cell1-server-0        1/1     Running   0             30m
rabbitmq-server-0              1/1     Running   1 (27m ago)   30m
swift-proxy-549bcf8f4d-t9jmh   3/3     Running   0             24m
swift-storage-0                16/16   Running   0             30m

dciabrin · 2024-01-15T07:32:32Z

/lgtm

gibizer

Looks good to me.

gibizer · 2024-01-15T08:05:32Z

controllers/mariadbaccount_controller.go

@@ -416,7 +398,7 @@ func (r *MariaDBAccountReconciler) reconcileDelete(
 	// referenced by the MariaDBDatabase which will lead us to the hostname


nit: galera/mariadb is now just galera

I wanted to add this to L397 instead of L398 but github does not allow it

zzzeek · 2024-01-15T18:42:39Z

waiting on @dciabrin to /approve

dciabrin · 2024-01-16T10:06:30Z

/approve

openshift-ci · 2024-01-16T10:06:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dciabrin, gibizer, zzzeek

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

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

zzzeek requested a review from dciabrin December 6, 2023 17:36

openshift-ci bot requested a review from lewisdenny December 6, 2023 17:36

zzzeek mentioned this pull request Dec 6, 2023

add controller to create and delete individual usernames in mariadb / galera #166

Merged

zzzeek commented Dec 6, 2023

View reviewed changes

config/rbac/role.yaml Outdated Show resolved Hide resolved

zzzeek force-pushed the remove_standalone_mariadb branch 3 times, most recently from bddf2b7 to 9072490 Compare December 6, 2023 18:02

zzzeek commented Dec 6, 2023

View reviewed changes

config/samples/mariadb_v1beta1_mariadb.yaml Outdated Show resolved Hide resolved

zzzeek force-pushed the remove_standalone_mariadb branch 2 times, most recently from ce1083b to 10931a0 Compare December 6, 2023 19:39

zzzeek mentioned this pull request Dec 11, 2023

remove "mariadb" CRD references in favor of Galera openstack-k8s-operators/openstack-operator#597

Merged

zzzeek force-pushed the remove_standalone_mariadb branch from 10931a0 to dcd387c Compare December 13, 2023 18:51

Remove MariaDB CRD

8af0ab9

openstack-k8s-operators now uses Galera exclusively for MySQL/MariaDB access, so the MariaDB CRD can be fully removed.

zzzeek force-pushed the remove_standalone_mariadb branch from dcd387c to 8af0ab9 Compare December 14, 2023 22:32

zzzeek requested review from gibizer and dprince December 18, 2023 15:29

gibizer reviewed Dec 18, 2023

View reviewed changes

api/v1beta1/galera_types.go Show resolved Hide resolved

openshift-ci bot assigned dciabrin Jan 15, 2024

openshift-ci bot added the lgtm label Jan 15, 2024

gibizer approved these changes Jan 15, 2024

View reviewed changes

openshift-ci bot assigned gibizer Jan 15, 2024

openshift-ci bot added the approved label Jan 16, 2024

openshift-merge-bot bot merged commit 6fb96fd into openstack-k8s-operators:main Jan 16, 2024
6 checks passed

zzzeek deleted the remove_standalone_mariadb branch January 25, 2024 20:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove MariaDB CRD #173

Remove MariaDB CRD #173

zzzeek commented Dec 6, 2023 •

edited

Loading

zzzeek commented Dec 6, 2023

zzzeek commented Dec 6, 2023

zzzeek commented Dec 15, 2023

zzzeek commented Dec 15, 2023

gibizer left a comment

zzzeek commented Dec 18, 2023

dprince commented Jan 4, 2024

zzzeek commented Jan 4, 2024

dciabrin commented Jan 8, 2024

zzzeek commented Jan 8, 2024

zzzeek commented Jan 11, 2024

dciabrin commented Jan 15, 2024

gibizer left a comment

gibizer Jan 15, 2024

gibizer Jan 15, 2024

zzzeek commented Jan 15, 2024

dciabrin commented Jan 16, 2024

openshift-ci bot commented Jan 16, 2024

	// When the MariaDB CR provides the Service it sets the "cr" label of the
	// Service to "mariadb-<name of the MariaDB CR>". So we use this label
	// to select the right Service. See:
	// https://github.com/openstack-k8s-operators/mariadb-operator/blob/5781b0cf1087d7d28fa285bd5c44689acba92183/pkg/service.go#L17
	// https://github.com/openstack-k8s-operators/mariadb-operator/blob/590ffdc5ad86fe653f9cd8a7102bb76dfe2e36d1/pkg/utils.go#L4
	selector := map[string]string{
	"app": "mariadb",
	"cr": fmt.Sprintf("mariadb-%s", name),
	}

		@@ -416,7 +398,7 @@ func (r *MariaDBAccountReconciler) reconcileDelete(
		// referenced by the MariaDBDatabase which will lead us to the hostname

Remove MariaDB CRD #173

Remove MariaDB CRD #173

Conversation

zzzeek commented Dec 6, 2023 • edited Loading

zzzeek commented Dec 6, 2023

zzzeek commented Dec 6, 2023

zzzeek commented Dec 15, 2023

zzzeek commented Dec 15, 2023

gibizer left a comment

Choose a reason for hiding this comment

zzzeek commented Dec 18, 2023

dprince commented Jan 4, 2024

zzzeek commented Jan 4, 2024

dciabrin commented Jan 8, 2024

zzzeek commented Jan 8, 2024

zzzeek commented Jan 11, 2024

dciabrin commented Jan 15, 2024

gibizer left a comment

Choose a reason for hiding this comment

gibizer Jan 15, 2024

Choose a reason for hiding this comment

gibizer Jan 15, 2024

Choose a reason for hiding this comment

zzzeek commented Jan 15, 2024

dciabrin commented Jan 16, 2024

openshift-ci bot commented Jan 16, 2024

zzzeek commented Dec 6, 2023 •

edited

Loading