-
Notifications
You must be signed in to change notification settings - Fork 1.8k
TELCODOCS-2304: Make the operator configuration persistent during clu… #95124
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
base: main
Are you sure you want to change the base?
Conversation
@StephenJamesSmith: This pull request references TELCODOCS-2304 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
🤖 Wed Jun 25 14:04:15 - Prow CI generated the docs preview: |
@StephenJamesSmith: This pull request references TELCODOCS-2304 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
affe1ab
to
16bafdc
Compare
/lgtm |
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
/label telco |
/label peer-review-needed |
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.
nice work! mostly just left some questions and suggestions below
modules/kmm-configuring-kmmo.adoc
Outdated
|
||
[NOTE] | ||
==== | ||
If you want to configure `KMM Hub`, create the `ConfigMap` using the name `kmm-operator-hub-manager-config` in the KMM-hub controller's namespace. |
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.
is it KMM-hub, or KMM Hub
? both variations being used here without any additional context is kind of confusing
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.
They are the same. You can rephrase this to use only KMM-hub
or KMM Hub
.
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.
Changed to "KMM Hub".
modules/kmm-configuring-kmmo.adoc
Outdated
|`webhook.disableHTTP2` | ||
|If `true`, disables HTTP/2 for the webhook server, as a mitigation for link:https://access.redhat.com/security/cve/cve-2023-44487[cve-2023-44487]. The recommended value is `true`. | ||
|`metrics.bindAddress` | ||
|Determines the bind address for the metrics server. If unspecified, the default is `:8080`. To disable the metrics server, set to `0`. The default value is `0.0.0.0:8443`. |
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.
so is the default :8080
or 0.0.0.0:8443
?
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.
:8080
is a mistake. I am currently fixing it in the dev documentation with kubernetes-sigs/kernel-module-management#1147.
@StephenJamesSmith you can use the diff from the PR I have linked to fix this PR.
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.
Made @ybettan's change.
modules/kmm-configuring-kmmo.adoc
Outdated
|`metrics.bindAddress` | ||
|Determines the bind address for the metrics server. If unspecified, the default is `:8080`. To disable the metrics server, set to `0`. The recommended value is `0.0.0.0:8443`. | ||
|`webhook.disableHTTP2` | ||
|If `true`, disables HTTP/2 for the webhook server, as a mitigation for link:https://access.redhat.com/security/cve/cve-2023-44487[cve-2023-44487]. The default value is `true`. |
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.
|If `true`, disables HTTP/2 for the webhook server, as a mitigation for link:https://access.redhat.com/security/cve/cve-2023-44487[cve-2023-44487]. The default value is `true`. | |
|If `true`, disables HTTP/2 for the webhook server, as a mitigation for link:https://access.redhat.com/security/cve/cve-2023-44487[CVE-2023-44487]. The default value is `true`. |
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.
Changed.
modules/kmm-configuring-kmmo.adoc
Outdated
|
||
|`worker.seLinuxType` | ||
|Determines the value of the `seLinuxOptions.type` field of the worker container's security context. For more information, see link:https://kubernetes.io/docs/tasks/configure-pod-container/security-context/[SecurityContext]. The recommended value is `spc_t`. | ||
|Determines the value of the `seLinuxOptions.type` field of the worker container's security context. For more information, see link:https://kubernetes.io/docs/tasks/configure-pod-container/security-context/[SecurityContext]. The default value is `spc_t`. |
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.
|Determines the value of the `seLinuxOptions.type` field of the worker container's security context. For more information, see link:https://kubernetes.io/docs/tasks/configure-pod-container/security-context/[SecurityContext]. The default value is `spc_t`. | |
|Determines the value of the `seLinuxOptions.type` field of the worker container's security context. For more information, see link:https://kubernetes.io/docs/tasks/configure-pod-container/security-context/[SecurityContext]. The default value is `spc_t`. |
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.
Changed.
modules/kmm-configuring-kmmo.adoc
Outdated
|`worker.setFirmwareClassPath` | ||
|Sets the kernel's firmware search path into the `/sys/module/firmware_class/parameters/path` file on the node. The recommended value is `/var/lib/firmware` if you need to set that value through the worker app. Otherwise, unset. | ||
|`worker.firmwareHostPath` | ||
|If set, the value of this field is written by the worker into the /sys/module/firmware_class/parameters/path file on the node. For more information see link:https://openshift-kmm.netlify.app/documentation/firmwares/#setting-the-kernels-firmware-search-path[Setting the kernel's firmware search path]. The default value is `/var/lib/firmware`. |
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.
worker container? might be useful to specify here to avoid confusion with worker node
|If set, the value of this field is written by the worker into the /sys/module/firmware_class/parameters/path file on the node. For more information see link:https://openshift-kmm.netlify.app/documentation/firmwares/#setting-the-kernels-firmware-search-path[Setting the kernel's firmware search path]. The default value is `/var/lib/firmware`. | |
|If set, the value of this field is written by the worker container into the /sys/module/firmware_class/parameters/path file on the node. For more information see link:https://openshift-kmm.netlify.app/documentation/firmwares/#setting-the-kernels-firmware-search-path[Setting the kernel's firmware search path]. The default value is `/var/lib/firmware`. |
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.
Added "container".
…ster/operator upgrades
16bafdc
to
6a4fa6b
Compare
New changes are detected. LGTM label has been removed. |
@StephenJamesSmith: all tests passed! Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
Make the operator configuration persistent during cluster/operator upgrades
Version(s):
openshift-4.19
KMMO 2.4
Issue:
https://issues.redhat.com/browse/TELCODOCS-2304
Link to docs preview:
https://95124--ocpdocs-pr.netlify.app/openshift-enterprise/latest/hardware_enablement/kmm-kernel-module-management.html#kmm-configuring-kmmo_kernel-module-management-operator
Dev: @ybettan
QE: @cdvultur
QE review: