Skip to content

Conversation

@errordeveloper
Copy link
Contributor

@errordeveloper errordeveloper commented Jan 31, 2017

This is to improve UX, it removes the need for users to click away to addon pages.

This a very simple version and only 3 addons are included right now, happy to extend this, if folks like the idea.

@lukemarsden @thockin @mikedanese @caseydavenport PTAL

Initial appearance:
screenshot 2017-01-31 14 15 00

Drop down choices:
screenshot 2017-01-31 14 15 13

Weave Net addon selected from dropdown:
screenshot 2017-01-31 14 15 23


This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 31, 2017
@errordeveloper errordeveloper changed the title Add simple dropdown menu for selecting a network addon [RFC (do not merge)] Add simple dropdown menu for selecting a network addon Jan 31, 2017
@chenopis chenopis added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Jan 31, 2017
@thockin
Copy link
Member

thockin commented Feb 1, 2017

This is cute. I'll leave it to Devin to decide if we want to support code in these docs.

@errordeveloper
Copy link
Contributor Author

IMO we could do a similar thing for package installation instructions.

@lukemarsden
Copy link
Contributor

lukemarsden commented Feb 1, 2017

Thanks @thockin! Hi @devin-donnelly - the context here is that we've had lots of users show up in SIG-cluster-lifecycle complaining that it's hard to find the instructions to install a network. At the same time, we also want to remain vendor neutral. This solution fixes the problem by showing users how to install a network inline in the setup guide, while remaining vendor neutral by making it clear that a choice is necessary at this stage. WDYT?

Regards maintaining the JS, sign us up ;)

@thockin
Copy link
Member

thockin commented Feb 1, 2017 via email

Copy link
Member

@luxas luxas left a comment

Choose a reason for hiding this comment

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

We should let Canal and Romana join the party as well

<script>
var networkAddons = {
"Calico": "kubectl apply -f \"http://docs.projectcalico.org/v2.0/getting-started/kubernetes/installation/hosted/calico.yaml\"",
"Flannel": "kubeclt apply -f \"https://github.com/coreos/flannel/blob/master/Documentation/kube-flannel.yml?raw=true\"",
Copy link
Member

Choose a reason for hiding this comment

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

kubectl


<p>
<select id="pickNetworkAddon" style="margin:1em;font-size:1.2em;">
<option>Please select a network addon...</option>
Copy link
Member

Choose a reason for hiding this comment

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

In case the user chooses this, I think the command should go back to # kubectl apply -f <selected-add-on.yaml>, now it does not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, makes sense, will update.

Copy link
Member

@caseydavenport caseydavenport left a comment

Choose a reason for hiding this comment

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

Still reluctant about adding more enumerations of (network) add-ons in numerous places in the k8s docs but I suppose this is OK.


<script>
var networkAddons = {
"Calico": "kubectl apply -f \"http://docs.projectcalico.org/v2.0/getting-started/kubernetes/installation/hosted/calico.yaml\"",
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

URL looks the same to me...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I can see the kubeadm path segment...

Choose a reason for hiding this comment

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

@caseydavenport I think we should try to avoid having a version number in the URL. Can we have a "latest" or similar that corresponds to the latest stable release?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've update the URL to a kubeadm specific one.

# kubectl apply -f <add-on.yaml>
Several projects provide Kubernetes pod networks using CNI, some of which also support [Network Policy](/docs/user-guide/networkpolicies/).

You must select a network addon from the drop-down menu below, and correct instructions will appear below.
Copy link
Member

Choose a reason for hiding this comment

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

This sort of implies that the add-on needs to be from the dropdown. We should make it clear that this is just a sampling of network add-ons and not the definitive list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, could you suggest a way to phrase it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A specific suggestion would be very appreciated!

Copy link
Member

Choose a reason for hiding this comment

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

How about something like this:

"The dropdown below includes some common network add-ons. Select one from the dropdown to view its installation instructions."

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry to be late to this thread. I'm not a fan of this drop down idea. Mainly because I can't' recall ever seeing it used in docs to address this kind of problem? It seems like a strange thing to see on a doc page. Are there examples of this that someone could point me to?

Also, it doesn't address the problem of enumerating all the options because every add-on provider will want to be include in the drop down. Finally, it introduces additional doc edit/management complexity. So, in addition to markdown, I have to know javascript to make a change? Some of my trivially simple markdown edits have broken the rendering of some pages, and IMO adding scripts is opening a real bag of worms.

That said, happy to defer to doc experts on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @chrismarino, I don't think it matters whether it's used in other docs, as long as it's obvious enough to users. IMHO, it's a very common GUI idiom to have a dropdown select which thing you're looking at. An alternative design could be tabs. But I don't think it makes much of a difference (and dropdown is more scalable than tabs).

I think what really does matter is that users are having a bad time:

So I understand that I need to complete Step 3 first, but I am totally lost on what to do for the network.I feel, this step is quite vague.

Hopefully it's fairly clear how the JS mapping works. Failing that, we have volunteered to maintain the JS.

@errordeveloper
Copy link
Contributor Author

We should let Canal and Romana join the party as well

Yes, who should we ping?

@luxas
Copy link
Member

luxas commented Feb 8, 2017

cc @chrismarino for Romana
cc @lxpollitt for Canal

Also cc @tomdee

@lukemarsden
Copy link
Contributor

lukemarsden commented Feb 10, 2017

Let's get this in, it dramatically improves the UX for the kubeadm guide! cc @devin-donnelly

@errordeveloper errordeveloper changed the title [RFC (do not merge)] Add simple dropdown menu for selecting a network addon Add simple dropdown menu for selecting a network addon Feb 10, 2017
@errordeveloper
Copy link
Contributor Author

I've addressed the comments above.

@lukemarsden
Copy link
Contributor

lukemarsden commented Feb 13, 2017

Hey @thockin, totally hear you. We keep getting feedback that the current guide isn't working for people, though. Users are having a bad time. This PR is a big step towards fixing that. Can you help us get this merged?

@errordeveloper errordeveloper force-pushed the network-addon-dropdown branch 2 times, most recently from 9b86b86 to 673f66d Compare February 13, 2017 12:04
- add simple dropdown menu for selecting a network addon,
  so users don't have go to other pages to find the commands
  they need to run

- remove unecessary information from the paragraph
@errordeveloper
Copy link
Contributor Author

@caseydavenport I've used the text you've suggested, thanks very much.

@thockin @lukemarsden I've also edited the paragraph a bit more, there was some info in bold that was just unnecessary for anyone to know at that stage.

@lukemarsden
Copy link
Contributor

Thanks @errordeveloper, LGTM

@thockin
Copy link
Member

thockin commented Feb 13, 2017

This is squarely in the domain of docs owners, who have an actual docs plan...

@devin-donnelly

Added Romana to drop down...
@devin-donnelly
Copy link
Contributor

Sorry it's taken me so long to get in on this.

@errordeveloper , @lukemarsden : I'm not a big fan of dropping a new script into the documentation and then having to maintain it. I understand the reasoning behind this from a UX perspective, though.

As a compromise, have you considered using the Tabs functionality that's already installed on the site?

See this file: https://github.com/kubernetes/kubernetes.github.io/blob/master/_includes/tabs.html

These display different included code files in tabs rather than drop-downs, but it has the advantage of already being installed and working on the site and also lives in our _includes directory.

@devin-donnelly
Copy link
Contributor

See this file for an example: kubernetes.github.io/docs/user-guide/services/operations.md

@errordeveloper
Copy link
Contributor Author

@devin-donnelly I've only noticed tabs after I wrote this, I will try to use tabs for this, thanks.

@chenopis
Copy link
Contributor

chenopis commented Mar 2, 2017

@errordeveloper I will close this PR for now then.

@chenopis chenopis closed this Mar 2, 2017
@errordeveloper
Copy link
Contributor Author

@devin-donnelly I am afraid _include/tabs.html doesn't cut it for this purpose because:

  • it requires me to specify a file for which it wants to generate a link
    • this makes sense with the common use-case, but doesn't make sense for commands
  • there's a way to have no default choice, which is what's required here
  • spaces in tab titles break it completely
    • I have to say "WeaveNet" instead of "Weave Net"
    • I cannot say "Please select one to the left" in the title of the first tab

More broadly, I've not been able to make meaningful UX out of it. I spent half-a-day yesterday.

We can try extending tabs, but the way that it's written right now makes my brain hurt. I've quite quickly written a simple Jekyll plugin that takes a JSON description of all command variants we want to put in separate tabs, but it turns out we cannot use plugins due to gh-pages gem, and @pwittrock confirmed the site is hosted on Github pages, so we cannot simply get rid of that gem. It would be really great if we could convert this page to use tabs for Debian/RedHat installation instructions also, let's figure it out.

@lukemarsden
Copy link
Contributor

lukemarsden commented Mar 6, 2017

@chenopis Please re-open.

@devin-donnelly Please note Ilya's comments above. We tried the tabs route, and it turned out to be more complicated than what we were initially suggesting here. Additionally, the dropdown list is:

  1. More scalable, as there's no horizontal width limit to the number of networks that can be featured.
  2. More natural to have an "unselected default", which is necessary to avoid biasing towards one vendor.

@chenopis
Copy link
Contributor

chenopis commented Mar 6, 2017

Ok, I'm reopening this for further discussion.

@chenopis chenopis reopened this Mar 6, 2017
<script>
var networkAddons = {
"Calico": "kubectl apply -f \"http://docs.projectcalico.org/v2.0/getting-started/kubernetes/installation/hosted/kubeadm/calico.yaml\"",
"Flannel": "kubectl apply -f \"https://github.com/coreos/flannel/blob/master/Documentation/kube-flannel.yml?raw=true\"",
Copy link
Contributor

Choose a reason for hiding this comment

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

Flannel also needs to apply a RBAC file to run properly: https://github.com/coreos/flannel/blob/master/Documentation/kube-flannel-rbac.yml
Would be helpful to have a single file a no external commands.

@errordeveloper
Copy link
Contributor Author

errordeveloper commented Mar 15, 2017 via email

@jaredbhatti
Copy link
Contributor

jaredbhatti commented Mar 24, 2017

Per @thockin and @devin-donnelly concerns, I am strongly against this PR. I ask that you reconsider using tabs. The drop-down is difficult to read and distinguish from the text, and I'm EXTREMELY reluctant to support new UX that has near-similar functionality to something we already have on the site.

@chenopis
Copy link
Contributor

FYI, we're going to discuss this at the next SIG Docs meeting on 4/4, tentatively scheduled for 10:30 am PDT:

Join from PC, Mac, Linux, iOS or Android: https://zoom.us/j/678394311
Or iPhone one-tap (US Toll): +16465588656,678394311# or +14086380968,678394311#

Or Telephone:
Dial: +1 646 558 8656 (US Toll) or +1 408 638 0968 (US Toll)
Meeting ID: 678 394 311
International numbers available: https://zoom.us/zoomconference?m=zy_s4GywCQIUgexWdq58vma1f9N8F8s8

@chenopis
Copy link
Contributor

chenopis commented May 4, 2017

I have refactored the tabs implementation in PR #3268, and you can test it out here: https://deploy-preview-3268--kubernetes-io-master-staging.netlify.com/docs/tabs-example/

@chenopis chenopis self-assigned this May 4, 2017
@chenopis chenopis removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label May 30, 2017
@luxas luxas removed their assignment Jun 19, 2017
@alexellis
Copy link

I write quite a few tutorials for Docker Swarm (new version) and I tried to get started with Kubernetes via kubeadm for the first time.. it was a complete fail. It seems like the docs are trying to be impartial but by doing so only give hints as to what is required to get flannel working.. In the end I had to rely on a Kubernetes expert to give me the answers.

It would be great if this could be improved.

  • It's not clear that the --pod-network-cidr=10.244.0.0/16 reference is meant for kubeadm init - you should show that on one line.
  • My provider - Ubuntu 16.04 on Packet didn't have proc config - this could be quite common, so you should mention it in the context of the flight checks
  • After applying the flannel script - nothing worked and I had to get help from someone else to find out how to apply the flannel RBAC script.

Even if trying to be impartial - this guide needs to leave you with a working system at the end and right now it just left me super confused and I'm not a newbie to orchestration.

Needed in addition:

kubectl create -f https://raw.githubusercontent.com/coreos/flannel/master/Documentation/kube-flannel-rbac.yml

Before / after:

kubectl create -f https://raw.githubusercontent.com/coreos/flannel/master/Documentation/kube-flannel.yml

@luxas
Copy link
Member

luxas commented Jun 26, 2017

Thanks a lot @alexellis for that write-up. I hope that comment can bring this thread to life...

So TL;DR; I agree with @alexellis, @lukemarsden and @errordeveloper and also think this would be a really nice feature to merge. Basically let the providers add themselves to this dropdown list. If the provider is lazy enough to not do it; well bad for them, but they are probably still listed in the addons page.

Of course we'll ping all people involved to notify them they can add supported solutions to this list.
Lately there have been a lot of churn in the community about the complexity of finding which solutions work, and I totally agree. I still want to be very clear regarding that whatever satisfies the CNI+Kubernetes integration will work for kubeadm as well and that kubeadm won't choose either of these solutions.

But making these visible in the docs will actually help us target the users we want to target -- people that set up Kubernetes for the very first time; people that want to kick the tires with Kubernetes. Sure, kubeadm fits a lot of other people like experienced hackers of k8s as well, but that person can in theory build whatever system he/she wants anyway.

I'm wondering how this kind of straightforward change could stall out like this.
If we have to use tabs to be consistent, let's use tabs. If that isn't a great fit for this use-case, let's use a dropdown. I'm agnostic of that CSS-kind of stuff, but I really care about the user.

I'm currently updating the kubeadm docs to reflect the v1.7 release (#4018), and I might just add this kind of functionality in a second PR and try to push for this again

ping @thockin @devin-donnelly @chenopis @errordeveloper @lukemarsden

@chenopis
Copy link
Contributor

I think @errordeveloper just hasn't had time to get back to this PR yet. The implementation of tabs is good to go, and I used this PR to model the example: https://kubernetes.io/docs/tabs-example/

@lukemarsden
Copy link
Contributor

I think @luxas is going to take a look at this, instead of @errordeveloper now – thanks!

@luxas
Copy link
Member

luxas commented Jul 3, 2017 via email

@luxas
Copy link
Member

luxas commented Jul 5, 2017

Implemented by: #4274

@luxas luxas closed this Jul 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.

Projects

None yet

Development

Successfully merging this pull request may close these issues.