Propagate manifests paths earlier to operator status (#2106)

Author: dsimansk

pkg/reconciler/common/install.go

@@ -55,6 +55,14 @@ func Install(ctx context.Context, manifest *mf.Manifest, instance base.KComponen
 		status.MarkInstallFailed(err.Error())
 		return fmt.Errorf("failed to apply (cluster)rolebindings: %w", err)
 	}
+	// Webhook configs are placeholder to be reconciled and configured by webhook controllers, not fully operational yet.
+	// They are owned by SYSTEM_NAMESPACE and reconciled once webhook deployment is started.
+	// In some cases a webhook config might be left on the cluster from previous uncompleted install/reinstall attempts.
+	// Such pre-existing webhook config can block creation of ConfigMap and other resource that are validated, resulting
+	// in a deadlock installation loop. For this reasons
+	if err := InstallWebhookConfigs(ctx, manifest, instance); err != nil {
+		return err
+	}
 	if err := manifest.Filter(mf.Not(mf.Any(role, rolebinding, webhook, webhookDependentResources))).Apply(); err != nil {
 		status.MarkInstallFailed(err.Error())
 		if ks, ok := instance.(*v1beta1.KnativeServing); ok && strings.Contains(err.Error(), gatewayNotMatch) &&
@@ -71,6 +79,7 @@ func Install(ctx context.Context, manifest *mf.Manifest, instance base.KComponen
 
 // InstallWebhookConfigs applies the Webhook manifest resources and updates the given status accordingly.
 func InstallWebhookConfigs(ctx context.Context, manifest *mf.Manifest, instance base.KComponent) error {
+	logging.FromContext(ctx).Debug("Installing webhook configurations")
 	status := instance.GetStatus()
 	if err := manifest.Filter(webhook).Apply(); err != nil {
 		status.MarkInstallFailed(err.Error())

pkg/reconciler/common/install_test.go

@@ -56,9 +56,9 @@ func TestInstall(t *testing.T) {
 		*clusterRole.DeepCopy(),
 		*roleBinding.DeepCopy(),
 		*clusterRoleBinding.DeepCopy(),
-		*deployment.DeepCopy(),
 		*mutatingWebhookConfiguration.DeepCopy(),
 		*validatingWebhookConfiguration.DeepCopy(),
+		*deployment.DeepCopy(),
 	}
 
 	client := &fakeClient{}
@@ -81,10 +81,6 @@ func TestInstall(t *testing.T) {
 		t.Fatalf("Install() = %v, want no error", err)
 	}
 
-	if err := InstallWebhookConfigs(context.TODO(), &manifest, instance); err != nil {
-		t.Fatalf("InstallWebhookConfigs() = %v, want no error", err)
-	}
-
 	if err := MarkStatusSuccess(context.TODO(), &manifest, instance); err != nil {
 		t.Fatalf("MarkStatusSuccess() = %v, want no error", err)
 	}

pkg/reconciler/knativeeventing/knativeeventing.go

@@ -141,9 +141,8 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, ke *v1beta1.KnativeEvent
 		r.transform,
 		r.handleTLSResources,
 		manifests.Install,
+		manifests.SetManifestPaths, // setting path right after applying manifests to populate paths
 		common.CheckDeployments,
-		common.InstallWebhookConfigs,
-		manifests.SetManifestPaths,
 		common.MarkStatusSuccess,
 		common.DeleteObsoleteResources(ctx, ke, r.installed),
 	}

pkg/reconciler/knativeserving/knativeserving.go

@@ -88,6 +88,7 @@ func (r *Reconciler) FinalizeKind(ctx context.Context, original *v1beta1.Knative
 	}
 
 	if manifest == nil {
+		logger.Warnf("No manifest found; no cluster-scoped resources will be finalized")
 		return nil
 	}
 
@@ -123,10 +124,9 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, ks *v1beta1.KnativeServi
 		r.appendExtensionManifests,
 		r.transform,
 		manifests.Install,
+		manifests.SetManifestPaths, // setting path right after applying manifests to populate paths
 		common.CheckDeployments,
-		common.InstallWebhookConfigs,
 		common.InstallWebhookDependentResources,
-		manifests.SetManifestPaths,
 		common.MarkStatusSuccess,
 		common.DeleteObsoleteResources(ctx, ks, r.installed),
 	}