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

Force recreation of ovs db cluster on first replica when protocol changes #361

Merged

Conversation

olliewalsh
Copy link
Contributor

@olliewalsh olliewalsh commented Oct 10, 2024

Cannot remove final cluster member to change protocol, but can convert to standalone schema. Cluster gets recreated with the correct address.

Jira: OSPRH-6954
Jira: OSPRH-10415

@olliewalsh
Copy link
Contributor Author

/hold

…nges

Cannot remove final cluster member to change protocol, but can convert
to standalone schema. Cluster gets recreated with the correct address.
@olliewalsh
Copy link
Contributor Author

/test ovn-operator-build-deploy-kuttl

1 similar comment
@olliewalsh
Copy link
Contributor Author

/test ovn-operator-build-deploy-kuttl

The liveness probe is quite agressive and could trigger before ovsdb-server
starts when toggling TLS. Add a startup probe to tolerate this.
@olliewalsh
Copy link
Contributor Author

/hold

Deleting clusters when relicas=3 does not work reliably, eventually
getting killed when hitting the termination grace period timeout.

ovn_db_delete and ovn_tls_enable kuttl test are affected by this so for now
just increase the timeout.
@olliewalsh
Copy link
Contributor Author

/remove-hold

@olliewalsh
Copy link
Contributor Author

/test ovn-operator-build-deploy-kuttl

1 similar comment
@olliewalsh
Copy link
Contributor Author

/test ovn-operator-build-deploy-kuttl

Comment on lines +24 to +30
template='{{.status.internalDbAddress}}{{"\n"}}'
regex="tcp:.*"
dbUri=$(oc get -n $NAMESPACE OVNDBCluster ovndbcluster-sb-sample -o go-template="$template")
matches=$(echo "$dbUri" | sed -e "s?$regex??")
if [[ -n "$matches" ]]; then
exit 1
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

would this be the same as ?

oc get -n $NAMESPACE OVNDBCluster ovndbcluster-sb -o jsonpath={.status.internalDbAddress} | grep -q tcp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, would use grep -q '^tcp:'.

# Check ovn connect is ptcp
- script: |
sb_pod=$(oc get pod -n $NAMESPACE -l service=ovsdbserver-sb -o name|head -1)
oc rsh -n $NAMESPACE ${sb_pod} ovn-sbctl --no-leader-only get-connection | grep -q ptcp
Copy link
Contributor

Choose a reason for hiding this comment

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

can use oc rsh or oc exec on the deployment to not have to check for the pod I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, seems to work

$ oc rsh statefulset/ovsdbserver-sb hostname
ovsdbserver-sb-0

This and the other assert were taken from the existing kutt tests so maybe clean them all up in a follow up patch?

Copy link
Contributor

Choose a reason for hiding this comment

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

works for me

Copy link

@otherwiseguy otherwiseguy left a comment

Choose a reason for hiding this comment

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

This looks right to me. I haven't been able to test it locally, but it makes sense.

Copy link
Contributor

openshift-ci bot commented Oct 18, 2024

@otherwiseguy: changing LGTM is restricted to collaborators

In response to this:

This looks right to me. I haven't been able to test it locally, but it makes sense.

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-sigs/prow repository.

Copy link
Contributor

@karelyatin karelyatin left a comment

Choose a reason for hiding this comment

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

/lgtm

Copy link
Contributor

openshift-ci bot commented Oct 18, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: karelyatin, olliewalsh, otherwiseguy

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 [karelyatin,olliewalsh]

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

@openshift-merge-bot openshift-merge-bot bot merged commit b2f1022 into openstack-k8s-operators:main Oct 18, 2024
6 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.

5 participants