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

fix kube-dns CrashLoopBackOff issue with RBAC enabled minikube #1107

Merged
merged 1 commit into from
Oct 24, 2017

Conversation

dangula
Copy link
Member

@dangula dangula commented Oct 19, 2017

[skip ci] fix #925

@galexrt
Copy link
Member

galexrt commented Oct 19, 2017

@dangula Have you tried updating minikube to v0.22.3? After the v0.22.x update I didn't had this error anymore.

Edit: Just saw the comments on the issue #925, I'm not having this with the latest minikube.

@dangula
Copy link
Member Author

dangula commented Oct 19, 2017

@galexrt I am using minikube v0.22.3.(with image minikube-v0.23.5.iso) And my kube-dns pod keeps restaring all the time. The kube-dns restart issue hasn't caused any issues but i thought i would be nice to fix it.

@galexrt
Copy link
Member

galexrt commented Oct 19, 2017

@dangula I just noticed it happens again for me. Kube-dns working fine for me, was caused because I was using a minikube in which I already fixed the RBAC by hand earlier.

Yes, it is definitely good to fix this.

Copy link
Member

@galexrt galexrt left a comment

Choose a reason for hiding this comment

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

Just two minor requests.
Tested the manifests and kube-dns works after them.

# workaround for kube-dns CrashLoopBackOff issue with RBAC enabled
#issue https://github.com/kubernetes/minikube/issues/1734 and https://github.com/kubernetes/minikube/issues/1722
enable_roles_for_RBAC() {
cat <<EOF | kubectl create -f -
Copy link
Member

Choose a reason for hiding this comment

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

Please add an indent here.

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

verbs: ["*"]
- nonResourceURLs: ["*"]
verbs: ["*"]

Copy link
Member

Choose a reason for hiding this comment

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

I would remove those empty lines in between the objects.

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

Copy link
Member

@galexrt galexrt left a comment

Choose a reason for hiding this comment

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

One last thing and it looks good to me.

# workaround for kube-dns CrashLoopBackOff issue with RBAC enabled
#issue https://github.com/kubernetes/minikube/issues/1734 and https://github.com/kubernetes/minikube/issues/1722
enable_roles_for_RBAC() {
cat <<EOF | kubectl create -f -
Copy link
Member

Choose a reason for hiding this comment

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

The indent from the other lines seems to be 4 spaces. Please change it here to look like the other lines.

Copy link
Member Author

Choose a reason for hiding this comment

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

oops fixed now.

Copy link
Member

@galexrt galexrt left a comment

Choose a reason for hiding this comment

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

LGTM 👍

name: cluster-writer
apiGroup: rbac.authorization.k8s.io
---
# Setup sd-build as a reader. This has to be a
Copy link
Member

Choose a reason for hiding this comment

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

why do we need these sd-* accounts, roles and bindings? in the original issue kubernetes/minikube#1734, it looks like the sample fix came from the Screwdriver project: http://blog.screwdriver.cd/post/161863341372/set-up-screwdriver-in-kubernetes, which is where I think this sd-* stuff is coming from.

We aren't using screwdriver, so we shouldn't add those RBAC configs to our scripts, right? I would think that only cluster-writer and cluster-reader are necessary. Can you please try this scenario after removing all the sd-* stuff that I suspect isn't necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks for catching that,i just coped the roles from the last comment in the issue. Cleaned them up now.

Copy link
Member

Choose a reason for hiding this comment

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

@dangula can you confirm that kube-dns worked OK with RBAC enabled once you removed those sd-* permissions?

Copy link
Member Author

Choose a reason for hiding this comment

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

@jbw976 it worked, i ran full regession and all tests passed.

Copy link
Member

Choose a reason for hiding this comment

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

ah, it looks like the cluster-reader role isn't used anymore now. it was only used by the sd-build account. let's get rid of the cluster-reader role too then I think this change will be complete. thanks @dangula

@jbw976 jbw976 merged commit d6c72f6 into rook:master Oct 24, 2017
@dangula dangula deleted the pr_fix_minikubeRBAC branch October 25, 2017 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

minikube should use RBAC
3 participants