Repository: google/kf (go)

Issue:
kf install does not support GKE private master IP address
<!-- If you need to report a security issue with kf, send an email to kf-security@google.com. -->

## What version of kf?

<!-- Paste the output of kf version -->
8cd8bdb08bf12af78d25d903c9d51063c2215e6e

## Expected Behavior

<!-- Briefly describe what you expect to happen -->
Kf install should work

## Actual Behavior

<!-- Briefly describe what is actually happening -->
Kf install always uses the public ip when targeting k8s clusters. This is problematic if installing a cluster from a VPC.

## Steps to Reproduce the Problem
1. Create GKE cluster on a private VPC and authorize a network where a VM is running.
2. Execute kf install from that VM (hangs)

<!-- How can a maintainer reproduce this issue (be detailed) -->


Pull request description:
Ask user for public/internal GKE server IP
<!-- Include the issue number below -->
Fixes #823

## Proposed Changes

* Add a question to the `kf install gke` command to ask which IP addr to hit when targeting the cluster

Note that more setup differences might be required for creating private GKE clusters, but this is an easy question to answer.

<!-- kf follows the Keep A Changelog standard for release notes.

https://keepachangelog.com/en/1.0.0/

Changelog entries should be one per line and start with one of the following
words:

`Added` for new features.
`Changed` for changes in existing functionality.
`Deprecated` for soon-to-be removed features.
`Removed` for now removed features.
`Fixed` for any bug fixes.
`Security` in case of vulnerabilities.

If one of the changes is breaking include that as a second word e.g.

  Removed BREAKING support for manifests v1
-->

```release-note

```


Patch:
diff --git a/pkg/apis/kf/v1alpha1/utils.go b/pkg/apis/kf/v1alpha1/utils.go
index c19bcf4..31a6970 100644
--- a/pkg/apis/kf/v1alpha1/utils.go
+++ b/pkg/apis/kf/v1alpha1/utils.go
@@ -24,6 +24,7 @@ import (
 	"strings"
 
 	corev1 "k8s.io/api/core/v1"
+	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	"knative.dev/pkg/apis"
 	duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1"
 	cv1alpha3 "knative.dev/pkg/client/clientset/versioned/typed/istio/v1alpha3"
@@ -107,7 +108,16 @@ func (ci *conditionImpl) MarkTemplateError(err error) error {
 func (ci *conditionImpl) MarkReconciliationError(action string, err error) error {
 	msg := fmt.Sprintf("Error occurred while %s %s: %s", action, ci.childType, err)
 
-	ci.manager.MarkFalse(ci.destination, "ReconciliationError", msg)
+	switch {
+	case apierrs.IsConflict(err):
+		ci.manager.MarkUnknown(ci.destination, "CacheOutdated", msg)
+
+		// In the future, additional retryable errors can be added here if
+		// Kubernetes starts returning other failures.
+
+	default:
+		ci.manager.MarkFalse(ci.destination, "ReconciliationError", msg)
+	}
 
 	return errors.New(msg)
 }
diff --git a/pkg/kf/apps/log_tailer.go b/pkg/kf/apps/log_tailer.go
index d4c0ae5..6fda02c 100644
--- a/pkg/kf/apps/log_tailer.go
+++ b/pkg/kf/apps/log_tailer.go
@@ -116,6 +116,9 @@ func (t *pushLogTailer) handleWatch() (bool, error) {
 	for e := range ws.ResultChan() {
 		switch obj := e.Object.(type) {
 		case *v1alpha1.App:
+			if obj.ResourceVersion != "" {
+				t.resourceVersion = obj.ResourceVersion
+			}
 			// skip out of date apps
 			if obj.Generation != obj.Status.ObservedGeneration {
 				continue
@@ -129,7 +132,9 @@ func (t *pushLogTailer) handleWatch() (bool, error) {
 				return true, nil
 			}
 		case *k8smeta.Status:
-			t.resourceVersion = obj.ResourceVersion
+			if obj.ResourceVersion != "" {
+				t.resourceVersion = obj.ResourceVersion
+			}
 			if obj.Status != k8smeta.StatusFailure {
 				// TODO: I'm not sure when/if we would get this.
 				continue
@@ -150,7 +155,6 @@ func (t *pushLogTailer) handleWatch() (bool, error) {
 func (t *pushLogTailer) handleUpdate(
 	app *v1alpha1.App,
 ) (bool, error) {
-
 	sourceReady := app.Status.GetCondition(v1alpha1.AppConditionSourceReady)
 	if sourceReady == nil {
 		// source might still be creating
diff --git a/pkg/kf/commands/install/gke/gke.go b/pkg/kf/commands/install/gke/gke.go
index 5653d2a..f580cde 100644
--- a/pkg/kf/commands/install/gke/gke.go
+++ b/pkg/kf/commands/install/gke/gke.go
@@ -526,15 +526,25 @@ func targetCluster(
 ) error {
 	ctx = SetLogPrefix(ctx, "Target Cluster")
 	Logf(ctx, "targeting cluster")
-	_, err := gcloud(
-		ctx,
+
+	_, selection, err := SelectPrompt(ctx, "GKE Master Server IP", "public", "internal")
+	if err != nil {
+		return err
+	}
+
+	args := []string{
 		"container",
 		"clusters",
 		"get-credentials",
 		clusterName,
 		"--zone", zone,
 		"--project", projID,
-	)
+	}
+
+	if selection == "internal" {
+		args = append(args, "--internal-ip")
+	}
+	_, err = gcloud(ctx, args...)
 
 	return err
 }
diff --git a/pkg/kf/istio/istio_client_options.go b/pkg/kf/istio/istio_client_options.go
index b7d55ed..4847a77 100644
--- a/pkg/kf/istio/istio_client_options.go
+++ b/pkg/kf/istio/istio_client_options.go
@@ -78,7 +78,7 @@ func WithListIngressesService(val string) ListIngressesOption {
 // ListIngressesOptionDefaults gets the default values for ListIngresses.
 func ListIngressesOptionDefaults() ListIngressesOptions {
 	return ListIngressesOptions{
-		WithListIngressesNamespace("istio-system"),
-		WithListIngressesService("istio-ingressgateway"),
+		WithListIngressesNamespace("gke-system"),
+		WithListIngressesService("istio-ingress"),
 	}
 }
diff --git a/pkg/kf/istio/istio_client_options.yml b/pkg/kf/istio/istio_client_options.yml
index 9300e75..28faf1c 100644
--- a/pkg/kf/istio/istio_client_options.yml
+++ b/pkg/kf/istio/istio_client_options.yml
@@ -23,8 +23,8 @@ configs:
   - name: Service
     type: string
     description: the name of the ingress service
-    default: '"istio-ingressgateway"'
+    default: '"istio-ingress"'
   - name: Namespace
     type: string
     description: the Kubernetes namespace to search for Ingresses
-    default: '"istio-system"'
+    default: '"gke-system"'
diff --git a/pkg/reconciler/route/resources/virtual_service.go b/pkg/reconciler/route/resources/virtual_service.go
index a02c82b..c5bfb3c 100644
--- a/pkg/reconciler/route/resources/virtual_service.go
+++ b/pkg/reconciler/route/resources/virtual_service.go
@@ -31,8 +31,8 @@ import (
 
 const (
 	ManagedByLabel        = "app.kubernetes.io/managed-by"
-	KnativeIngressGateway = "knative-ingress-gateway.knative-serving.svc.cluster.local"
-	GatewayHost           = "istio-ingressgateway.istio-system.svc.cluster.local"
+	KnativeIngressGateway = "knative-serving/gke-system-gateway"
+	GatewayHost           = "cluster-local-gateway.gke-system.svc.cluster.local"
 )
 
 // MakeVirtualServiceLabels creates Labels that can be used to tie a


Test patch:
diff --git a/pkg/apis/kf/v1alpha1/utils_test.go b/pkg/apis/kf/v1alpha1/utils_test.go
index 4646ba3..d376085 100644
--- a/pkg/apis/kf/v1alpha1/utils_test.go
+++ b/pkg/apis/kf/v1alpha1/utils_test.go
@@ -24,6 +24,8 @@ import (
 
 	"github.com/google/kf/pkg/kf/testutil"
 	corev1 "k8s.io/api/core/v1"
+	apierrs "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"knative.dev/pkg/apis"
 	duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1"
 )
@@ -139,6 +141,32 @@ func ExampleSingleConditionManager_MarkReconciliationError() {
 	// Error: Error occurred while updating Dummy: update err
 }
 
+func ExampleSingleConditionManager_MarkReconciliationError_conflict() {
+	manager := apis.NewLivingConditionSet("DummyReady").Manage(&duckv1beta1.Status{})
+	scm := NewSingleConditionManager(manager, "DummyReady", "Dummy")
+
+	conflict := apierrs.NewConflict(
+		schema.GroupResource{
+			Group:    "serving.knative.dev",
+			Resource: "services",
+		},
+		"MyService",
+		errors.New("the object has been modified; please apply your changes to the latest version and try again"),
+	)
+	err := scm.MarkReconciliationError("updating", conflict)
+
+	result := manager.GetCondition("DummyReady")
+	fmt.Println("Status:", result.Status)
+	fmt.Println("Message:", result.Message)
+	fmt.Println("Reason:", result.Reason)
+	fmt.Println("Error:", err)
+
+	// Output: Status: Unknown
+	// Message: Error occurred while updating Dummy: Operation cannot be fulfilled on services.serving.knative.dev "MyService": the object has been modified; please apply your changes to the latest version and try again
+	// Reason: CacheOutdated
+	// Error: Error occurred while updating Dummy: Operation cannot be fulfilled on services.serving.knative.dev "MyService": the object has been modified; please apply your changes to the latest version and try again
+}
+
 func TestGenerateName_Deterministic(t *testing.T) {
 	t.Parallel()
 
diff --git a/pkg/kf/istio/istio_client_test.go b/pkg/kf/istio/istio_client_test.go
index 506e640..c491b4d 100644
--- a/pkg/kf/istio/istio_client_test.go
+++ b/pkg/kf/istio/istio_client_test.go
@@ -47,8 +47,8 @@ func TestIstioClient_ListIngresses(t *testing.T) {
 		"default values": {
 			setup: func(mockK8s kubernetes.Interface) {
 				svc := &corev1.Service{}
-				svc.Name = "istio-ingressgateway"
-				mockK8s.CoreV1().Services("istio-system").Create(svc)
+				svc.Name = "istio-ingress"
+				mockK8s.CoreV1().Services("gke-system").Create(svc)
 			},
 		},
 		"custom values": {
diff --git a/pkg/reconciler/route/resources/virtual_service_test.go b/pkg/reconciler/route/resources/virtual_service_test.go
index ec7f142..949c424 100644
--- a/pkg/reconciler/route/resources/virtual_service_test.go
+++ b/pkg/reconciler/route/resources/virtual_service_test.go
@@ -185,7 +185,7 @@ func TestMakeVirtualService(t *testing.T) {
 						},
 						Route: []networking.HTTPRouteDestination{
 							{
-								Destination: networking.Destination{Host: "istio-ingressgateway.istio-system.svc.cluster.local"},
+								Destination: networking.Destination{Host: "cluster-local-gateway.gke-system.svc.cluster.local"},
 								Weight:      100,
 								Headers: &networking.Headers{
 									Request: &networking.HeaderOperations{
@@ -228,7 +228,7 @@ func TestMakeVirtualService(t *testing.T) {
 						},
 						Route: []networking.HTTPRouteDestination{
 							{
-								Destination: networking.Destination{Host: "istio-ingressgateway.istio-system.svc.cluster.local"},
+								Destination: networking.Destination{Host: "cluster-local-gateway.gke-system.svc.cluster.local"},
 								Weight:      34,
 								Headers: &networking.Headers{
 									Request: &networking.HeaderOperations{
@@ -239,7 +239,7 @@ func TestMakeVirtualService(t *testing.T) {
 								},
 							},
 							{
-								Destination: networking.Destination{Host: "istio-ingressgateway.istio-system.svc.cluster.local"},
+								Destination: networking.Destination{Host: "cluster-local-gateway.gke-system.svc.cluster.local"},
 								Weight:      33,
 								Headers: &networking.Headers{
 									Request: &networking.HeaderOperations{
@@ -250,7 +250,7 @@ func TestMakeVirtualService(t *testing.T) {
 								},
 							},
 							{
-								Destination: networking.Destination{Host: "istio-ingressgateway.istio-system.svc.cluster.local"},
+								Destination: networking.Destination{Host: "cluster-local-gateway.gke-system.svc.cluster.local"},
 								Weight:      33,
 								Headers: &networking.Headers{
 									Request: &networking.HeaderOperations{