Add new driver "SSH" to bootstrap generic minkube clusters over ssh #10099

afbjorklund · 2021-01-06T17:25:30Z

New year, new attempt of the generic driver...

This driver connects to an already existing virtual machine,
using the provided IP address and SSH user/key/port parameters.

On this machine, libmachine will provision the docker daemon
and minikube will bootstrap the kubernetes cluster (as usual).

Rebased version of ~~#9545~~ ~~#6873~~ ~~#4734~~

Closes #4733 #9583

This driver connects to an already existing virtual machine, using the provided IP address and SSH user/key/port parameters. On this machine, libmachine will provision the docker daemon and minikube will bootstrap the kubernetes cluster (as usual).

It is not supported anyway, and just throws errors. There is no use to restart or to retry, just give up. This should never be a problem with "none", though. That always return running, while generic tests ssh.

All drivers *must* be in "supportedDrivers" Delete Podman on Darwin (Mac), not available

It was failing to install the "docker" package Since the provisioning was defaulting to ISO

Since we don't know the remote machine type anyway, it could be a virtual machine or a physical server. But we don't add any _extra_ isolation for minikube, so go with "bare metal machine" machine type for now.

It was only called for existing machines, not for new. But other commands expect to be able to run "docker".

k8s-ci-robot · 2021-01-06T17:25:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afbjorklund

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [afbjorklund]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

This makes the machine driver specific to minikube

The "none" driver has redefined stop and start to mean the kubelet rather than the host itself. Use the same API here. Killing actual containers remotely remains to be implemented. Only stop the kubelet service for now, and assuming systemd.

afbjorklund · 2021-01-06T19:16:28Z

@medyagh : I forked the "generic" driver and changed the start/stop implementation, so now it looks more like the "none" driver.

This also means that both drivers are now free to be renamed or aliased or whatever, since they are no longer using libmachine.

Not using the default libmachine drivers anymore

Setting the config directly instead in minikube

medyagh

mostly looks good just a couple knits.
btw do u mind pasting an example in the output I am curious how it looks for minikube start

pkg/drivers/generic/generic.go

cmd/minikube/cmd/start_flags.go

But most of it is identical to the "none" driver

tstromberg · 2021-01-08T19:57:39Z

What would you think about an alternate name for this, such as "ssh" or "remote"?

generic feels difficult to reason about.

afbjorklund · 2021-01-08T21:00:38Z

What would you think about an alternate name for this, such as "ssh" or "remote"?

generic feels difficult to reason about.

The fork (from libmachine) is now completed, so the drivers can be renamed.

We had originally planned to do it as a separate feature, though: #7772

The boring names were local and remote, but there were also some more creative suggestions

Like "bare metal" or "cloud" and also "native". But not stuck with none and generic anymore...

afbjorklund · 2021-01-08T21:05:23Z

Think we will leave the provisioner as-is, and leave that to the other feature: #9989

But right now it works differently for ubuntu and for any other distributions.

So we might want to add some specific KIC-check, to the "ubuntu" provisioner ?

func (p *UbuntuProvisioner) String() string {
        return "ubuntu"
}

// CompatibleWithHost checks if provisioner is compatible with host
func (p *UbuntuProvisioner) CompatibleWithHost() bool {
        return p.OsReleaseInfo.ID == "ubuntu"
}

Because it is hard-coded for KIC, so we don't really "detect" it anyway.

medyagh · 2021-01-08T21:58:45Z

What would you think about an alternate name for this, such as "ssh" or "remote"?

generic feels difficult to reason about.

I would vote for SSH name since everything in the flags points to SSH.
wdyt @afbjorklund to change the name to SSH driver?

afbjorklund · 2021-01-09T13:08:44Z

What would you think about an alternate name for this, such as "ssh" or "remote"?
generic feels difficult to reason about.

I would vote for SSH name since everything in the flags points to SSH.
wdyt @afbjorklund to change the name to SSH driver?

I thought all drivers were using ssh to talk to the machine, even docker ?
Well, maybe with the exception of the none driver that uses regular exec.

But we can go with ssh...

Will change the code, to see what we end up with. Like: IsBareMetal or IsSSH

Now that we have forked the machine driver

medyagh · 2021-01-13T01:34:40Z

@afbjorklund there seem to be a build error need to be fixed

afbjorklund · 2021-01-13T19:22:20Z

@medyagh : yeah, happened in 36d94a2

Basic integration error, from not doing CI

medyagh · 2021-01-13T21:22:08Z

/ok-to-test

cmd/minikube/cmd/start.go

cmd/minikube/cmd/start_flags.go

pkg/drivers/ssh/ssh.go

pkg/minikube/config/types.go

afbjorklund · 2021-01-16T13:48:30Z

Things got a bit hard to read, when the machine driver was renamed.

i.e sshIPAddress, sshSSHUser, sshSSHKey, sshSSHPort

Easy to confuse the name of the driver (ssh) with the protocol (ssh)

Normally the IP of the VM is the same for all uses, though

medyagh · 2021-01-16T21:53:04Z

/retest-this-please

afbjorklund added 11 commits January 6, 2021 18:03

Don't download ISO for the generic VM driver

c076669

Generic driver does not add user to docker group

7c5ba62

Try to uninstall kubernetes on delete, for generic

2b5c5b9

Don't try to start/stop drivers without VMs

6dad258

It is not supported anyway, and just throws errors. There is no use to restart or to retry, just give up. This should never be a problem with "none", though. That always return running, while generic tests ssh.

Allow actually using the generic driver

d0260cc

All drivers *must* be in "supportedDrivers" Delete Podman on Darwin (Mac), not available

Call DetectProvisioner for the generic driver

d3ea174

It was failing to install the "docker" package Since the provisioning was defaulting to ISO

Fix failing unit test for MachineType

c3a23cd

Since we don't know the remote machine type anyway, it could be a virtual machine or a physical server. But we don't add any _extra_ isolation for minikube, so go with "bare metal machine" machine type for now.

Log the os-release also for the generic driver

d96d3be

Need to set up docker group in start - not in fix

a0d2c1a

It was only called for existing machines, not for new. But other commands expect to be able to run "docker".

Add helper for checking the generic driver name

a3b3b55

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 6, 2021

k8s-ci-robot requested review from blueelvis and tstromberg January 6, 2021 17:25

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 6, 2021

afbjorklund added 2 commits January 6, 2021 18:35

Fork the generic driver to the minikube code base

fa05fcd

Use CommonDrivers and drop machine GetCreateFlags

a77e3de

This makes the machine driver specific to minikube

afbjorklund removed the request for review from blueelvis January 6, 2021 17:44

afbjorklund added 3 commits January 6, 2021 20:24

Move the constants for flag defaults from drivers

72a0fd6

Not using the default libmachine drivers anymore

Also remove unused machine SetConfigFromFlags

1d6c86c

Setting the config directly instead in minikube

OK to call Stop when implementation is changed

1d5ee9e

afbjorklund changed the title ~~WIP: Add minikube support for the "generic" VM driver~~ Add minikube support for the "generic" VM driver Jan 6, 2021

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 6, 2021

afbjorklund requested a review from medyagh January 6, 2021 20:17

medyagh requested changes Jan 7, 2021

View reviewed changes

pkg/drivers/generic/generic.go Outdated Show resolved Hide resolved

pkg/drivers/generic/generic.go Outdated Show resolved Hide resolved

cmd/minikube/cmd/start_flags.go Outdated Show resolved Hide resolved

cmd/minikube/cmd/start_flags.go Outdated Show resolved Hide resolved

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 8, 2021

Add placeholder for docs for the generic driver

fb6cf6b

But most of it is identical to the "none" driver

afbjorklund force-pushed the generic-2021 branch from 86281db to fb6cf6b Compare January 8, 2021 17:17

Avoid the hard-coded dependency on systemctl

40ec0e8

afbjorklund added 2 commits January 9, 2021 14:11

Merge branch 'master' into generic

e53f537

Rename the generic driver to the ssh driver

b2121ea

Now that we have forked the machine driver

afbjorklund changed the title ~~Add minikube support for the "generic" VM driver~~ Add minikube support for the "ssh" driver Jan 9, 2021

afbjorklund added 2 commits January 11, 2021 18:12

Only add docker group for the docker runtime

1663a03

Prevent trying to use localhost as address

529d2c3

afbjorklund added 2 commits January 13, 2021 20:15

Merge branch 'master' into generic

abb556f

The MachineName function was moved to config

099c6b7

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jan 13, 2021

medyagh changed the title ~~Add minikube support for the "ssh" driver~~ Add new driver support for the "ssh" driver Jan 13, 2021

medyagh changed the title ~~Add new driver support for the "ssh" driver~~ Add new driver "SSH" to bootstrap generic minkube clusters over ssh Jan 13, 2021

medyagh requested changes Jan 13, 2021

View reviewed changes

cmd/minikube/cmd/start.go Show resolved Hide resolved

medyagh self-requested a review January 15, 2021 22:29

medyagh requested changes Jan 15, 2021

View reviewed changes

cmd/minikube/cmd/start_flags.go Outdated Show resolved Hide resolved

pkg/drivers/ssh/ssh.go Outdated Show resolved Hide resolved

pkg/minikube/config/types.go Outdated Show resolved Hide resolved

afbjorklund added 2 commits January 16, 2021 14:34

Merge branch 'master' into generic

4611e2f

Address review comments, rename to ssh-ip-address

541193c

medyagh merged commit ce655bc into kubernetes:master Jan 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new driver "SSH" to bootstrap generic minkube clusters over ssh #10099

Add new driver "SSH" to bootstrap generic minkube clusters over ssh #10099

afbjorklund commented Jan 6, 2021 •

edited

Loading

k8s-ci-robot commented Jan 6, 2021

afbjorklund commented Jan 6, 2021

medyagh left a comment

tstromberg commented Jan 8, 2021

afbjorklund commented Jan 8, 2021 •

edited

Loading

afbjorklund commented Jan 8, 2021 •

edited

Loading

medyagh commented Jan 8, 2021

afbjorklund commented Jan 9, 2021 •

edited

Loading

medyagh commented Jan 13, 2021

afbjorklund commented Jan 13, 2021 •

edited

Loading

medyagh commented Jan 13, 2021

afbjorklund commented Jan 16, 2021

medyagh commented Jan 16, 2021

Add new driver "SSH" to bootstrap generic minkube clusters over ssh #10099

Add new driver "SSH" to bootstrap generic minkube clusters over ssh #10099

Conversation

afbjorklund commented Jan 6, 2021 • edited Loading

k8s-ci-robot commented Jan 6, 2021

afbjorklund commented Jan 6, 2021

medyagh left a comment

Choose a reason for hiding this comment

tstromberg commented Jan 8, 2021

afbjorklund commented Jan 8, 2021 • edited Loading

afbjorklund commented Jan 8, 2021 • edited Loading

medyagh commented Jan 8, 2021

afbjorklund commented Jan 9, 2021 • edited Loading

medyagh commented Jan 13, 2021

afbjorklund commented Jan 13, 2021 • edited Loading

medyagh commented Jan 13, 2021

afbjorklund commented Jan 16, 2021

medyagh commented Jan 16, 2021

afbjorklund commented Jan 6, 2021 •

edited

Loading

afbjorklund commented Jan 8, 2021 •

edited

Loading

afbjorklund commented Jan 8, 2021 •

edited

Loading

afbjorklund commented Jan 9, 2021 •

edited

Loading

afbjorklund commented Jan 13, 2021 •

edited

Loading