Skip to content
This repository has been archived by the owner on Feb 5, 2020. It is now read-only.

Update to Kubernetes 1.6 and Bootkube 0.4.0 #246

Merged
merged 11 commits into from
Apr 19, 2017
Merged

Update to Kubernetes 1.6 and Bootkube 0.4.0 #246

merged 11 commits into from
Apr 19, 2017

Conversation

Quentin-M
Copy link
Contributor

@Quentin-M Quentin-M commented Apr 17, 2017

Good evening,

This PR updates bootkube (0.4.0), its manifests (Kubernetes 1.6.1), introduces liveness probes to CM/Scheduler and add tolerations/anti-affinity for the critical components to run next to each other but avoid running multiple times on the same node.

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"5", GitVersion:"v1.5.2", GitCommit:"08e099554f3c31f6e6f07b448ab3ed78d0520507", GitTreeState:"clean", BuildDate:"2017-01-12T04:57:25Z", GoVersion:"go1.7.4", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"6", GitVersion:"v1.6.1+coreos.0", GitCommit:"9212f77ed8c169a0afa02e58dce87913c6387b3e", GitTreeState:"clean", BuildDate:"2017-04-04T00:32:53Z", GoVersion:"go1.7.5", Compiler:"gc", Platform:"linux/amd64"}

$ kubectl get nodes
NAME                                        STATUS    AGE
ip-10-0-10-234.us-west-2.compute.internal   Ready     9m
ip-10-0-19-89.us-west-2.compute.internal    Ready     9m
ip-10-0-42-19.us-west-2.compute.internal    Ready     9m
ip-10-0-54-106.us-west-2.compute.internal   Ready     9m
ip-10-0-74-57.us-west-2.compute.internal    Ready     9m

$ kubectl get pods -n=kube-system
NAME                                                               READY     STATUS    RESTARTS   AGE
heapster-961268227-wh7mh                                           2/2       Running   0          52s
kube-apiserver-984hh                                               1/1       Running   0          9m
kube-apiserver-g909c                                               1/1       Running   0          9m
kube-apiserver-r83s1                                               1/1       Running   6          9m
kube-controller-manager-1744054037-274tf                           1/1       Running   0          9m
kube-controller-manager-1744054037-g3fc2                           1/1       Running   0          9m
kube-dns-2891576090-tqf6h                                          3/3       Running   0          9m
kube-flannel-29tqb                                                 2/2       Running   2          9m
kube-flannel-5z3hm                                                 2/2       Running   3          9m
kube-flannel-k6bqk                                                 2/2       Running   3          9m
kube-flannel-qmfpq                                                 2/2       Running   3          9m
kube-flannel-tqp2w                                                 2/2       Running   2          9m
kube-proxy-0v670                                                   1/1       Running   0          7m
kube-proxy-3clvk                                                   1/1       Running   0          7m
kube-proxy-3pxlb                                                   1/1       Running   0          7m
kube-proxy-7sv92                                                   1/1       Running   0          7m
kube-proxy-n56n9                                                   1/1       Running   0          7m
kube-scheduler-2496325411-ct3jf                                    1/1       Running   0          9m
kube-scheduler-2496325411-rrw2z                                    1/1       Running   0          9m
pod-checkpointer-hmdw4                                             1/1       Running   0          3m
pod-checkpointer-hmdw4-ip-10-0-10-234.us-west-2.compute.internal   1/1       Running   0          3m
pod-checkpointer-jjcx2                                             1/1       Running   0          3m
pod-checkpointer-jjcx2-ip-10-0-42-19.us-west-2.compute.internal    1/1       Running   0          3m
pod-checkpointer-jwzq5                                             1/1       Running   0          3m
pod-checkpointer-jwzq5-ip-10-0-19-89.us-west-2.compute.internal    1/1       Running   0          3m

config.tf Outdated
kubednsmasq = "gcr.io/google_containers/kube-dnsmasq-amd64:1.4.1"
dnsmasq_metrics = "gcr.io/google_containers/dnsmasq-metrics-amd64:1.0.1"
exechealthz = "gcr.io/google_containers/exechealthz-amd64:1.2"
kubedns = "gcr.io/google_containers/k8s-dns-kube-dns-amd64:1.14.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@Quentin-M Quentin-M Apr 17, 2017

Choose a reason for hiding this comment

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

Thanks :) Addressed.

@alekssaul
Copy link
Contributor

Error Loading Jobs.

extensions/v1beta1.Job was in process of being deprecated in favor of batch/v1 [0] . I had https://github.com/coreos-inc/tectonic-issues/issues/151 open regarding this.

[0]https://kubernetes.io/docs/concepts/jobs/run-to-completion-finite-workloads/#extensionsv1beta1job-is-deprecated

@Quentin-M Quentin-M mentioned this pull request Apr 17, 2017
@philips
Copy link
Contributor

philips commented Apr 17, 2017

cc @squat @sym3tri

@Quentin-M
Copy link
Contributor Author

Quentin-M commented Apr 17, 2017

Note: also need to take a look at kubernetes-retired/bootkube#434.

@squat
Copy link
Contributor

squat commented Apr 17, 2017

@Quentin-M and also kubernetes-retired/bootkube#421 <- this includes anti-affinity for critical control plane components

@Quentin-M
Copy link
Contributor Author

@squat This one should be implemented already. I used git blame on templates.go - so anything that is already merged should be there.

@@ -43,7 +49,7 @@ spec:
- --service-account-key-file=/etc/kubernetes/secrets/service-account.pub
- --client-ca-file=/etc/kubernetes/secrets/ca.crt
- --authorization-mode=RBAC
- --runtime-config=rbac.authorization.k8s.io/v1alpha1
- --runtime-config=rbac.authorization.k8s.io/v1beta1
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be unnecessary since all beta API groups are enabled by default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Smart! I didn't know that. Let me remove that line then.

- --insecure-port=8080
- --kubelet-client-certificate=/etc/kubernetes/secrets/apiserver.crt
- --kubelet-client-key=/etc/kubernetes/secrets/apiserver.key
- --runtime-config=api/all=true
Copy link
Contributor

Choose a reason for hiding this comment

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

We should remove this since we are not enabling any alpha API groups and do not want to unknowingly depend on them.

Copy link
Contributor Author

@Quentin-M Quentin-M Apr 17, 2017

Choose a reason for hiding this comment

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

I agree. I simply copy-pasted whatever is in bootkube here without modifications but for templating purposes. Generally want to stay as close as their manifests as possible. Do you think we should still drop 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.

@aaronlevy Is there a specific reason this is here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I think so. We had a story that explicitly asks to disable allowing all alpha features by default. If a user really wants to use alpha API features then they can always modify this daemonset.

https://github.com/coreos-inc/tectonic/blob/master/docs-internal/alpha-features.md
https://www.pivotaltracker.com/story/show/137556789

Copy link
Contributor

Choose a reason for hiding this comment

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

Only exists for flexibility - but I am 100% for removing this.

Copy link
Contributor

Choose a reason for hiding this comment

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

We may need to explicitly allow TPRs though -- that is still technically an alpha feature

Copy link
Contributor

Choose a reason for hiding this comment

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

@aaronlevy TPR is v1beta1

Copy link
Contributor

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.

Addressed.

@Quentin-M
Copy link
Contributor Author

/cc @amrutac. Pinged you with credentials to a Kubernetes 1.6 deployment to witness the Console breakages. Tell me if there is anything I can do to help you!

- /var/lock/api-server.lock
- /hyperkube
- apiserver
- --admission-control=NamespaceLifecycle,ServiceAccount
Copy link
Contributor

Choose a reason for hiding this comment

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

We should update this to the upstream recommended list: kubernetes-retired/bootkube#438

Copy link
Contributor

Choose a reason for hiding this comment

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

Added issue to track this generally: #249

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed.

@s-urbaniak
Copy link
Contributor

Setting the "do not merge" label as long as we don't have a working console.

@alexsomesan
Copy link
Contributor

@sym3tri Looks like this is causing some misbehaviour in the console? I presume we don't want to integrate it before the console has addressed those, right?

@amrutac @squat Could you please ping back here when the related console changes are in?

@Quentin-M
Copy link
Contributor Author

Quentin-M commented Apr 18, 2017

- key: "CriticalAddonsOnly"
operator: "Exists"
- key: "node-role.kubernetes.io/master"
operator: Equal
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just use operator: "Exists" here?

Copy link
Contributor Author

@Quentin-M Quentin-M Apr 18, 2017

Choose a reason for hiding this comment

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

- key: "CriticalAddonsOnly"
operator: "Exists"
- key: "node-role.kubernetes.io/master"
operator: Equal
Copy link
Contributor

Choose a reason for hiding this comment

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

operator: Exists

Copy link
Contributor Author

Choose a reason for hiding this comment

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

- key: "CriticalAddonsOnly"
operator: "Exists"
- key: "node-role.kubernetes.io/master"
operator: Equal
Copy link
Contributor

Choose a reason for hiding this comment

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

operator: Exists

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hostNetwork: true
tolerations:
- key: "node-role.kubernetes.io/master"
operator: Equal
Copy link
Contributor

Choose a reason for hiding this comment

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

operator: Exists

Copy link
Contributor Author

Choose a reason for hiding this comment

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

- key: "CriticalAddonsOnly"
operator: "Exists"
- key: "node-role.kubernetes.io/master"
operator: Equal
Copy link
Contributor

Choose a reason for hiding this comment

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

operator: Exists

Copy link
Contributor Author

Choose a reason for hiding this comment

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

- key: "CriticalAddonsOnly"
operator: "Exists"
- key: "node-role.kubernetes.io/master"
operator: Equal
Copy link
Contributor

Choose a reason for hiding this comment

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

operator: Exists

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Quentin-M
Copy link
Contributor Author

Quentin-M commented Apr 18, 2017

Have to wait for flannel-io/flannel#681.

@aaronlevy
Copy link
Contributor

I opened kubernetes-retired/bootkube#451 to track changing to operator: Exists in bootkube too

@s-urbaniak
Copy link
Contributor

Tentatively setting this as do-not-merge, as long as it is in WIP state.

@Quentin-M
Copy link
Contributor Author

Just bumped flannel and fixed a bunch of other stuff. Tested quite a bit by a few people on AWS - other platforms are left untested. Can we still merge and then have @squat test/follow-up later as necessary please? We need all platforms and everybody to start using it actively. We can't afford waiting until release day.

Copy link
Contributor

@alexsomesan alexsomesan left a comment

Choose a reason for hiding this comment

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

Did a build of this branch myself.
Works nicely, didn't notice any anomalies on a quick glance.
Let's get this in and keep testing around it.

@alexsomesan alexsomesan merged commit 3ac7f75 into coreos:master Apr 19, 2017
@Quentin-M Quentin-M deleted the bootkube_v040 branch April 19, 2017 21:27
@Quentin-M
Copy link
Contributor Author

Quentin-M commented Apr 19, 2017

THANK YOU! 💯

@s-urbaniak
Copy link
Contributor

Also tested this myself and it is looking great, thanks!!! I also quickly tested this on openstack/neutron and openstack/nova which worked fine with a minor fix on the nova-side, that I'll file as a separate PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants