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

Add DB purge CronJob #698

Merged
merged 4 commits into from
Feb 29, 2024
Merged

Conversation

gibizer
Copy link
Contributor

@gibizer gibizer commented Feb 20, 2024

We will have one CronJob per cell to do db archiving and purging per
cell.

Pointer types are needed for nested struct field defaulting.

Operator-lint update is needed to handle the pointer types properly in
the omitempty check and to handle an edge case with generic types.

Implements: OSPRH-104

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/e23e37eea1df4f6fa3eb5ce8d767e07d

nova-operator-content-provider FAILURE in 11m 37s
⚠️ nova-operator-kuttl SKIPPED Skipped due to failed job nova-operator-content-provider
⚠️ nova-operator-tempest-multinode SKIPPED Skipped due to failed job nova-operator-content-provider

@gibizer gibizer force-pushed the purge branch 2 times, most recently from 3fb6584 to 7fcaa77 Compare February 21, 2024 20: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/f2ee19be261e4bdca52a6a087bfa70b0

✔️ nova-operator-content-provider SUCCESS in 56m 25s
✔️ nova-operator-kuttl SUCCESS in 37m 36s
nova-operator-tempest-multinode RETRY_LIMIT in 11m 07s

@gibizer gibizer force-pushed the purge branch 2 times, most recently from 9f0dda8 to 4e075f1 Compare February 24, 2024 19:26
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/fbe5f49981fd4f96a7f30ef994f506ab

nova-operator-content-provider FAILURE in 15m 22s
⚠️ nova-operator-kuttl SKIPPED Skipped due to failed job nova-operator-content-provider
⚠️ nova-operator-tempest-multinode SKIPPED Skipped due to failed job nova-operator-content-provider

@gibizer gibizer changed the title [CRD]Add DB purge API to NovaCell Add DB purge CronJob Feb 24, 2024
@gibizer gibizer marked this pull request as ready for review February 25, 2024 16:47
@openshift-ci openshift-ci bot requested a review from bogdando February 25, 2024 16:47
@gibizer gibizer force-pushed the purge branch 3 times, most recently from 2e86c5a to 9476d52 Compare February 25, 2024 19:28
@gibizer
Copy link
Contributor Author

gibizer commented Feb 25, 2024

kuttl failure seems to be unrelated but probably persistent

fatal: [localhost]: FAILED! => {"msg": "The conditional check 'cifmw_edpm_prepare_update_os_containers | bool' failed. The error was: error while evaluating conditional (cifmw_edpm_prepare_update_os_containers | bool): 'cifmw_edpm_prepare_update_os_containers' is undefined. 'cifmw_edpm_prepare_update_os_containers' is undefined\n\nThe error appears to be in '/home/zuul/src/github.com/openstack-k8s-operators/nova-operator/ci/nova-operator-base/playbooks/pre.yaml': line 128, column 20, but may\nbe elsewhere in the file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n              --for=jsonpath='{.status.phase}'=Complete --timeout=20m\n                   ^ here\n\nThere appears to be both 'k=v' shorthand syntax and YAML in this task. Only one syntax may be used.\n"}

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/19c4c742400f4105a4ab58b7fe433fc0

✔️ nova-operator-content-provider SUCCESS in 2h 01m 06s
nova-operator-kuttl FAILURE in 41m 49s
✔️ nova-operator-tempest-multinode SUCCESS in 1h 42m 36s

Copy link
Contributor

@SeanMooney SeanMooney left a comment

Choose a reason for hiding this comment

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

overall i think this looks good. im a little confused why some of the enve tests are suing "1 0 * * *" instead of "0 0 * * *"

templates/novaconductor/bin/dbpurge.sh Show resolved Hide resolved
test/functional/base_test.go Show resolved Hide resolved
@@ -135,6 +136,14 @@ var _ = Describe("Nova controller", func() {
Expect(instance.Status.RegisteredCells).To(BeEmpty())
})

It("defaults Spec fields", func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not really default its the test defaults

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can drop the assert of the schedule filed as that is not the default, but the two age fields are defaults here

pkg/novaconductor/dbpurge.go Show resolved Hide resolved
templates/novaconductor/bin/dbpurge.sh Show resolved Hide resolved
@SeanMooney
Copy link
Contributor

SeanMooney commented Feb 26, 2024

the funcitonal test failure is

not to contain substring
: novncproxy_base_url
In [It] at: /go/src/github.com/openstack-k8s-operators/operator/test/functional/novacell_controller_test.go:405 @ 02/25/24 20:49:23.25

flaky test?

or do we need to similate this being ready or something like that to make it work properly?

@gibizer
Copy link
Contributor Author

gibizer commented Feb 26, 2024

the funcitonal test failure is

not to contain substring : novncproxy_base_url In [It] at: /go/src/github.com/openstack-k8s-operators/operator/test/functional/novacell_controller_test.go:405 @ 02/25/24 20:49:23.25

flaky test?

probably yes. but the kuttl job is broken on main

@gibizer
Copy link
Contributor Author

gibizer commented Feb 26, 2024

/test functional

@gibizer
Copy link
Contributor Author

gibizer commented Feb 26, 2024

recheck

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/6be432437726415bb039591b0135768b

✔️ nova-operator-content-provider SUCCESS in 1h 59m 11s
nova-operator-kuttl FAILURE in 38m 57s
✔️ nova-operator-tempest-multinode SUCCESS in 1h 40m 12s

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/b06e332b494246ee8d9dd28b94aca4be

✔️ nova-operator-content-provider SUCCESS in 2h 14m 22s
nova-operator-kuttl FAILURE in 40m 45s
✔️ nova-operator-tempest-multinode SUCCESS in 1h 54m 42s

@gibizer
Copy link
Contributor Author

gibizer commented Feb 27, 2024

kuttl test failure is relevant, probably we have a different name in kuttl

2024-02-26 13:34:48.219366 | controller |     case.go:364: failed in step 1-deploy
2024-02-26 13:34:48.219432 | controller |     case.go:366: cronjobs.batch "nova-cell0-conductor-db-purge" not found
2024-02-26 13:34:48.219464 | controller |     case.go:366: cronjobs.batch "nova-cell1-conductor-db-purge" not found

@gibizer
Copy link
Contributor Author

gibizer commented Feb 27, 2024

rebased after memcache support landed

We will have one CronJob per cell to do db archiving and purging per
cell.

Pointer types are needed for nested struct field defaulting.

Operator-lint update is needed to handle the pointer types properly in
the omitempty check and to handle an edge case with generic types.

Implements: OSPRH-104
@gibizer gibizer force-pushed the purge branch 2 times, most recently from 8b76a7b to 6990025 Compare February 27, 2024 16:24
@gibizer
Copy link
Contributor Author

gibizer commented Feb 27, 2024

kuttl is finally green \o/

@gibizer
Copy link
Contributor Author

gibizer commented Feb 27, 2024

/test

Copy link
Contributor

openshift-ci bot commented Feb 27, 2024

@gibizer: The /test command needs one or more targets.
The following commands are available to trigger required jobs:

  • /test functional
  • /test images
  • /test precommit-check

Use /test all to run all jobs.

In response to this:

/test

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.

@gibizer
Copy link
Contributor Author

gibizer commented Feb 27, 2024

/retest required

Copy link
Contributor

openshift-ci bot commented Feb 27, 2024

@gibizer: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test functional
  • /test images
  • /test precommit-check

Use /test all to run all jobs.

In response to this:

/retest required

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.

@gibizer
Copy link
Contributor Author

gibizer commented Feb 27, 2024

/test all

@gibizer
Copy link
Contributor Author

gibizer commented Feb 27, 2024

/retest-required

api/v1beta1/novacell_types.go Show resolved Hide resolved
pkg/novaconductor/dbpurge.go Outdated Show resolved Hide resolved
Copy link
Contributor

openshift-ci bot commented Feb 29, 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

@mrkisaolamb mrkisaolamb added lgtm and removed lgtm labels Feb 29, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 6a99bf2 into openstack-k8s-operators:main Feb 29, 2024
7 checks passed
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