kubernetes-coreos-units icon indicating copy to clipboard operation
kubernetes-coreos-units copied to clipboard

Slight overhaul

Open devurandom opened this issue 10 years ago • 13 comments

This is a slight overhaul over the existing service files.

  • The first commit uses internal CoreOS mechanisms instead of an additional setup-network-environment unit.
  • The 2nd commit uses a binary (that is expected to be dropped via cloud-config) to download the k8s binaries.
  • The last commit is a bit larger and adds several command arguments that are needed for k8s v0.20 and/or are present in hack/local-up-cluster.sh. But even more importantly, it makes almost every IP / port setting configurable (via a /etc/kube-config.env file that is supposed to be dropped by cloud-config).

devurandom avatar Jul 01 '15 17:07 devurandom

Thanks for this- let me spend some time going through and most likely I'll merge it in.

mhamrah avatar Jul 02 '15 00:07 mhamrah

I redid the commits to make the review easier. Please use git reset --hard in case you already checked out the branch!

I also undid the configurable k8s version. Splitting the Kubernetes setup (fleet) from the network setup (cloud-config) seems a more important goal. Thus I will redo this using another method that does not involve cloud-config.

devurandom avatar Jul 02 '15 10:07 devurandom

That's great, thanks. I started looking at this last night. I like the new approach of moving as much as possible out of cloud-config. I'm not happy with the original curling of binaries in ExecStartPre; I noticed some issues with my setup (that's why I had to add the rm as well). As an alternative to the original approach, and also what you had, what about deploying a separate "oneshot" unit file that downloads the entire zip package to /opt/bin and setting that as requirement for other units? That's what I was thinking in terms of downloading.

On Thu, Jul 2, 2015 at 6:21 AM Dennis Schridde [email protected] wrote:

I redid the commits to make the review easier. Please use git reset --hard in case you already checked out the branch!

I also undid the configurable k8s version. Splitting the Kubernetes setup (fleet) from the network setup (cloud-config) seems a more important goal. Thus I will redo this using another method that does not involve cloud-config.

— Reply to this email directly or view it on GitHub https://github.com/mhamrah/kubernetes-coreos-units/pull/2#issuecomment-117991901 .

mhamrah avatar Jul 02 '15 11:07 mhamrah

This looks good overall; I'm not ready to merge in just yet for three reasons:

  1. Clarification on the usage of the key generation, and what that means for multiple-master slaves
  2. Removal of the update binary script; I think the oneshot approach is best.
  3. I'd like an updated readme so new users have clear guidance on what they need to do. For instance, there is now a need to source /etc/kube-config.env with settings like KUBE_MASTER_PORT, etc.

Make sense?

mhamrah avatar Jul 02 '15 12:07 mhamrah

Regarding the oneshot appreach to binary downloads: I was trying to use unit templating to deal with versioning in an easy and obvious way that also properly deals with dependencies (i.e. [email protected] and so on). There are currently some issues (somehow dependencies don't appear to work for instanced units; cause unknown), but I think that approach is even superior to the oneshot download unit, as it allows to immediately see which node is running which version of Kubernetes, and to automatically (disregarding abovementioned issues) enforce matching versions on all nodes.

devurandom avatar Jul 02 '15 13:07 devurandom

I wonder if you could do something like this with apiserver_0.19.3.service:

/bin/bash -c \"KUBE_VERSION=`/bin/echo %p | cut -d_ -f 2`; #download cube with $KUBE_VERSION"

mhamrah avatar Jul 02 '15 13:07 mhamrah

Current status:

  • [ ] Figure out how serviceaccount keys work and how they interact with multi-master setups.
  • [ ] Think about replacing curl-update-binary.
  • [x] Update README.md with clear guidance on the new configuration options.
  • [x] Use comma-separated KUBE_MASTER_URLS to support multi-master setups.

Anything missing?

devurandom avatar Jul 02 '15 15:07 devurandom

That's great!

On Thu, Jul 2, 2015 at 11:03 AM Dennis Schridde [email protected] wrote:

Current status:

  • Figure out how serviceaccount keys work and how they interact with multi-master setups.
  • Think about replacing curl-update-binary.
  • Update README.md with clear guidance on the new configuration options.
  • Use comma-separated KUBE_MASTER_URLS to support multi-master setups.

Anything missing?

— Reply to this email directly or view it on GitHub https://github.com/mhamrah/kubernetes-coreos-units/pull/2#issuecomment-118060926 .

mhamrah avatar Jul 02 '15 15:07 mhamrah

I believe the previously mentioned issues with instanced units might be caused by the problem that coreos/fleet#1273 fixed. After removing the broken units and restarting fleetd, some machines still refuse to run globally scheduled units. While I am rebooting the cluster to get rid of any stale state and confused fleetd instances, I tried my '[email protected]` unit on a node that still worked. Let me know what you think about it!

devurandom avatar Jul 02 '15 16:07 devurandom

I wanted to keep the binaries out of /opt/bin to prevent version conflicts, and intended to modify the other kube-*.services to run the binaries from the versioned path. However, systemd does not expand %i in the executable filename. Thus I just committed a revision of [email protected] that syncs the binaries to /opt/bin.

devurandom avatar Jul 02 '15 17:07 devurandom

Please have a look at both variants in devurandom/kubernetes-coreos-units@a76815422f36458e859741e73738ab3008a5f3ad ([email protected]) and devurandom/kubernetes-coreos-units@d55a3c0347d1b13512abf5327cf5083119cd58eb (kube-*@.service).

devurandom avatar Jul 02 '15 17:07 devurandom

Apologies for the delay in looking at this- got sidetracked with a bunch of stuff. DO you think there's a use case in running multiple versions, or different versions, of kubernetes on a cluster? I probably think the version-in-unit-file is the best route, as it puts the version up front.

I'm going to play around with your fork and hopefully merge in this weekend.

mhamrah avatar Jul 17 '15 19:07 mhamrah

As the MachineMetadata key is the same for all unit files, it does not make much sense to run multiple versions from this template on the same cluster.

Buuuut: The Version number as a template parameter achieves two things:

  1. It makes it easy to update just by resubmitting the unit. No editing of the files. No overlooking to edit a file.
  2. It makes it immediately obvious which version of Kubernetes is running.

Please refrain from merging for now, as I still have not solved following question:

  • [ ] Figure out how serviceaccount keys work and how they interact with multi-master setups. (cf. GoogleCloudPlatform/kubernetes#11000)

Also, I still use curl-update-binary. It still seems to be very convenient. The version presented here contains a bug, though. I will submit a fix later (probably when I solved the question about authentication/authorisation).

devurandom avatar Jul 17 '15 23:07 devurandom