Avoid deleting scale set if annotation is not parsable or if it does …

…not exist

pkg/apis/internalversion/selector.go

@@ -26,5 +26,8 @@ type NamespaceNameSelector struct {
 
 // Matches returns true if name and namespace is specified within the selector
 func (s *NamespaceNameSelector) Matches(namespace, name string) bool {
-	return s != nil && slices.Contains(s.MatchNamespace, namespace) && slices.Contains(s.MatchName, name)
+	if s == nil {
+		return true
+	}
+	return slices.Contains(s.MatchNamespace, namespace) && slices.Contains(s.MatchName, name)
 }

pkg/apis/internalversion/selector_test.go

@@ -22,8 +22,8 @@ func TestSelector_Matches(t *testing.T) {
 	t.Run("nil selector should return false always", func(t *testing.T) {
 		var selector *NamespaceNameSelector
 		podNamespace, podName := "podNamespace", "podName"
-		if selector.Matches(podNamespace, podName) {
-			t.Fatalf("Matches(%q, %q) returned true on nil selector", podNamespace, podName)
+		if !selector.Matches(podNamespace, podName) {
+			t.Fatalf("expected nil selector to match everything")
 		}
 	})
 

pkg/apis/v1alpha1/selector.go

@@ -26,5 +26,8 @@ type NamespaceNameSelector struct {
 
 // Matches returns true if name and namespace is specified within the selector
 func (s *NamespaceNameSelector) Matches(namespace, name string) bool {
-	return s != nil && slices.Contains(s.MatchNamespace, namespace) && slices.Contains(s.MatchName, name)
+	if s == nil {
+		return true
+	}
+	return slices.Contains(s.MatchNamespace, namespace) && slices.Contains(s.MatchName, name)
 }

pkg/apis/v1alpha1/selector_test.go

@@ -22,8 +22,8 @@ func TestSelector_Matches(t *testing.T) {
 	t.Run("nil selector should return false always", func(t *testing.T) {
 		var selector *NamespaceNameSelector
 		podNamespace, podName := "podNamespace", "podName"
-		if selector.Matches(podNamespace, podName) {
-			t.Fatalf("Matches(%q, %q) returned true on nil selector", podNamespace, podName)
+		if !selector.Matches(podNamespace, podName) {
+			t.Fatalf("expected nil selector to match everything")
 		}
 	})
 

pkg/kwok/server/debugging_port_forword.go

@@ -24,13 +24,13 @@ import (
 	"net/http"
 
 	"github.com/emicklei/go-restful/v3"
-	"golang.org/x/exp/slices"
 	"k8s.io/apimachinery/pkg/types"
 
 	"sigs.k8s.io/kwok/pkg/apis/internalversion"
 	"sigs.k8s.io/kwok/pkg/kwok/server/portforward"
 	"sigs.k8s.io/kwok/pkg/log"
 	"sigs.k8s.io/kwok/pkg/utils/exec"
+	"sigs.k8s.io/kwok/pkg/utils/slices"
 )
 
 // PortForward handles a port forwarding request.
@@ -44,12 +44,18 @@ func (s *Server) PortForward(ctx context.Context, podName, podNamespace string,
 		return err
 	}
 
+	fmt.Printf("Found forward: %v\n", *forward)
 	if len(forward.Command) > 0 {
 		cmdStream := exec.IOStreams{
 			In:  stream,
 			Out: stream,
 		}
-		return exec.Exec(context.Background(), "", cmdStream, forward.Command[0], forward.Command[1:]...)
+		fmt.Printf("executing: %v, %v\n", forward.Command[0], forward.Command[1:])
+		if err := exec.Exec(ctx, "", cmdStream, forward.Command[0], forward.Command[1:]...); err != nil {
+			fmt.Printf("returning from exec: %v\n", err)
+		}
+		fmt.Println("Returning without error")
+		return nil
 	}
 
 	addr := fmt.Sprintf("%s:%d", forward.TargetAddress, forward.TargetPort)

pkg/kwok/server/portforward/httpstream.go

@@ -263,7 +263,7 @@ func (h *httpStreamHandler) portForward(ctx context.Context, p *httpStreamPair)
 	h.logger.Debug("Connection request done invoking forwarder.PortForward for port", "connection", h.conn, "request", p.requestID, "port", portString)
 
 	if err != nil {
-		err := fmt.Errorf("error forwarding port %d to pod %s/%s, uid %v: %w", port, h.podNamespace, h.podName, h.uid, err)
+		err := fmt.Errorf("error forwarding port %d to pod %s/%s: %w", port, h.podNamespace, h.podName, err)
 		logger := log.FromContext(ctx)
 		logger.Error("PortForward", err)
 		fmt.Fprint(p.errorStream, err.Error())

pkg/kwok/server/portforward/websocket.go

@@ -194,7 +194,7 @@ func (h *websocketStreamHandler) portForward(ctx context.Context, p *websocketSt
 	h.logger.Debug("Connection done invoking forwarder.PortForward for port", "connection", h.conn, "port", p.port)
 
 	if err != nil {
-		err := fmt.Errorf("error forwarding port %d to pod %s/%s, uid %v: %w", p.port, h.podNamespace, h.podName, h.uid, err)
+		err := fmt.Errorf("error forwarding port %d to pod %s/%s: %w", p.port, h.podNamespace, h.podName, err)
 		logger := log.FromContext(ctx)
 		logger.Error("PortForward", err)
 		fmt.Fprint(p.errorStream, err.Error())

pkg/utils/slices/slices.go

@@ -45,3 +45,13 @@ func Filter[S ~[]T, T any](s S, f func(T) bool) []T {
 	}
 	return out
 }
+
+// Contains returns true if the slice contains the given element.
+func Contains[S ~[]T, T comparable](s S, t T) bool {
+	for _, v := range s {
+		if v == t {
+			return true
+		}
+	}
+	return false
+}

test/kwokctl/kwokctl_port_forward.test.sh

@@ -0,0 +1,112 @@
+#!/usr/bin/env bash
+# Copyright 2022 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.
+
+DIR="$(dirname "${BASH_SOURCE[0]}")"
+
+DIR="$(realpath "${DIR}")"
+
+RELEASES=()
+
+function usage() {
+  echo "Usage: $0 <kube-version...>"
+  echo "  <kube-version> is the version of kubernetes to test against."
+}
+
+function args() {
+  if [[ $# -eq 0 ]]; then
+    usage
+    exit 1
+  fi
+  while [[ $# -gt 0 ]]; do
+    RELEASES+=("${1}")
+    shift
+  done
+}
+
+function test_create_cluster() {
+  local release="${1}"
+  local name="${2}"
+
+  KWOK_KUBE_VERSION="${release}" kwokctl -v=-4 create cluster --name "${name}" --timeout 10m --wait 10m --quiet-pull --config="${DIR}/port-forward.yaml"
+  if [[ $? -ne 0 ]]; then
+    echo "Error: Cluster ${name} creation failed"
+    exit 1
+  fi
+}
+
+function test_port_forward() {
+  local name="${1}"
+  local pid
+  local result
+  kwokctl --name "${name}" kubectl port-forward deployment/fake-pod 8080:"${2}" 2>&1 > /dev/null &
+  pid=$!
+
+  # allow some time for port forward to start
+  sleep 1
+  result=$(curl localhost:8080/healthz 2>/dev/null)
+  kill -INT $pid
+  if [[ ! "${result}" == "ok" ]]; then
+    echo "Error: failed to port-forward to ${2}"
+    return 1
+  fi
+  echo "${result}"
+}
+
+function test_apply_node_and_deployment() {
+  kwokctl --name "${name}" kubectl apply -f "${DIR}/fake-node.yaml"
+  if [[ $? -ne 0 ]]; then
+    echo "Error: fake-node apply failed"
+    exit 1
+  fi
+  kwokctl --name "${name}" kubectl apply -f "${DIR}/fake-deployment.yaml"
+  if [[ $? -ne 0 ]]; then
+    echo "Error: fake-deployment apply failed"
+    exit 1
+  fi
+}
+
+function test_delete_cluster() {
+  local release="${1}"
+  local name="${2}"
+  kwokctl delete cluster --name "${name}"
+}
+
+function main() {
+  local failed=()
+  for release in "${RELEASES[@]}"; do
+    echo "------------------------------"
+    echo "Testing port-forwarding on ${KWOK_RUNTIME} for ${release}"
+    name="port-forwarding-cluster-${KWOK_RUNTIME}-${release//./-}"
+    test_create_cluster "${release}" "${name}" || failed+=("create_cluster_${name}")
+    test_apply_node_and_deployment || failed+=("apply_node_and_deployment")
+    test_port_forward "${name}" "8888" || failed+=("port_forward_${name}_default_forward")
+    test_port_forward "${name}" "8001" || failed+=("port_forward_${name}_target_forward")
+    test_port_forward "${name}" "8002" || failed+=("port_forward_${name}_command_forward")
+    test_delete_cluster "${release}" "${name}" || failed+=("delete_cluster_${name}")
+  done
+
+  if [[ "${#failed[@]}" -ne 0 ]]; then
+    echo "------------------------------"
+    echo "Error: Some tests failed"
+    for test in "${failed[@]}"; do
+      echo " - ${test}"
+    done
+    exit 1
+  fi
+}
+
+args "$@"
+
+main

test/kwokctl/port-forward.yaml

@@ -0,0 +1,22 @@
+kind: ClusterPortForward
+apiVersion: kwok.x-k8s.io/v1alpha1
+metadata:
+  name: cluster-port-forward-rules
+spec:
+  forwards:
+    # match the port 8001 forwards to the targetAddress:targetPort
+    - ports:
+        - 8001
+      targetPort: 10247
+      targetAddress: localhost
+
+    # # match the port 8002 forwards with the stdin/stdout of nc
+    - ports:
+      - 8002
+      command:
+        - nc
+        - localhost
+        - "10247"
+
+    - targetPort: 10247
+      targetAddress: localhost