Improve documentation for Webhook deployment configuration #1312
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Oops my branch was branched off of #1310 causing CI to fail. Will fix this quickly |
AObuchow
force-pushed
the
webhook-config-docs
branch
2 times, most recently
from
488c32a
to
1893b82
Compare
dkwon17
reviewed
Sep 6, 2024
docs/additional-configuration.adoc
Outdated
Comment on lines
372
to
375
| **Note:** In order for the `devworkspace-webhook-server` configuration options to take effect: | ||
| - You must place them in the https://github.com/devfile/devworkspace-operator?tab=readme-ov-file#global-configuration-for-the-devworkspace-operator[global DWOC], which has the name `devworkspace-operator-config` and exists in the namespace where the DevWorkspaceOperator is installed. If it does not already exist on the cluster, you must create it. | ||
| - You'll need to terminate the `devworkspace-controller-manager` pod and restart it so that the `devworkspace-webhook-server` deployment can be adjusted accordingly. |
There was a problem hiding this comment.
Suggested change
| **Note:** In order for the `devworkspace-webhook-server` configuration options to take effect: | |
| - You must place them in the https://github.com/devfile/devworkspace-operator?tab=readme-ov-file#global-configuration-for-the-devworkspace-operator[global DWOC], which has the name `devworkspace-operator-config` and exists in the namespace where the DevWorkspaceOperator is installed. If it does not already exist on the cluster, you must create it. | |
| - You'll need to terminate the `devworkspace-controller-manager` pod and restart it so that the `devworkspace-webhook-server` deployment can be adjusted accordingly. | |
| **Note:** For the `devworkspace-webhook-server` configuration options to take effect: | |
| - You must place them in the https://github.com/devfile/devworkspace-operator?tab=readme-ov-file#global-configuration-for-the-devworkspace-operator[global DWOC], which has the name `devworkspace-operator-config` and exists in the namespace where the DevWorkspace Operator is installed. If it does not already exist on the cluster, you must create it. | |
| - You'll need to delete the current `devworkspace-controller-manager` pod so that the replicaset can recreate a new pod. The new pod will update the `devworkspace-webhook-server` deployment. |
Here is my suggestion. We also need to have the newline in between the Note: and first bullet point, otherwise the bullet points are not rendered properly:
There was a problem hiding this comment.
Thanks so much for the review and for catching this David, I'll update the PR accordingly!
There was a problem hiding this comment.
Fixed :) Should be good now 🙏
Comment on lines
+37
to
+44
| // Note: In order for changes made to the webhook configuration to take effect: | ||
| // | ||
| // - The changes must be made in the global DevWorkspaceOperatorConfig, which has the | ||
| // name 'devworkspace-operator-config' and exists in the same namespace where the | ||
| // DevWorkspaceOperator is deployed. | ||
| // | ||
| // - The devworkspace-controller-manager pod must be terminated and recreated for the | ||
| // DevWorkspace Webhook Server deployment to be updated. |
There was a problem hiding this comment.
Suggested change
| // Note: In order for changes made to the webhook configuration to take effect: | |
| // | |
| // - The changes must be made in the global DevWorkspaceOperatorConfig, which has the | |
| // name 'devworkspace-operator-config' and exists in the same namespace where the | |
| // DevWorkspaceOperator is deployed. | |
| // | |
| // - The devworkspace-controller-manager pod must be terminated and recreated for the | |
| // DevWorkspace Webhook Server deployment to be updated. | |
| // For the changes made to the webhook configuration to take effect: | |
| // | |
| // - The changes must be made in the global DevWorkspaceOperatorConfig, which has the | |
| // name 'devworkspace-operator-config' and exists in the same namespace where the | |
| // DevWorkspace Operator is deployed. This DevWorkspaceOperatorConfig can be created | |
| // manually if it does not exist. | |
| // | |
| // - The devworkspace-controller-manager pod must be manually terminated. This allows the | |
| // replicaset to recreate the pod which will update the DevWorkspace Webhook Server deployment. |
AObuchow
force-pushed
the
webhook-config-docs
branch
from
1893b82
to
fe1e978
Compare
dkwon17
reviewed
Sep 9, 2024
docs/additional-configuration.adoc
Outdated
| **Note:** In order for the `devworkspace-webhook-server` configuration options to take effect: | ||
|
|
||
| - You must place them in the https://github.com/devfile/devworkspace-operator?tab=readme-ov-file#global-configuration-for-the-devworkspace-operator[global DWOC], which has the name `devworkspace-operator-config` and exists in the namespace where the DevWorkspaceOperator is installed. If it does not already exist on the cluster, you must create it. | ||
| - You'll need to terminate the `devworkspace-controller-manager` pod and restart it so that the `devworkspace-webhook-server` deployment can be adjusted accordingly. |
There was a problem hiding this comment.
Can we update to:
Suggested change
| - You'll need to terminate the `devworkspace-controller-manager` pod and restart it so that the `devworkspace-webhook-server` deployment can be adjusted accordingly. | |
| - You'll need to terminate the `devworkspace-controller-manager` pod so that the replicaset can recreate it. The new pod will update the `devworkspace-webhook-server` deployment. |
it's to make it clear that the user doesn't have to restart anything, all the user has to do is terminate the pod and wait until it's recreated
There was a problem hiding this comment.
Good call, done :)
AObuchow
force-pushed
the
webhook-config-docs
branch
from
fe1e978
to
20ed1b6
Compare
|
Thank you @AObuchow , I will merge soon |
dkwon17
approved these changes
Sep 9, 2024
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AObuchow, dkwon17 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 |
|
I cannot merge because of a failing check: I am checking |
|
CI is failing due to #1314 |
AObuchow
commented
Sep 10, 2024
docs/additional-configuration.adoc
Outdated
|
|
||
| These configuration options exist in the **global** DWOC's `config.webhook` field: | ||
|
|
||
| ```YAML |
There was a problem hiding this comment.
This is using the wrong code block formatting style. It should be:
[source,yaml]
----
(content)
----Will fix this.
AObuchow
force-pushed
the
webhook-config-docs
branch
from
20ed1b6
to
352db0f
Compare
|
New changes are detected. LGTM label has been removed. |
Mention requirements for DWOC webhook configuration changes to take effect. Fix devfile#1289 Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
AObuchow
force-pushed
the
webhook-config-docs
branch
from
352db0f
to
b4dc6ef
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
What issues does this PR fix or reference?
Fix #1289
Is it tested? How?
kubectl explain dwoc.config.webhook. The DESCRIPTION field should show the new mentions of needing to use the global DWOC and restarting the devworkspace-controller-manager podPR Checklist
/test v8-devworkspace-operator-e2e, v8-che-happy-pathto trigger)v8-devworkspace-operator-e2e: DevWorkspace e2e testv8-che-happy-path: Happy path for verification integration with Che