do not setup ownerrefs for clusterscoped/cross-namespace objects

Author: bparees

pkg/resource/caconfig.go

@@ -11,7 +11,6 @@ import (
 	configlisters "github.com/openshift/client-go/config/listers/config/v1"
 	imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1"
 	"github.com/openshift/cluster-image-registry-operator/pkg/parameters"
-	"github.com/openshift/cluster-image-registry-operator/pkg/util"
 )
 
 var _ Mutator = &generatorCAConfig{}
@@ -24,7 +23,6 @@ type generatorCAConfig struct {
 	imageConfigName       string
 	name                  string
 	namespace             string
-	owner                 metav1.OwnerReference
 }
 
 func newGeneratorCAConfig(lister corelisters.ConfigMapNamespaceLister, imageConfigLister configlisters.ImageLister, openshiftConfigLister corelisters.ConfigMapNamespaceLister, client coreset.CoreV1Interface, params *parameters.Globals, cr *imageregistryv1.Config) *generatorCAConfig {
@@ -36,7 +34,6 @@ func newGeneratorCAConfig(lister corelisters.ConfigMapNamespaceLister, imageConf
 		imageConfigName:       params.ImageConfig.Name,
 		name:                  params.CAConfig.Name,
 		namespace:             params.Deployment.Namespace,
-		owner:                 util.AsOwner(cr),
 	}
 }
 
@@ -82,8 +79,6 @@ func (gcac *generatorCAConfig) expected() (runtime.Object, error) {
 		}
 	}
 
-	util.AddOwnerRefToObject(cm, gcac.owner)
-
 	return cm, nil
 }
 
@@ -106,3 +101,7 @@ func (gcac *generatorCAConfig) Update(o runtime.Object) (bool, error) {
 func (gcac *generatorCAConfig) Delete(opts *metav1.DeleteOptions) error {
 	return gcac.client.ConfigMaps(gcac.GetNamespace()).Delete(gcac.GetName(), opts)
 }
+
+func (g *generatorCAConfig) Owned() bool {
+	return true
+}

pkg/resource/clusterrole.go

@@ -8,22 +8,19 @@ import (
 	rbaclisters "k8s.io/client-go/listers/rbac/v1"
 
 	imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1"
-	"github.com/openshift/cluster-image-registry-operator/pkg/util"
 )
 
 var _ Mutator = &generatorClusterRole{}
 
 type generatorClusterRole struct {
 	lister rbaclisters.ClusterRoleLister
 	client rbacset.RbacV1Interface
-	owner  metav1.OwnerReference
 }
 
 func newGeneratorClusterRole(lister rbaclisters.ClusterRoleLister, client rbacset.RbacV1Interface, cr *imageregistryv1.Config) *generatorClusterRole {
 	return &generatorClusterRole{
 		lister: lister,
 		client: client,
-		owner:  util.AsOwner(cr),
 	}
 }
 
@@ -94,8 +91,6 @@ func (gcr *generatorClusterRole) expected() (runtime.Object, error) {
 		},
 	}
 
-	util.AddOwnerRefToObject(role, gcr.owner)
-
 	return role, nil
 }
 
@@ -118,3 +113,7 @@ func (gcr *generatorClusterRole) Update(o runtime.Object) (bool, error) {
 func (gcr *generatorClusterRole) Delete(opts *metav1.DeleteOptions) error {
 	return gcr.client.ClusterRoles().Delete(gcr.GetName(), opts)
 }
+
+func (g *generatorClusterRole) Owned() bool {
+	return true
+}

pkg/resource/clusterrolebinding.go

@@ -9,7 +9,6 @@ import (
 
 	imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1"
 	"github.com/openshift/cluster-image-registry-operator/pkg/parameters"
-	"github.com/openshift/cluster-image-registry-operator/pkg/util"
 )
 
 var _ Mutator = &generatorClusterRoleBinding{}
@@ -19,7 +18,6 @@ type generatorClusterRoleBinding struct {
 	client      rbacset.RbacV1Interface
 	saName      string
 	saNamespace string
-	owner       metav1.OwnerReference
 }
 
 func newGeneratorClusterRoleBinding(lister rbaclisters.ClusterRoleBindingLister, client rbacset.RbacV1Interface, params *parameters.Globals, cr *imageregistryv1.Config) *generatorClusterRoleBinding {
@@ -28,7 +26,6 @@ func newGeneratorClusterRoleBinding(lister rbaclisters.ClusterRoleBindingLister,
 		client:      client,
 		saName:      params.Pod.ServiceAccount,
 		saNamespace: params.Deployment.Namespace,
-		owner:       util.AsOwner(cr),
 	}
 }
 
@@ -66,8 +63,6 @@ func (gcrb *generatorClusterRoleBinding) expected() (runtime.Object, error) {
 		},
 	}
 
-	util.AddOwnerRefToObject(crb, gcrb.owner)
-
 	return crb, nil
 }
 
@@ -90,3 +85,7 @@ func (gcrb *generatorClusterRoleBinding) Update(o runtime.Object) (bool, error)
 func (gcrb *generatorClusterRoleBinding) Delete(opts *metav1.DeleteOptions) error {
 	return gcrb.client.ClusterRoleBindings().Delete(gcrb.GetName(), opts)
 }
+
+func (g *generatorClusterRoleBinding) Owned() bool {
+	return true
+}

pkg/resource/deployment.go

@@ -12,7 +12,6 @@ import (
 	imageregistryv1 "github.com/openshift/cluster-image-registry-operator/pkg/apis/imageregistry/v1"
 	"github.com/openshift/cluster-image-registry-operator/pkg/parameters"
 	"github.com/openshift/cluster-image-registry-operator/pkg/storage"
-	"github.com/openshift/cluster-image-registry-operator/pkg/util"
 )
 
 var _ Mutator = &generatorDeployment{}
@@ -84,8 +83,6 @@ func (gd *generatorDeployment) expected() (runtime.Object, error) {
 		},
 	}
 
-	util.AddOwnerRefToObject(deploy, util.AsOwner(gd.cr))
-
 	return deploy, nil
 }
 
@@ -108,3 +105,7 @@ func (gd *generatorDeployment) Update(o runtime.Object) (bool, error) {
 func (gd *generatorDeployment) Delete(opts *metav1.DeleteOptions) error {
 	return gd.client.Deployments(gd.GetNamespace()).Delete(gd.GetName(), opts)
 }
+
+func (g *generatorDeployment) Owned() bool {
+	return true
+}

pkg/resource/generator.go

@@ -235,6 +235,9 @@ func (g *Generator) Remove(cr *imageregistryv1.Config) error {
 		PropagationPolicy:  &propagationPolicy,
 	}
 	for _, gen := range generators {
+		if !gen.Owned() {
+			continue
+		}
 		if err := gen.Delete(opts); err != nil {
 			if errors.IsNotFound(err) {
 				continue

pkg/resource/imageconfig.go

@@ -133,6 +133,11 @@ func (gic *generatorImageConfig) Delete(opts *metav1.DeleteOptions) error {
 	return gic.configClient.Images().Delete(gic.GetName(), opts)
 }
 
+func (g *generatorImageConfig) Owned() bool {
+	// the registry operator can create and contribute to the imageconfig, but it doesn't own it.
+	return false
+}
+
 func (gic *generatorImageConfig) getRouteHostnames() ([]string, error) {
 	var externalHostnames []string
 

pkg/resource/nodecadaemon.go

@@ -1,23 +1,23 @@
 package resource
 
 import (
-	"os"
+  "os"
 
-	appsv1 "k8s.io/api/apps/v1"
-	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	appsclientv1 "k8s.io/client-go/kubernetes/typed/apps/v1"
-	appslisters "k8s.io/client-go/listers/apps/v1"
-	kcorelisters "k8s.io/client-go/listers/core/v1"
+  appsv1 "k8s.io/api/apps/v1"
+  corev1 "k8s.io/api/core/v1"
+  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+  "k8s.io/apimachinery/pkg/runtime"
+  appsclientv1 "k8s.io/client-go/kubernetes/typed/apps/v1"
+  appslisters "k8s.io/client-go/listers/apps/v1"
+  kcorelisters "k8s.io/client-go/listers/core/v1"
 
-	"github.com/openshift/library-go/pkg/operator/resource/resourceread"
+  "github.com/openshift/library-go/pkg/operator/resource/resourceread"
 
-	"github.com/openshift/cluster-image-registry-operator/pkg/parameters"
+  "github.com/openshift/cluster-image-registry-operator/pkg/parameters"
 )
 
 const (
-	nodeCADaemonSetDefinition = `
+  nodeCADaemonSetDefinition = `
 apiVersion: apps/v1
 kind: DaemonSet
 metadata:
@@ -113,101 +113,106 @@ spec:
 var _ Mutator = &generatorNodeCADaemonSet{}
 
 type generatorNodeCADaemonSet struct {
-	daemonSetLister appslisters.DaemonSetNamespaceLister
-	serviceLister   kcorelisters.ServiceNamespaceLister
-	client          appsclientv1.AppsV1Interface
-	owner           metav1.OwnerReference
-	params          *parameters.Globals
+  daemonSetLister appslisters.DaemonSetNamespaceLister
+  serviceLister   kcorelisters.ServiceNamespaceLister
+  client          appsclientv1.AppsV1Interface
+  owner           metav1.OwnerReference
+  params          *parameters.Globals
 }
 
 func newGeneratorNodeCADaemonSet(daemonSetLister appslisters.DaemonSetNamespaceLister, serviceLister kcorelisters.ServiceNamespaceLister, client appsclientv1.AppsV1Interface, params *parameters.Globals) *generatorNodeCADaemonSet {
-	return &generatorNodeCADaemonSet{
-		daemonSetLister: daemonSetLister,
-		serviceLister:   serviceLister,
-		client:          client,
-		params:          params,
-	}
+  return &generatorNodeCADaemonSet{
+    daemonSetLister: daemonSetLister,
+    serviceLister:   serviceLister,
+    client:          client,
+    params:          params,
+  }
 }
 
 func (ds *generatorNodeCADaemonSet) Type() runtime.Object {
-	return &appsv1.DaemonSet{}
+  return &appsv1.DaemonSet{}
 }
 
 func (ds *generatorNodeCADaemonSet) GetNamespace() string {
-	return ds.params.Deployment.Namespace
+  return ds.params.Deployment.Namespace
 }
 
 func (ds *generatorNodeCADaemonSet) GetName() string {
-	return "node-ca"
+  return "node-ca"
 }
 
 func (ds *generatorNodeCADaemonSet) Get() (runtime.Object, error) {
-	return ds.daemonSetLister.Get(ds.GetName())
+  return ds.daemonSetLister.Get(ds.GetName())
 }
 
 func (ds *generatorNodeCADaemonSet) Create() error {
 
-	internalHostname, err := getServiceHostname(ds.serviceLister, ds.params.Service.Name)
-	if err != nil {
-		return err
-	}
-
-	daemonSet := resourceread.ReadDaemonSetV1OrDie([]byte(nodeCADaemonSetDefinition))
-	env := corev1.EnvVar{
-		Name:  "internalRegistryHostname",
-		Value: internalHostname,
-	}
-
-	daemonSet.Spec.Template.Spec.Containers[0].Image = os.Getenv("IMAGE")
-	daemonSet.Spec.Template.Spec.Containers[0].Env = append(daemonSet.Spec.Template.Spec.Containers[0].Env, env)
-	_, err = ds.client.DaemonSets(ds.GetNamespace()).Create(daemonSet)
-	return err
+  internalHostname, err := getServiceHostname(ds.serviceLister, ds.params.Service.Name)
+  if err != nil {
+    return err
+  }
+
+  daemonSet := resourceread.ReadDaemonSetV1OrDie([]byte(nodeCADaemonSetDefinition))
+  env := corev1.EnvVar{
+    Name:  "internalRegistryHostname",
+    Value: internalHostname,
+  }
+
+  daemonSet.Spec.Template.Spec.Containers[0].Image = os.Getenv("IMAGE")
+  daemonSet.Spec.Template.Spec.Containers[0].Env = append(daemonSet.Spec.Template.Spec.Containers[0].Env, env)
+  _, err = ds.client.DaemonSets(ds.GetNamespace()).Create(daemonSet)
+  return err
 }
 
 func (ds *generatorNodeCADaemonSet) Update(o runtime.Object) (bool, error) {
-	internalHostname, err := getServiceHostname(ds.serviceLister, ds.params.Service.Name)
-	if err != nil {
-		return false, err
-	}
-
-	daemonSet := o.(*appsv1.DaemonSet)
-	modified := false
-	exists := false
-
-	newImage := os.Getenv("IMAGE")
-	oldImage := daemonSet.Spec.Template.Spec.Containers[0].Image
-	if newImage != oldImage {
-		daemonSet.Spec.Template.Spec.Containers[0].Image = newImage
-		modified = true
-	}
-
-	for i, env := range daemonSet.Spec.Template.Spec.Containers[0].Env {
-		if env.Name == "internalRegistryHostname" {
-			exists = true
-			if env.Value != internalHostname {
-				daemonSet.Spec.Template.Spec.Containers[0].Env[i].Value = internalHostname
-				modified = true
-			}
-			break
-		}
-	}
-	if !exists {
-		env := corev1.EnvVar{
-			Name:  "internalRegistryHostname",
-			Value: internalHostname,
-		}
-		daemonSet.Spec.Template.Spec.Containers[0].Env = append(daemonSet.Spec.Template.Spec.Containers[0].Env, env)
-		modified = true
-	}
-
-	if !modified {
-		return false, nil
-	}
-
-	_, err = ds.client.DaemonSets(ds.GetNamespace()).Update(daemonSet)
-	return err == nil, err
+  internalHostname, err := getServiceHostname(ds.serviceLister, ds.params.Service.Name)
+  if err != nil {
+    return false, err
+  }
+
+  daemonSet := o.(*appsv1.DaemonSet)
+  modified := false
+  exists := false
+
+  newImage := os.Getenv("IMAGE")
+  oldImage := daemonSet.Spec.Template.Spec.Containers[0].Image
+  if newImage != oldImage {
+    daemonSet.Spec.Template.Spec.Containers[0].Image = newImage
+    modified = true
+  }
+
+  for i, env := range daemonSet.Spec.Template.Spec.Containers[0].Env {
+    if env.Name == "internalRegistryHostname" {
+      exists = true
+      if env.Value != internalHostname {
+        daemonSet.Spec.Template.Spec.Containers[0].Env[i].Value = internalHostname
+        modified = true
+      }
+      break
+    }
+  }
+  if !exists {
+    env := corev1.EnvVar{
+      Name:  "internalRegistryHostname",
+      Value: internalHostname,
+    }
+    daemonSet.Spec.Template.Spec.Containers[0].Env = append(daemonSet.Spec.Template.Spec.Containers[0].Env, env)
+    modified = true
+  }
+
+  if !modified {
+    return false, nil
+  }
+
+  _, err = ds.client.DaemonSets(ds.GetNamespace()).Update(daemonSet)
+  return err == nil, err
 }
 
 func (ds *generatorNodeCADaemonSet) Delete(opts *metav1.DeleteOptions) error {
-	return ds.client.DaemonSets(ds.GetNamespace()).Delete(ds.GetName(), opts)
+  return ds.client.DaemonSets(ds.GetNamespace()).Delete(ds.GetName(), opts)
+}
+
+func (ds *generatorNodeCADaemonSet) Owned() bool {
+  // the nodeca daemon's lifecycle is not tied to the lifecycle of the registry
+  return false
 }

pkg/resource/resource.go

@@ -21,6 +21,9 @@ type Mutator interface {
 	Create() error
 	Update(o runtime.Object) (bool, error)
 	Delete(opts *metaapi.DeleteOptions) error
+	// Owned indicates whether this resource is explicitly owned by the registry operator
+	// and therefore should be removed when the registry config resource is removed.
+	Owned() bool
 }
 
 func Name(o Getter) string {