-
Notifications
You must be signed in to change notification settings - Fork 79
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
Update openstack-k8s-operators #690
Conversation
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/4dad7e48f5c8449cac8037c7af11600f ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 32m 58s |
c585df2
to
bff33c8
Compare
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/5b66c9b0b2104d4a899f5bfe3fc355d0 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 27m 45s |
bff33c8
to
1b7f867
Compare
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/50d1262d05cd4c1f9dd69f5a50a72ab9 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 28m 50s |
82b0831
to
0639560
Compare
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/e7fe228c50e545a99d455143c9aeb2b3 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 59m 26s |
/test openstack-operator-build-deploy-kuttl |
recheck |
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/d27d4d022fd84f5e8322f9b695f5bdcf ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 31m 40s |
0639560
to
62637f3
Compare
62637f3
to
c70f363
Compare
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/4029c8e962b1424b8028b660bdb35897 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 49m 29s |
c70f363
to
e2cb7fe
Compare
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/250014d39e4343bcbddbc55a7b7b2399 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 02m 57s |
e2cb7fe
to
8a274d3
Compare
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/07051a9a66944f93a824cb951411c4e1 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 29m 23s |
8a274d3
to
15de2f5
Compare
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/186118a89a4743cd98fea7d240c1428c ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 58m 37s |
/hold we need to land #694 first then ask renovate to regenerate this PR |
/unhold |
15de2f5
to
d6ea2b3
Compare
Edited/Blocked NotificationRenovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR. You can manually request rebase by checking the rebase/retry box above. ⚠ Warning: custom changes will be lost. |
This patch updates the cinder-operator/api from 49edc0d to 038a5ec and also updates the functional tests. Tests need to be updated after adding MemcachedSpecCore openstack-k8s-operators/infra-operator#191 Related-PR: openstack-k8s-operators/openstack-operator#690
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/2b6a23261fcc49ad9b45e92a0d3ad28a ✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 03m 14s |
kuttl test failed in nova with:
there is a mariadbaccount for nova-api, but not for cells: first tests had the cell0/cell1 account , e.g. later logs only show waiting |
I believe this means the crd ( |
if i can deploy a control plane locally using "make openstack" with the latest nova operator, that would indicate nova works, right? |
OK I can reproduce this with the current openstack-operator, I didnt try with this PR but it's likely the same thing. The nova operator has the correct templates, but the cellDatabaseAccount is not filled in becuase openstack-operator has to send it. So when doing a "make openstack_deploy", the nova CR looks like this:
so that's wrong, that cellDatatabaseAccount should be filled in (and it's required, nova doesn't generate this, which I thought it did for a minute). I'm not sure what kubebuilder-required or whatever gets us here if it's happy to put a blank string there, I guess it just means "the key is present" :). well thanks :) ideally it would throw a big error for the blank string like that. then over on the openstack-operator side inside the openstackcontrolplane CR we see this (note this is not with this PR), it wants to do the right thing (which means it can be fixed) but it's currently wrong:
so that's also wrong. I think we earlier had said we would just remove these, but that's wrong. that databaseuser with underscore needs to become databaseaccount with a dash, like this:
right now, with the blank cellDatabaseAccount, my nova has the error we see:
and I've got the other log message:
and there's no MariaDBAccount, because there can't be, there's no account name. so yes I can reproduce it, easy to fix. What I do not see, which is also phew makes sense, are those messages regarding "Successfully ensured MariaDBAccount nova-cell0 exists;" on this end. that's not possible because there is no "nova-cell0" name indicated anywhere. So im not sure if a kuttl template is filling those in, maybe they conflict with the openstackoperator CR and they get deleted? i dont know this stuff that deeply yet to really know. OK so I go and
I also blow away those then...using the way I know how to do this at the moment, I delete the "nova" so that the openstackcontrolplane recreates it,
then we finally see those accounts:
the databases are there too, here's with cell0
then after 5 minutes of panic I found cell1 which is of course in a separate galera
OK so TL;DR; I can reproduce the problems above and we need to modify openstack-operator to fully specify and transfer to the nova CR it creates the |
This look like a defaulting issue somewhere. The NovaSpec.cellTemplate is defaulted to have the account names defined https://github.com/openstack-k8s-operators/nova-operator/blob/2fc1351673afc28eb493b8d4d9feb2810197a199/api/v1beta1/nova_types.go#L52 Then in the CellTemplate the field is required https://github.com/openstack-k8s-operators/nova-operator/blob/2fc1351673afc28eb493b8d4d9feb2810197a199/api/v1beta1/novacell_types.go#L42-L44
If openstack-operator generates this then openstack-operator does not see the new Nova CRD where the cellDatabaseUser field is removed. |
It seems like this CRD definition does not contain our default but this has the proper defaults: |
Wait a sec. This needs to be something in the kuttl job as the zuul job https://review.rdoproject.org/zuul/build/cf9b91a2f939442c86ead8babc495082/logs managed to deploy the control plane and data plane and run a tempest smoke test successfully. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will push a fix
cellDatabaseUser: nova_cell0 | ||
conductorServiceTemplate: | ||
replicas: 0 | ||
hasAPIAccess: true | ||
cell1: | ||
cellDatabaseUser: nova_cell1 | ||
cellDatabaseInstance: openstack |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I see what is happening here. Originally we defined the cellDatabaseUser fields here explicitly as the defaulting only works properly if the celltemplate is not defined at all. If a field is defined in it then the rest is not defaulted according to the kubebuilder defaults. Which make sense as the defaults are defined in the template field level https://github.com/openstack-k8s-operators/nova-operator/blob/2fc1351673afc28eb493b8d4d9feb2810197a199/api/v1beta1/nova_types.go#L52
not on the individual field level within the template struct. (we tried to do that defaulting as well but that fails due to webhhooks ser/des the struct). So the solution here is to explicitly confiuger the database account name as well. In the other sample files we don't define the cell template at all and there the defaulting works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually what I wrote here belongs to the collapsed cell sample file that is used as an input in the kuttl test. This is just the assert where we could drop the user and don't have to assert the account. But the sample file needs to explicitlly provide the account due to defaulting quirks
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: openstack-k8s-ci-robot, stuggi 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:
Approvers can indicate their approval by writing |
The cellDatabaseUser field is replaced with the cellDatabaseAccount. Most of the sample and kuttl test assert files does not define the cellTemplates and therefore they are defaulted by nova-operator. However in the collapsed cell sample we need to finetune the cell template fields and therefore some of the cellTemplate fields are explicitly defined. However if at least one of the field is defined then the struct level defaulting does not triggered, so the rest of the fields needs to be define too, otherwise we get the golang default values. This patch makes sure that cellDatabaseAccount field is defined in the samples where we explicitly define cellTemplates.
863148e
to
33b7fa9
Compare
keystone failed with :
I had seen this in my env once or so where cleanup of mariadbdatabase failed. the keystone mariadbdatabase is still the one from previous test which has the deletionTimestamp set: lets rerun the kuttl and see if this is a permanent thing or we can fix as a follow up |
/test openstack-operator-build-deploy-kuttl |
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/0014205479fa4f649f527f2a0df5492b ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 57m 38s |
for reference, we suppose that openstack-k8s-operators/mariadb-operator#206 might fix the issue. |
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/da6f39c6d11b49c4b07e7cfdae4476ff ❌ openstack-k8s-operators-content-provider TIMED_OUT in 30m 57s |
/retest |
recheck |
This PR contains the following updates:
cb4e05f
->95cf5d9
f14a9a6
->c2e2433
992e8b3
->fdd88ea
6b52675
->073a542
4e4efb5
->6b5d600
dc7919e
->f66e438
8c50610
->946bc7d
49edc0d
->038a5ec
b1b853e
->033a606
169ced5
->dc65ab4
169ced5
->dc65ab4
169ced5
->dc65ab4
169ced5
->dc65ab4
169ced5
->dc65ab4
169ced5
->dc65ab4
76fef73
->81216f1
fe5c58a
->acb164b
17a8992
->2fc1351
aa54e85
->2dcd200
f9db481
->4221008
b764323
->51d0de7
d42793e
->826f326
d2a5a5a
->ab60211
1394175
->1fa0278
444ba30
->25f01ea
Configuration
📅 Schedule: Branch creation - "every weekend" in timezone America/New_York, Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.
This PR has been generated by Renovate Bot.