NGF: Add Deploy data plane doc #530
Conversation
github-actions
bot
added
documentation
There was a problem hiding this comment.
Generally LGTM, but I have a lot of edit suggestions.
Shell examples are inconsistent: the start of the document delineates the shell command and the output into different code blocks, while later the command and output are put into the same blocks.
In the former case, a reader can easily copy the command - in the latter case, they cannot, and if they go to manually copy the command, they'll be forced to also strip away the shell prompts and other decorations.
I favour the former for general UX, but you can also just block the shell block and the text output together: no need to add text like "You should see these conditions".
| ... | ||
| ``` | ||
|
|
||
| {{< note >}} In order for the changes to propagate onto the Service, it needs to be manually restarted. {{< /note >}} |
There was a problem hiding this comment.
| {{< note >}} In order for the changes to propagate onto the Service, it needs to be manually restarted. {{< /note >}} | |
| {{< note >}} You must manually restart the service for changes to take effect. {{< /note >}} |
There was a problem hiding this comment.
We need to restart the service to have its labels and annotations set properly? I wouldn't think those fields needed to have the happen. Does that mean delete it and have it re-created?
There was a problem hiding this comment.
@sjberman yea, just double checked, it needs to be deleted, and NGF re-creates it.
I wouldn't think those fields need to happen that way either, but that seems to be the current behavior. Do you think that should be changed? Weird that the type of Service doesn't require manual intervention but labels/annotations do
There was a problem hiding this comment.
Do the labels/annotations on the Deployment get set properly? Or any other provisioned resource?
There may be a bug if none of that is working.
There was a problem hiding this comment.
yea the labels/annotations on the Deployment, replicaset, and pod all get set properly, just not the Service
There was a problem hiding this comment.
Hm, that may be worth investigating. For some fields like IP it makes sense that the Service would need to restart, but labels and annotations should just be applied.
There was a problem hiding this comment.
FWIW, running kubectl label on the service works without needing to restart the service, so I feel like there's a bug here
There was a problem hiding this comment.
I also checked and the labels don't get applied to any of the ConfigMaps either, so it's not limited to Services.
There was a problem hiding this comment.
issue has been identified and fixed, so i will remove this note
| ... | ||
| ``` | ||
|
|
||
| {{< note >}} In order for the changes to propagate onto the Service, it needs to be manually restarted. {{< /note >}} |
There was a problem hiding this comment.
We need to restart the service to have its labels and annotations set properly? I wouldn't think those fields needed to have the happen. Does that mean delete it and have it re-created?
|
FYI, comment only: we have a definition for |
|
|
||
| ```shell | ||
| kubectl get service cafe-nginx | ||
| ``` |
There was a problem hiding this comment.
shouldn't we want spacing between code blocks? @ADubhlaoich i remember it being a rule before
Proposed changes
Add "Deploy Data plane document" to guide users on how to create and modify the NGINX Data Plane.
Problem: Users want a guide on how to create and modify the NGINX Data Plane.
Solution: Added the guide.
Testing: Tested with
make watchCloses nginx/nginx-gateway-fabric#3237
Checklist
Before merging a pull request, run through this checklist and mark each as complete.
README.mdandCHANGELOG.mdFootnotes
Potentially sensitive changes include anything involving code, personally identify information (PII), live URLs or significant amounts of new or revised documentation. Please refer to our style guide for guidance about placeholder content. ↩