add NFD rule to QAT resource driver #66
Conversation
Signed-off-by: Oksana Baranova <oksana.baranova@intel.com>
| nameOverride: intel-qat-nfd | ||
| enableNodeFeatureApi: true |
There was a problem hiding this comment.
enableNodeFeatureApi is used instead of nodeFeatureRules.enabled simplifying the management of the chart. nameOverride is used to set custom name for nfd components themselves, replacing default values
There was a problem hiding this comment.
Ok, I found nameOverride use from templates/_helpers.tpl (= already in repo), but I still do not see anything using enableNodeFeatureApi, neither in this PR, or already in repo. Where it's used?
There was a problem hiding this comment.
@eero-t , this is "nfd" section, nfd is an alias for https://kubernetes-sigs.github.io/node-feature-discovery/v0.16/deployment/helm.html?highlight=enableNodeFeatureApi#general-parameters , these are values for the dependency chart with one exception: line 14 in chart.yaml tells to install this dependency when nfd.enabled is true - enabled param is for this current chart.
There was a problem hiding this comment.
Comment in the indicated docs state:
DEPRECATED: will be removed in NFD v0.17, use
featureGates.NodeFeatureAPIinstead.
Looking at the older NFD docs, enableNodeFeatureApi was referring to gRPC API that was deprecated already in NFD v0.14, but the featureGates options were added only in v0.16 (which is still the latest version).
There was a problem hiding this comment.
| nameOverride: intel-qat-nfd | |
| enableNodeFeatureApi: true | |
| nameOverride: intel-qat-nfd | |
| # TODO: this deprecated NFD option will be replaced in NFD v0.17 with "featureGates.NodeFeatureAPI" (added in v0.16): | |
| # https://kubernetes-sigs.github.io/node-feature-discovery/v0.16/deployment/helm.html#general-parameters | |
| enableNodeFeatureApi: true |
| nameOverride: intel-qat-nfd | ||
| enableNodeFeatureApi: true |
There was a problem hiding this comment.
@eero-t , this is "nfd" section, nfd is an alias for https://kubernetes-sigs.github.io/node-feature-discovery/v0.16/deployment/helm.html?highlight=enableNodeFeatureApi#general-parameters , these are values for the dependency chart with one exception: line 14 in chart.yaml tells to install this dependency when nfd.enabled is true - enabled param is for this current chart.
|
Not a real review, but a reminder to test this several times in a cluster which doesn't yet have NFD deployed, and with the nfd.enabled turned to true for the values. There's high likelihood of CRD related issues arising. |
The chart has been tested in two cluster setups: one without QAT and one with QAT. Both configurations work as expected |
eero-t
left a comment
eero-t
left a comment
There was a problem hiding this comment.
Approved, with couple of doc suggestions.
tkatila
left a comment
tkatila
left a comment
There was a problem hiding this comment.
lgtm.
Can you squash the fix commits before this is merged?
* add NFD rule to QAT resource driver Signed-off-by: Oksana Baranova <oksana.baranova@intel.com>
6 participants
No description provided.