OSDOCS-475 revisions#16983
Conversation
openshift-ci-robot
added
the
size/XL
jboxman
force-pushed
the
OSDOCS-475-remix
branch
from
396b757 to
533da2a
Compare
jboxman
added
the
peer-review-needed
|
The preview will be available shortly at: |
|
@openshift/team-documentation, PTAL. Thanks! |
jboxman
force-pushed
the
OSDOCS-475-remix
branch
2 times, most recently
from
dd45a0d to
ea6406d
Compare
|
@kalexand-rh @huffmanca @mburke5678 peer review please. |
There was a problem hiding this comment.
Do we need mark-up? NetworkAttachmentDefinition
There was a problem hiding this comment.
I'm not sure if we have a specific style for this; I'd prefer to only use a code block to reference values and commands. This is a CR Kind, so I think of it as the name of the CR thing, since all CRs of Kind: NetworkAttachmentDefinition describe an additional network. Although it's also a parameter value, but not in this context. So the use is bifurcated and thus I'm not sure which is appropriate universally.
There was a problem hiding this comment.
Probably my ignorance. You said above the CNI plug-in configuration is JSON format??
Can you get a more robust example with more of the parameters from the template?
There was a problem hiding this comment.
Replied on a later comment.
There was a problem hiding this comment.
Initial cap? This looks odd.
There was a problem hiding this comment.
@mburke5678, it does feel wrong. I asked in #forum-multus and all the Multus CNI plug-ins are lowercase names. I'm not completely surprised, as it is a plug-in rather than a complex software project or product.
There was a problem hiding this comment.
Initial cap? This looks odd.
There was a problem hiding this comment.
Agreed, but see above.
There was a problem hiding this comment.
Same here: You said above the CNI plug-in configuration is JSON format??
There was a problem hiding this comment.
@mburke5678, yeah, the plug-in configuration is not very intuitive. The configuration is a JSON object, but it's provided as a string value for a YAML parameter, and then the CNO injects it to the plug-in during initialization.
That's where the single quotes at the beginning and end of the multi-line value come from. I wonder if providing this as a YAML multi-line string might make it more obvious?
There was a problem hiding this comment.
s/ipam/IPAM?
If so, change throughout topic
There was a problem hiding this comment.
@mburke5678, based on comments in #forum-multus, the concept is IPAM, but the actual plug-in is just ipam. It's the same for the others. It's nonetheless jarring to read.
There was a problem hiding this comment.
The way it works, the DNS resolver adds the domain to the end of a host name that isn't qualified. So fedora is a host name, unqualified. And thus the resolver will append example.com, giving us fedora.example.com to resolve.
Actually I think it's fedora.example.com. or some such, but I've long forgotten what DNS I knew.
mburke5678
added
peer-review-done
|
@mburke5678 thanks! |
jboxman
force-pushed
the
OSDOCS-475-remix
branch
from
ea6406d to
c66d65c
Compare
|
@anuragthehatter, I've updated the multiple networks content based on peer review feedback. Can you take a look? Thanks! |
|
@weliang1 Can you help verifying @jboxman chnages in regards to Multiple Networks here https://OSDOCS-475-remix--ocpdocs.netlify.com/? |
|
… On Mon, Oct 14, 2019 at 9:46 PM Anurag Saxena ***@***.***> wrote:
@weliang1 <https://github.com/weliang1> Can you help verifying @jboxman
<https://github.com/jboxman> chnages in regards to Multiple Networks here
https://OSDOCS-475-remix--ocpdocs.netlify.com/?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#16983?email_source=notifications&email_token=AEFRCOD4GSC2JZQJIJRXLNTQOUOI7A5CNFSM4I4OKW62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBHEAXA#issuecomment-541999196>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEFRCOEK5ID2SBXHE56A46DQOUOI7ANCNFSM4I4OKW6Q>
.
|
|
@weliang1 thanks! |
|
@vikram-redhat, this went through QE already in #16349. Only the content has been reorganized. I'm going to merge this, and then fix https://bugzilla.redhat.com/show_bug.cgi?id=1761891. |
|
/cherrypick enterprise-4.2 |
|
@jboxman: new pull request created: #17312 DetailsIn 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. |
8 participants
This includes changes stemming from #16349.