Initial (experimental) CoreOS support #1813

aledbf · 2017-02-07T16:39:15Z

replaces #1480

Detect CoreOS
Move key manifests to code, to tolerate read-only mounts
Run nodeup as a oneshot systemd service, rather than relying on
cloud-init behaviour which varies across distros

Edit: tested with --bastion using weave and calico

Edit 2: this change requires nodeup with flag --install

Edit 3: also tested with kopeio-vxlan

This change is

chrislovecnm · 2017-02-08T00:34:22Z

Should we use a feature flag for this? You can look at how I am using on in my rolling-update PR https://github.com/kubernetes/kops/pull/1134/files#diff-7095872b64962f5aefa022d844e0dc40R283

aledbf · 2017-02-08T12:23:16Z

@chrislovecnm what do you mean? Add a feature flag to install in CoreOS?

aledbf · 2017-02-09T19:50:39Z

TODO:

change default --volume-plugin-dir=/usr/libexec/kubernetes/kubelet-plugins/volume/exec/" to /opt/kubernetes/kubelet-plugins/volume/exec/
(/usr/libexec is read-only)

chrislovecnm · 2017-02-09T22:31:35Z

@aledbf this is experimental, and should only be used in my mind if the user deliberately turns on the feature flag.

Question for you: how do rolling updates work of not work with CoreOS.

philk · 2017-02-10T00:33:12Z

Testing this out (seems to be working, thanks!) and noticed something.

Without using kubelet-wrapper or providing a socat binary this will still run into the socat issue which breaks helm

chrislovecnm

Questions for u

chrislovecnm · 2017-02-10T01:23:48Z

nodeup/pkg/model/docker.go

@@ -227,6 +227,11 @@ func (d *dockerVersion) matches(arch Architecture, dockerVersion string, distro
 }

 func (b *DockerBuilder) Build(c *fi.ModelBuilderContext) error {
+	if b.Distribution == distros.DistributionCoreOS {
+		glog.Infof("Detected CoreOS; won't install Docker")


So are we running rkt or docker?

It's preinstalled though (we can't really install complicated things on CoreOS)

chrislovecnm · 2017-02-10T01:24:21Z

nodeup/pkg/model/etcd.go

+		return nil
+	}
+
+	// TODO: Do we actually use the user anywhere?


How do we figure this out?

It's unused; we remove and verify nothing breaks. This PR takes the first step on that.

chrislovecnm · 2017-02-10T01:25:49Z

nodeup/pkg/model/kubeapiserver.go

+	{
+		pod, err := b.buildPod()
+		if err != nil {
+			return fmt.Errorf("error building kube-apiserver pod: %v", err)


error building kube-apiserver pod

Kinda sound like we failed to deploy a pod, instead of failing to build the struct that we use to build the api server yaml that is used by kubelet. Thoughts on rewording?

chrislovecnm · 2017-02-10T01:26:47Z

nodeup/pkg/model/kubeapiserver.go

+		return nil, fmt.Errorf("error building kube-apiserver flags: %v", err)
+	}
+
+	redirectCommand := []string{


Can you add a todo for setting this up to get logs from kubectl logs we have another issue in to allow for that.

To be clear, can you add a comment in the code to fix this to send logs to docker so that kubectl logs works with these pods.

~~The redirect to file was removed~~

We already have #1813 ; I think that is separate from this PR

aledbf · 2017-02-10T12:46:40Z

@philk the "problem" with kubelet-wrapper is that we need to use hyperkube and that means kops for coreos ends being different.

philk · 2017-02-10T18:36:11Z

@aledbf I don't disagree that it sucks to ship kubelet two different ways but shipping a (slightly) broken kubelet isn't great either. Do you know the progress on using kubelet in a container for all installs? (or has that idea been abandoned?)

aledbf · 2017-02-10T18:39:15Z

Do you know the progress on using kubelet in a container for all installs? (or has that idea been abandoned?)

ping @justinsb @chrislovecnm

* Detect CoreOS * Move key manifests to code, to tolerate read-only mounts * Misc refactorings so more code can be shared * Change lots of ints to int32s in the models * Run nodeup as a oneshot systemd service, rather than relying on cloud-init behaviour which varies across distros

justinsb · 2017-02-11T20:22:52Z

I opened #1861 to track the problems of #NoSocat, as it feels separable from this PR.

kubelet in a container has not made any progress AFAIK; my understanding is that is has caused problems in the past (metrics?) and so it isn't the default.

I don't think it's a huge deal to do, but I'm also not sure that socat should be the deciding factor...

justinsb

This LGTM, but we need a second LGTM as I authored some of it. @kris-nova ?

justinsb · 2017-02-11T20:25:33Z

nodeup/pkg/model/docker.go

@@ -227,6 +227,11 @@ func (d *dockerVersion) matches(arch Architecture, dockerVersion string, distro
 }

 func (b *DockerBuilder) Build(c *fi.ModelBuilderContext) error {
+	if b.Distribution == distros.DistributionCoreOS {
+		glog.Infof("Detected CoreOS; won't install Docker")


It's preinstalled though (we can't really install complicated things on CoreOS)

krisnova · 2017-02-12T00:19:17Z

cmd/nodeup/main.go

@@ -50,6 +52,9 @@ func main() {
 	target := "direct"
 	flag.StringVar(&target, "target", target, "Target - direct, cloudinit")

+	install := false
+	flag.BoolVar(&install, "install", install, "If true, will install a systemd unit instead of running directly")


Is the --install flag too ambiguous here? At a glance I didn't really understand what this was doing. Should we consider a more verbose name?

krisnova · 2017-02-12T00:20:18Z

cmd/nodeup/main.go

+				}
+				if i == 0 {
+					// We could also try to evaluate based on cwd
+					s, err = os.Readlink("/proc/self/exe")


Should we

Check that file exists

Check that it is in fact a symlink

Before just bailing out here?

krisnova · 2017-02-12T00:21:07Z

cmd/nodeup/main.go

+				Command:         command,
+				FSRoot:          flagRootFS,
+			}
+			err = i.Run()


Doesn't look like we do anything if we get an Error back from i.Run() can we at least log it please?

Actually, we logged and retried, using the same logic as before.

krisnova · 2017-02-12T00:22:20Z

docs/images.md

+
+## CoreOS
+
+CoreOS support is highly experimental.  Please report any issues.


If this is super experimental should we have something in the software that suggests/warns about that?

Ideas

log message that yells and says "HEY THIS IS EXPERIMENTAL" (Not literally)

whatever flags we use to trigger the behavior could have the word experimental in them a la VENDOREXPERIMENT

We don't have an easy way to know the image OS in kops, so I'm not sure how practical this is.

krisnova · 2017-02-12T00:24:14Z

nodeup/pkg/model/kubectl.go

+	return nil
+}
+
+func (b *KubectlBuilder) kubectlPath() string {


Wondering if we should borrow some functionality from the which command here?

where I can find the referenced command?

I am referring to the which command commonly found on *nix flavored file systems. I think it ships in coreutils

I think it can be done in Go, but I guess the real point I am getting at is:

Would it be helpful to look up the kubectl executable at runtime instead of assuming where it is based on the OS?

~~done. Using https://golang.org/pkg/os/exec/#LookPath~~

We actually don't want to rely on the PATH here. We want to run our version of kubectl,

But in this case, this is the path where we're installing kubectl anyway, so we can't find it.

Why can't you just use hyperkube on all platforms for kubectl?

Interesting. What is the advantage of doing so @chancez?

More containers less binaries is always a good thing IMO. Plus, you'll already be using hyperkube on each host, so it isn't additional overhead either. It'll also make updating the binary easier, and will handle verification.

krisnova

Can we please advertise the "experimental"ness of this PR a bit more before merging? See notes on including language in the logs, or in the flag itself.

LGTM as far as code is concerned. Nice PR!

justinsb · 2017-02-13T14:18:34Z

I'm not really sure where we would advertise it, because kops doesn't know the OS when it is installing. I guess we could check the image owner and warn if it matches the well known coreos.com value, but we don't do this for the other images also...

aledbf · 2017-02-13T15:54:19Z

@justinsb @kris-nova anything else to add/change in this PR?

aledbf · 2017-02-14T14:20:53Z

@justinsb ping

justinsb · 2017-02-14T16:08:51Z

Looks great - let's get some miles on it :-)

ahasnaini · 2017-02-23T14:58:37Z

Is there any info available on how to deploy a k8s cluster on coreOS, currently it deploy on debian, which flas am I missing ?

aledbf · 2017-02-23T15:04:38Z

@ahasnaini you need to change the image flag to something like --image 595879546273/CoreOS-stable-1235.9.0-hvm

ahasnaini · 2017-02-23T19:14:56Z

thanks that worked, I used the following command
kops create cluster --name=xxx.xxx.xxxx --state=s3://kops1213 --zones=eu-west-1a --node-count=1 --node-size=t2.micro --master-size=t2.micro --dns-zone=xxx.xxx.xx

EC2 instances got started but dnszone didn't get updated, without the image it did. when i logged on the master instance nothing was running under docker. do I need any more flags ?

vmrm · 2019-05-24T21:36:26Z

Hello, can't find any info - does kops support CoreOS images in Google Cloud Platform? Tried to deploy one of ig of existing cluster - VMs are started, but it seems that they aren't provisioned with kubelet and etc.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 7, 2017

chrislovecnm suggested changes Feb 10, 2017

View reviewed changes

aledbf mentioned this pull request Feb 10, 2017

Map kubelet flag volume-plugin-dir #1843

Merged

aledbf force-pushed the coreos branch from ca11eac to 7368e28 Compare February 10, 2017 12:35

aledbf force-pushed the coreos branch 2 times, most recently from db3e6e1 to 477ce0b Compare February 10, 2017 16:49

justinsb modified the milestone: 1.5.2 Feb 11, 2017

justinsb and others added 3 commits February 11, 2017 13:57

Fix build

da26306

Address comments

6715bd5

aledbf force-pushed the coreos branch 3 times, most recently from 19ccaea to 6715bd5 Compare February 11, 2017 20:03

justinsb mentioned this pull request Feb 11, 2017

CoreOS is #NoSocat #1861

Closed

justinsb approved these changes Feb 11, 2017

View reviewed changes

krisnova reviewed Feb 12, 2017

View reviewed changes

krisnova approved these changes Feb 12, 2017

View reviewed changes

aledbf force-pushed the coreos branch 5 times, most recently from 494a064 to 6715bd5 Compare February 12, 2017 14:58

Address comments

1619766

aledbf force-pushed the coreos branch from aa22340 to 1619766 Compare February 13, 2017 14:21

justinsb merged commit 1c78188 into kubernetes:master Feb 14, 2017

justinsb mentioned this pull request Feb 16, 2017

WIP: Initial (experimental) CoreOS support #1480

Closed

aledbf mentioned this pull request Feb 23, 2017

Add support for CoreOS #274

Closed


		## CoreOS

		CoreOS support is highly experimental. Please report any issues.

Initial (experimental) CoreOS support #1813

Initial (experimental) CoreOS support #1813

Conversation

aledbf commented Feb 7, 2017 • edited Loading

chrislovecnm commented Feb 8, 2017

aledbf commented Feb 8, 2017

aledbf commented Feb 9, 2017

chrislovecnm commented Feb 9, 2017

philk commented Feb 10, 2017 • edited Loading

chrislovecnm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aledbf Feb 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aledbf commented Feb 10, 2017

philk commented Feb 10, 2017

aledbf commented Feb 10, 2017

justinsb commented Feb 11, 2017

justinsb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krisnova Feb 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Would it be helpful to look up the kubectl executable at runtime instead of assuming where it is based on the OS?

aledbf Feb 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krisnova left a comment

Choose a reason for hiding this comment

justinsb commented Feb 13, 2017

aledbf commented Feb 13, 2017

aledbf commented Feb 14, 2017

justinsb commented Feb 14, 2017

ahasnaini commented Feb 23, 2017

aledbf commented Feb 23, 2017

ahasnaini commented Feb 23, 2017

vmrm commented May 24, 2019

aledbf commented Feb 7, 2017 •

edited

Loading

philk commented Feb 10, 2017 •

edited

Loading

aledbf Feb 10, 2017 •

edited

Loading

krisnova Feb 12, 2017 •

edited

Loading

Would it be helpful to look up the `kubectl` executable at runtime instead of assuming where it is based on the OS?

aledbf Feb 12, 2017 •

edited

Loading