Merge pull request #1721 from njhale/fix-crd-adopt

Bug 1861636: fix(operator): re-adopt manually disowned crds

pkg/controller/operators/adoption_controller.go

@@ -59,9 +59,14 @@ func (r *AdoptionReconciler) SetupWithManager(mgr ctrl.Manager) error {
  return err
  }
 
- enqueueCSV := &handler.EnqueueRequestsFromMapFunc{
- ToRequests: handler.ToRequestsFunc(r.mapToClusterServiceVersions),
- }
+ var (
+ enqueueCSV = &handler.EnqueueRequestsFromMapFunc{
+ ToRequests: handler.ToRequestsFunc(r.mapToClusterServiceVersions),
+ }
+ enqueueProviders = &handler.EnqueueRequestsFromMapFunc{
+ ToRequests: handler.ToRequestsFunc(r.mapToProviders),
+ }
+ )
  err = ctrl.NewControllerManagedBy(mgr).
  For(&operatorsv1alpha1.ClusterServiceVersion{}).
  Watches(&source.Kind{Type: &appsv1.Deployment{}}, enqueueCSV).
@@ -74,7 +79,7 @@ func (r *AdoptionReconciler) SetupWithManager(mgr ctrl.Manager) error {
  Watches(&source.Kind{Type: &rbacv1.RoleBinding{}}, enqueueCSV).
  Watches(&source.Kind{Type: &rbacv1.ClusterRole{}}, enqueueCSV).
  Watches(&source.Kind{Type: &rbacv1.ClusterRoleBinding{}}, enqueueCSV).
- Watches(&source.Kind{Type: &apiextensionsv1.CustomResourceDefinition{}}, enqueueCSV).
+ Watches(&source.Kind{Type: &apiextensionsv1.CustomResourceDefinition{}}, enqueueProviders).
  Watches(&source.Kind{Type: &apiregistrationv1.APIService{}}, enqueueCSV).
  Watches(&source.Kind{Type: &operatorsv1alpha1.Subscription{}}, enqueueCSV).
  Complete(reconcile.Func(r.ReconcileClusterServiceVersion))
@@ -110,7 +115,7 @@ func NewAdoptionReconciler(cli client.Client, log logr.Logger, scheme *runtime.S
 func (r *AdoptionReconciler) ReconcileSubscription(req ctrl.Request) (reconcile.Result, error) {
  // Set up a convenient log object so we don't have to type request over and over again
  log := r.log.WithValues("request", req)
- log.V(1).Info("reconciling subscription")
+ log.V(4).Info("reconciling subscription")
 
  // Fetch the Subscription from the cache
  ctx := context.TODO()
@@ -173,7 +178,7 @@ func (r *AdoptionReconciler) ReconcileSubscription(req ctrl.Request) (reconcile.
 func (r *AdoptionReconciler) ReconcileClusterServiceVersion(req ctrl.Request) (reconcile.Result, error) {
  // Set up a convenient log object so we don't have to type request over and over again
  log := r.log.WithValues("request", req)
- log.V(1).Info("reconciling ClusterServiceVersion")
+ log.V(4).Info("reconciling csv")
 
  // Fetch the CSV from the cache
  ctx := context.TODO()
@@ -228,7 +233,6 @@ func (r *AdoptionReconciler) adoptComponents(ctx context.Context, csv *operators
  defer mu.Unlock()
  errs = append(errs, err)
  }()
- continue
  }
 
  for _, component := range components {
@@ -371,11 +375,7 @@ func (r *AdoptionReconciler) adoptees(ctx context.Context, operator decorators.O
  components = append(components, crd)
  }
 
- if err := utilerrors.NewAggregate(errs); err != nil {
- return nil, err
- }
-
- return components, nil
+ return components, utilerrors.NewAggregate(errs)
 }
 
 func (r *AdoptionReconciler) adoptInstallPlan(ctx context.Context, operator *decorators.Operator, latest *operatorsv1alpha1.InstallPlan) error {
@@ -423,12 +423,20 @@ func (r *AdoptionReconciler) mapToSubscriptions(obj handler.MapObject) (requests
  ctx := context.TODO()
  subs := &operatorsv1alpha1.SubscriptionList{}
  if err := r.List(ctx, subs, client.InNamespace(obj.Meta.GetNamespace())); err != nil {
- r.log.Error(err, "couldn't list subscriptions")
+ r.log.Error(err, "error listing subscriptions")
  }
 
+ visited := map[types.NamespacedName]struct{}{}
  for _, sub := range subs.Items {
  nsn := types.NamespacedName{Namespace: sub.GetNamespace(), Name: sub.GetName()}
+
+ if _, ok := visited[nsn]; ok {
+ // Already requested
+ continue
+ }
+
  requests = append(requests, reconcile.Request{NamespacedName: nsn})
+ visited[nsn] = struct{}{}
  }
 
  return
@@ -440,24 +448,51 @@ func (r *AdoptionReconciler) mapToClusterServiceVersions(obj handler.MapObject)
  }
 
  // Get all owner CSV from owner labels if cluster scoped
- if obj.Meta.GetNamespace() == metav1.NamespaceAll {
+ namespace := obj.Meta.GetNamespace()
+ if namespace == metav1.NamespaceAll {
  name, ns, ok := ownerutil.GetOwnerByKindLabel(obj.Meta, operatorsv1alpha1.ClusterServiceVersionKind)
  if ok {
  nsn := types.NamespacedName{Namespace: ns, Name: name}
  requests = append(requests, reconcile.Request{NamespacedName: nsn})
  }
-
  return
  }
 
  // Get all owner CSVs from OwnerReferences
  owners := ownerutil.GetOwnersByKind(obj.Meta, operatorsv1alpha1.ClusterServiceVersionKind)
  for _, owner := range owners {
- nsn := types.NamespacedName{Namespace: obj.Meta.GetNamespace(), Name: owner.Name}
+ nsn := types.NamespacedName{Namespace: namespace, Name: owner.Name}
  requests = append(requests, reconcile.Request{NamespacedName: nsn})
  }
 
- // TODO(njhale): Requeue CSVs on CRD changes
+ return
+}
+
+func (r *AdoptionReconciler) mapToProviders(obj handler.MapObject) (requests []reconcile.Request) {
+ if obj.Meta == nil {
+ return nil
+ }
+
+ var (
+ ctx = context.TODO()
+ csvs = &operatorsv1alpha1.ClusterServiceVersionList{}
+ )
+ if err := r.List(ctx, csvs); err != nil {
+ r.log.Error(err, "error listing csvs")
+ return
+ }
+
+ for _, csv := range csvs.Items {
+ request := reconcile.Request{
+ NamespacedName: types.NamespacedName{Namespace: csv.GetNamespace(), Name: csv.GetName()},
+ }
+ for _, provided := range csv.Spec.CustomResourceDefinitions.Owned {
+ if provided.Name == obj.Meta.GetName() {
+ requests = append(requests, request)
+ break
+ }
+ }
+ }
 
  return
 }

pkg/controller/operators/adoption_controller_test.go

@@ -184,9 +184,23 @@ var _ = Describe("Adoption Controller", func() {
 
  Context("that has an existing installed csv", func() {
 
+ var (
+ providedCRD *apiextensionsv1.CustomResourceDefinition
+ )
+
  BeforeEach(func() {
+ providedCRD = fixtures.Fill(&apiextensionsv1.CustomResourceDefinition{}).(*apiextensionsv1.CustomResourceDefinition)
  installed = &operatorsv1alpha1.ClusterServiceVersion{
  Spec: operatorsv1alpha1.ClusterServiceVersionSpec{
+ CustomResourceDefinitions: operatorsv1alpha1.CustomResourceDefinitions{
+ Owned: []operatorsv1alpha1.CRDDescription{
+ {
+ Name: providedCRD.GetName(),
+ Kind: providedCRD.Spec.Names.Kind,
+ Version: providedCRD.Spec.Versions[0].Name,
+ },
+ },
+ },
  InstallStrategy: operatorsv1alpha1.NamedInstallStrategy{
  StrategyName: operatorsv1alpha1.InstallStrategyNameDeployment,
  StrategySpec: operatorsv1alpha1.StrategyDetailsDeployment{
@@ -215,6 +229,49 @@ var _ = Describe("Adoption Controller", func() {
  })
  })
 
+ Context("with an existing provided CRD", func() {
+ BeforeEach(func() {
+ Eventually(func() error {
+ return k8sClient.Create(ctx, providedCRD)
+ }, timeout, interval).Should(Succeed())
+ created = append(created, providedCRD)
+
+ Eventually(func() (map[string]string, error) {
+ latest := &apiextensionsv1.CustomResourceDefinition{}
+ err := k8sClient.Get(ctx, testobj.NamespacedName(providedCRD), latest)
+
+ return latest.GetLabels(), err
+ }, timeout, interval).Should(HaveKey(componentLabelKey))
+ })
+
+ Context("when its component label is removed", func() {
+ BeforeEach(func() {
+ Eventually(func() error {
+ latest := &apiextensionsv1.CustomResourceDefinition{}
+ if err := k8sClient.Get(ctx, testobj.NamespacedName(providedCRD), latest); err != nil {
+ return err
+ }
+
+ if len(latest.GetLabels()) == 0 {
+ return nil
+ }
+ delete(latest.GetLabels(), componentLabelKey)
+
+ return k8sClient.Update(ctx, latest)
+ }, timeout, interval).Should(Succeed())
+ })
+
+ It("should be relabelled", func() {
+ Eventually(func() (map[string]string, error) {
+ latest := &apiextensionsv1.CustomResourceDefinition{}
+ err := k8sClient.Get(ctx, testobj.NamespacedName(providedCRD), latest)
+
+ return latest.GetLabels(), err
+ }, timeout, interval).Should(HaveKey(componentLabelKey))
+ })
+ })
+ })
+
  Context("that has resources owned by the installed csv", func() {
  var (
  components []testobj.RuntimeMetaObject
@@ -289,10 +346,6 @@ var _ = Describe("Adoption Controller", func() {
  ownerLabels,
  fixtures.Fill(&rbacv1.ClusterRoleBinding{}),
  ),
- testobj.WithLabels(
- ownerLabels,
- fixtures.Fill(&apiextensionsv1.CustomResourceDefinition{}),
- ),
  testobj.WithLabels(
  ownerLabels,
  fixtures.Fill(&apiregistrationv1.APIService{}),
@@ -306,7 +359,7 @@ var _ = Describe("Adoption Controller", func() {
  }
  })
 
- Specify("component label", func() {
+ Specify("a component label", func() {
  for _, component := range components {
  Eventually(func() (map[string]string, error) {
  err := k8sClient.Get(ctx, testobj.NamespacedName(component), component)