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.

Sign up for GitHub

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

Jump to bottom

Use database helpers from mariadb-operator/api #554

Merged
merged 1 commit into from
Oct 6, 2023
Merged

Use database helpers from mariadb-operator/api #554

merged 1 commit into from
Oct 6, 2023

Conversation

gibizer
Copy link
Contributor

@gibizer gibizer commented Oct 2, 2023

The lib-common/modules/database is moved to mariadb-operator/api to remove a circular dependency.

@openshift-ci openshift-ci bot requested review from bogdando and viroel October 2, 2023 09:39
@gibizer
Copy link
Contributor Author

gibizer commented Oct 2, 2023

/hold dependency need to land

@gibizer gibizer mentioned this pull request Oct 2, 2023
Merged
15 tasks
mrkisaolamb
mrkisaolamb approved these changes Oct 2, 2023
View reviewed changes
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

@openshift-ci openshift-ci bot added the lgtm label Oct 2, 2023
@softwarefactory-project-zuul
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/d24ebbcff92645a8b41dc5357dddf821

✔️ nova-operator-content-provider SUCCESS in 42m 00s
❌ nova-operator-kuttl RETRY_LIMIT in 26m 49s

@SeanMooney
Copy link
Contributor

this also looks good to me for what its worth. what is not clear to me is what the depency is but ill assume you will remove the hold once its merged.

SeanMooney
SeanMooney reviewed Oct 2, 2023
View reviewed changes
go.mod Outdated
@@ -95,3 +94,5 @@ replace github.com/openshift/api => github.com/openshift/api v0.0.0-202304141430
// needed to to cert-manager v1.11.4 see https://github.com/cert-manager/cert-manager/blob/v1.11.4/go.mod#L263C1-L264C104
// remove this once we bump to cert-manager v1.12.x
replace github.com/Venafi/vcert/v4 => github.com/jetstack/vcert/v4 v4.9.6-0.20230519122548-219f317ae107 //allow-merging

replace github.com/openstack-k8s-operators/mariadb-operator/api => github.com/gibizer/mariadb-operator/api v0.0.0-20230929123006-6644f4172777
Copy link
Contributor

Choose a reason for hiding this comment

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

this will obvioulsy be dropped when the dep is merged but is here for testing.
the ci job will technically prevent the merge without the hold until this is removed

@gibizer
Copy link
Contributor Author

gibizer commented Oct 2, 2023

this also looks good to me for what its worth. what is not clear to me is what the depency is but ill assume you will remove the hold once its merged.

You can see the full picture of the in flight PRs related to this work here openstack-k8s-operators/lib-common#352

@gibizer
Copy link
Contributor Author

gibizer commented Oct 2, 2023

recheck
cert manager issue has been fixed in install_yamls

No resources found in cert-manager-operator namespace.
No resources found in cert-manager-operator namespace.
No resources found in cert-manager-operator namespace.
No resources found in cert-manager-operator namespace.
No resources found in cert-manager namespace.
error: timed out waiting for the condition on pods/cert-manager-cainjector-9dd8574d-dvxg4

@gibizer gibizer mentioned this pull request Oct 3, 2023
Merged
@gibizer
Use database helpers from mariadb-operator/api
8bf7d07 
The lib-common/modules/database is moved to mariadb-operator/api to
remove a circular dependency.
@openshift-ci openshift-ci bot removed the lgtm label Oct 5, 2023
@gibizer
Copy link
Contributor Author

gibizer commented Oct 5, 2023

/unhold

stuggi
stuggi approved these changes Oct 6, 2023
View reviewed changes
Copy link
Contributor

@stuggi stuggi left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Oct 6, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 6, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gibizer, mrKisaoLamb, 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:
  • OWNERS [gibizer,mrKisaoLamb,stuggi]

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

@openshift-ci openshift-ci bot merged commit 07ad73a into openstack-k8s-operators:main Oct 6, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SeanMooney SeanMooney SeanMooney left review comments

@mrkisaolamb mrkisaolamb mrkisaolamb approved these changes

@stuggi stuggi stuggi approved these changes

@bogdando bogdando Awaiting requested review from bogdando

@viroel viroel Awaiting requested review from viroel

Assignees

@stuggi stuggi

@mrkisaolamb mrkisaolamb

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gibizer @SeanMooney @stuggi @mrkisaolamb @openshift-merge-robot