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

Switch to use new CreateServiceReadyCondition #883

Conversation

stuggi
Copy link
Contributor

@stuggi stuggi commented Oct 24, 2024

The current ExposeServiceReadyCondition can be confusing since it just reflects if the k8s service got created, but not the route to expose the public endpoint. This changes to a new CreateServiceReadyCondition to prevent this.

Depends-On: openstack-k8s-operators/lib-common#572

Jira: OSPRH-4164

@stuggi
Copy link
Contributor Author

stuggi commented Oct 24, 2024

@gibizer that would be the follow up we discussed in #522 (comment)

Copy link

This change depends on a change that failed to merge.

Change openstack-k8s-operators/lib-common#572 is needed.

Copy link
Contributor

@bogdando bogdando left a comment

Choose a reason for hiding this comment

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

lgtm but static replacements

Copy link
Contributor

@gibizer gibizer left a comment

Choose a reason for hiding this comment

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

Looks good to me. This does clarify the situation. I think the original meaning was that the openstack service is now exposed in k8s via a k8s service. But you are right that it does not mean it is exposed to the end user outside of k8s as that will require the creation of the route.

The current ExposeServiceReadyCondition can be confusing since
it just reflects if the k8s service got created, but not the route
to expose the public endpoint. This changes to a new
CreateServiceReadyCondition to prevent this.

Jira: OSPRH-4164

Depends-On: openstack-k8s-operators/lib-common#572

Signed-off-by: Martin Schuppert <[email protected]>
@stuggi stuggi force-pushed the createservice_condition branch from 1e99a4b to c88313a Compare October 29, 2024 15:37
Copy link
Contributor

@gibizer gibizer left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
Contributor

openshift-ci bot commented Oct 29, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gibizer, 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

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://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/3f6cb8d2a1ed40ee85f47b720182df81

✔️ openstack-meta-content-provider SUCCESS in 3h 50m 47s
✔️ nova-operator-kuttl SUCCESS in 44m 17s
✔️ nova-operator-tempest-multinode SUCCESS in 2h 20m 54s
nova-operator-tempest-multinode-ceph FAILURE in 34m 27s

@stuggi
Copy link
Contributor Author

stuggi commented Oct 30, 2024

recheck

@openshift-merge-bot openshift-merge-bot bot merged commit 447af09 into openstack-k8s-operators:main Oct 30, 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.

3 participants