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 1925524: Migrate to shared informers and watchers removed #386
Bug 1925524: Migrate to shared informers and watchers removed #386
Conversation
…gs in ConfigMapInformer
@akram: This pull request references Bugzilla bug 1925524, 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
Requesting review from QA contact: 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. |
This should fail build as it requires a version of kubernetes-client-api-plugin. |
35c64d4
to
d6a986f
Compare
/retest |
1 similar comment
/retest |
/test e2e-aws-jenkins |
/retest |
/retest |
/bugzilla refresh |
@adambkaplan: This pull request references Bugzilla bug 1925524, which is invalid:
Comment 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. |
/bugzilla refresh |
@adambkaplan: This pull request references Bugzilla bug 1925524, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
/retest |
The built image still references plugins from the stock image version instead of using the ones specified by the currently tests sync plugin:
Retesting that to see if it fixes the issue. |
needs openshift/jenkins#1297 |
openshift/jenkins#1297 will not pass e2e tests, because, openshift-sync 1.0.45 cannot start with kubernetes-1.30 plugin, as it requires an older version on kubernetes-client-api-plugin. I will have to /override this one having done the tests manually and then we will have to update openshift/jenkins#1297 to make it use openshift-sync 1.0.46 after it is released. That's a net less, but, because of the build dependency, we are are forced to do it. |
/override ci/prow/e2e-aws-jenkins |
@akram: Overrode contexts on behalf of akram: ci/prow/e2e-aws-jenkins 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. |
/lgtm |
@jkhelil: changing LGTM is restricted to collaborators 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. |
/lgtm |
@jkhelil: changing LGTM is restricted to collaborators 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Adding lgtm on behalf of @jkhelil
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adambkaplan, akram, jkhelil 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 |
@akram: All pull requests linked via external trackers have merged: Bugzilla bug 1925524 has 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. |
Important refactoring to fix conccurency issue which appeared after the introduction of the Priority and Fairness API in kubernetes.
This new API made the plugin subject to performance degration while using an important number of connexion ( > 100 ). As the openshift sync plugins uses 5 connections per namespaces that it watches, Jenkins instances watching more that 20 namespaces where affected.
Before the introduction of P&F API, the connections were silently dropped and connections in queue had a chance to get their resources to be synced. After the introduction, this connection recycling was not happening because important number of connections are queued and throttled.
This PR introduces major changes:
SharedInformers
implementations instead ofwatch
API. This introduces an automatic caching system with list refreshes and a cleaner programming style as we receive events already filtered by types in their respective methods (onAdd
onModify
onDelete
)Builds+BuildConfigs
,Secrets
,ConfigMap
andImageStreams
)