[wip] debug stale webhook config map

.golangci.yaml

@@ -182,9 +182,9 @@ linters:
       - legacy
       - std-error-handling
     rules:
-      - linters:
-          - staticcheck
-        text: corev1.Endpoint.* is deprecated
+      # - linters:
+      #     - staticcheck
+      #   text: corev1.Endpoint.* is deprecated
       - linters:
           - staticcheck
         text: NewSimpleClientset is deprecated

test/e2e-tests.sh

@@ -67,106 +67,109 @@ if (( SHORT )); then
   GO_TEST_FLAGS+=("-short")
 fi
 
-go_test_e2e -timeout=50m \
-  "${GO_TEST_FLAGS[@]}" \
-  ./test/conformance/api/... \
-  ./test/conformance/runtime/... \
-  ./test/e2e \
-  "${E2E_TEST_FLAGS[@]}" || failed=1
-
-toggle_feature tag-header-based-routing Enabled
-go_test_e2e -timeout=2m ./test/e2e/tagheader "${E2E_TEST_FLAGS[@]}" || failed=1
-toggle_feature tag-header-based-routing Disabled
-
-toggle_feature allow-zero-initial-scale true config-autoscaler || fail_test
-go_test_e2e -timeout=2m ./test/e2e/initscale "${E2E_TEST_FLAGS[@]}" || failed=1
-toggle_feature allow-zero-initial-scale false config-autoscaler || fail_test
-
-go_test_e2e -timeout=2m ./test/e2e/domainmapping "${E2E_TEST_FLAGS[@]}" || failed=1
-
-toggle_feature cluster-local-domain-tls enabled config-network || fail_test
-go_test_e2e -timeout=2m ./test/e2e/clusterlocaldomaintls "${E2E_TEST_FLAGS[@]}" || failed=1
-toggle_feature cluster-local-domain-tls disabled config-network || fail_test
-
-toggle_feature system-internal-tls enabled config-network || fail_test
-toggle_feature "logging.enable-request-log" true config-observability || fail_test
-toggle_feature "logging.request-log-template" "TLS: {{.Request.TLS}}" config-observability || fail_test
-# with current implementation, Activator must be restarted when configuring system-internal-tls. See https://github.com/knative/serving/issues/13754
-restart_pod "${SYSTEM_NAMESPACE}" "app=activator"
-
-# we need to restart the pod in order to start the net-certmanager-controller
-if (( ! HTTPS )); then
-  restart_pod "${SYSTEM_NAMESPACE}" "app=controller"
-fi
-go_test_e2e -timeout=3m ./test/e2e/systeminternaltls "${E2E_TEST_FLAGS[@]}" || failed=1
-toggle_feature system-internal-tls disabled config-network || fail_test
-toggle_feature "logging.enable-request-log" false config-observability || fail_test
-toggle_feature "logging.request-log-template" '' config-observability || fail_test
-# with the current implementation, Activator is always in the request path, and needs to be restarted after configuring system-internal-tls
-restart_pod "${SYSTEM_NAMESPACE}" "app=activator"
-
-# we need to restart the pod to stop the net-certmanager-controller
-if (( ! HTTPS )); then
-  restart_pod "${SYSTEM_NAMESPACE}" "app=controller"
-  kubectl get leases -n "${SYSTEM_NAMESPACE}" -o json | jq -r '.items[] | select(.metadata.name | test("controller.knative.dev.serving.pkg.reconciler.certificate.reconciler")).metadata.name' | xargs kubectl delete lease -n "${SYSTEM_NAMESPACE}"
-fi
-
-kubectl get cm "config-gc" -n "${SYSTEM_NAMESPACE}" -o yaml > "${TMP_DIR}"/config-gc.yaml
-add_trap "kubectl replace cm 'config-gc' -n ${SYSTEM_NAMESPACE} -f ${TMP_DIR}/config-gc.yaml" SIGKILL SIGTERM SIGQUIT
-immediate_gc
-go_test_e2e -timeout=2m ./test/e2e/gc "${E2E_TEST_FLAGS[@]}" || failed=1
-kubectl replace cm "config-gc" -n "${SYSTEM_NAMESPACE}" -f "${TMP_DIR}"/config-gc.yaml
-
-# Run scale tests.
-# Note that we use a very high -parallel because each ksvc is run as its own
-# sub-test. If this is not larger than the maximum scale tested then the test
-# simply cannot pass.
-# TODO - Renable once we get this reliably passing on GKE 1.21
-# go_test_e2e -timeout=20m -parallel=300 ./test/scale "${E2E_TEST_FLAGS[@]}" || failed=1
-
-# Run HPA tests
-go_test_e2e -timeout=30m -tags=hpa ./test/e2e "${E2E_TEST_FLAGS[@]}" || failed=1
-
-# Run initContainers tests with alpha enabled avoiding any issues with the testing options guard above
-# InitContainers test uses emptyDir.
-toggle_feature kubernetes.podspec-init-containers Enabled
-go_test_e2e -timeout=2m ./test/e2e/initcontainers "${E2E_TEST_FLAGS[@]}" || failed=1
-toggle_feature kubernetes.podspec-init-containers Disabled
-
-# Run multi-container probe tests
-toggle_feature multi-container-probing Enabled
-go_test_e2e -timeout=2m ./test/e2e/multicontainerprobing "${E2E_TEST_FLAGS[@]}" || failed=1
-toggle_feature multi-container-probing Disabled
-
-# RUN PVC tests with default storage class.
-toggle_feature kubernetes.podspec-persistent-volume-claim Enabled
-toggle_feature kubernetes.podspec-persistent-volume-write Enabled
-toggle_feature kubernetes.podspec-securitycontext Enabled
-go_test_e2e -timeout=5m ./test/e2e/pvc "${E2E_TEST_FLAGS[@]}" || failed=1
-toggle_feature kubernetes.podspec-securitycontext Disabled
-toggle_feature kubernetes.podspec-persistent-volume-write Disabled
-toggle_feature kubernetes.podspec-persistent-volume-claim Disabled
-
-# RUN secure pod defaults test in a separate install.
-toggle_feature secure-pod-defaults Enabled
-go_test_e2e -timeout=3m ./test/e2e/securedefaults "${E2E_TEST_FLAGS[@]}" || failed=1
-toggle_feature secure-pod-defaults Disabled
-
-# Run HA tests separately as they're stopping core Knative Serving pods.
-# Define short -spoofinterval to ensure frequent probing while stopping pods.
-go_test_e2e -timeout=30m -failfast -parallel=1 ./test/ha \
-  "${E2E_TEST_FLAGS[@]}" \
-  -replicas="${REPLICAS:-1}" \
-  -buckets="${BUCKETS:-1}" \
-  -spoofinterval="10ms" || failed=1
-
-if (( HTTPS )); then
-  kubectl delete -f "${E2E_YAML_DIR}"/test/config/externaldomaintls/certmanager/caissuer/ --ignore-not-found
-  toggle_feature external-domain-tls Disabled config-network
-  # we need to restart the pod to stop the net-certmanager-controller
-  restart_pod "${SYSTEM_NAMESPACE}" "app=controller"
-  kubectl get leases -n "${SYSTEM_NAMESPACE}" -o json | jq -r '.items[] | select(.metadata.name | test("controller.knative.dev.serving.pkg.reconciler.certificate.reconciler")).metadata.name' | xargs kubectl delete lease -n "${SYSTEM_NAMESPACE}"
-fi
+# go_test_e2e -timeout=50m \
+#   "${GO_TEST_FLAGS[@]}" \
+#   ./test/conformance/api/... \
+#   ./test/conformance/runtime/... \
+#   ./test/e2e \
+#   "${E2E_TEST_FLAGS[@]}" || failed=1
+#
+# toggle_feature tag-header-based-routing Enabled
+# go_test_e2e -timeout=2m ./test/e2e/tagheader "${E2E_TEST_FLAGS[@]}" || failed=1
+# toggle_feature tag-header-based-routing Disabled
+
+for i in {1..20}; do
+  export TEST_RUN=$i
+  toggle_feature allow-zero-initial-scale true config-autoscaler || fail_test
+  go_test_e2e -timeout=2m ./test/e2e/initscale "${E2E_TEST_FLAGS[@]}" || fail_test
+  toggle_feature allow-zero-initial-scale false config-autoscaler || fail_test
+done
+
+# go_test_e2e -timeout=2m ./test/e2e/domainmapping "${E2E_TEST_FLAGS[@]}" || failed=1
+#
+# toggle_feature cluster-local-domain-tls enabled config-network || fail_test
+# go_test_e2e -timeout=2m ./test/e2e/clusterlocaldomaintls "${E2E_TEST_FLAGS[@]}" || failed=1
+# toggle_feature cluster-local-domain-tls disabled config-network || fail_test
+#
+# toggle_feature system-internal-tls enabled config-network || fail_test
+# toggle_feature "logging.enable-request-log" true config-observability || fail_test
+# toggle_feature "logging.request-log-template" "TLS: {{.Request.TLS}}" config-observability || fail_test
+# # with current implementation, Activator must be restarted when configuring system-internal-tls. See https://github.com/knative/serving/issues/13754
+# restart_pod "${SYSTEM_NAMESPACE}" "app=activator"
+#
+# # we need to restart the pod in order to start the net-certmanager-controller
+# if (( ! HTTPS )); then
+#   restart_pod "${SYSTEM_NAMESPACE}" "app=controller"
+# fi
+# go_test_e2e -timeout=3m ./test/e2e/systeminternaltls "${E2E_TEST_FLAGS[@]}" || failed=1
+# toggle_feature system-internal-tls disabled config-network || fail_test
+# toggle_feature "logging.enable-request-log" false config-observability || fail_test
+# toggle_feature "logging.request-log-template" '' config-observability || fail_test
+# # with the current implementation, Activator is always in the request path, and needs to be restarted after configuring system-internal-tls
+# restart_pod "${SYSTEM_NAMESPACE}" "app=activator"
+#
+# # we need to restart the pod to stop the net-certmanager-controller
+# if (( ! HTTPS )); then
+#   restart_pod "${SYSTEM_NAMESPACE}" "app=controller"
+#   kubectl get leases -n "${SYSTEM_NAMESPACE}" -o json | jq -r '.items[] | select(.metadata.name | test("controller.knative.dev.serving.pkg.reconciler.certificate.reconciler")).metadata.name' | xargs kubectl delete lease -n "${SYSTEM_NAMESPACE}"
+# fi
+#
+# kubectl get cm "config-gc" -n "${SYSTEM_NAMESPACE}" -o yaml > "${TMP_DIR}"/config-gc.yaml
+# add_trap "kubectl replace cm 'config-gc' -n ${SYSTEM_NAMESPACE} -f ${TMP_DIR}/config-gc.yaml" SIGKILL SIGTERM SIGQUIT
+# immediate_gc
+# go_test_e2e -timeout=2m ./test/e2e/gc "${E2E_TEST_FLAGS[@]}" || failed=1
+# kubectl replace cm "config-gc" -n "${SYSTEM_NAMESPACE}" -f "${TMP_DIR}"/config-gc.yaml
+#
+# # Run scale tests.
+# # Note that we use a very high -parallel because each ksvc is run as its own
+# # sub-test. If this is not larger than the maximum scale tested then the test
+# # simply cannot pass.
+# # TODO - Renable once we get this reliably passing on GKE 1.21
+# # go_test_e2e -timeout=20m -parallel=300 ./test/scale "${E2E_TEST_FLAGS[@]}" || failed=1
+#
+# # Run HPA tests
+# go_test_e2e -timeout=30m -tags=hpa ./test/e2e "${E2E_TEST_FLAGS[@]}" || failed=1
+#
+# # Run initContainers tests with alpha enabled avoiding any issues with the testing options guard above
+# # InitContainers test uses emptyDir.
+# toggle_feature kubernetes.podspec-init-containers Enabled
+# go_test_e2e -timeout=2m ./test/e2e/initcontainers "${E2E_TEST_FLAGS[@]}" || failed=1
+# toggle_feature kubernetes.podspec-init-containers Disabled
+#
+# # Run multi-container probe tests
+# toggle_feature multi-container-probing Enabled
+# go_test_e2e -timeout=2m ./test/e2e/multicontainerprobing "${E2E_TEST_FLAGS[@]}" || failed=1
+# toggle_feature multi-container-probing Disabled
+#
+# # RUN PVC tests with default storage class.
+# toggle_feature kubernetes.podspec-persistent-volume-claim Enabled
+# toggle_feature kubernetes.podspec-persistent-volume-write Enabled
+# toggle_feature kubernetes.podspec-securitycontext Enabled
+# go_test_e2e -timeout=5m ./test/e2e/pvc "${E2E_TEST_FLAGS[@]}" || failed=1
+# toggle_feature kubernetes.podspec-securitycontext Disabled
+# toggle_feature kubernetes.podspec-persistent-volume-write Disabled
+# toggle_feature kubernetes.podspec-persistent-volume-claim Disabled
+#
+# # RUN secure pod defaults test in a separate install.
+# toggle_feature secure-pod-defaults Enabled
+# go_test_e2e -timeout=3m ./test/e2e/securedefaults "${E2E_TEST_FLAGS[@]}" || failed=1
+# toggle_feature secure-pod-defaults Disabled
+#
+# # Run HA tests separately as they're stopping core Knative Serving pods.
+# # Define short -spoofinterval to ensure frequent probing while stopping pods.
+# go_test_e2e -timeout=30m -failfast -parallel=1 ./test/ha \
+#   "${E2E_TEST_FLAGS[@]}" \
+#   -replicas="${REPLICAS:-1}" \
+#   -buckets="${BUCKETS:-1}" \
+#   -spoofinterval="10ms" || failed=1
+#
+# if (( HTTPS )); then
+#   kubectl delete -f "${E2E_YAML_DIR}"/test/config/externaldomaintls/certmanager/caissuer/ --ignore-not-found
+#   toggle_feature external-domain-tls Disabled config-network
+#   # we need to restart the pod to stop the net-certmanager-controller
+#   restart_pod "${SYSTEM_NAMESPACE}" "app=controller"
+#   kubectl get leases -n "${SYSTEM_NAMESPACE}" -o json | jq -r '.items[] | select(.metadata.name | test("controller.knative.dev.serving.pkg.reconciler.certificate.reconciler")).metadata.name' | xargs kubectl delete lease -n "${SYSTEM_NAMESPACE}"
+# fi
 
 (( failed )) && fail_test
 

test/e2e/initscale/initial_scale_zero_test.go

@@ -20,6 +20,7 @@ limitations under the License.
 package initscale
 
 import (
+	"os"
 	"testing"
 
 	"knative.dev/serving/test"
@@ -30,16 +31,18 @@ import (
 // the revision level. This test runs after the cluster wide flag allow-zero-initial-scale
 // is set to true.
 func TestInitScaleZero(t *testing.T) {
-	t.Parallel()
+	t.Run(os.Getenv("TEST_RUN"), func(t *testing.T) {
+		t.Parallel()
 
-	clients := e2e.Setup(t)
-	names := test.ResourceNames{
-		Config: test.ObjectNameForTest(t),
-		Image:  test.HelloWorld,
-	}
+		clients := e2e.Setup(t)
+		names := test.ResourceNames{
+			Config: test.ObjectNameForTest(t),
+			Image:  test.HelloWorld,
+		}
 
-	test.EnsureTearDown(t, clients, &names)
+		test.EnsureTearDown(t, clients, &names)
 
-	t.Log("Creating a new Configuration with initial scale zero and verifying that no pods are created")
-	e2e.CreateAndVerifyInitialScaleConfiguration(t, clients, names, 0)
+		t.Log("Creating a new Configuration with initial scale zero and verifying that no pods are created")
+		e2e.CreateAndVerifyInitialScaleConfiguration(t, clients, names, 0)
+	})
 }

vendor/knative.dev/pkg/configmap/informer/informed_watcher.go

@@ -1,3 +1,3 @@
 /*
 Copyright 2018 The Knative Authors
 
@@ -20,6 +20,7 @@
 	"errors"
 	"fmt"
 
+	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
@@ -32,6 +33,7 @@
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/cache"
 	"knative.dev/pkg/configmap"
+	"knative.dev/pkg/kmap"
 )
 
 // NewInformedWatcherFromFactory watches a Kubernetes namespace for ConfigMap changes.
@@ -99,6 +101,8 @@
 	// of registering and notifying observers. This simplifies the
 	// InformedWatcher to just setting up the Kubernetes informer.
 	configmap.ManualWatcher
+
+	Logger *zap.SugaredLogger
 }
 
 // Asserts that InformedWatcher implements Watcher.
@@ -218,15 +222,29 @@
 func (i *InformedWatcher) addConfigMapEvent(obj interface{}) {
 	configMap := obj.(*corev1.ConfigMap)
 	i.OnChange(configMap)
+
+	if i.Logger != nil {
+		data := kmap.ExcludeKeyList(configMap.Data, []string{"_example"})
+		i.Logger.Warnf("config map %q added: %#v", configMap.Name, data)
+	}
 }
 
 func (i *InformedWatcher) updateConfigMapEvent(o, n interface{}) {
+	configMap := n.(*corev1.ConfigMap)
+
 	// Ignore updates that are idempotent. We are seeing those
 	// periodically.
 	if equality.Semantic.DeepEqual(o, n) {
+		if i.Logger != nil {
+			i.Logger.Warnf("config map update ignored %s", configMap.Name)
+		}
 		return
 	}
-	configMap := n.(*corev1.ConfigMap)
+
+	if i.Logger != nil {
+		data := kmap.ExcludeKeyList(configMap.Data, []string{"_example"})
+		i.Logger.Warnf("config map %q updated: %#v", configMap.Name, data)
+	}
 	i.OnChange(configMap)
 }
 
@@ -246,6 +264,11 @@
 		return
 	}
 
+	if i.Logger != nil {
+		data := kmap.ExcludeKeyList(configMap.Data, []string{"_example"})
+		i.Logger.Warnf("config map %q deleted: %#v", configMap.Name, data)
+	}
+
 	if def, ok := i.defaults[configMap.Name]; ok {
 		i.OnChange(def)
 	}

vendor/knative.dev/pkg/injection/config.go

@@ -1,3 +1,3 @@
 /*
 Copyright 2020 The Knative Authors
 
@@ -31,6 +31,9 @@
 	env := new(environment.ClientConfig)
 	env.InitFlags(flag.CommandLine)
 	klog.InitFlags(flag.CommandLine)
+
+	flag.CommandLine.Set("v", "8")
+
 	flag.Parse()
 	cfg, err := env.GetRESTConfig()
 	if err != nil {

vendor/knative.dev/pkg/injection/sharedmain/main.go

@@ -1,3 +1,3 @@
 /*
 Copyright 2019 The Knative Authors
 
@@ -240,7 +240,11 @@
 
 // MainWithConfig runs the generic main flow for controllers and webhooks
 // with the given config.
-func MainWithConfig(ctx context.Context, component string, cfg *rest.Config, ctors ...injection.ControllerConstructor) {
+func MainWithConfig(parentCtx context.Context, component string, cfg *rest.Config, ctors ...injection.ControllerConstructor) {
+	ctx := context.WithoutCancel(parentCtx)
+	ctx, ctxCancel := context.WithCancel(ctx)
+	defer ctxCancel()
+
 	log.Printf("Registering %d clients", len(injection.Default.GetClients()))
 	log.Printf("Registering %d informer factories", len(injection.Default.GetInformerFactories()))
 	log.Printf("Registering %d informers", len(injection.Default.GetInformers()))
@@ -315,13 +319,23 @@
 	// and pass them in.
 	var wh *webhook.Webhook
 	if len(webhooks) > 0 {
+		// we use ctx that has the informers etc.
 		wh, err = webhook.New(ctx, webhooks)
 		if err != nil {
 			logger.Fatalw("Failed to create webhook", zap.Error(err))
 		}
 		eg.Go(func() error {
-			return wh.Run(ctx.Done())
+			// we use the parent context because we want the webhook
+			// to stop first prior to stopping the informers
+			defer ctxCancel()
+			return wh.Run(parentCtx.Done())
 		})
+	} else {
+		// no webhooks cancel the context when the parent is cancelled
+		go func() {
+			<-parentCtx.Done()
+			ctxCancel()
+		}()
 	}
 
 	// Start the injection clients and informers.
@@ -479,7 +493,9 @@
 		cmLabelReqs = append(cmLabelReqs, *req)
 	}
 	// TODO(mattmoor): This should itself take a context and be injection-based.
-	return cminformer.NewInformedWatcher(kc, system.Namespace(), cmLabelReqs...)
+	cmw := cminformer.NewInformedWatcher(kc, system.Namespace(), cmLabelReqs...)
+	cmw.Logger = logger.Named("cmw")
+	return cmw
 }
 
 // WatchLoggingConfigOrDie establishes a watch of the logging config or dies by
@@ -520,7 +536,8 @@
 
 // ControllersAndWebhooksFromCtors returns a list of the controllers and a list
 // of the webhooks created from the given constructors.
-func ControllersAndWebhooksFromCtors(ctx context.Context,
+func ControllersAndWebhooksFromCtors(
+	ctx context.Context,
 	cmw *cminformer.InformedWatcher,
 	ctors ...injection.ControllerConstructor,
 ) ([]*controller.Impl, []any) {