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 2070929: Downstream Merge: 04-05-2022 #1078

Merged
merged 19 commits into from May 5, 2022

Conversation

tssurya
Copy link
Contributor

@tssurya tssurya commented May 4, 2022

martinkennelly and others added 19 commits April 30, 2022 13:45
We do not need to start a recording of pod setup within
OVN when theres an error.
This was introduced in recent changes.

Signed-off-by: Martin Kennelly <mkennell@redhat.com>
Signed-off-by: Martin Kennelly <mkennell@redhat.com>
Theres no need to hold it in memory anymore when we have calculated
all the metrics required for this pod.

Signed-off-by: Martin Kennelly <mkennell@redhat.com>
NOOP is not enough when we want to actually
test controllers that rely on SetIPs.
Deleting all IPs and adding the provided ones
should be good enough.

Signed-off-by: Ori Braunshtein <obraunsh@redhat.com>
When using the namespace's address_set for the
EgressQoS rule we should return an empty map rather
than nil, otherwise when we try to load from it
in the pod sync path we get a nil pointer dereference.

Using an empty map is fine since we don't want to
change anything in the namespace's address_set here.

Signed-off-by: Ori Braunshtein <obraunsh@redhat.com>
Adding E2E tests that cover EgressQoS functionality for both
IPv4 and IPv6:
* First table verifies that a pod's egress traffic is marked with
the correct DSCP value on EgressQoS resource updates regardless
if it was created before or after the resource.

* Second table verifies that updating a pod's labels results
in the right QoS rules being applied to its egress traffic.

Signed-off-by: Ori Braunshtein <obraunsh@redhat.com>
If the pod isn't yet scheduled, we should just ignore the event and try again later
when we get the update event.

Signed-off-by: Ori Braunshtein <obraunsh@redhat.com>
The way we currently log is too noisy for
cases we don't care about EgressQoS.
Logging pods now only when we're going to
do something with them.

Signed-off-by: Ori Braunshtein <obraunsh@redhat.com>
This commit ensures invalid or unassigned egress ip entry is also
removed from cloudprivateipconfig when egressip resource is
deleted.

Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
We should be deleting the snat towards nodeIP before
adding snat towards egressIP only from the node
where the pod lives if that same node is serving
as the egressIPnode for the pod.

A second problem that was fixed was in deleteEgressIPAssignments
we were re-adding the setup for SNAT towards nodeIP only if
pod had no egressIPs attached to it, which is wrong since depending
on the location of the pod, we'd need to re-add the SNAT towards
nodeIP even if the pod is being served by another egressIP.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
v1beta1 will be removed in 1.25

Signed-off-by: Ori Braunshtein <obraunsh@redhat.com>
v1beta1 will be removed in 1.25

Signed-off-by: Ori Braunshtein <obraunsh@redhat.com>
Delete invalid egress ip from cloudprivateipconfig
Don't warn on failure to create pod when it isn't scheduled
delete SNAT2NIP if pod.node == egressNodeServingPod
Bump PodDisruptionBudget and EndpointSlice to v1
@openshift-ci openshift-ci bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels May 4, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 4, 2022

@tssurya: This pull request references Bugzilla bug 2070929, which is invalid:

  • expected the bug to target the "4.11.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 2070929: Downstream Merge: 04-05-2022

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.

@tssurya
Copy link
Contributor Author

tssurya commented May 4, 2022

/bugzilla refresh

@openshift-ci openshift-ci bot requested review from jcaamano and trozet May 4, 2022 10:36
@openshift-ci openshift-ci bot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels May 4, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 4, 2022

@tssurya: This pull request references Bugzilla bug 2070929, which is valid. 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 POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @huiran0826

In response to this:

/bugzilla refresh

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 requested a review from huiran0826 May 4, 2022 10:36
@tssurya
Copy link
Contributor Author

tssurya commented May 4, 2022

/retest-required

@tssurya
Copy link
Contributor Author

tssurya commented May 5, 2022

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 5, 2022

@tssurya: 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/e2e-openstack-ovn ac5b231 link false /test e2e-openstack-ovn
ci/prow/e2e-metal-ipi-ovn-ipv6 ac5b231 link false /test e2e-metal-ipi-ovn-ipv6
ci/prow/okd-e2e-gcp-ovn ac5b231 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.

@trozet
Copy link
Contributor

trozet commented May 5, 2022

/lgtm

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

openshift-ci bot commented May 5, 2022

[APPROVALNOTIFIER] This PR is APPROVED

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

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 openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 5, 2022
@openshift-merge-robot openshift-merge-robot merged commit 401cfc2 into openshift:master May 5, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 5, 2022

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

Bugzilla bug 2070929 has been moved to the MODIFIED state.

In response to this:

Bug 2070929: Downstream Merge: 04-05-2022

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.

@pperiyasamy
Copy link
Member

Thanks @tssurya !

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-high Referenced Bugzilla bug's severity is high 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.

None yet

7 participants