Bug 1900989: unidling: switch away from endpoints to the service #165
Conversation
The router (in 4.7) has moved to using endpointslices. Rather than annotating both endpoints and endpointslices oc/idle will now annotate the service associated with the endpoints. Similarly unidling now needs to remove those idle annotations from the service. - openshift/oc#720 - openshift/router#225
|
@frobware: This pull request references Bugzilla bug 1900989, 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
In response to this:
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-robot
added
bugzilla/severity-high
|
Requires: |
|
/cc @aojea |
frobware
force-pushed
the
unidle-switch-from-endpoints-to-service
branch
from
d44e796
to
ce9b4b9
Compare
As oc/idle currently annotates endpoints for backwards compatibilty we remove any idle annotations after they have been removed from the service and the scaled resources have been scaled back. If we don't continue with annotating endpoints this commit can be dropped. openshift/oc#720
frobware
force-pushed
the
unidle-switch-from-endpoints-to-service
branch
from
ce9b4b9
to
5083147
Compare
|
cc @openshift/openshift-team-network-edge |
oc/idle is now updating the service in addition to the endpoints when idling. Related PRs: - openshift/openshift-controller-manager#165 - openshift/oc#720 - openshift/router#225 - openshift/sdn#252
|
Also needs openshift/openshift-apiserver#180 |
|
@frobware: This pull request references Bugzilla bug 1900989, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
|
/refresh |
|
/retest |
oc/idle will now additionally annotate the service. The test explicitly deletes it so removing that call so that we can verify the following PRs, all related to annotating the service when idling. - openshift/oc#720 - openshift/router#225 - openshift/sdn#252 - openshift/openshift-apiserver#180 - openshift/openshift-controller-manager#165
|
Why didn't we notice that idling got broken? Is there no e2e test? Does one of the PRs add a test? |
|
@frobware: This pull request references Bugzilla bug 1900989, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
We have an idling test that is currently excluded; PR to re-enable openshift/origin#25847. |
|
@frobware: This pull request references Bugzilla bug 1900989, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
3 similar comments
|
@frobware: This pull request references Bugzilla bug 1900989, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
|
@frobware: This pull request references Bugzilla bug 1900989, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
|
@frobware: This pull request references Bugzilla bug 1900989, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
|
@frobware: This pull request references Bugzilla bug 1900989, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: frobware, soltysh 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 |
openshift-ci-robot
added
the
approved
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
Why does it mean that? |
|
This change would also mean that if you tried to use a 4.6 |
Correct, that's why we should ensure the oc bits gets backported asap to 4.6 |
|
@frobware: This pull request references Bugzilla bug 1900989, which is valid. 3 validation(s) were run on this bug
In response to this:
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. |
Yes, this is true. Uniding should remove idle annotations from services as well. |
|
|
|
openshift/oc#720 has merged. /hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
|
@frobware: Some pull requests linked via external trackers have merged:
The following pull requests linked via external trackers have not merged:
These pull request must merge or be unlinked from the Bugzilla bug in order for it to move to the next state. Once unlinked, request a bug refresh with Bugzilla bug 1900989 has not been moved to the MODIFIED state. In response to this:
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. |
oc/idle is now updating the service in addition to the endpoints when idling. Related PRs: - openshift/openshift-controller-manager#165 - openshift/oc#720 - openshift/router#225 - openshift/sdn#252
|
/cherry-pick release-4.6 |
|
@frobware: new pull request created: #167 In response to this:
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. |
Previously idle annotations were added to endpoints. In 4.6 the router
switched to watching endpointslices which don't have the idle
annotation. There was some upstream PRs/discussion to automatically
propagate existing annotations from endpoints to endpointslices:
kubernetes/kubernetes#98116
As that was a non-starter we changed oc/idle to annotate the service
(and the endpoints for backwards compatibility). In the router we are
already watching service and and when we get endpoint changes we now
check for the idle annotation on the service. If it is present we
write the cluster-ip address into the haproxy.config as we previously
did prior to switching to endpointslices. We watch endpointslices for
dual-stack and IPv6 support.
This change means that undiling needs to watch for services with idle
annotations as opposed to endpoints. Switch the unidling controller to
look for idle annotations on the service.
Requires:
Bug 1900989: Unidling use Services annotations, switch away from Endpoints sdn#252TODO: amend origin test (openshift/origin#25847) to exercise idle/unidle via a route and not just the service.