perf improvement for unskipped streams

Author: gabemontero

pkg/stub/config.go

@@ -67,14 +67,15 @@ func (h *Handler) SpecValidation(cfg *v1.Config) error {
 // second boolean: does the config change require a samples upsert; for example, simply adding
 // to a skip list does not require a samples upsert
 // third boolean: even if an upsert is not needed, update the config instance to clear out image import errors
-func (h *Handler) VariableConfigChanged(cfg *v1.Config) (bool, bool, bool) {
+// map: imagestreams that were unskipped (imagestream updates can be expensive, so an optimizatino)
+func (h *Handler) VariableConfigChanged(cfg *v1.Config) (bool, bool, bool, map[string]bool) {
+	unskippedStreams := map[string]bool{}
 	if cfg.Spec.SamplesRegistry != cfg.Status.SamplesRegistry {
 		logrus.Printf("SamplesRegistry changed from %s to %s", cfg.Status.SamplesRegistry, cfg.Spec.SamplesRegistry)
-		return true, true, false
+		return true, true, false, unskippedStreams
 	}
 
 	logrus.Debugf("cfg skipped streams %#v", cfg.Spec.SkippedImagestreams)
-	unskippedStreams := map[string]bool{}
 	streamsThatWereSkipped := map[string]bool{}
 	for _, stream := range cfg.Status.SkippedImagestreams {
 		streamsThatWereSkipped[stream] = true
@@ -88,12 +89,12 @@ func (h *Handler) VariableConfigChanged(cfg *v1.Config) (bool, bool, bool) {
 		logrus.Printf("SkippedImagestreams changed in size from %#v to %#v", cfg.Status.SkippedImagestreams, cfg.Spec.SkippedImagestreams)
 		if len(cfg.Spec.SkippedImagestreams) < len(cfg.Status.SkippedImagestreams) {
 			// skip list reduced, meaning we need to upsert some samples we were ignoring
-			return true, true, false
+			return true, true, false, unskippedStreams
 		}
 		// even if the skipped list has been increased, if a stream we were skipping
 		// has been removed, we need to upsert; assumes buildSkipFilters called beforehand
 		if len(unskippedStreams) > 0 {
-			return true, true, true
+			return true, true, true, unskippedStreams
 		}
 
 		// otherwise, we've only added to the skip list, so don't upsert,but also see if we
@@ -110,7 +111,7 @@ func (h *Handler) VariableConfigChanged(cfg *v1.Config) (bool, bool, bool) {
 				// we do not break here cause we want to clear out all possible streams
 			}
 		}
-		return true, false, clearImageImportErrors
+		return true, false, clearImageImportErrors, unskippedStreams
 	}
 
 	clearImageImportErrors := false
@@ -129,33 +130,33 @@ func (h *Handler) VariableConfigChanged(cfg *v1.Config) (bool, bool, bool) {
 		}
 	}
 	if changeInContent {
-		return changeInContent, changeInContent, clearImageImportErrors
+		return changeInContent, changeInContent, clearImageImportErrors, unskippedStreams
 	}
 
 	if len(cfg.Spec.SkippedTemplates) != len(cfg.Status.SkippedTemplates) {
 		logrus.Printf("SkippedTemplates changed from %#v to %#v", cfg.Status.SkippedTemplates, cfg.Spec.SkippedTemplates)
 		if len(cfg.Spec.SkippedTemplates) < len(cfg.Status.SkippedTemplates) {
-			return true, true, false
+			return true, true, false, unskippedStreams
 		}
 		for _, tpl := range cfg.Status.SkippedTemplates {
 			// even if the skipped list has been increased, if a tpl we were skipping
 			// has been removed, we need to upsert; assumes buildSkipFilters called beforehand
 			if _, ok := h.skippedTemplates[tpl]; !ok {
-				return true, true, false
+				return true, true, false, unskippedStreams
 			}
 		}
-		return true, false, false
+		return true, false, false, unskippedStreams
 	}
 
 	for _, skip := range cfg.Status.SkippedTemplates {
 		if _, ok := h.skippedTemplates[skip]; !ok {
 			logrus.Printf("SkippedTemplates changed in content ... minimally %s has been removed", skip)
-			return true, true, false
+			return true, true, false, unskippedStreams
 		}
 	}
 
 	logrus.Debugf("Incoming Config unchanged from last processed version")
-	return false, false, false
+	return false, false, false, unskippedStreams
 }
 
 func (h *Handler) buildSkipFilters(opcfg *v1.Config) {

pkg/stub/handler.go

@@ -537,8 +537,9 @@ func (h *Handler) Handle(event v1.Event) error {
 		configChanged := false
 		configChangeRequiresUpsert := false
 		configChangeRequiresImportErrorUpdate := false
+		unskippedStreams := map[string]bool{}
 		if cfg.Spec.ManagementState == cfg.Status.ManagementState {
-			configChanged, configChangeRequiresUpsert, configChangeRequiresImportErrorUpdate = h.VariableConfigChanged(cfg)
+			configChanged, configChangeRequiresUpsert, configChangeRequiresImportErrorUpdate, unskippedStreams = h.VariableConfigChanged(cfg)
 			logrus.Debugf("config changed %v upsert needed %v import error upd needed %v exists/true %v progressing/false %v op version %s status version %s",
 				configChanged,
 				configChangeRequiresUpsert,
@@ -559,7 +560,7 @@ func (h *Handler) Handle(event v1.Event) error {
 				// and see if any samples were deleted while samples operator was down
 				h.buildFileMaps(cfg, false)
 				// passing in false means if the samples is present, we leave it alone
-				return h.createSamples(cfg, false)
+				return h.createSamples(cfg, false, unskippedStreams)
 			}
 			// if config changed requiring an upsert, but a prior config action is still in progress,
 			// reset in progress to false and return; the next event should drive the actual
@@ -668,7 +669,7 @@ func (h *Handler) Handle(event v1.Event) error {
 				return err
 			}
 
-			err = h.createSamples(cfg, true)
+			err = h.createSamples(cfg, true, unskippedStreams)
 			if err != nil {
 				h.processError(cfg, v1.ImageChangesInProgress, corev1.ConditionUnknown, err, "error creating samples: %v")
 				logrus.Printf("CRDUPDATE setting in progress to unknown")
@@ -686,6 +687,11 @@ func (h *Handler) Handle(event v1.Event) error {
 			progressing.Status = corev1.ConditionTrue
 			for isName := range h.imagestreamFile {
 				_, skipped := h.skippedImagestreams[isName]
+				unskipping := len(unskippedStreams) > 0
+				_, unskipped := unskippedStreams[isName]
+				if unskipping && !unskipped {
+					continue
+				}
 				if !cfg.NameInReason(progressing.Reason, isName) && !skipped {
 					progressing.Reason = progressing.Reason + isName + " "
 				}
@@ -780,7 +786,7 @@ func (h *Handler) setSampleManagedLabelToFalse(kind, name string) error {
 	return nil
 }
 
-func (h *Handler) createSamples(cfg *v1.Config, updateIfPresent bool) error {
+func (h *Handler) createSamples(cfg *v1.Config, updateIfPresent bool, unskippedStreams map[string]bool) error {
 	// first, got through the list and prime our upsert cache
 	// prior to any actual upserts
 	imagestreams := []*imagev1.ImageStream{}
@@ -789,6 +795,15 @@ func (h *Handler) createSamples(cfg *v1.Config, updateIfPresent bool) error {
 		if err != nil {
 			return err
 		}
+
+		// if unskippedStreams has >0 entries, then we are down this path to only upsert the streams
+		// listed there
+		if len(unskippedStreams) > 0 {
+			if _, ok := unskippedStreams[imagestream.Name]; !ok {
+				continue
+			}
+		}
+
 		if _, isok := h.skippedImagestreams[imagestream.Name]; !isok {
 			if updateIfPresent {
 				cache.AddUpsert(imagestream.Name)