Merge pull request #1533 from openshift-cherrypick-robot/cherry-pick-1532-to-release-4.12

[release-4.12] OCPBUGS-17139: make webhook connection failure a warning in log

Author: openshift-merge-robot

pkg/operator/webhooksupportabilitycontroller/degraded_webhook.go

@@ -22,6 +22,9 @@ type webhookInfo struct {
 	Service               *serviceReference
 	CABundle              []byte
 	FailurePolicyIsIgnore bool
+	// TimeoutSeconds specifies the timeout for a webhook.
+	// After the timeout passes, the webhook call will be ignored or the API call will fail
+	TimeoutSeconds *int32
 }
 
 // serviceReference generically represents a service reference
@@ -49,7 +52,7 @@ func (c *webhookSupportabilityController) updateWebhookConfigurationDegraded(ctx
 				serviceMsgs = append(serviceMsgs, msg)
 				continue
 			}
-			err = c.assertConnect(ctx, webhook.Service, webhook.CABundle)
+			err = c.assertConnect(ctx, webhook.Name, webhook.Service, webhook.CABundle, webhook.TimeoutSeconds)
 			if err != nil {
 				msg := fmt.Sprintf("%s: %s", webhook.Name, err)
 				if webhook.FailurePolicyIsIgnore {
@@ -94,7 +97,7 @@ func (c *webhookSupportabilityController) assertService(reference *serviceRefere
 }
 
 // assertConnect performs a dns lookup of service, opens a tcp connection, and performs a tls handshake.
-func (c *webhookSupportabilityController) assertConnect(ctx context.Context, reference *serviceReference, caBundle []byte) error {
+func (c *webhookSupportabilityController) assertConnect(ctx context.Context, webhookName string, reference *serviceReference, caBundle []byte, webhookTimeoutSeconds *int32) error {
 	host := reference.Name + "." + reference.Namespace + ".svc"
 	port := "443"
 	if reference.Port != nil {
@@ -104,6 +107,10 @@ func (c *webhookSupportabilityController) assertConnect(ctx context.Context, ref
 	if len(caBundle) > 0 {
 		rootCAs.AppendCertsFromPEM(caBundle)
 	}
+	timeout := 10 * time.Second
+	if webhookTimeoutSeconds != nil {
+		timeout = time.Duration(*webhookTimeoutSeconds) * time.Second
+	}
 	// the last error that occurred in the loop below
 	var err error
 	// retry up to 3 times on error
@@ -114,7 +121,7 @@ func (c *webhookSupportabilityController) assertConnect(ctx context.Context, ref
 		case <-time.After(time.Duration(i) * time.Second):
 		}
 		dialer := &tls.Dialer{
-			NetDialer: &net.Dialer{Timeout: 1 * time.Second},
+			NetDialer: &net.Dialer{Timeout: timeout},
 			Config: &tls.Config{
 				ServerName: host,
 				RootCAs:    rootCAs,
@@ -124,8 +131,8 @@ func (c *webhookSupportabilityController) assertConnect(ctx context.Context, ref
 		conn, err = dialer.DialContext(ctx, "tcp", net.JoinHostPort(host, port))
 		if err != nil {
 			if i != 2 {
-				// log err since only last one is reported
-				runtime.HandleError(err)
+				// log warning since only last one is reported
+				klog.Warningf("failed to connect to webhook %q via service %q: %v", webhookName, net.JoinHostPort(host, port), err)
 			}
 			continue
 		}

pkg/operator/webhooksupportabilitycontroller/degraded_webhook_admission.go

@@ -27,6 +27,7 @@ func (c *webhookSupportabilityController) updateMutatingAdmissionWebhookConfigur
 				Name:                  webhook.Name,
 				CABundle:              webhook.ClientConfig.CABundle,
 				FailurePolicyIsIgnore: webhook.FailurePolicy != nil && *webhook.FailurePolicy == admissionregistrationv1.Ignore,
+				TimeoutSeconds:        webhook.TimeoutSeconds,
 			}
 			if webhook.ClientConfig.Service != nil {
 				info.Service = &serviceReference{
@@ -58,6 +59,7 @@ func (c *webhookSupportabilityController) updateValidatingAdmissionWebhookConfig
 				Name:                  webhook.Name,
 				CABundle:              webhook.ClientConfig.CABundle,
 				FailurePolicyIsIgnore: webhook.FailurePolicy != nil && (*webhook.FailurePolicy == v1.Ignore),
+				TimeoutSeconds:        webhook.TimeoutSeconds,
 			}
 
 			if webhook.ClientConfig.Service != nil {

pkg/operator/webhooksupportabilitycontroller/degraded_webhook_conversion.go

@@ -2,9 +2,10 @@ package webhooksupportabilitycontroller
 
 import (
 	"context"
-
 	operatorv1 "github.com/openshift/api/operator/v1"
 	"github.com/openshift/library-go/pkg/operator/v1helpers"
+
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/labels"
 )
@@ -21,24 +22,31 @@ func (c *webhookSupportabilityController) updateCRDConversionWebhookConfiguratio
 	}
 	var webhookInfos []webhookInfo
 	for _, crd := range crds {
-		conversion := crd.Spec.Conversion
-		if conversion == nil || conversion.Strategy != v1.WebhookConverter {
-			continue
-		}
-		clientConfig := conversion.Webhook.ClientConfig
-		if clientConfig == nil || clientConfig.Service == nil {
+		if !hasCRDConversionWebhookConfiguration(crd) {
 			continue
 		}
 		info := webhookInfo{
 			Name:     crd.Name,
-			CABundle: clientConfig.CABundle,
+			CABundle: crd.Spec.Conversion.Webhook.ClientConfig.CABundle,
 			Service: &serviceReference{
-				Namespace: clientConfig.Service.Namespace,
-				Name:      clientConfig.Service.Name,
-				Port:      clientConfig.Service.Port,
+				Namespace: crd.Spec.Conversion.Webhook.ClientConfig.Service.Namespace,
+				Name:      crd.Spec.Conversion.Webhook.ClientConfig.Service.Name,
+				Port:      crd.Spec.Conversion.Webhook.ClientConfig.Service.Port,
 			},
 		}
 		webhookInfos = append(webhookInfos, info)
 	}
 	return c.updateWebhookConfigurationDegraded(ctx, condition, webhookInfos)
 }
+
+func hasCRDConversionWebhookConfiguration(crd *apiextensionsv1.CustomResourceDefinition) bool {
+	conversion := crd.Spec.Conversion
+	if conversion == nil || conversion.Strategy != v1.WebhookConverter {
+		return false
+	}
+	clientConfig := conversion.Webhook.ClientConfig
+	if clientConfig == nil || clientConfig.Service == nil {
+		return false
+	}
+	return true
+}

pkg/operator/webhooksupportabilitycontroller/webhook_supportability_controller.go

@@ -7,6 +7,8 @@ import (
 	"github.com/openshift/library-go/pkg/operator/events"
 	"github.com/openshift/library-go/pkg/operator/management"
 	"github.com/openshift/library-go/pkg/operator/v1helpers"
+
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextensionsinformers "k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions"
 	apiextensionslistersv1 "k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1"
 	admissionregistrationlistersv1 "k8s.io/client-go/listers/admissionregistration/v1"
@@ -43,14 +45,19 @@ func NewWebhookSupportabilityController(
 			kubeInformersForAllNamespaces.Admissionregistration().V1().MutatingWebhookConfigurations().Informer(),
 			kubeInformersForAllNamespaces.Admissionregistration().V1().ValidatingWebhookConfigurations().Informer(),
 			kubeInformersForAllNamespaces.Core().V1().Services().Informer(),
-			apiExtensionsInformers.Apiextensions().V1().CustomResourceDefinitions().Informer(),
 		).
+		WithFilteredEventsInformers(func(obj interface{}) bool {
+			if crd, ok := obj.(*apiextensionsv1.CustomResourceDefinition); ok {
+				return hasCRDConversionWebhookConfiguration(crd)
+			}
+			return true // re-queue just in case, the checks are fairly cheap
+		}, apiExtensionsInformers.Apiextensions().V1().CustomResourceDefinitions().Informer()).
 		WithSync(c.sync).
 		ToController("webhookSupportabilityController", recorder)
 	return c
 }
 
-func (c *webhookSupportabilityController) sync(ctx context.Context, controllerContext factory.SyncContext) error {
+func (c *webhookSupportabilityController) sync(ctx context.Context, _ factory.SyncContext) error {
 	operatorSpec, _, _, err := c.operatorClient.GetOperatorState()
 	if err != nil {
 		return err