Skip to content

Adding connectivity test to troubleshooting doc#11643

Merged
joestringer merged 4 commits intocilium:masterfrom
jedsalazar:add-connectiviy-test-to-troubleshooting-doc
Jun 2, 2020
Merged

Adding connectivity test to troubleshooting doc#11643
joestringer merged 4 commits intocilium:masterfrom
jedsalazar:add-connectiviy-test-to-troubleshooting-doc

Conversation

@jedsalazar
Copy link
Copy Markdown
Contributor

@jedsalazar jedsalazar commented May 21, 2020 •
edited by aanm
Loading

Previously the troubleshooting documentation lacked a reference to the connectivity tests specified in the getting started guide.

Added the connectivity guide reference as well as some minor style updates

Fixes: #11581

Signed-off-by: Jed Salazar jed@isovalent.com

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Thanks for contributing!

Fixes: #issue-number

Add connectivity test to troubleshooting

@jedsalazar jedsalazar requested a review from a team as a code owner May 21, 2020 17:48
@maintainer-s-little-helper
Copy link
Copy Markdown

maintainer-s-little-helper Bot commented May 21, 2020

Please set the appropriate release note label.

@jedsalazar jedsalazar added the release-note/misc This PR makes changes that have no direct user impact. label May 21, 2020
@michi-covalent
Copy link
Copy Markdown
Contributor

michi-covalent commented May 21, 2020

@jedsalazar looks like you need to update the spelling word list.

you can also run make render-docs locally to see if there are any syntax/spelling errors.

https://docs.cilium.io/en/latest/contributing/testing/documentation/

@coveralls
Copy link
Copy Markdown

coveralls commented May 21, 2020 •
edited
Loading

Coverage Status

Coverage decreased (-0.04%) to 36.886% when pulling ec22fa4 on jedsalazar:add-connectiviy-test-to-troubleshooting-doc into 60bffa4 on cilium:master.

@tgraf tgraf added the area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. label May 21, 2020
@maintainer-s-little-helper
Copy link
Copy Markdown

maintainer-s-little-helper Bot commented May 21, 2020

Commit 33d89c40cb6bb80a2c152f9cbb5bf5974032b2e3 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper Bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 21, 2020
@netlify
Copy link
Copy Markdown

netlify Bot commented May 21, 2020 •
edited
Loading

Deploy preview for cilium-docs failed.

Built with commit 33d89c40cb6bb80a2c152f9cbb5bf5974032b2e3

https://app.netlify.com/sites/cilium-docs/deploys/5ec6cbc594e7af00079df10e

Adding connectivity test to troubleshooting doc
675797a 
Previously the troubleshooting documentation lacked a reference to the connectivity tests specified in the getting started guide.

Added the connectivity guide reference as well as some minor style updates

Fixes: cilium#11581

Signed-off-by: Jed Salazar <jed@isovalent.com>

Updating issues in the docs

Fixing ts_agent_logs line, adding a link to the test and fixing a spelling error
@jedsalazar jedsalazar force-pushed the add-connectiviy-test-to-troubleshooting-doc branch from 33d89c4 to 675797a Compare May 21, 2020 19:33
@maintainer-s-little-helper maintainer-s-little-helper Bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 21, 2020
joestringer
joestringer requested changes May 21, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a welcome change :)

Couple of comments below.

Comment thread Documentation/troubleshooting.rst
michi-covalent
michi-covalent reviewed May 21, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@michi-covalent michi-covalent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm overall, one minor comment about not hardcoding the release tag.

Comment thread Documentation/troubleshooting.rst Outdated
variant and the readiness and liveness gate indicates success or failure of the
test:

.. _test: https://raw.githubusercontent.com/cilium/cilium/1.7.4/examples/kubernetes/connectivity-check/connectivity-check.yaml
Copy link
Copy Markdown
Contributor

@michi-covalent michi-covalent May 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think you need to use the SCM_WEB variable defined in Documentation/conf.py here to avoid hardcoding branches/tags. here is an example: https://github.com/cilium/cilium/blame/054dd163fdb792c44b3c493b8d6fd111e5e1a2bc/Documentation/gettingstarted/dns.rst#L33

@maintainer-s-little-helper
Copy link
Copy Markdown

maintainer-s-little-helper Bot commented May 21, 2020

Commit d4ae623733662b7e504d201be61c8fba9074c353 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper Bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 21, 2020
jedsalazar
jedsalazar commented May 21, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@jedsalazar jedsalazar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm overall, one minor comment about not hardcoding the release tag.

thanks, Michi! added. it doesn't seem to render on my local build. going see if it works in netlify

Comment thread Documentation/troubleshooting.rst
@maintainer-s-little-helper
Copy link
Copy Markdown

maintainer-s-little-helper Bot commented May 22, 2020

Commit d4ae623733662b7e504d201be61c8fba9074c353 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

1 similar comment
@maintainer-s-little-helper
Copy link
Copy Markdown

maintainer-s-little-helper Bot commented May 22, 2020

Commit d4ae623733662b7e504d201be61c8fba9074c353 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@aanm
Copy link
Copy Markdown
Member

aanm commented May 27, 2020

@jedsalazar some commits are missing sign-offs

jedsalazar added 3 commits May 27, 2020 08:00
Update link to reference SCM_WEB var
18e6fa7 
Hard-coding is bad, variables are good.

Fixes: cilium#11581

Signed-off-by: Jed Salazar <jed@isovalent.com>
Adding cilium-test namespace creation and depth of test descriptions
1b171e7 
Given that the connectivity testing is referenced in the troubleshooting docs, it's worth deploying them to an empty namespace, `cilium-test`. h/t @joe

Also included an initial effort to inlcude a table describing the coverage that each testing type provides.

Fixes: cilium#11581

Signed-off-by: Jed Salazar <jed@isovalent.com>
Fix spelling error and add intra to wordlist
ec22fa4 
Fixes: cilium#11581

Signed-off-by: Jed Salazar <jed@isovalent.com>
@jedsalazar jedsalazar force-pushed the add-connectiviy-test-to-troubleshooting-doc branch from a3b97c3 to ec22fa4 Compare May 27, 2020 14:02
@maintainer-s-little-helper maintainer-s-little-helper Bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 27, 2020
@jedsalazar
Copy link
Copy Markdown
Contributor Author

jedsalazar commented May 27, 2020

@jedsalazar some commits are missing sign-offs

Thanks, @aanm. I updated the commits. PTAL

michi-covalent
michi-covalent approved these changes May 29, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@michi-covalent michi-covalent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's roll

@joestringer joestringer merged commit 3da5bcf into cilium:master Jun 2, 2020
@tklauser tklauser mentioned this pull request Jun 3, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joestringer joestringer joestringer approved these changes

@michi-covalent michi-covalent michi-covalent approved these changes

Assignees

No one assigned

Labels

area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Docs: Add connectivity-check.yaml instructions Cilium troubleshooting section

7 participants

@jedsalazar @michi-covalent @coveralls @aanm @joestringer @tklauser @tgraf