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 1908068: Enable DownwardAPIHugePages feature gate #821
Bug 1908068: Enable DownwardAPIHugePages feature gate #821
Conversation
"DownwardAPIHugePages" an alpha feature. The effect of enabling the feature gate is to allow the existing resource "request" and "limit" fields of "hugepages-1Gi" and "hugepages-2Mi" to be exposed to a container via the Downward API. From an API stability point of view, based on the hugepages KEP, it's clear that it's only "alpha" because new features have to start out as alpha, but it seems very clearly guaranteed that it's going to go to GA. There is minimal chance that this behaviour will change as the feature moves to beta and GA upstream. There's no real danger that we're going to get stuck supporting some dropped or deprecated API as a result of enabling this feature gate now. From a code-untestedness point of view, the effect of enabling the feature gate is also small. Currently the SR-IOV DP Admission Controller (mutating webhook) is the only controller using this feature. If it sees that secondary interfaces are being requested, and the container has also requested hugepages, then it will inject requests for these fields to be included via Downward API. Downward API is already being used to pass annotations to the container, so this is just 2 more fields being requested. |
/retest |
c8a9619
to
e2718ca
Compare
@Billy99: This pull request references Bugzilla bug 1908068, 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 |
@Billy99: An error was encountered adding this pull request to the external tracker bugs for bug 1908068 on the Bugzilla server at https://bugzilla.redhat.com:
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 |
@Billy99: This pull request references Bugzilla bug 1908068, 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. |
/assign @derekwaynecarr cc @knobunc |
Removing WIP, latest CI builds now have kubelet 1.20 |
@deads2k can you take a look at this please? I'm not sure if @derekwaynecarr will get a chance. Thanks (And yes, this is related to a late feature, but it was one of the ones dependent on the rebase) |
@deads2k @derekwaynecarr @knobunc PTAL Let me know if you need more information on this PR. Need to wrap up this feature now the kubelet 1.20 is in OCP 4.7. |
e2718ca
to
d4f99e7
Compare
config/v1/types_feature.go
Outdated
@@ -126,6 +126,7 @@ var defaultFeatures = &FeatureGateEnabledDisabled{ | |||
"NodeDisruptionExclusion", // sig-scheduling, ccoleman | |||
"ServiceNodeExclusion", // sig-scheduling, ccoleman | |||
"SCTPSupport", // sig-network, ccallend | |||
"DownwardAPIHugePages", // sig-node, bmcfall |
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.
This is a node feature. It needs a name and an ack from the node team to enable this by default.
This adds the DownwardAPIHugePages to the enabled-by-default feature gate flags. Signed-off-by: Billy McFall <22157057+Billy99@users.noreply.github.com>
d4f99e7
to
677cdb1
Compare
/lgtm |
@sjenning PTAL |
/approved |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Billy99, rphillips, sjenning 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 |
@Billy99: All pull requests linked via external trackers have merged: Bugzilla bug 1908068 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. |
This adds the DownwardAPIHugePages to the enabled-by-default feature gate flags.
Signed-off-by: Billy McFall 22157057+Billy99@users.noreply.github.com