Add more logging around catalog source sync

Signed-off-by: Per Goncalves da Silva <[email protected]>

Author: Per Goncalves da Silva

pkg/controller/operators/catalog/operator.go

@@ -897,6 +897,11 @@ func (o *Operator) handleCatSrcDeletion(obj interface{}) {
 }
 
 func validateSourceType(logger *logrus.Entry, in *v1alpha1.CatalogSource) (out *v1alpha1.CatalogSource, continueSync bool, _ error) {
+	logger = logger.WithField("step", "validateSourceType")
+	logger.Info("validate source type start")
+	defer func() {
+		logger.WithField("continueSync", continueSync).Info("validate source type end")
+	}()
 	out = in
 	var err error
 	switch sourceType := out.Spec.SourceType; sourceType {
@@ -912,41 +917,44 @@ func validateSourceType(logger *logrus.Entry, in *v1alpha1.CatalogSource) (out *
 		err = fmt.Errorf("unknown sourcetype: %s", sourceType)
 	}
 	if err != nil {
+		logger.WithError(err).Error("error validating catalog source type")
 		out.SetError(v1alpha1.CatalogSourceSpecInvalidError, err)
 		return
 	}
 
 	// The sourceType is valid, clear all status (other than status conditions array) if there's existing invalid spec reason
 	if out.Status.Reason == v1alpha1.CatalogSourceSpecInvalidError {
+		logger.Info("catalog source has invalid spec")
 		out.Status = v1alpha1.CatalogSourceStatus{
 			Conditions: out.Status.Conditions,
 		}
 	}
 	continueSync = true
-
 	return
 }
 
 func (o *Operator) syncConfigMap(logger *logrus.Entry, in *v1alpha1.CatalogSource) (out *v1alpha1.CatalogSource, continueSync bool, syncError error) {
+	logger = logger.WithFields(logrus.Fields{
+		"configmap.namespace": in.Namespace,
+		"configmap.name":      in.Spec.ConfigMap,
+	})
+	logger.Info("checking catsrc configmap state")
+
 	out = in
 	if !(in.Spec.SourceType == v1alpha1.SourceTypeInternal || in.Spec.SourceType == v1alpha1.SourceTypeConfigmap) {
 		continueSync = true
+		logger.Info("catalog type is neither internal not configmap - bailing without error")
 		return
 	}
 
 	out = in.DeepCopy()
 
-	logger = logger.WithFields(logrus.Fields{
-		"configmap.namespace": in.Namespace,
-		"configmap.name":      in.Spec.ConfigMap,
-	})
-	logger.Info("checking catsrc configmap state")
-
 	var updateLabel bool
 	// Get the catalog source's config map
 	configMap, err := o.lister.CoreV1().ConfigMapLister().ConfigMaps(in.GetNamespace()).Get(in.Spec.ConfigMap)
 	// Attempt to look up the CM via api call if there is a cache miss
 	if apierrors.IsNotFound(err) {
+		logger.Info("configmap not found in cache - attempting with live client")
 		configMap, err = o.opClient.KubernetesInterface().CoreV1().ConfigMaps(in.GetNamespace()).Get(context.TODO(), in.Spec.ConfigMap, metav1.GetOptions{})
 		// Found cm in the cluster, add managed label to configmap
 		if err == nil {
@@ -961,6 +969,7 @@ func (o *Operator) syncConfigMap(logger *logrus.Entry, in *v1alpha1.CatalogSourc
 		}
 	}
 	if err != nil {
+		logger.WithError(err).Error("error getting configmap")
 		syncError = fmt.Errorf("failed to get catalog config map %s: %s", in.Spec.ConfigMap, err)
 		out.SetError(v1alpha1.CatalogSourceConfigMapError, syncError)
 		return
@@ -969,11 +978,11 @@ func (o *Operator) syncConfigMap(logger *logrus.Entry, in *v1alpha1.CatalogSourc
 	if wasOwned := ownerutil.EnsureOwner(configMap, in); !wasOwned || updateLabel {
 		configMap, err = o.opClient.KubernetesInterface().CoreV1().ConfigMaps(configMap.GetNamespace()).Update(context.TODO(), configMap, metav1.UpdateOptions{})
 		if err != nil {
+			logger.WithError(err).Error("error updating configmap")
 			syncError = fmt.Errorf("unable to write owner onto catalog source configmap - %v", err)
 			out.SetError(v1alpha1.CatalogSourceConfigMapError, syncError)
 			return
 		}
-
 		logger.Info("adopted configmap")
 	}
 
@@ -992,30 +1001,39 @@ func (o *Operator) syncConfigMap(logger *logrus.Entry, in *v1alpha1.CatalogSourc
 	}
 
 	continueSync = true
+	logger.Info("configmap catalog source sync'ed successfully")
 	return
 }
 
 func (o *Operator) syncRegistryServer(logger *logrus.Entry, in *v1alpha1.CatalogSource) (out *v1alpha1.CatalogSource, continueSync bool, syncError error) {
 	out = in.DeepCopy()
 
-	logger.Info("synchronizing registry server")
+	logger = logger.WithField("step", "syncRegistryServer")
+	logger.Info("registry server sync start")
+	defer func() {
+		logger.WithError(syncError).WithField("continueSync", continueSync).Info("registry server sync end")
+	}()
 	sourceKey := registry.CatalogKey{Name: in.GetName(), Namespace: in.GetNamespace()}
+
+	logger.Info("getting reconciler for catalog source")
 	srcReconciler := o.reconciler.ReconcilerForSource(in)
 	if srcReconciler == nil {
-		// TODO: Add failure status on catalogsource and remove from sources
+		// TODO: Add failure status on catalogsource and remove from sources )
 		syncError = fmt.Errorf("no reconciler for source type %s", in.Spec.SourceType)
 		out.SetError(v1alpha1.CatalogSourceRegistryServerError, syncError)
 		return
 	}
 
+	logger.Info("checking registry server health")
 	healthy, err := srcReconciler.CheckRegistryServer(logger, in)
 	if err != nil {
 		syncError = err
 		out.SetError(v1alpha1.CatalogSourceRegistryServerError, syncError)
 		return
 	}
 
-	logger.WithField("health", healthy).Infof("checked registry server health")
+	logger = logger.WithField("health", healthy)
+	logger.Info("checked registry server health")
 
 	if healthy && in.Status.RegistryServiceStatus != nil {
 		logger.Info("registry state good")
@@ -1052,7 +1070,9 @@ func (o *Operator) syncRegistryServer(logger *logrus.Entry, in *v1alpha1.Catalog
 			return
 		}
 		if out.Spec.UpdateStrategy.RegistryPoll.ParsingError != "" && out.Status.Reason != v1alpha1.CatalogSourceIntervalInvalidError {
-			out.SetError(v1alpha1.CatalogSourceIntervalInvalidError, errors.New(out.Spec.UpdateStrategy.RegistryPoll.ParsingError))
+			err := errors.New(out.Spec.UpdateStrategy.RegistryPoll.ParsingError)
+			logger.WithError(err).Info("failed to parse registry poll interval")
+			out.SetError(v1alpha1.CatalogSourceIntervalInvalidError, err)
 		}
 		logger.Infof("requeuing registry server sync based on polling interval %s", out.Spec.UpdateStrategy.Interval.Duration.String())
 		resyncPeriod := reconciler.SyncRegistryUpdateInterval(out, time.Now())
@@ -1061,13 +1081,18 @@ func (o *Operator) syncRegistryServer(logger *logrus.Entry, in *v1alpha1.Catalog
 	}
 
 	if err := o.sources.Remove(sourceKey); err != nil {
-		o.logger.WithError(err).Debug("error closing client connection")
+		o.logger.WithError(err).Info("error closing client connection")
 	}
 
 	return
 }
 
 func (o *Operator) syncConnection(logger *logrus.Entry, in *v1alpha1.CatalogSource) (out *v1alpha1.CatalogSource, continueSync bool, syncError error) {
+	logger = logger.WithField("step", "syncConnection")
+	logger.Info("sync connection start")
+	defer func() {
+		logger.WithError(syncError).WithField("continueSync", continueSync).Info("sync connection end")
+	}()
 	out = in.DeepCopy()
 
 	sourceKey := registry.CatalogKey{Name: in.GetName(), Namespace: in.GetNamespace()}
@@ -1101,6 +1126,7 @@ func (o *Operator) syncConnection(logger *logrus.Entry, in *v1alpha1.CatalogSour
 
 	source := o.sources.GetMeta(sourceKey)
 	if source == nil {
+		logger.Info("source not found - attempting to establish connection")
 		source, syncError = connectFunc()
 		if syncError != nil {
 			out.SetError(v1alpha1.CatalogSourceRegistryServerError, syncError)
@@ -1109,10 +1135,15 @@ func (o *Operator) syncConnection(logger *logrus.Entry, in *v1alpha1.CatalogSour
 
 		// Set connection status and return.
 		updateConnectionStateFunc(out, source)
+		logger.Info("successfully created source and updated connection state")
 		return
 	}
 
 	if source.Address != address {
+		logger.WithFields(logrus.Fields{
+			"source.address":        source.Address,
+			"catalogsource.address": address,
+		}).Info("source and catalog source addresses differ - creating a new source")
 		source, syncError = connectFunc()
 		if syncError != nil {
 			out.SetError(v1alpha1.CatalogSourceRegistryServerError, syncError)
@@ -1125,6 +1156,7 @@ func (o *Operator) syncConnection(logger *logrus.Entry, in *v1alpha1.CatalogSour
 
 	// GRPCConnectionState update must fail before
 	if out.Status.GRPCConnectionState == nil {
+		logger.Info("connection state is nil - attempting to update connection state")
 		updateConnectionStateFunc(out, source)
 	}
 
@@ -1152,7 +1184,10 @@ func (o *Operator) syncCatalogSources(obj interface{}) (syncError error) {
 		"catalogsource.name":      catsrc.Name,
 		"id":                      queueinformer.NewLoopID(),
 	})
-	logger.Info("syncing catalog source")
+	logger.Info("catalog source sync start")
+	defer func() {
+		logger.WithError(syncError).Info("catalog source sync end")
+	}()
 
 	syncFunc := func(in *v1alpha1.CatalogSource, chain []CatalogSourceSyncFunc) (out *v1alpha1.CatalogSource, syncErr error) {
 		out = in

pkg/controller/registry/reconciler/grpc.go

@@ -247,11 +247,18 @@ func correctImages(source grpcCatalogSourceDecorator, pod *corev1.Pod) bool {
 
 // EnsureRegistryServer ensures that all components of registry server are up to date.
 func (c *GrpcRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry, catalogSource *v1alpha1.CatalogSource) error {
+	logger = logger.WithField("step", "ensureRegistryServer")
+	logger.Info("ensure registry server start")
+	defer func() {
+		logger.Info("ensure registry service end")
+	}()
 	source := grpcCatalogSourceDecorator{CatalogSource: catalogSource, createPodAsUser: c.createPodAsUser, opmImage: c.opmImage, utilImage: c.utilImage}
 
 	// if service status is nil, we force create every object to ensure they're created the first time
+	logger.Info("validating registry service status")
 	valid, err := isRegistryServiceStatusValid(&source)
 	if err != nil {
+		logger.WithError(err).Info("couldn't validate registry service status")
 		return err
 	}
 	overwrite := !valid
@@ -260,24 +267,30 @@ func (c *GrpcRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry, cata
 	}
 
 	//TODO: if any of these error out, we should write a status back (possibly set RegistryServiceStatus to nil so they get recreated)
+	logger.Info("ensuring registry server service account")
 	sa, err := c.ensureSA(source)
 	if err != nil && !apierrors.IsAlreadyExists(err) {
+		logger.WithError(err).Error("couldn't ensure registry service account")
 		return pkgerrors.Wrapf(err, "error ensuring service account: %s", source.GetName())
 	}
 
 	sa, err = c.OpClient.GetServiceAccount(sa.GetNamespace(), sa.GetName())
 	if err != nil {
+		logger.WithError(err).Error("couldn't get registry service account")
 		return err
 	}
 
+	logger.Info("ensuring registry server pod")
 	defaultPodSecurityConfig, err := getDefaultPodContextConfig(c.OpClient, catalogSource.GetNamespace())
 	if err != nil {
+		logger.WithError(err).Error("couldn't get default pod security config")
 		return err
 	}
 
 	// recreate the pod if no existing pod is serving the latest image or correct spec
 	current, err := c.currentPodsWithCorrectImageAndSpec(logger, source, sa, defaultPodSecurityConfig)
 	if err != nil {
+		logger.WithError(err).Error("couldn't get current pods with correct image and spec")
 		return err
 	}
 	overwritePod := overwrite || len(current) == 0
@@ -287,29 +300,39 @@ func (c *GrpcRegistryReconciler) EnsureRegistryServer(logger *logrus.Entry, cata
 
 	pod, err := source.Pod(sa, defaultPodSecurityConfig)
 	if err != nil {
+		logger.WithError(err).Error("couldn't create registry pod")
 		return err
 	}
 	if err := c.ensurePod(logger, source, sa, defaultPodSecurityConfig, overwritePod); err != nil {
+		logger.WithError(err).Error("couldn't ensure registry pod")
 		return pkgerrors.Wrapf(err, "error ensuring pod: %s", pod.GetName())
 	}
 	if err := c.ensureUpdatePod(logger, sa, defaultPodSecurityConfig, source); err != nil {
+		logger.WithError(err).Info("error ensuring registry server pod")
 		if _, ok := err.(UpdateNotReadyErr); ok {
+			logger.WithError(err).Error("ensure update pod error is not of type UpdateNotReadyErr")
 			return err
 		}
 		return pkgerrors.Wrapf(err, "error ensuring updated catalog source pod: %s", pod.GetName())
 	}
+
+	logger.Info("ensuring registry server service")
 	service, err := source.Service()
 	if err != nil {
+		logger.WithError(err).Error("couldn't get service")
 		return err
 	}
 	if err := c.ensureService(source, overwrite); err != nil {
+		logger.WithError(err).Error("couldn't ensure service")
 		return pkgerrors.Wrapf(err, "error ensuring service: %s", service.GetName())
 	}
 
 	if overwritePod {
+		logger.Info("overwriting pod registry service status")
 		now := c.now()
 		service, err := source.Service()
 		if err != nil {
+			logger.WithError(err).Error("couldn't get service")
 			return err
 		}
 		catalogSource.Status.RegistryServiceStatus = &v1alpha1.RegistryServiceStatus{
@@ -581,14 +604,22 @@ func (c *GrpcRegistryReconciler) removePods(pods []*corev1.Pod, namespace string
 
 // CheckRegistryServer returns true if the given CatalogSource is considered healthy; false otherwise.
 func (c *GrpcRegistryReconciler) CheckRegistryServer(logger *logrus.Entry, catalogSource *v1alpha1.CatalogSource) (bool, error) {
+	logger = logger.WithField("step", "checkRegistryServer")
+	logger.Info("check registry server start")
+	defer func() {
+		logger.Info("check registry server end")
+	}()
+
 	source := grpcCatalogSourceDecorator{CatalogSource: catalogSource, createPodAsUser: c.createPodAsUser, opmImage: c.opmImage, utilImage: c.utilImage}
 
 	// The CheckRegistryServer function is called by the CatalogSoruce controller before the registry resources are created,
 	// returning a IsNotFound error will cause the controller to exit and never create the resources, so we should
 	// only return an error if it is something other than a NotFound error.
+	logger.Info("getting registry server resources (sa, svc, pods)")
 	serviceAccount := source.ServiceAccount()
 	serviceAccount, err := c.OpClient.GetServiceAccount(serviceAccount.GetNamespace(), serviceAccount.GetName())
 	if err != nil {
+		logger.WithError(err).Infof("registry service not healthy: could not get service account")
 		if !apierrors.IsNotFound(err) {
 			return false, err
 		}
@@ -597,27 +628,43 @@ func (c *GrpcRegistryReconciler) CheckRegistryServer(logger *logrus.Entry, catal
 
 	registryPodSecurityConfig, err := getDefaultPodContextConfig(c.OpClient, catalogSource.GetNamespace())
 	if err != nil {
+		logger.WithError(err).Info("registry service not healthy: could not get registry pod security config")
 		return false, err
 	}
 
 	// Check on registry resources
 	// TODO: add gRPC health check
 	service, err := c.currentService(source)
 	if err != nil {
+		logger.WithError(err).Info("registry service not healthy: could not get current service")
 		return false, err
 	}
+
 	currentPods, err := c.currentPodsWithCorrectImageAndSpec(logger, source, serviceAccount, registryPodSecurityConfig)
 	if err != nil {
+		logger.WithError(err).Info("registry service not healthy: could not get current pods")
 		return false, err
 	}
+
+	logger.Info("checking registry server resources")
+	currentServiceAccount := c.currentServiceAccount(source)
 	if len(currentPods) < 1 ||
-		service == nil || c.currentServiceAccount(source) == nil {
+		service == nil || currentServiceAccount == nil {
+		logger.WithFields(logrus.Fields{
+			"numCurrentPods":      len(currentPods),
+			"isServiceNil":        service == nil,
+			"isServiceAccountNil": serviceAccount == nil,
+		}).Info("registry service not healthy: one or more required resources are missing")
 		return false, nil
 	}
+
+	logger.Info("cleaning up registry server dead pods")
 	podsAreLive, e := detectAndDeleteDeadPods(logger, c.OpClient, currentPods, source.GetNamespace())
 	if e != nil {
+		logger.WithError(e).Info("registry service not healthy: could not detect and delete dead pods")
 		return false, fmt.Errorf("error deleting dead pods: %v", e)
 	}
+
 	return podsAreLive, nil
 }