Conversation
jboxman
commented
Feb 26, 2020
- https://issues.redhat.com/browse/OSDOCS-929
|
The preview will be available shortly at: |
1eba9c9 to
df2ab8c
Compare
modules/nw-sctp-enabling.adoc
Outdated
There was a problem hiding this comment.
"you can load and enable blacklisted SCTP Kernel module"
modules/nw-sctp-verifying.adoc
Outdated
There was a problem hiding this comment.
This command is to run the sctp server, we need example that shows how to launch deployment with one pod from where user can fire sctp client to connect to this server. @danwinship @trozet do you have instructions to start pod and run sctp sample application, otherwise i will dig into the ci tests to find something.
There was a problem hiding this comment.
I found a good example which can fire both sctp server pod and sctp client in https://bugzilla.redhat.com/show_bug.cgi?id=1796157, the configuration steps are from @fedepaol who can comment on this
There was a problem hiding this comment.
The instructions on the bz are fine. They were for the default namespace but they can be modified to apply to a custom namespace.
There was a problem hiding this comment.
Should a customer install an image from quay.io? Or is there a different image that makes sense?
Should this be done in a different namespace than default?
Thanks!
There was a problem hiding this comment.
I asked @sferich888, and we can't use the quay.io image for this -- Is there a different way to validate that uses a Red Hat supported image?
There was a problem hiding this comment.
Sorry for the late reply.
Just plain fedora is fine I think. It is what I used in the kba.
There was a problem hiding this comment.
Or any other image that has installed lskctp-tools.
There was a problem hiding this comment.
@fedepaol, do you know how to find images that might include lskctp-tools? Fedora isn't permissible, either.
Could I just log in to a node and confirm sctp is loaded in by running lsmod?
There was a problem hiding this comment.
@jboxman this needs to be changed I think. Once you have the server pod running, you need to spin up a new pod (client) to which you bash into and try to connect to the server (as described in https://bugzilla.redhat.com/show_bug.cgi?id=1796157 )
The pod description is the one named sctpclient. Client connection instructions in:
5. Connect to the client pod and launch sctp_test in client mode, using the server pod's address:
There was a problem hiding this comment.
@fedepaol, okay, I didn't realize a second Pod was still necessary for this.
modules/nw-sctp-about.adoc
Outdated
There was a problem hiding this comment.
The docs don't expand "TCP" and "UDP", so I wouldn't expand "SCTP" either. People always just refer to it by the acronym. (I am sure that most of our customers who have demanded SCTP support could not actually tell you what "SCTP" stands for. I'm not sure I would have guessed correctly if I hadn't just read it... I might have gone with "Sequence" rather than "Stream"...)
There was a problem hiding this comment.
@danwinship, that makes sense; I may have been overzealous in expanding acronyms. But for SCTP, I think it may be valuable for search engine traffic.
modules/nw-sctp-about.adoc
Outdated
There was a problem hiding this comment.
(We don't expand "IP" anywhere else in the docs either.)
The customers who actually need SCTP support already know exactly what it is, so it might be useful to aim this description at people who don't need SCTP support, to assure them that they don't need to care about it and can just skip this section. eg, maybe add something about "it is not widely used except in the telecom industry".
|
@fedepaol, to get this merged for 4.4, I'm going to omit the verification steps for now and revisit them in a separate PR. |
Sorry I missed your reply. That is fine. Can you list what images are allowed? Ubi8 would be fine as well, but it is more tricky to get the subscription manager right from there. |
|
@fedepaol, images from https://catalog.redhat.com/software/containers/explore are okay. |
|
@openshift/team-documentation, this currently excludes |
|
@jboxman just did a quick try with ubi8 and it works. Pod definition: |
|
Is lkctp-tools in the ubi repo? If not the dnf install will fail. |
@sferich888 I tried that pod before posting it. |
|
Pod logs: |
ca799b5 to
4db73ea
Compare
|
So, together with @weliang1 we had some fun in debugging sctp_tool not working. For this reason, I think the following are more suitable for docs: Together with using nc: On client side: On server side: |
|
@fedepaol, thanks, I'll make those changes. |
f54b047 to
fa066e4
Compare
512b054 to
b7780f7
Compare
|
@weliang1, I updated the PR with your suggestions. Thanks! |
|
/LGTM |
kalexand-rh
left a comment
There was a problem hiding this comment.
Just a few picks and questions, and it LGTM.
modules/nw-sctp-about.adoc
Outdated
There was a problem hiding this comment.
On {op-system-first},
Can you add a little more context about why it's blacklisted or what that means for the user?
There was a problem hiding this comment.
@kalexand-rh, it's due to an oversight. Perhaps blacklisted is too harsh. (Although that's the technical term, as the kernel cannot load the SCTP module until it is removed from the blacklist.) Not sure how to diplomatically say "sorry, our bad."
Could say
On {op-system-first}, the SCTP module is disabled by default.
modules/nw-sctp-verifying.adoc
Outdated
There was a problem hiding this comment.
Is the nc package available by default to RHEL? Support's filed several tickets asking that I purge 'jq' commands from the collection because the package isn't always available, so it might be worth checking before we publish this method.
There was a problem hiding this comment.
@kalexand-rh, I wondered that myself. The UBI image used for the verification pods includes nc.
|
New changes are detected. LGTM label has been removed. |
|
/cherry-pick enterprise-4.4 |
|
@jboxman: new pull request created: #21237 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. |