Copy LSP.tag to LSP.tag_request if it is unset#584
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: otherwiseguy The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @otherwiseguy. Thanks for your PR. I'm waiting for a openstack-k8s-operators member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
| @@ -0,0 +1,32 @@ | |||
| #!/bin/bash | |||
| set -e | |||
There was a problem hiding this comment.
ok this will exit on errors, but may be better if we also have some wait for ovndbs on this script startup?
considering the case where both ovndbs(single replica setup) and ovn-northd started or rolled out together.
There was a problem hiding this comment.
Sorry, I should have commented on that in the commit message. Using the nbctl daemon does cause it to wait on the connection.
|
In northd logs https://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/logs//558/rdoproject.org/558bdff04a364d57a3cb4bbcbf95a0a9/controller/ci-framework-data/logs/openstack-must-gather/quay-io-openstack-k8s-operators-openstack-must-gather-sha256-60f18c13dd16095f37313b68f86965210c8ccdb0badbf530f299f3accfed406a/namespaces/openstack/pods/ovn-northd-0/logs/ovn-northd.log observed the below warning:- |
otherwiseguy
force-pushed
the
OSPRH-31613
branch
from
054ac96 to
69cf2ee
Compare
@karelyatin Ah, good catch. I moved the --db option to only the nbctl daemon setup. |
|
/ok-to-test |
| const ( | ||
| // ServiceCommand - | ||
| ServiceCommand = "/usr/bin/ovn-northd" | ||
| ServiceCommand = "/usr/local/bin/container-scripts/setup.sh" |
There was a problem hiding this comment.
IIUC the problem it is solving, this is needed only temporary and we should be fine with removing it in one of the next releases. We actually should need it only in 18, maybe even in one FR release and then it is not needed anymore, right? If that's correct understanding, maybe you could add some small TODO note to remove it in future, wdyt?
There was a problem hiding this comment.
Actually we don't have control from which release to which release customer will update so atleast for 18.0 looks we need this for 18.0 Lifetime. i.e if any release before this fix is the source version then this will be required. For any future update(i.e once all the tag are switched to tag_request) it will not be needed
For 19 we likely would not need it depending on how the pre-requisite of rhoso 19 are layered out(like if pre-requisite is env should be on latest FR like FR7 then we should be fine to drop it), but if update is allowed from FR6 to RHOSO 19 then we can't get rid of it.
If we are sure of this we can push this patch only in 18.0-fr6 branch which will be the base of all FR and bug fix releases for 18.0
But yes good to add NOTE on the code why this all is needed with a TODO to remove once this is no longer needed
otherwiseguy
force-pushed
the
OSPRH-31613
branch
2 times, most recently
from
fe297b0 to
de47e56
Compare
OVN 26.03.1 contains a fix that clears tags when the tag_request is cleared. For years, neutron has set the tag directly without setting a request. This if fixed in the neutron code, but on upgrade, it is possible that ports with tags and no tag_request will have the tags cleared before neutron could restart to fix them. Due to upgrade races between ovndbcluster and northd, this hacks the fix into northd startup. Resolves: OSPRH-31613 Signed-off-by: Terry Wilson <twilson@redhat.com>
otherwiseguy
force-pushed
the
OSPRH-31613
branch
from
de47e56 to
607960c
Compare
|
@otherwiseguy: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
3 participants
OVN 26.03.1 contains a fix that clears tags when the tag_request is cleared. For years, neutron has set the tag directly without setting a request. This if fixed in the neutron code, but on upgrade, it is possible that ports with tags and no tag_request will have the tags cleared before neutron could restart to fix them. Due to upgrade races between ovndbcluster and northd, this hacks the fix into northd startup.
Resolves: OSPRH-31613