Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: docker: update some command outputs #13695

Merged
merged 2 commits into from
Oct 22, 2020
Merged

docs: docker: update some command outputs #13695

merged 2 commits into from
Oct 22, 2020

Conversation

jibi
Copy link
Member

@jibi jibi commented Oct 22, 2020

This PR addresses a couple of small nits in the docker getting started guide (just updating the output of some commands to make the document consistent with the current revision of cilium)

@jibi jibi requested a review from a team as a code owner October 22, 2020 10:11
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 22, 2020
@jibi jibi mentioned this pull request Oct 22, 2020
20 tasks
Proxy Status: OK, ip 10.15.66.191, 0 redirects active on ports 10000-20000
Cluster health: 0/1 reachable (2020-10-22T09:54:59Z)
Name IP Reachable Endpoints reachable
vagrant (localhost) 10.0.2.15 true false
Copy link
Member

Choose a reason for hiding this comment

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

It should become reachable after a while.

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, and look something like:

Cluster health:         4/4 reachable   (2020-10-22T10:59:19Z)

Also, it looks like Hubble status was removed. With 1.9 enabling Hubble by default, it should actually look something like this:

Hubble:                 Ok              Current/Max Flows: 4096/4096 (100.0%), Flows/s: 1.29   Metrics: Disabled

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, I should have waited a few seconds more to see that endpoint in a reachable state

Copy link
Member Author

Choose a reason for hiding this comment

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

This made me realize that the Vagrantfile for that tutorial is still using a docker image for cilium based on version 1.7.
I bumped it in this PR

Copy link
Member

Choose a reason for hiding this comment

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

@jibi How come Hubble is marked as "Disabled"? It should be enabled by default as stated in my comment above.

Copy link
Member Author

Choose a reason for hiding this comment

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

I suspect it's because we are not pulling the hubble images in the docker-compose.yml file (although I have still not much context on how the docker setup works for hubble)

Copy link
Member

Choose a reason for hiding this comment

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

Starting with Cilium 1.8, Hubble server is embedded in Cilium so not extra image is required. Actually, the reason why it does not show up is because the Cilium version is 1.7.2 as you found out in #13697. I think that the command output should be updated for v1.9.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that the command output should be updated for v1.9.

I actually had already updated the image before running that command 🤔

vagrant@vagrant:~/cilium/examples/getting-started$ cilium version
Client: 1.9.0-rc2 fae41eead 2020-10-21T21:03:17+11:00 go version go1.15.3 linux/amd64
Daemon: 1.9.0-rc2 fae41eead 2020-10-21T21:03:17+11:00 go version go1.15.3 linux/amd64
vagrant@vagrant:~/cilium/examples/getting-started$ cilium status | grep Hubble
Hubble:                 Disabled

Copy link
Member

Choose a reason for hiding this comment

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

Mmh, not sure why Hubble is disabled then 🤔 .

Copy link
Member

Choose a reason for hiding this comment

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

Is it worth opening an issue? Sounds like Docker integration is best effort (no large users).

Documentation/gettingstarted/docker.rst Show resolved Hide resolved
For #13627

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
For #13627

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
@pchaigno pchaigno added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. needs-backport/1.9 release-note/misc This PR makes changes that have no direct user impact. labels Oct 22, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 22, 2020
@aanm aanm merged commit 2c2b5de into cilium:master Oct 22, 2020
@jibi jibi deleted the pr/jibi/docker-docs-nits branch October 23, 2020 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

5 participants