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

Bug 2094088: Fixes Updating non-default columns as well as libovsdb fixes for empty values #1146

Merged
merged 5 commits into from
Jun 16, 2022

Conversation

trozet
Copy link
Contributor

@trozet trozet commented Jun 15, 2022

Pulls in fixes to ensure we always set columns back to default values as well as the fix for libovsdb which correctly interprets empty values for pointers.

Reverts part of ovn-org/ovn-kubernetes#2800

onModelUpdatesAll was not really updating all...only things that had non
default values. Therefore if there was an update operation using
onModelUpdatesAll, where the desired update was to set a column back to
its default state, it would be ignored. This true for specific types
like maps, slices and pointers.

This commit reverts the behavior to its previous state before
onModelUpdatesAll was added. Additionally for those models which used
the same behavior of only updating default previously, onModelUpdatesAll
is renamed to onModelUpdatesAllNonDefault.

In the future, someone may want to implement a real onModelUpdatesAll
function which would dynamically figure out all of the fields of a model
to update and return that list. For now the code is commented with TODOs
and reverted back to its previous state.

Signed-off-by: Tim Rozet <trozet@redhat.com>
(cherry picked from commit 5d84deb)
Signed-off-by: Tim Rozet <trozet@redhat.com>
(cherry picked from commit 2796c94)
Signed-off-by: Tim Rozet <trozet@redhat.com>
(cherry picked from commit aacdd6e)
The test "reconciles a deleted pod referenced by a networkpolicy in
another namespace" was never actually changing the expected data in
libovsdb after deleting the pod. Due to a race the check would sometimes
just work if the policy had not updated after the pod delete yet.

Signed-off-by: Tim Rozet <trozet@redhat.com>
(cherry picked from commit b89fb48)
Signed-off-by: Tim Rozet <trozet@redhat.com>
(cherry picked from commit 292b884)
@trozet
Copy link
Contributor Author

trozet commented Jun 15, 2022

/assign @jcaamano

@openshift-ci openshift-ci bot added bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Jun 15, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 15, 2022

@trozet: This pull request references Bugzilla bug 2094088, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.11.0) matches configured target release for branch (4.11.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @anuragthehatter

In response to this:

Bug 2094088: Fixes Updating non-default columns as well as libovsdb fixes for empty values

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/test-infra repository.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 15, 2022
@jcaamano
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 16, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 16, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jcaamano, trozet

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

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 2 against base HEAD 54c45b5 and 8 for PR HEAD 34a26a5 in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 16, 2022

@trozet: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/4.11-upgrade-from-stable-4.10-local-gateway-e2e-aws-ovn-upgrade 34a26a5 link false /test 4.11-upgrade-from-stable-4.10-local-gateway-e2e-aws-ovn-upgrade
ci/prow/e2e-openstack-ovn 34a26a5 link false /test e2e-openstack-ovn
ci/prow/okd-e2e-gcp-ovn 34a26a5 link false /test okd-e2e-gcp-ovn

Full PR test history. Your PR dashboard.

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/test-infra repository. I understand the commands that are listed here.

@openshift-ci openshift-ci bot merged commit 4607b7d into openshift:master Jun 16, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 16, 2022

@trozet: All pull requests linked via external trackers have merged:

Bugzilla bug 2094088 has been moved to the MODIFIED state.

In response to this:

Bug 2094088: Fixes Updating non-default columns as well as libovsdb fixes for empty values

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/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants