Update minikube docs post rename #1332

bobcatfish · 2018-06-23T17:46:53Z

The docs for setting up minikube were using the namespaces and
resource names from elafros instead of knative. The naming changed
slightly, e.g. a knative controller is now called controller
instead of knative-serving-controller, so one of the loops had
to be broken into 2 statements.

Added steps about redeploying pods after setting up GCR
secrets b/c there is a chicken and egg problem where the namespaces
must exist before you can setup the secrets, but the secrets must
exist before the images can be pulled.

The PR that enabled MutatingAdmissionWebhook by default
(kubernetes/minikube#2547) was merged, but
the latest minikube (0.28.0) still did not enable this option
by default b/c providing any arugments overrides all of the defaults,
so we must still set it explicitly.

Made it clear in the setting up knative serving docs that the cluster
admin binding is required, not just for istio.

Proposed Changes

Update minikube and GCR for minikube docs

NONE

bobcatfish · 2018-06-24T17:32:15Z

DEVELOPMENT.md

 kubectl apply -f ./third_party/istio-0.8.0/istio.yaml
 ```

-Then label namespaces with `istio-injection=enabled`:
+Optionally label namespaces with `istio-injection=enabled`:


@tcnghia this IS optional right? I don't enable injection for any of my non-default namespaces, and it's not enabled for the e2e namespaces in the automated tests, and things still work, but I'm not sure if that's intentional

trisberg · 2018-06-25T14:29:19Z

I saw some issues with the image pull secrets following the sequence in the docs. worked better when first creating the knative-serving namespace and the service account using:

ko apply -f config/100-namespace.yaml
ko apply -f config/200-serviceaccount.yaml

and then adding the imagePullSecret to the "controller" service account before doing the full knative-serving deploy.

bobcatfish · 2018-06-25T17:26:18Z

worked better when first creating the knative-serving namespace and the service account using:

kk, I'll update the docs, thanks @trisberg !

bobcatfish · 2018-06-25T18:47:11Z

docs/creating-a-kubernetes-cluster.md

+        1. Deploy the rest of Knative:
+           ```bash
+           ko apply -f config/
+           ```


@trisberg The instructions are a bit rough, let me know if you can think of a clearer way of explaining this, or if I've missed anything!

(I also found this a more reliable sequence of actions than recreating the controllers, etc., after deploying!)

it's hard because these steps depend on the GCR instructions below. I think it is fine the way it you have it now

bobcatfish · 2018-06-25T19:05:05Z

/test pull-knative-serving-go-coverage

trisberg · 2018-06-25T19:39:02Z

docs/creating-a-kubernetes-cluster.md

+           ko apply -f config/
+           ```
+
+    1. [Enable log and metric collection](../DEVELOPMENT.md#enable-log-and-metric-collection)


we should mention that 8GB might not be enough for the elasticsearch components - I've found that 12GB works better

trisberg · 2018-06-25T20:02:39Z

DEVELOPMENT.md


-### Deploy Istio
+1. [Make your user a cluster admin](#setup-cluster-admin)


one more thing, don't think you need this for minikube, I always run without this step

great, removed!

... okay it wasn't removed but NOW it will be :D

actually - since this is in DEVELOPMENT.md it covers both Minikube and GKE, so we should leave it in and maybe add a comment that it can be skipped for Minikube

lol thanks for catching this, just re-fixed it XD

trisberg · 2018-06-26T20:43:01Z

docs/creating-a-kubernetes-cluster.md

+    cluster, e.g. [GCR](#minikube-with-gcr), you will need to deploy Knative
+    itself a bit differently:
+
+    1. [Deploy istio](../DEVELOPMENT.md#deploy-istio)


We should add: For Minikube replace LoadBalancer with NodePort using:

sed 's/LoadBalancer/NodePort/' third_party/istio-0.8.0/istio.yaml | \ kubectl apply -f -

i will add that! that might actually solve most of #609 I have a feeling - I want to follow up and see if we can make this process a bit more automatic for the minikube case as well.

trisberg · 2018-06-26T21:04:16Z

docs/creating-a-kubernetes-cluster.md

-    for prefix in ela build; do
-      kubectl create secret docker-registry "gcr" \
+    export [email protected]
+    for prefix in knative-serving build; do


the knative-serving namespace no longer has a system suffix, I suspect that this will need to be adjusted again if the build namespace changes

oh yeah i missed that, thanks @trisberg !!

evankanderson · 2018-06-27T17:10:42Z

/approve

I'm going to let @trisberg do the /lgtm, assuming that I understand how Prow works.

google-prow-robot · 2018-06-27T17:10:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bobcatfish, evankanderson

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~~ [evankanderson]

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

evankanderson · 2018-06-27T17:20:31Z

/assign @trisberg

bobcatfish · 2018-06-27T21:11:38Z

docs/creating-a-kubernetes-cluster.md

+
+       (Then optionally [enable istio injection](../DEVELOPMENT.md#deploy-istio).)
+
+    1. [Deploy build](../DEVELOPMENT.md#deploy-build)


i think at this point i should duplicate this line in this doc, since the rest of the steps are explicit

The docs for setting up minikube were using the namespaces and resource names from elafros instead of knative. The naming changed slightly, e.g. a knative controller is now called `controller` instead of `knative-serving-controller`, so one of the loops had to be broken into 2 statements. Added steps about redeploying pods after setting up GCR secrets b/c there is a chicken and egg problem where the namespaces must exist before you can setup the secrets, but the secrets must exist before the images can be pulled. The PR that enabled `MutatingAdmissionWebhook` by default (kubernetes/minikube#2547) was merged, but the latest minikube (0.28.0) still did not enable this option by default b/c providing any arugments overrides all of the defaults, so we must still set it explicitly. Made it clear in the setting up knative serving docs that the cluster admin binding is required, not just for istio. Use a `NodePort` instead of a `LoadBalancer` (see kubernetes/minikube#384) - another step along the road to #608.

trisberg · 2018-06-27T22:02:27Z

/lgtm

bobcatfish requested review from trisberg, tcnghia and mattmoor June 23, 2018 17:46

google-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 23, 2018

bobcatfish force-pushed the minikube_doc_update branch 3 times, most recently from 1dc6984 to c9fcbc0 Compare June 23, 2018 22:45

google-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 23, 2018

bobcatfish commented Jun 24, 2018

View reviewed changes

bobcatfish force-pushed the minikube_doc_update branch 3 times, most recently from ee7e5f1 to 01805ed Compare June 25, 2018 18:45

bobcatfish commented Jun 25, 2018

View reviewed changes

trisberg reviewed Jun 25, 2018

View reviewed changes

bobcatfish force-pushed the minikube_doc_update branch from 01805ed to f366308 Compare June 25, 2018 19:56

trisberg reviewed Jun 25, 2018

View reviewed changes

bobcatfish force-pushed the minikube_doc_update branch from f366308 to 2036c47 Compare June 25, 2018 20:38

trisberg reviewed Jun 26, 2018

View reviewed changes

google-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 27, 2018

google-prow-robot assigned trisberg Jun 27, 2018

bobcatfish mentioned this pull request Jun 27, 2018

Local development story for conformance tests #608

Closed

bobcatfish force-pushed the minikube_doc_update branch from 2036c47 to fb63486 Compare June 27, 2018 21:07

bobcatfish force-pushed the minikube_doc_update branch from fb63486 to 404b15c Compare June 27, 2018 21:08

bobcatfish commented Jun 27, 2018

View reviewed changes

bobcatfish force-pushed the minikube_doc_update branch from 404b15c to b4ea5c6 Compare June 27, 2018 21:13

google-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 27, 2018

google-prow-robot merged commit 8752b6b into knative:master Jun 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update minikube docs post rename #1332

Update minikube docs post rename #1332

bobcatfish commented Jun 23, 2018 •

edited

Loading

bobcatfish Jun 24, 2018

trisberg commented Jun 25, 2018

bobcatfish commented Jun 25, 2018

bobcatfish Jun 25, 2018

trisberg Jun 25, 2018

bobcatfish commented Jun 25, 2018

trisberg Jun 25, 2018

bobcatfish Jun 25, 2018

trisberg Jun 25, 2018

bobcatfish Jun 25, 2018

bobcatfish Jun 27, 2018

trisberg Jun 27, 2018

bobcatfish Jun 27, 2018

trisberg Jun 26, 2018

bobcatfish Jun 27, 2018

trisberg Jun 26, 2018 •

edited

Loading

bobcatfish Jun 27, 2018

evankanderson commented Jun 27, 2018

google-prow-robot commented Jun 27, 2018

evankanderson commented Jun 27, 2018

bobcatfish Jun 27, 2018

trisberg commented Jun 27, 2018


		### Deploy Istio
		1. [Make your user a cluster admin](#setup-cluster-admin)


		(Then optionally [enable istio injection](../DEVELOPMENT.md#deploy-istio).)

		1. [Deploy build](../DEVELOPMENT.md#deploy-build)

Update minikube docs post rename #1332

Update minikube docs post rename #1332

Conversation

bobcatfish commented Jun 23, 2018 • edited Loading

Proposed Changes

Choose a reason for hiding this comment

trisberg commented Jun 25, 2018

bobcatfish commented Jun 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobcatfish commented Jun 25, 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

trisberg Jun 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evankanderson commented Jun 27, 2018

google-prow-robot commented Jun 27, 2018

evankanderson commented Jun 27, 2018

Choose a reason for hiding this comment

trisberg commented Jun 27, 2018

bobcatfish commented Jun 23, 2018 •

edited

Loading

trisberg Jun 26, 2018 •

edited

Loading