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

Use protobuf instead of rest to connect to apiserver host and add troubleshooting doc #143

Merged
merged 1 commit into from
Jan 21, 2017

Conversation

aledbf
Copy link
Member

@aledbf aledbf commented Jan 19, 2017

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

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 19, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@aledbf aledbf changed the title Use protobuf instead of rest to connect to apiserver host WIP: Use protobuf instead of rest to connect to apiserver host Jan 19, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 42.338% when pulling 00862cfab6cc485f5bdc3d034ed53ebc2b85587b on aledbf:improve-errors into ac5b84c on kubernetes:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 42.276% when pulling 157ea2232a7d0758d4e84350b617302e6b0e8290 on aledbf:improve-errors into ac5b84c on kubernetes:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 43.036% when pulling 4e03205 on aledbf:improve-errors into 7553ab3 on kubernetes:master.

@aledbf aledbf changed the title WIP: Use protobuf instead of rest to connect to apiserver host Use protobuf instead of rest to connect to apiserver host and add troubleshooting doc Jan 19, 2017
Copy link
Member

@justinsb justinsb left a 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.
Copy link
Member

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" ?

Copy link
Member Author

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
Copy link
Member

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
Copy link
Member

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?

Copy link
Member Author

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"
Copy link
Member

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!)

Copy link
Member Author

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)

Copy link
Member

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!

Copy link
Member Author

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

Copy link
Member

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 :-)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 42.983% when pulling e5b02b6 on aledbf:improve-errors into 30a6229 on kubernetes:master.

@justinsb
Copy link
Member

Thanks for tweaks - LGTM!

@aledbf aledbf merged commit 728c1c9 into kubernetes:master Jan 21, 2017
@aledbf aledbf deleted the improve-errors branch February 2, 2017 11:14
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. nginx
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve how errors connecting to api-server are handled
6 participants