Update minikube dev tooling #1906

markmandel · 2020-11-20T00:13:17Z

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking
/kind bug

/kind cleanup

/kind documentation
/kind feature
/kind hotfix

What this PR does / Why we need it:

Needed to update minikube to Kubernetes 1.17.x and I figured I would also go through the minikube dev experience and update it.

This includes:

Switch to default to the Docker driver, since everyone should have Docker installed.
Removing the Windows hacks, because they were awful and I feel bad I even wrote them in the first place.
Migrate tooling to use new minikube functionality
Update minikube commands to up to date release conformity.
Updated the documentation

Which issue(s) this PR fixes:

Work on #1824

Special notes for your reviewer:

N/A

Needed to update minikube to Kubernetes 1.17.x and I figured I would also go through the minikube dev experience and update it. This includes: * Switch to default to the Docker driver, since everyone should have Docker installed. * Removing the Windows hacks, because they were awful and I feel bad I even wrote them in the first place. * Migrate tooling to use new minikube functionality * Update minikube commands to up to date release conformity. * Updated the documentation Work on googleforgames#1824

agones-bot · 2020-11-20T00:35:32Z

Build Succeeded 👏

Build Id: b30af332-2458-48cb-b732-a454237dd365

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.11.0-cad662f
image: gcr.io/agones-images/agones-sdk:1.11.0-cad662f
image: gcr.io/agones-images/agones-ping:1.11.0-cad662f
Linux C++ SDK (build): agonessdk-1.11.0-cad662f-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.11.0-cad662f.zip

A preview of the website (the last 30 builds are retained):

https://cad662f-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1906/head:pr_1906 && git checkout pr_1906
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.11.0-cad662f

aLekSer · 2020-11-20T13:16:50Z

build/README.md

@@ -653,20 +638,22 @@ Since Minikube runs locally, there are some targets that need to be used instead

 #### `make minikube-test-cluster`
 Switches to an "agones" profile, and starts a kubernetes cluster


I see that additional command which changes profile was removed. Was it redundant?

This is now powered by the -p argument in minikube start - which also creates the profile. minikube profile foo will not create the profile if it doesn't exist.

https://minikube.sigs.k8s.io/docs/commands/start/

aLekSer · 2020-11-20T13:18:03Z

build/includes/windows.mk

- export DOCKER_CERT_PATH=$$(echo $$DOCKER_CERT_PATH | $(win_to_wsl_path))
-
-# minikube shell mount for certificates
-minikube_cert_mount = $(cert_path):$(cert_path)


I think this one should stay there

So the previous Windows support was super hacky, brittle and I expect it doesn't work anymore.

We were rewriting windows path to linux paths in make files. If we decide to continue supporting it, it needs a whole new rewrite. So I've removed it for now. If we need to add new os specific tooling, then lets revisit then. Right now, between OSX and Linux, it's all the same.

aLekSer · 2020-11-20T13:18:47Z

build/includes/linux.mk

-MINIKUBE_DOCKER_ENV ?= eval $$($(MINIKUBE) docker-env)
-
-# minikube shell mount for certificates
-minikube_cert_mount := ~/.minikube:$(HOME)/.minikube


This one also might be needed and be different for different OSs. WDYT?

For OSX and Linux it's the same.

It may be the same for minikube under WSL, but it requires testing. But I figure we can revisit if we decide it's possible to support windows without hackery.

aLekSer · 2020-11-20T13:19:37Z

build/includes/minikube.mk

+MINIKUBE_DRIVER ?= docker
+
+# minikube shell mount for certificates
+minikube_cert_mount := ~/.minikube:$(HOME)/.minikube


It seems that Windows would not have this folder. Am I missing something? I can test on my other PC.

This is why I changed the README to:
https://github.com/googleforgames/agones/pull/1906/files#diff-4b71ec736aee936feb417e5193e800a1014a8e2df2a5b5b92fd78fbd3d781dbdR122

The Minikube setup on Windows has not been tested. Pull Requests would be appreciated!

I expect it will all fall apart. Last time I looked at this (many years ago now), kubectl on WSL wouldn't work with minikube because the minikube config referenced certs hosted at C:\ which doesn't work for Linux.

I expect the entire Windows dev environment setup would need a review anyway. (do we even have anyone developing on windows?)

Got an idea. That would be a leftover from this PR.

Since we have the git history to go back and look at if we need it, I'm not convinced it's necessary to leave this in, given that we don't actually know what the correct path is for windows+minikube dev (or if we even want to support it).

This feels to me like leaving in commented out code in the current source repo.

+1 to not leaving stuff in that is "dead code" in the hope that we will fix it later. It's better to clean it up and we can always look at repo history if necessary to pull it back in.

In which case, I will merge this, and we can revisit Windows at a later date 👍

aLekSer

Left small comments regarding minikube_cert_mount which might need to be OS-specific.

roberthbailey

LGTM. Please address @aLekSer's comments before merging.

google-oss-robot · 2020-11-20T22:45:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: markmandel, roberthbailey

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~~ [markmandel,roberthbailey]

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

markmandel · 2020-11-24T18:41:31Z

@roberthbailey do you want to tiebreak on this PR?

google-oss-robot · 2020-11-30T18:52:00Z

New changes are detected. LGTM label has been removed.

agones-bot · 2020-11-30T19:11:51Z

Build Succeeded 👏

Build Id: 2106ebf5-b282-41fc-b5c4-46559c717234

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.11.0-28b1659
image: gcr.io/agones-images/agones-sdk:1.11.0-28b1659
image: gcr.io/agones-images/agones-ping:1.11.0-28b1659
Linux C++ SDK (build): agonessdk-1.11.0-28b1659-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.11.0-28b1659.zip

A preview of the website (the last 30 builds are retained):

https://28b1659-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1906/head:pr_1906 && git checkout pr_1906
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.11.0-28b1659

markmandel added kind/cleanup Refactoring code, fixing up documentation, etc area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. labels Nov 20, 2020

markmandel requested review from aLekSer, roberthbailey and pooneh-m November 20, 2020 00:13

google-oss-robot requested a review from dzlier-gcp November 20, 2020 00:13

google-oss-robot added the approved label Nov 20, 2020

google-cla bot added the cla: yes label Nov 20, 2020

google-oss-robot added the size/L label Nov 20, 2020

aLekSer reviewed Nov 20, 2020

View reviewed changes

roberthbailey approved these changes Nov 20, 2020

View reviewed changes

google-oss-robot assigned roberthbailey Nov 20, 2020

google-oss-robot added the lgtm label Nov 20, 2020

Merge branch 'master' into build/minikube

28b1659

google-oss-robot removed the lgtm label Nov 30, 2020

markmandel merged commit 40e6e50 into googleforgames:master Nov 30, 2020

markmandel deleted the build/minikube branch November 30, 2020 19:30

markmandel added this to the 1.11.0 milestone Dec 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update minikube dev tooling #1906

Update minikube dev tooling #1906

markmandel commented Nov 20, 2020

agones-bot commented Nov 20, 2020

aLekSer Nov 20, 2020 •

edited

Loading

markmandel Nov 20, 2020

aLekSer Nov 20, 2020

markmandel Nov 20, 2020

aLekSer Nov 20, 2020

markmandel Nov 20, 2020

aLekSer Nov 20, 2020

markmandel Nov 20, 2020 •

edited

Loading

aLekSer Nov 21, 2020

markmandel Nov 23, 2020 •

edited

Loading

roberthbailey Nov 30, 2020

markmandel Nov 30, 2020

aLekSer left a comment

roberthbailey left a comment

google-oss-robot commented Nov 20, 2020

markmandel commented Nov 24, 2020

google-oss-robot commented Nov 30, 2020

agones-bot commented Nov 30, 2020

		@@ -653,20 +638,22 @@ Since Minikube runs locally, there are some targets that need to be used instead

		#### `make minikube-test-cluster`
		Switches to an "agones" profile, and starts a kubernetes cluster

Update minikube dev tooling #1906

Update minikube dev tooling #1906

Conversation

markmandel commented Nov 20, 2020

agones-bot commented Nov 20, 2020

aLekSer Nov 20, 2020 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markmandel Nov 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markmandel Nov 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aLekSer left a comment

Choose a reason for hiding this comment

roberthbailey left a comment

Choose a reason for hiding this comment

google-oss-robot commented Nov 20, 2020

markmandel commented Nov 24, 2020

google-oss-robot commented Nov 30, 2020

agones-bot commented Nov 30, 2020

aLekSer Nov 20, 2020 •

edited

Loading

markmandel Nov 20, 2020 •

edited

Loading

markmandel Nov 23, 2020 •

edited

Loading