Restore Unidler #15383
Restore Unidler #15383
Conversation
|
@DirectXMan12 I commented out a test related to this. Can you check the original disable commit and re-enable the test? |
|
/assign dcbw |
|
[test][testextended][extended: networking] |
|
@deads2k I uncommented the unidling extended tests, if that's what you're asking... |
|
[test][testextended][extended: networking] looks like the test cluster was up late drinking again |
|
Evaluated for origin testextended up to 9a454d8 |
|
Evaluated for origin test up to 9a454d8 |
|
continuous-integration/openshift-jenkins/testextended FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_extended/900/) (Base Commit: 421b57e) (PR Branch Commit: 9a454d8) (Extended Tests: networking) |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/3417/) (Base Commit: 421b57e) (PR Branch Commit: 9a454d8) |
openshift-merge-robot
added
the
size/XXL
|
|
||
| p.usingUserspaceLock.Lock() | ||
| defer p.usingUserspaceLock.Unlock() | ||
|
|
||
| for _, service := range services { | ||
| if !apihelper.IsServiceIPSet(service) { |
There was a problem hiding this comment.
Do we need to preserve the IsServiceIPSet() bit?
There was a problem hiding this comment.
I don't think so, no
|
|
||
| usingUserspace, knownEndpoints := p.usingUserspace[svcName] | ||
|
|
||
| if !knownEndpoints { |
There was a problem hiding this comment.
Any chance we could get endpoints before a service shows up, just because perhaps we have different watches for each and one watch is stalled in a goroutine somewhere but the other keeps going?
There was a problem hiding this comment.
we should have a delete for an endpoints set that we haven't already gotten an add for, though. Getting them in different orders shouldn't be a problem.
|
/approve |
openshift-merge-robot
added
approved
This commit moves the hybrid proxier onto the event-driven model adopted upstream by the iptables and userspace proxiers.
Upstream removed our ObjectReference in the ServiceInfo. Now, we have to fake an object reference when we submit events using just the name and namespace from the service info.
DirectXMan12
force-pushed
the
rebase/fix-unidler
branch
from
9a454d8 to
e2c49a8
Compare
|
someone change the meaning of |
|
/retest #14385 ? |
|
/retest |
|
The bot seems to be confused on the issue front. Let's try: /approve no-issue |
|
@dcbw can I get an LGTM? |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DirectXMan12, deads2k, knobunc The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
Automatic merge from submit-queue (batch tested with PRs 15881, 15905, 15383, 15938) |
Fixes #15101
The unidler was broken by changes to the structure of the proxies upstream.
This PR refactors the hybrid proxier to follow the event-driven model used by the
upstream proxies, and works around the lack of the ObjectReference in ServiceInfo
(we fake a lightweight ObjectReference using service name and namespace, which is
just enough for the unidler controller).