Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update minikube dev tooling #1906

Merged
merged 2 commits into from
Nov 30, 2020
Merged

Update minikube dev tooling #1906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cad662f
Update minikube dev tooling
markmandel Nov 20, 2020
28b1659
Merge branch 'master' into build/minikube
markmandel Nov 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 15 additions & 32 deletions build/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,6 @@ Table of Contents
### Linux
- Install Make, either via `apt install make` or `yum install make` depending on platform.
- [Install Docker](https://docs.docker.com/engine/installation/) for your Linux platform.
- (optional) Minikube will require [VirtualBox](https://www.virtualbox.org) and will need to be installed if you wish
to develop on Minikube

### Windows
Building and developing Agones requires you to use the
Expand All @@ -121,16 +119,12 @@ as this makes it easy to create a (relatively) cross platform development and bu
- Agones will need to be cloned somewhere on your `/c` (or drive of your choice) path, as that is what Docker will support mounts from
- All interaction with Agones must be on the `/c` (or drive of your choice) path, otherwise Docker mounts will not work
- Now the `make` commands can all be run from within your WSL shell
- (optional) Minikube is supported via the [HyperV](https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/index)
driver - the same virtualisation platform as the Docker installation.
- **Note**: If you want to dev and test with Minikube, you **must** run WSL as Administrator, otherwise Minikube can't control HyperV.
- The Minikube setup on Windows has not been tested. Pull Requests would be appreciated!

### macOS

- Install Make, `brew install make`, if it's not installed already
- Install [Docker for Mac](https://docs.docker.com/docker-for-mac/install/)
- (optional) Minikube will require [VirtualBox](https://www.virtualbox.org) and will need to be installed if you wish
to develop on Minikube

## GOPATH

Expand Down Expand Up @@ -251,13 +245,10 @@ When your are finished, you can run `make clean-gcloud-test-cluster` to tear dow
### Running a Test Minikube cluster
This will setup a [Minikube](https://github.com/kubernetes/minikube) cluster, running on an `agones` profile,

Because Minikube runs on a virtualisation layer on the host, some of the standard build and development Make targets
need to be replaced by Minikube specific targets.
Because Minikube runs on a virtualisation layer on the host (usually Docker), some of the standard build and development
Make targets need to be replaced by Minikube specific targets.

First, [install Minikube](https://github.com/kubernetes/minikube#installation), which may also require you to install
a virtualisation solution, such as [VirtualBox](https://www.virtualbox.org) as well.
Check the [Building on Different Platforms](#building-on-different-platforms) above for details on what virtualisation
solution to use.
First, [install Minikube](https://github.com/kubernetes/minikube#installation).

Next we will create the Agones Minikube cluster. Run `make minikube-test-cluster` to create the `agones` profile,
and a Kubernetes cluster of the supported version under this profile.
Expand Down Expand Up @@ -287,14 +278,8 @@ It's worth noting that Minikube does let you [reuse its Docker daemon](https://g
and build directly on Minikube, but in this case this approach is far simpler,
and makes cross-platform support for the build system much easier.

If you find you also want to push your own images into Minikube,
the convenience make target `make minikube-transfer-image` can be run with the `TAG` argument specifying
the tag of the Docker image you wish to transfer into Minikube.

For example:
```bash
$ make minikube-transfer-image TAG=myimage:0.1
```
To push your own images into the cluster, take a look at Minikube's
[Pushing Images](https://minikube.sigs.k8s.io/docs/handbook/pushing/) guide.

Running end-to-end tests on Minikube is done via the `make minikube-test-e2e` target. This target use the same `make test-e2e` but also setup some prerequisites for use with a Minikube cluster.

Expand Down Expand Up @@ -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
Copy link
Collaborator

@aLekSer aLekSer Nov 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/

of the right version.
of the right version. Uses "docker" as the default driver.

Use MINIKUBE_DRIVER variable to change the VM driver
(defaults virtualbox for Linux and macOS, hyperv for windows) if you so desire.

#### `make minikube-push`
Push the local Agones Docker images that have already been built
via `make build` or `make build-images` into the "agones" minikube instance.
If needed, use MINIKUBE_DRIVER variable to change the VM driver.

#### `make minikube-install`

Installs the current development version of Agones into the Kubernetes cluster.
Use this instead of `make install`, as it disables PullAlways on the install.yaml

#### `make minikube-push`

Push the local Agones Docker images that have already been built
via `make build` or `make build-images` into the "agones" minikube instance with `minikube cache add`

#### `make minikube-setup-prometheus`

Installs prometheus metric backend into the Kubernetes cluster.
Use this instead of `make setup-prometheus`, as it disables Persistent Volume Claim.

Expand All @@ -693,10 +680,6 @@ These tests validate Agones flow from start to finish.
Connecting to Minikube requires so enhanced permissions, so use this target
instead of `make shell` to start an interactive shell for development on Minikube.

#### `make minikube-transfer-image`
Convenience target for transferring images into minikube.
Use TAG to specify the image to transfer into minikube

#### `make minikube-controller-portforward`
The minikube version of [`make controller-portforward`](#make-controller-portforward) to setup
port forwarding to the controller deployment.
Expand Down
12 changes: 0 additions & 12 deletions build/includes/linux.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,6 @@ sha_dir = $(shell sha256sum $(1) | cut -d' ' -f1 | sha256sum | head -c 10 )

# Minikube executable
MINIKUBE ?= minikube
# Default minikube driver
MINIKUBE_DRIVER ?= virtualbox
# set docker env for minikube
MINIKUBE_DOCKER_ENV ?= eval $$($(MINIKUBE) docker-env)

# minikube shell mount for certificates
minikube_cert_mount := ~/.minikube:$(HOME)/.minikube
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


# _____ _
# |_ _|_ _ _ __ __ _ ___| |_ ___
Expand All @@ -46,11 +39,6 @@ minikube_cert_mount := ~/.minikube:$(HOME)/.minikube
# |_|\__,_|_| \__, |\___|\__|___/
# |___/

# Does not do anything
minikube-post-start:
# kubectl > 1.11 may have --address flag, but for the time being,
# we will use --network=host, as port-forward binds to localhost

# port forward the agones controller.
# useful for pprof and stats viewing, etc
controller-portforward: PORT ?= 8080
Expand Down
10 changes: 0 additions & 10 deletions build/includes/macos.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,6 @@ sha_dir = $(shell shasum -a 256 $(1) | cut -d' ' -f1 | shasum -a 256 | head -c 1

# Minikube executable
MINIKUBE ?= minikube
# Default minikube driver
MINIKUBE_DRIVER ?= virtualbox
# set docker env for minikube
MINIKUBE_DOCKER_ENV ?= eval $$($(MINIKUBE) docker-env)

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

# _____ _
# |_ _|_ _ _ __ __ _ ___| |_ ___
Expand All @@ -46,9 +39,6 @@ minikube_cert_mount := ~/.minikube:$(HOME)/.minikube
# |_|\__,_|_| \__, |\___|\__|___/
# |___/

# Does nothing
minikube-post-start:

# port forward the agones controller.
# useful for pprof and stats viewing, etc
controller-portforward: PORT ?= 8080
Expand Down
47 changes: 16 additions & 31 deletions build/includes/minikube.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
# See the License for the specific language governing permissions and
# limitations under the License.

MINIKUBE_DRIVER ?= docker

# minikube shell mount for certificates
minikube_cert_mount := ~/.minikube:$(HOME)/.minikube
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@markmandel markmandel Nov 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@markmandel markmandel Nov 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


# __ __ _ _ _ _
# | \/ (_)_ __ (_) | ___ _| |__ ___
Expand All @@ -22,58 +26,39 @@

# Switches to an "agones" profile, and starts a kubernetes cluster
# of the right version.
#
# Use MINIKUBE_DRIVER variable to change the VM driver
# (defaults virtualbox for Linux and macOS, hyperv for windows) if you so desire.
minikube-test-cluster: DOCKER_RUN_ARGS+=--network=host -v $(minikube_cert_mount)
minikube-test-cluster: $(ensure-build-image) minikube-agones-profile
$(MINIKUBE) start --kubernetes-version v1.16.13 --vm-driver $(MINIKUBE_DRIVER)
# wait until the master is up
until docker run --rm $(common_mounts) $(DOCKER_RUN_ARGS) $(build_tag) kubectl cluster-info; \
do \
echo "Waiting for cluster to start..."; \
sleep 1; \
done
$(MAKE) minikube-post-start

# switch to the agones cluster
minikube-agones-profile:
$(MINIKUBE) profile $(MINIKUBE_PROFILE)
minikube-test-cluster: $(ensure-build-image)
$(MINIKUBE) start --kubernetes-version v1.17.13 -p $(MINIKUBE_PROFILE) --vm-driver $(MINIKUBE_DRIVER)

# Connecting to minikube requires so enhanced permissions, so use this target
# instead of `make shell` to start an interactive shell for development on minikube.
minikube-shell: $(ensure-build-image) minikube-agones-profile
minikube-shell: $(ensure-build-image)
$(MAKE) shell DOCKER_RUN_ARGS="--network=host -v $(minikube_cert_mount) $(DOCKER_RUN_ARGS)"

# Push the local Agones Docker images that have already been built
# via `make build` or `make build-images` into the "agones" minikube instance.
minikube-push: minikube-agones-profile
$(MAKE) minikube-transfer-image TAG=$(sidecar_tag)
$(MAKE) minikube-transfer-image TAG=$(controller_tag)
$(MAKE) minikube-transfer-image TAG=$(ping_tag)
$(MAKE) minikube-transfer-image TAG=$(allocator_tag)
minikube-push:
$(MINIKUBE) cache add $(sidecar_tag)
$(MINIKUBE) cache add $(controller_tag)
$(MINIKUBE) cache add $(ping_tag)
$(MINIKUBE) cache add $(allocator_tag)

# Installs the current development version of Agones into the Kubernetes cluster.
# Use this instead of `make install`, as it disables PullAlways on the install.yaml
minikube-install: minikube-agones-profile
minikube-install:
$(MAKE) install DOCKER_RUN_ARGS="--network=host -v $(minikube_cert_mount)" ALWAYS_PULL_SIDECAR=false \
IMAGE_PULL_POLICY=IfNotPresent PING_SERVICE_TYPE=NodePort ALLOCATOR_SERVICE_TYPE=NodePort

minikube-uninstall: $(ensure-build-image) minikube-agones-profile
minikube-uninstall: $(ensure-build-image)
$(MAKE) uninstall DOCKER_RUN_ARGS="--network=host -v $(minikube_cert_mount)"

# Convenience target for transferring images into minikube.
# Use TAG to specify the image to transfer into minikube
minikube-transfer-image:
docker save $(TAG) | ($(MINIKUBE_DOCKER_ENV) && docker load)

# Runs e2e tests against our minikube
minikube-test-e2e: DOCKER_RUN_ARGS=--network=host -v $(minikube_cert_mount)
minikube-test-e2e: minikube-agones-profile test-e2e
minikube-test-e2e: test-e2e

# Runs stress tests against our minikube
minikube-stress-test-e2e: DOCKER_RUN_ARGS=--network=host -v $(minikube_cert_mount)
minikube-stress-test-e2e: minikube-agones-profile stress-test-e2e
minikube-stress-test-e2e: stress-test-e2e

# prometheus on minkube
# we have to disable PVC as it's not supported on minkube.
Expand Down
26 changes: 0 additions & 26 deletions build/includes/windows.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,6 @@ sha_dir = $(shell sha256sum $(1) | cut -d' ' -f1 | sha256sum | head -c 10 )

# Minikube executable
MINIKUBE ?= minikube.exe
# Default minikube driver
MINIKUBE_DRIVER ?= hyperv
# set docker env for minikube
MINIKUBE_DOCKER_ENV ?= eval $$($(MINIKUBE) docker-env --shell=bash) && \
export DOCKER_CERT_PATH=$$(echo $$DOCKER_CERT_PATH | $(win_to_wsl_path))

# minikube shell mount for certificates
minikube_cert_mount = $(cert_path):$(cert_path)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one should stay there

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


# transform the path from windows to WSL
win_to_wsl_path := sed -e 's|\([A-Z]\):|/\L\1|' -e 's|\\|/|g'

# find the cert path
cert_path = $(realpath $(shell $(MINIKUBE) docker-env --shell bash | grep DOCKER_CERT_PATH | awk -F "=" '{ print $$2 }' | sed 's/"//g' | $(win_to_wsl_path))/..)

# _____ _
# |_ _|_ _ _ __ __ _ ___| |_ ___
Expand All @@ -53,18 +39,6 @@ cert_path = $(realpath $(shell $(MINIKUBE) docker-env --shell bash | grep DOCKER
# |_|\__,_|_| \__, |\___|\__|___/
# |___/

# Sets minikube credentials
minikube-post-start:
echo "Creating minikube credentials"
export CERT_PATH=$(cert_path) && \
docker run --rm $(common_mounts) $(DOCKER_RUN_ARGS) $(build_tag) kubectl config set-cluster $(MINIKUBE_PROFILE) \
--certificate-authority=$$CERT_PATH/ca.crt --server=https://$$($(MINIKUBE) ip):8443 && \
docker run --rm $(common_mounts) $(DOCKER_RUN_ARGS) $(build_tag) kubectl config set-credentials $(MINIKUBE_PROFILE) \
--client-certificate=$$CERT_PATH/client.crt --client-key=$$CERT_PATH/client.key
docker run --rm $(common_mounts) $(DOCKER_RUN_ARGS) $(build_tag) kubectl config set-context $(MINIKUBE_PROFILE) \
--cluster=$(MINIKUBE_PROFILE) --user=$(MINIKUBE_PROFILE)
docker run --rm $(common_mounts) $(DOCKER_RUN_ARGS) $(build_tag) kubectl config use-context $(MINIKUBE_PROFILE)

# port forward the agones controller.
# useful for pprof and stats viewing, etc
controller-portforward: PORT ?= 8080
Expand Down