Merge pull request kubernetes-sigs#100 from amwat/gce_network

[GCE] Explicitly set KUBE_GCE_NETWORK

kubetest2-gce/deployer/common.go

@@ -129,10 +129,14 @@ func (d *deployer) buildEnv() []string {
 	// the run of kubetest2 and so should be placed in the artifacts directory
 	env = append(env, fmt.Sprintf("KUBECONFIG=%s", d.kubeconfigPath))
 
-	// kube-up and kube-down get this as a default ("kubernetes") but log-dump
-	// does not. opted to set it manually here for maximum consistency
+	// kube-up and kube-down get this as a default ("kubernetes" / "e2e-test-${USER}")
+	// but log-dump does not, set it explicitly here for maximum consistency
 	env = append(env, fmt.Sprintf("KUBE_GCE_INSTANCE_PREFIX=%s", d.instancePrefix))
 
+	// kube-up and kube-down get this as a default ("default" / "e2e-test-${USER}")
+	// but log-dump does not, set it explicitly here for maximum consistency
+	env = append(env, fmt.Sprintf("KUBE_GCE_NETWORK=%s", d.network))
+
 	// Pass through number of nodes and associated IP range. In the future,
 	// IP range will be configurable.
 	env = append(env, fmt.Sprintf("NUM_NODES=%d", d.NumNodes))

kubetest2-gce/deployer/deployer.go

@@ -81,6 +81,27 @@ type deployer struct {
 	CreateCustomNetwork         bool   `desc:"Sets the environment variable CREATE_CUSTOM_NETWORK=true during deployment."`
 }
 
+// pseudoUniqueSubstring returns a substring of a UUID
+// that can be reasonably used in resource names
+// where length is constrained
+// e.g https://cloud.google.com/compute/docs/naming-resources
+// but still retain as much uniqueness as possible
+// also easily lets us tie it back to a run
+func pseudoUniqueSubstring(uuid string) string {
+	// both KUBETEST2_RUN_ID and PROW_JOB_ID uuids are generated
+	// following RFC 4122 https://tools.ietf.org/html/rfc4122
+	// e.g. 09a2565a-7ac6-11eb-a603-2218f636630c
+	// extract the first 13 characters (09a2565a-7ac6) as they are the ones that depend on
+	// timestamp and has the best avalanche effect (https://en.wikipedia.org/wiki/Avalanche_effect)
+	// as compared to the other bytes
+	// 13 characters is also <= the no. of character being used previously
+	const maxResourceNamePrefixLength = 13
+	if len(uuid) <= maxResourceNamePrefixLength {
+		return uuid
+	}
+	return uuid[:maxResourceNamePrefixLength]
+}
+
 // New implements deployer.New for gce
 func New(opts types.Options) (types.Deployer, *pflag.FlagSet) {
 	d := &deployer{
@@ -92,11 +113,12 @@ func New(opts types.Options) (types.Deployer, *pflag.FlagSet) {
 				Strategy: "make",
 			},
 		},
-		kubeconfigPath:              filepath.Join(opts.RunDir(), "kubetest2-kubeconfig"),
-		logsDir:                     filepath.Join(opts.RunDir(), "cluster-logs"),
-		boskosHeartbeatClose:        make(chan struct{}),
-		instancePrefix:              "kubetest2",
-		network:                     "default",
+		kubeconfigPath:       filepath.Join(opts.RunDir(), "kubetest2-kubeconfig"),
+		logsDir:              filepath.Join(opts.RunDir(), "cluster-logs"),
+		boskosHeartbeatClose: make(chan struct{}),
+		// names need to start with an alphabet
+		instancePrefix:              "kt2-" + pseudoUniqueSubstring(opts.RunID()),
+		network:                     "kt2-" + pseudoUniqueSubstring(opts.RunID()),
 		BoskosAcquireTimeoutSeconds: 5 * 60,
 		BoskosLocation:              "http://boskos.test-pods.svc.cluster.local.",
 		NumNodes:                    3,

kubetest2-gce/deployer/deployer_test.go

@@ -0,0 +1,54 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package deployer
+
+import "testing"
+
+func TestPseudoUniqueSubstring(t *testing.T) {
+	testCases := []struct {
+		name              string
+		uuid              string
+		expectedSubstring string
+	}{
+		{
+			name:              "actual uuid",
+			uuid:              "09a2565a-7ac6-11eb-a603-2218f636630c",
+			expectedSubstring: "09a2565a-7ac6",
+		},
+		{
+			name:              "<= 13 length uuid",
+			uuid:              "09a2565a-7ac6",
+			expectedSubstring: "09a2565a-7ac6",
+		},
+		{
+			name:              "empty string",
+			uuid:              "",
+			expectedSubstring: "",
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			actualSubstring := pseudoUniqueSubstring(tc.uuid)
+			if actualSubstring != tc.expectedSubstring {
+				t.Errorf("invalid substring: expected %s, but got %s", tc.expectedSubstring, actualSubstring)
+			}
+		})
+	}
+}

kubetest2-gce/deployer/down.go

@@ -51,9 +51,9 @@ func (d *deployer) Down() error {
 	}
 
 	klog.V(2).Info("about to delete nodeport firewall rule")
-	if err := d.deleteFirewallRuleNodePort(); err != nil {
-		return fmt.Errorf("failed to delete firewall rule: %s", err)
-	}
+	// best-effort try to delete the explicitly created firewall rules
+	// ideally these should already be deleted by kube-down
+	d.deleteFirewallRuleNodePort()
 
 	if d.boskos != nil {
 		klog.V(2).Info("releasing boskos project")

kubetest2-gce/deployer/firewall.go

@@ -19,6 +19,7 @@ package deployer
 import (
 	"fmt"
 
+	"k8s.io/klog"
 	"sigs.k8s.io/kubetest2/pkg/exec"
 )
 
@@ -50,16 +51,14 @@ func (d *deployer) createFirewallRuleNodePort() error {
 	return nil
 }
 
-func (d *deployer) deleteFirewallRuleNodePort() error {
+func (d *deployer) deleteFirewallRuleNodePort() {
 	cmd := exec.Command(
 		"gcloud", "compute", "firewall-rules", "delete",
 		"--project", d.GCPProject,
 		d.nodePortRuleName(),
 	)
 	exec.InheritOutput(cmd)
 	if err := cmd.Run(); err != nil {
-		return fmt.Errorf("failed to delete nodeports firewall rules: %s", err)
+		klog.Warning("failed to delete nodeports firewall rules: might be deleted already?")
 	}
-
-	return nil
 }