-
Notifications
You must be signed in to change notification settings - Fork 19
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
Correct placement flow to expose service only when deployment is ready #197
Correct placement flow to expose service only when deployment is ready #197
Conversation
50db196
to
034c332
Compare
This patch hit similarr issue like nova and tests timeouts when wait for correct state in db so I bump up timeout for tests |
034c332
to
edd7b96
Compare
// Only expose the service is the deployment succeeded | ||
if !instance.Status.Conditions.IsTrue(condition.DeploymentReadyCondition) { | ||
Log.Info("Waiting for the Deployment to become Ready before exposing the sevice in Keystone") | ||
return ctrl.Result{}, nil |
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.
shouldn't we requeue, or will we get another event to retrigger this, once things get ready?
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.
we are watching the Deployment so there will be an automatic reconcile when the deployment status changes.
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.
This is stable for me after 40 run. The logic in the controller looks good. We might want to cleanup the envtest if possible later separately.
[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:
Approvers can indicate their approval by writing |
0863f22
into
openstack-k8s-operators:main
Depends-On: openstack-k8s-operators/mariadb-operator#222