Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[go.mod]Bump from v0.3.0 to pseudoversion #660

Conversation

gibizer
Copy link
Contributor

@gibizer gibizer commented Jan 26, 2024

Our intention is to track service operator and lib-common dependencies
via pseudoversions. However renovate does not automatically bump from a
tagged version (e.g. v0.3.0) to a newer but not tagged pseudoversion. So
this patch does the manual bump. After this renovate will bump the newer
pseduoversions.

Adapt to the new version by:

  • adding RBAC rules for MariaDBAccount manipulation
  • adding Own(MariaDBAccount) to detect status changes
  • separating DB user name and schema name
  • adapting test to simulate MariaDBAccount success
  • adapting the tests to the interim changes of mariadb TLS

@gibizer gibizer requested a review from mrkisaolamb January 26, 2024 12:44
@openshift-ci openshift-ci bot requested review from abays and jamepark4 January 26, 2024 12:44
@gibizer
Copy link
Contributor Author

gibizer commented Jan 26, 2024

/test functional
known instability that is being fixed in parallel

Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/7b16f76656fd4211ab0f031e50f3bbd5

✔️ nova-operator-content-provider SUCCESS in 1h 22m 41s
nova-operator-kuttl FAILURE in 38m 14s
nova-operator-tempest-multinode FAILURE in 1h 06m 05s

@SeanMooney
Copy link
Contributor

so this seams to be breakign how we use the db
so i guess we need to adapt that first

@gibizer
Copy link
Contributor Author

gibizer commented Jan 26, 2024

❯ oc logs nova-api-db-create-ksmvh --follow
ERROR 1115 (42000) at line 1: Unknown character set: ''

@gibizer
Copy link
Contributor Author

gibizer commented Jan 26, 2024

❯ oc logs nova-api-db-create-ksmvh --follow
ERROR 1115 (42000) at line 1: Unknown character set: ''

Suspecting openstack-k8s-operators/mariadb-operator#172

@gibizer
Copy link
Contributor Author

gibizer commented Jan 26, 2024

❯ oc logs nova-api-db-create-ksmvh --follow
ERROR 1115 (42000) at line 1: Unknown character set: ''

Suspecting openstack-k8s-operators/mariadb-operator#172

Wrong suspicion.

But. The current openstack-operator is using mariadb-operator openstack-k8s-operators/mariadb-operator@6fb96fd
see
https://github.com/openstack-k8s-operators/openstack-operator/blob/06eb0b3ab8dc2e591f0ba21d99684d7b579e55cc/go.mod#L26
and that mariadb has the field definition of
https://github.com/openstack-k8s-operators/mariadb-operator/blob/6fb96fd3a8bcc04e45d0a80893e40274c26d0dd2/api/v1beta1/mariadbdatabase_types.go#L39
that only defaults the field to utf8 if the field is not provided in the request.
So all our old service operator works as they don't even know about the existence of that recent field. But as soon as the service operator bumps to mariadb-operator/api 6fb96fd3a8bcc04e45d0a80893e40274c26d0dd2 it starts sending the field with "" value and that causing the db create jobs to fail.

The mariadb-operator has a fix for that (adding omitempty ) in
openstack-k8s-operators/mariadb-operator@4d759dd#diff-6273db9c3b1f1dd7dcaae35f91de6f3bb1688c8a76dd16d775ff5c5fdf629866

So I will try to bump the mariadb in openstack-operator to 4d759dd283b312905366b45f0d93059334738b2c and then bump the service operators to 4d759dd283b312905366b45f0d93059334738b2c as well to see if that helps.

@gibizer gibizer force-pushed the bump-to-pseudoversion branch 2 times, most recently from 82924f5 to 5051883 Compare January 26, 2024 18:20
@gibizer
Copy link
Contributor Author

gibizer commented Jan 26, 2024

So I will try to bump the mariadb in openstack-operator to 4d759dd283b312905366b45f0d93059334738b2c and then bump the service operators to 4d759dd283b312905366b45f0d93059334738b2c as well to see if that helps.

I ended up bumping mariadb here to 331c633c453c as in the meantime openstack-operator bumped to it too. Locally with an openstack operator build (OPENSTACK_IMG=quay.io/gibi/openstack-operator-index:v0.0.380 make openstack) using this PR was able to successfully do DB creation from nova. But

  1. the functional tests needs some adjustment
  2. we need to do some changes to adapt to the existence of MariaDBAccounts CRs
E0126 18:47:04.390760       1 reflector.go:140] pkg/mod/k8s.io/[email protected]/tools/cache/reflector.go:169: Failed to watch *v1beta1.MariaDBAccount: failed to list *v1beta1.MariaDBAccount: mariadbaccounts.mariadb.openstack.org is forbidden: User "system:serviceaccount:openstack-operators:nova-operator-controller-manager" cannot list resource "mariadbaccounts" in API group "mariadb.openstack.org" at the cluster scope

@gibizer gibizer force-pushed the bump-to-pseudoversion branch from 5051883 to c274999 Compare January 26, 2024 18:51
@gibizer
Copy link
Contributor Author

gibizer commented Jan 26, 2024

So I will try to bump the mariadb in openstack-operator to 4d759dd283b312905366b45f0d93059334738b2c and then bump the service operators to 4d759dd283b312905366b45f0d93059334738b2c as well to see if that helps.

I ended up bumping mariadb here to 331c633c453c as in the meantime openstack-operator bumped to it too. Locally with an openstack operator build (OPENSTACK_IMG=quay.io/gibi/openstack-operator-index:v0.0.380 make openstack) using this PR was able to successfully do DB creation from nova. But

  1. the functional tests needs some adjustment
  2. we need to do some changes to adapt to the existence of MariaDBAccounts CRs
E0126 18:47:04.390760       1 reflector.go:140] pkg/mod/k8s.io/[email protected]/tools/cache/reflector.go:169: Failed to watch *v1beta1.MariaDBAccount: failed to list *v1beta1.MariaDBAccount: mariadbaccounts.mariadb.openstack.org is forbidden: User "system:serviceaccount:openstack-operators:nova-operator-controller-manager" cannot list resource "mariadbaccounts" in API group "mariadb.openstack.org" at the cluster scope

added RBAC for MariaDBAccount and now I'm getting

      message: 'DB creation failed for cell0(Error create or update account object
        nova_cell0 *v1beta1.MariaDBAccount openstack/nova_cell0: MariaDBAccount.mariadb.openstack.org
        "nova_cell0" is invalid: metadata.name: Invalid value: "nova_cell0": a lowercase
        RFC 1123 subdomain must consist of lower case alphanumeric characters, ''-''
        or ''.'', and must start and end with an alphanumeric character (e.g. ''example.com'',
        regex used for validation is ''[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'')),cell1(Error
        create or update account object nova_cell1 *v1beta1.MariaDBAccount openstack/nova_cell1:
        MariaDBAccount.mariadb.openstack.org "nova_cell1" is invalid: metadata.name:
        Invalid value: "nova_cell1": a lowercase RFC 1123 subdomain must consist of
        lower case alphanumeric characters, ''-'' or ''.'', and must start and end
        with an alphanumeric character (e.g. ''example.com'', regex used for validation
        is ''[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*''))'

This is because mariadb-operator uses the passed in database user name as the name of the MariaDB account. Our samples uses nova_cell0 (nova_<cell_mame>) as db user name by default and we use underscore in it as mariadb itself allows underscore in the user name. However k8s does not allow underscore in the resource names.
https://github.com/openstack-k8s-operators/mariadb-operator/blob/331c633c453c493a80c315289137b08fc9601ba0/api/v1beta1/mariadbdatabase_funcs.go#L174

So I manually edited the OpenStackControlPlane CR to use db users as novaapi, novacell0, novacell1 and this triggered MariaDBAccount creation. But then db sync failed with error:

  File "/usr/lib/python3.9/site-packages/pymysql/err.py", line 107, in raise_mysql_exception
    raise errorclass(errno, errval)
pymysql.err.OperationalError: (1044, "Access denied for user 'novaapi'@'%' to database 'novaapi'")

And this is probably due the another change in behavior. Mariadb in the past used the database schema name as the user. But not any more.

@gibizer gibizer force-pushed the bump-to-pseudoversion branch from c274999 to f0c299f Compare January 26, 2024 20:02
@gibizer
Copy link
Contributor Author

gibizer commented Jan 26, 2024

And this is probably due the another change in behavior. Mariadb in the past used the database schema name as the user. But not any more.

so I changed nova to use the schema name and the user name separately.

With these changes (+ manually editing the OpenStackControlPlane CR to avoid underscore in the username) nova control plane can be deployed now

Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/ba9bb4bc694e4e9ca5f375286217645b

✔️ nova-operator-content-provider SUCCESS in 1h 24m 05s
nova-operator-kuttl FAILURE in 39m 59s
nova-operator-tempest-multinode FAILURE in 1h 06m 01s

@zzzeek
Copy link
Contributor

zzzeek commented Jan 27, 2024

And this is probably due the another change in behavior. Mariadb in the past used the database schema name as the user. But not any more.

Right, so I had assumed that changing nothing in the downstream operators, they would still send Database{} using the same string for the database name and the username, where username was previously ignored.

however, you have a username that has an underscore in it, and then due to this:

https://github.com/openstack-k8s-operators/mariadb-operator/blob/331c633c453c493a80c315289137b08fc9601ba0/api/v1beta1/mariadbdatabase_funcs.go#L174-L181

I'm using that same name as the name of the mariadbaccount and that's being rejected.

would we accept as an interim solution to apply a simple search and replace of underscore to dash in mariadb-operator in that section when it comes up with the metadata name?

@gibizer
Copy link
Contributor Author

gibizer commented Jan 29, 2024

And this is probably due the another change in behavior. Mariadb in the past used the database schema name as the user. But not any more.

Right, so I had assumed that changing nothing in the downstream operators, they would still send Database{} using the same string for the database name and the username, where username was previously ignored.

however, you have a username that has an underscore in it, and then due to this:

https://github.com/openstack-k8s-operators/mariadb-operator/blob/331c633c453c493a80c315289137b08fc9601ba0/api/v1beta1/mariadbdatabase_funcs.go#L174-L181

I'm using that same name as the name of the mariadbaccount and that's being rejected.

would we accept as an interim solution to apply a simple search and replace of underscore to dash in mariadb-operator in that section when it comes up with the metadata name?

That is one way to fix it. The other is that I can change the nova-operator user defaults not to use special characters. I think other operators has one schema per operator and they use simple names like glance or placement for both the schema name and the user name. So this only affect nova-operator and I anyhow need to change the nova operator to adapt.

Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/9ab2be8d9bce4c268f0635c627cd7dce

✔️ nova-operator-content-provider SUCCESS in 1h 23m 57s
nova-operator-kuttl RETRY_LIMIT in 12m 15s
nova-operator-tempest-multinode FAILURE in 1h 06m 17s

@gibizer gibizer force-pushed the bump-to-pseudoversion branch from a400d45 to ae76f9c Compare January 29, 2024 10:12
Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/3c0f9dce520345a7bfe31e72cf8d5a17

✔️ nova-operator-content-provider SUCCESS in 2h 03m 56s
nova-operator-kuttl FAILURE in 39m 54s
✔️ nova-operator-tempest-multinode SUCCESS in 1h 46m 40s

@zzzeek
Copy link
Contributor

zzzeek commented Jan 29, 2024

And this is probably due the another change in behavior. Mariadb in the past used the database schema name as the user. But not any more.

Right, so I had assumed that changing nothing in the downstream operators, they would still send Database{} using the same string for the database name and the username, where username was previously ignored.
however, you have a username that has an underscore in it, and then due to this:
https://github.com/openstack-k8s-operators/mariadb-operator/blob/331c633c453c493a80c315289137b08fc9601ba0/api/v1beta1/mariadbdatabase_funcs.go#L174-L181
I'm using that same name as the name of the mariadbaccount and that's being rejected.
would we accept as an interim solution to apply a simple search and replace of underscore to dash in mariadb-operator in that section when it comes up with the metadata name?

That is one way to fix it. The other is that I can change the nova-operator user defaults not to use special characters. I think other operators has one schema per operator and they use simple names like glance or placement for both the schema name and the user name. So this only affect nova-operator and I anyhow need to change the nova operator to adapt.

whether or not you do that, I think mariadb-operators logic here definitely should accommodate this, mariadb identifiers with underscores in them are very common. since the Database helper is generating a name for the CR automatically it should absolutely be using k8s-compliant names when it does so. similar logic is already in the operator here: https://github.com/openstack-k8s-operators/mariadb-operator/blob/331c633c453c493a80c315289137b08fc9601ba0/pkg/mariadb/database.go#L88

@gibizer
Copy link
Contributor Author

gibizer commented Jan 29, 2024

And this is probably due the another change in behavior. Mariadb in the past used the database schema name as the user. But not any more.

Right, so I had assumed that changing nothing in the downstream operators, they would still send Database{} using the same string for the database name and the username, where username was previously ignored.
however, you have a username that has an underscore in it, and then due to this:
https://github.com/openstack-k8s-operators/mariadb-operator/blob/331c633c453c493a80c315289137b08fc9601ba0/api/v1beta1/mariadbdatabase_funcs.go#L174-L181
I'm using that same name as the name of the mariadbaccount and that's being rejected.
would we accept as an interim solution to apply a simple search and replace of underscore to dash in mariadb-operator in that section when it comes up with the metadata name?

That is one way to fix it. The other is that I can change the nova-operator user defaults not to use special characters. I think other operators has one schema per operator and they use simple names like glance or placement for both the schema name and the user name. So this only affect nova-operator and I anyhow need to change the nova operator to adapt.

whether or not you do that, I think mariadb-operators logic here definitely should accommodate this, mariadb identifiers with underscores in them are very common. since the Database helper is generating a name for the CR automatically it should absolutely be using k8s-compliant names when it does so. similar logic is already in the operator here: https://github.com/openstack-k8s-operators/mariadb-operator/blob/331c633c453c493a80c315289137b08fc9601ba0/pkg/mariadb/database.go#L88

Currently such translation is used from the db schema name to the MariaDBDatabase. As far as I know all our db schema names are coming from the service operator implementation and not from the user. So there it is less likely that two names will clash due to translated to the same name by the mariadb-operator. If at the end the DB user name will also come from some generation sw then I'm fine to have the translation in the mariadb-operator. But if it can come from the user then I would rather make a restriction on the usable characters of the username field.

@gibizer
Copy link
Contributor Author

gibizer commented Jan 29, 2024

tempest passed so the adaptation seems to work OK.
The kuttl test needs some love:

case.go:366: resource Nova:nova-kuttl-default/nova-kuttl: .spec.apiDatabaseUser: value mismatch, expected: nova_api != actual: novaapi

@gibizer
Copy link
Contributor Author

gibizer commented Jan 30, 2024

Bumped to openstack-k8s-operators/mariadb-operator@5ad30f7 so the underscore in the user names are OK again.

@gibizer gibizer force-pushed the bump-to-pseudoversion branch 3 times, most recently from 871d6fb to 94c1c6e Compare January 30, 2024 17:36
Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/190aa60ff1ab419ab0acd2c27055e09f

✔️ nova-operator-content-provider SUCCESS in 1h 44m 01s
✔️ nova-operator-kuttl SUCCESS in 40m 31s
nova-operator-tempest-multinode FAILURE in 1h 23m 23s

@gibizer
Copy link
Contributor Author

gibizer commented Jan 31, 2024

We still need a fix in mariadb-operator for the finalizer handling. Currently GetDatabaseByName() cannot be used to load the MairaDBAccount and DeleteFinalizer does not remove the finalizer from the MariaDBAccount.

https://github.com/openstack-k8s-operators/mariadb-operator/blob/dd95ba886c6cd2b3714940eb3e9590f427d895d4/api/v1beta1/mariadbdatabase_funcs.go#L397-L425

Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/9829bc9c60b14237b9bfa239f82523b0

✔️ nova-operator-content-provider SUCCESS in 2h 25m 25s
✔️ nova-operator-kuttl SUCCESS in 41m 51s
nova-operator-tempest-multinode FAILURE in 2h 07m 59s

@gibizer
Copy link
Contributor Author

gibizer commented Feb 1, 2024

I expect a green CI run on this now

@gibizer
Copy link
Contributor Author

gibizer commented Feb 1, 2024

/test functional
the whole functional build log is

Contained sensitive information -- content removed.

@gibizer gibizer force-pushed the bump-to-pseudoversion branch from 73d87d1 to 6cab55f Compare February 1, 2024 14:10
Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/d67cb93de0f645fdbb8f5dcb4eaa23c3

✔️ nova-operator-content-provider SUCCESS in 2h 18m 07s
nova-operator-kuttl FAILURE in 40m 46s
✔️ nova-operator-tempest-multinode SUCCESS in 2h 00m 42s

Our intention is to track service operator and lib-common dependencies
via pseudoversions. However renovate does not automatically bump from a
tagged version (e.g. v0.3.0) to a newer but not tagged pseudoversion. So
this patch does the manual bump. After this renovate will bump the newer
pseduoversions.

Adapt to the new version by:
* adding RBAC rules for MariaDBAccount manipulation
* adding Own(MariaDBAccount) to detect status changes
* separating DB user name and schema name
* adapting test to simulate MariaDBAccount success
* adapting the tests to the interim changes of mariadb TLS
@gibizer gibizer force-pushed the bump-to-pseudoversion branch from 6cab55f to 7246fb6 Compare February 2, 2024 09:23
@gibizer
Copy link
Contributor Author

gibizer commented Feb 2, 2024

adapted the kuttl test as well to the fqdn mariadb hostnames now

Copy link
Contributor

@mrkisaolamb mrkisaolamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Contributor

openshift-ci bot commented Feb 2, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gibizer, mrkisaolamb

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 [gibizer,mrkisaolamb]

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants