-
Notifications
You must be signed in to change notification settings - Fork 462
Bug 1849217: Sync kublelet config across platforms #1852
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 1849217: Sync kublelet config across platforms #1852
Conversation
This came up because of a bug report showing that workloads were scheduled on masters for the baremetal platform regardless of the schedulableMasters scheduler configuration. This is due to the NoSchedule taint not being applied by default on this platform. The baremetal platform specific kubelet caused this behavior. We were going to remove the custom kubelet config once this behavior was configurable (see PR openshift#993), but it was never removed because we ended up needing to make some IPv6 related customizations in this file. We also forgot to re-add the default taint. Meanwhile, some other changes to the kubelet unit were not applied to the baremetal version. This is the simplest fix, which is to get these files in sync again. The differences are very minor, so a better follow-up would be to get back to a single kubelet unit, or at least share the duplicated content somehow. I'm leaving that further cleanup as another change, since the most straight forward fix will be the simpler one to backport. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1828250
|
@russellb: No Bugzilla bug is referenced in the title of this pull request. DetailsIn 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. |
|
/retitle Bug 1849217: Sync kublelet config across platforms |
|
@russellb: This pull request references Bugzilla bug 1849217, which is invalid:
Comment DetailsIn 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 |
|
@russellb: This pull request references Bugzilla bug 1849217, which is invalid:
Comment DetailsIn 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 |
|
@runcom: This pull request references Bugzilla bug 1849217, 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. 6 validation(s) were run on this bug
DetailsIn 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. |
|
cc @rphillips /approve |
|
/test e2e-metal-ipi |
|
I'd prefer to see this pass e2e-metal-ipi before merging, but it does look fine to me. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hardys, runcom, russellb The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@russellb: All pull requests linked via external trackers have merged: openshift/machine-config-operator#1852. Bugzilla bug 1849217 has been moved to the MODIFIED state. DetailsIn 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 came up because of a bug report showing that workloads were
scheduled on masters for the baremetal platform regardless of the
schedulableMasters scheduler configuration. This is due to the
NoSchedule taint not being applied by default on this platform. The
baremetal platform specific kubelet caused this behavior. We were
going to remove the custom kubelet config once this behavior was
configurable (see PR #993), but it was never removed because we ended
up needing to make some IPv6 related customizations in this file. We
also forgot to re-add the default taint.
Meanwhile, some other changes to the kubelet unit were not applied to
the baremetal version.
This is the simplest fix, which is to get these files in sync again.
The differences are very minor, so a better follow-up would be to get
back to a single kubelet unit, or at least share the duplicated
content somehow.
I'm leaving that further cleanup as another change, since the most
straight forward fix will be the simpler one to backport.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1828250
master: #1817
4.5: #1835