-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Use protobuf instead of rest to connect to apiserver host and add troubleshooting doc #143
Conversation
Coverage decreased (-0.2%) to 42.338% when pulling 00862cfab6cc485f5bdc3d034ed53ebc2b85587b on aledbf:improve-errors into ac5b84c on kubernetes:master. |
00862cf
to
157ea22
Compare
Coverage decreased (-0.3%) to 42.276% when pulling 157ea2232a7d0758d4e84350b617302e6b0e8290 on aledbf:improve-errors into ac5b84c on kubernetes:master. |
157ea22
to
4e03205
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but a few docs suggestions and a question on making protobuf optional
1. _Service Account:_ This is recommended, because nothing has to be configured. The Ingress controller will use information provided by the system to communicate with the API server. See 'Service Account' section for details. | ||
|
||
2. _Kubeconfig file:_ In some Kubernetes environments service accounts are not available. In this case a manual configuration is required. The Ingress controller binary can be started with the `--kubeconfig` flag. The value of the flag is a path to a file specifying how to connect to the API server. | ||
The contents of the file is identical to `~/.kube/config` which is used by kubectl to connect to the API server. See 'kubeconfig' section for details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe "The format of the file" rather than "The content of the file" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
$ TOKEN_VALUE=$(kubectl exec test-701078429-s5kca -- cat /var/run/secrets/kubernetes.io/serviceaccount/token) | ||
$ echo $TOKEN_VALUE | ||
eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3Mi....9A | ||
$ kubectl exec test-701078429-s5kca -- curl --cacert /var/run/secrets/kubernetes.io/serviceaccount/ca.crt -H "Authorization: Bearer $TOKEN_VALUE" https://10.0.0.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, but FYI I think they might be making this open-access :-(
- /nginx-ingress-controller | ||
- --default-backend-service=$(POD_NAMESPACE)/nginx-default-backend | ||
- --nginx-configmap=$(POD_NAMESPACE)/ingress-nginx | ||
- --kubeconfig=/etc/kubernetes/kubeconfig.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These last few lines are the only change right? It's difficult to see the change here. Maybe pull out the changed few lines first, and then give the full example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point
|
||
cfg.QPS = defaultQPS | ||
cfg.Burst = defaultBurst | ||
cfg.ContentType = "application/vnd.kubernetes.protobuf" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do all k8s versions support protobuf? Do we want to make this optional? (I don't know the answer to either suggestion!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commit that contains the change appeared in k8s 1.3
(I am not sure if I can say is ok to use it by default)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know either! I guess/hope any errors would be pretty self-descriptive!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@justinsb just in case this code is from kubernetes/dashboard
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah - then we're good I think - ignore me :-)
4e03205
to
e5b02b6
Compare
Thanks for tweaks - LGTM! |
I hope this will help new users with the most common issue related to service accounts and bad certificates that are used when the communication with api server is stablished.
This PR also adds a troubleshooting document that appears in the log
(idea/code from kubernetes/dashboard)
fixes #129