up: copy configs to remote host #19383

mfojtik · 2018-04-17T10:46:24Z

WARNING: Crazy ahead.

This is will enable the cluster up case where the Docker is running on a remote host (a DOCKER_HOST is used), which is typically a case on Windows and OSX (if not DockerForMac is used).

The way this works is that we are copying the generated configs to remote host via privileged Docker container.

Current cluster up code assumes that every config live on the host, this PR will make it less obvious and you will have to be careful which path you want to use. I spend a lot of time and a lot of cluster up runs to tweak all places where we generate the local files and need to copy them into host. This might even less obvious for the components and we will have to be careful on adding components to not forget copy the files they generate to remote...

I tested this code on OSX where I run the centos7 VM and exposing the Docker from that VM via DOCKER_HOST.

/cc @smarterclayton @praveenkumar

@deads2k I don't like this, but I can't think of a better way to do this :-(

Fixes: #19360
Fixes: #19154

mfojtik · 2018-04-17T10:48:03Z

pkg/oc/bootstrap/docker/run_self_hosted.go

@@ -133,6 +134,7 @@ func (c *ClusterUpConfig) StartSelfHosted(out io.Writer) error {
 		return err
 	}

+	clientConfig.Host = c.ServerIP + ":8443"


@deads2k not sure if this is needed, but without this I was not able to get the --public-hostname to work. In order to bind the cluster up on the VM, I had to specify the VM IP address...

@deads2k also I think I saw somebody on IRC reporting that --public-hostname is broken

mfojtik · 2018-04-17T10:48:47Z

pkg/oc/bootstrap/docker/run_self_hosted.go

@@ -282,7 +334,7 @@ func (c *ClusterUpConfig) makeMasterConfig() (string, error) {
 	container.MasterImage = c.openshiftImage()
 	container.Args = []string{
 		"--write-config=/var/lib/origin/openshift.local.config",
-		"--master=127.0.0.1",
+		fmt.Sprintf("--master=%s", c.ServerIP),


@deads2k not sure if this fixes the comment above.

@deads2k not sure if this fixes the comment above.

You had to change this because you needed to serve from a different nic

mfojtik · 2018-04-17T10:53:30Z

/hold

Need to also handle volumes, right now the directory is created into /Users/mfojtik/... in the host. It should all live in /var/lib/origin

mfojtik · 2018-04-17T11:12:03Z

pkg/oc/bootstrap/docker/run_self_hosted.go

+		if err != nil {
+			return nil, err
+		}
+		configs.openshiftControllerConfigDir, err = c.copyToRemote(configs.openshiftControllerConfigDir, kubeapiserver.OpenShiftControllerManagerDirName)


@deads2k the reason I have to do this after every step is that some steps depend on files produced by previous step...

mfojtik · 2018-04-17T11:56:49Z

problem with volume and data dir solved.

/hold cancel

deads2k · 2018-04-17T12:00:12Z

pkg/oc/bootstrap/clusteradd/cmd.go

@@ -111,7 +111,9 @@ func NewCmdAdd(name, fullName string, out, errout io.Writer) *cobra.Command {
 // Start runs the start tasks ensuring that they are executed in sequence
 func (c *ClusterAddConfig) Run() error {
 	componentsToInstall := []componentinstall.Component{}
-	installContext, err := componentinstall.NewComponentInstallContext(c.openshiftImage(), c.imageFormat(), c.BaseDir, c.ServerLogLevel)
+	dockerHelper := dockerhelper.NewHelper(c.dockerClient)


I really don't want this in the context. Can't we continue passing it through the install step?

deads2k · 2018-04-17T12:01:49Z

pkg/oc/bootstrap/docker/host/host.go

@@ -17,24 +20,25 @@ nsenter --mount=/rootfs/proc/1/ns/mnt mkdir -p %[1]s
 grep -F %[1]s /rootfs/proc/1/mountinfo || nsenter --mount=/rootfs/proc/1/ns/mnt mount -o bind %[1]s %[1]s
 grep -F %[1]s /rootfs/proc/1/mountinfo | grep shared || nsenter --mount=/rootfs/proc/1/ns/mnt mount --make-shared %[1]s
 `
+
+	// RemoteHostOriginDir is a directory on the remote machine that runs Docker
+	RemoteHostOriginDir = "/var/lib/origin"


This shouldn't be necessary. Can't you just make the filesystem match the user filesystem. This was the source of a lot of ugly before and this is going to cause conflicts when you try to run with two different configs in two different CWDs.

per IRC, I reworked this to be RemoteHostOriginDir + baseDir

(I really don't want to mess up with SELinux or something similar when mirroring the OSX filesystem tree)

deads2k · 2018-04-17T12:05:34Z

pkg/oc/bootstrap/clusteradd/components/registry/registry_install.go

@@ -48,7 +50,13 @@ func (r *RegistryComponentOptions) Install(dockerClient dockerhelper.Interface,
 		return err
 	}

-	masterConfigDir := path.Join(r.InstallContext.BaseDir(), kubeapiserver.KubeAPIServerDirName)
+	// If docker is on remote host, the base dir is different.
+	baseDir := r.InstallContext.BaseDir()


Do you need the isntallcontext to list a different basedir perhaps?

the baseDir should be the same, if you are talking to remote host, you just need to prefix

deads2k · 2018-04-17T12:07:46Z

pkg/oc/bootstrap/clusteradd/components/router/router_install.go

+	// the Docker host master config dir.
+	if len(os.Getenv("DOCKER_HOST")) > 0 {
+		hostHelper := host.NewHostHelper(c.InstallContext.DockerHelper(), c.InstallContext.ClientImage())
+		remoteMasterConfigDir := path.Join(host.RemoteHostOriginDir, kubeapiserver.KubeAPIServerDirName)


you should not be copying into this destination. The add on components don't get to mess with the running kube/openshift control plane. Copy to the remote host if you must, but this should be a different directory off the basedir. Perhaps I didn't notice when I was fixing the rest.

this was pre-existing (we generated the cert in the master config dir)... I can move this somewhere else.

deads2k · 2018-04-17T12:17:31Z

pkg/oc/bootstrap/docker/run_self_hosted.go

-	if _, err := os.Stat(configLocations.openshiftAPIServerConfigDir); os.IsNotExist(err) {
-		_, err = c.makeOpenShiftAPIServerConfig(configLocations.masterConfigDir)
+	if c.isRemoteDocker {
+		configs.masterConfigDir, err = c.copyToRemote(configs.masterConfigDir, kubeapiserver.KubeAPIServerDirName)


so this value gets overwritten to be something else?

yes, it will become the remote host dir

deads2k · 2018-04-17T12:18:30Z

pkg/oc/bootstrap/docker/run_self_hosted.go

-		_, err = c.makeOpenShiftControllerConfig(configLocations.masterConfigDir)
+
+	if _, err := os.Stat(configs.nodeConfigDir); os.IsNotExist(err) {
+		_, err = c.makeNodeConfig(configs.masterConfigDir)


Why does this one get the remote master config dir?

because it ran in container on remote host? (so it needs the remote masterConfigDir to get admin.kubeconfig (if I remember this one)

mfojtik · 2018-04-17T12:24:01Z

@deads2k added support for multiple CWD (will use baseDir as part of remote host path), removed the docker helper from context.

mfojtik · 2018-04-17T12:50:22Z

/retest

(centos mirrors are down, again)

deads2k · 2018-04-17T17:59:39Z

Well, it's nasty. All installers are ugly. If you want it, I can deal with it, but hopefully we don't have to touch it much.

/lgtm
/retest

openshift-ci-robot · 2018-04-17T17:59:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, mfojtik

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:

~~pkg/oc/bootstrap/OWNERS~~ [deads2k,mfojtik]

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

openshift-bot · 2018-04-17T22:38:44Z

/retest

Please review the full test history for this PR and help us cut down flakes.

smarterclayton · 2018-04-18T20:15:22Z

Worked for me, thanks!

…

On Tue, Apr 17, 2018 at 6:46 AM, OpenShift CI Robot < ***@***.***> wrote: [APPROVALNOTIFIER] This PR is *APPROVED* This pull-request has been approved by: *mfojtik <#19383#>* The full list of commands accepted by this bot can be found here <https://go.k8s.io/bot-commands>. The pull request process is described here <https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process> Needs approval from an approver in each of these files: - pkg/oc/bootstrap/OWNERS <https://github.com/openshift/origin/blob/master/pkg/oc/bootstrap/OWNERS> [mfojtik] Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#19383 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p3w2coWQG2H5BJFrK9H7Jr7A1aZWks5tpcgPgaJpZM4TYEJE> .

mfojtik requested review from smarterclayton, csrwng and deads2k April 17, 2018 10:46

openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 17, 2018

mfojtik commented Apr 17, 2018

View reviewed changes

mfojtik changed the title ~~up: copy configs into remote host~~ up: copy configs to remote host Apr 17, 2018

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 17, 2018

mfojtik commented Apr 17, 2018

View reviewed changes

mfojtik force-pushed the up-40-volumes branch 2 times, most recently from 14f13b6 to 67d0c86 Compare April 17, 2018 11:54

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 17, 2018

deads2k reviewed Apr 17, 2018

View reviewed changes

up: copy configs into remote host

0252f6e

mfojtik force-pushed the up-40-volumes branch from 67d0c86 to 0252f6e Compare April 17, 2018 12:20

openshift-ci-robot assigned deads2k Apr 17, 2018

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 17, 2018

openshift-merge-robot merged commit bb34ae4 into openshift:master Apr 18, 2018

mfojtik deleted the up-40-volumes branch September 5, 2018 21:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

up: copy configs to remote host #19383

up: copy configs to remote host #19383

mfojtik commented Apr 17, 2018 •

edited

Loading

mfojtik Apr 17, 2018

mfojtik Apr 17, 2018

mfojtik Apr 17, 2018

deads2k Apr 17, 2018

mfojtik commented Apr 17, 2018

mfojtik Apr 17, 2018

mfojtik commented Apr 17, 2018

deads2k Apr 17, 2018

mfojtik Apr 17, 2018

deads2k Apr 17, 2018

mfojtik Apr 17, 2018

mfojtik Apr 17, 2018

deads2k Apr 17, 2018

mfojtik Apr 17, 2018

deads2k Apr 17, 2018

mfojtik Apr 17, 2018

deads2k Apr 17, 2018

mfojtik Apr 17, 2018

deads2k Apr 17, 2018

mfojtik Apr 17, 2018

mfojtik commented Apr 17, 2018

mfojtik commented Apr 17, 2018

deads2k commented Apr 17, 2018

openshift-ci-robot commented Apr 17, 2018

openshift-bot commented Apr 17, 2018

smarterclayton commented Apr 18, 2018 via email

up: copy configs to remote host #19383

up: copy configs to remote host #19383

Conversation

mfojtik commented Apr 17, 2018 • 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

mfojtik commented Apr 17, 2018

Choose a reason for hiding this comment

mfojtik commented Apr 17, 2018

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

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

mfojtik commented Apr 17, 2018

mfojtik commented Apr 17, 2018

deads2k commented Apr 17, 2018

openshift-ci-robot commented Apr 17, 2018

openshift-bot commented Apr 17, 2018

smarterclayton commented Apr 18, 2018 via email

mfojtik commented Apr 17, 2018 •

edited

Loading