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.

Sign up for GitHub

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

Jump to bottom

Workaround race condition on default sa creation #247

Merged
merged 1 commit into from
Feb 14, 2019
Merged

Workaround race condition on default sa creation #247

merged 1 commit into from
Feb 14, 2019

Conversation

jorhett
Copy link
Contributor

@jorhett jorhett commented Feb 5, 2019

This is a workaround for a known race condition where you can try to add a Pod before the cluster has created the default serviceaccount kubernetes/kubernetes#66689

Notice: /Stage[main]/Kubernetes::Cluster_roles/Kubernetes::Kubeadm_init[kube-master]/Exec[kubeadm init]/returns: executed successfully
Notice: /Stage[main]/Kubernetes::Kube_addons/Exec[Install cni network provider]/returns: serviceaccount/weave-net created
Notice: /Stage[main]/Kubernetes::Kube_addons/Exec[Install cni network provider]/returns: clusterrole.rbac.authorization.k8s.io/weave-net created
Notice: /Stage[main]/Kubernetes::Kube_addons/Exec[Install cni network provider]/returns: clusterrolebinding.rbac.authorization.k8s.io/weave-net created
Notice: /Stage[main]/Kubernetes::Kube_addons/Exec[Install cni network provider]/returns: role.rbac.authorization.k8s.io/weave-net created
Notice: /Stage[main]/Kubernetes::Kube_addons/Exec[Install cni network provider]/returns: rolebinding.rbac.authorization.k8s.io/weave-net created
Notice: /Stage[main]/Kubernetes::Kube_addons/Exec[Install cni network provider]/returns: daemonset.extensions/weave-net created
Notice: /Stage[main]/Kubernetes::Kube_addons/Exec[Install cni network provider]/returns: executed successfully
Notice: /Stage[main]/Profile::Kubectl::Templates/Exec[kubectl_template_busybox]/returns: Error from server (Forbidden): error when creating "/k8s/templates/busybox.yaml": pods "busybox" is forbidden: error looking up service account default/default: serviceaccount "default" not found
Error: 'kubectl apply -f /k8s/templates/busybox.yaml' returned 1 instead of one of [0]
Error: /Stage[main]/Profile::Kubectl::Templates/Exec[kubectl_template_busybox]/returns: change from 'notrun' to ['0'] failed: 'kubectl apply -f /k8s/templates/busybox.yaml' returned 1 instead of one of [0]

This adds a test for the SA, which will trigger the accounting query to speed up its creation.

davejrt
davejrt reviewed Feb 7, 2019
View reviewed changes
manifests/kubeadm_init.pp Outdated Show resolved Hide resolved
@jorhett
Copy link
Contributor Author

jorhett commented Feb 14, 2019

I've restructured so it as a new defined type so it can be used with other namespaces.

In doing so I realized that the exec path was (A) marked optional everywhere and it's not, and (B) hardcoded in the cluster file. I've moved this out to a module parameter and adjusted the things which use it.

@jorhett jorhett force-pushed the wait_for_default_sa branch 4 times, most recently from 8d6635e to 1f42b36 Compare February 14, 2019 23:03
New defined type to wait for existence of default SA in new namespace
e7257a0 
Workaround race condition on default sa creation
  kubernetes/kubernetes#66689

Make exec path a module parameter which can be overridden
Remove path's Optional marking since no exec will work without it
davejrt
davejrt approved these changes Feb 14, 2019
View reviewed changes
manifests/kubeadm_init.pp Outdated
command => 'kubectl -n default get serviceaccount default -o name'
path => $path,
timeout => 50,
retries => 5,
Copy link
Contributor

Choose a reason for hiding this comment

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

and also 'tries'

@davejrt
Copy link
Contributor

davejrt commented Feb 14, 2019

LGTM, thanks for the PR.

@davejrt davejrt merged commit 8528320 into puppetlabs:master Feb 14, 2019
@jorhett jorhett deleted the wait_for_default_sa branch February 19, 2019 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davejrt davejrt davejrt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jorhett @davejrt