Merge pull request #215 from bparees/owners

do not setup ownerrefs for clusterscoped/cross-namespace objects

Makefile

@@ -21,7 +21,7 @@ test-unit: verify
 	./hack/test-go.sh ./cmd/... ./pkg/...
 
 test-e2e:
-	GOCACHE=off ./hack/test-go.sh -timeout 20m -v$${WHAT:+ -run="$$WHAT"} ./test/e2e/
+	GOCACHE=off ./hack/test-go.sh -timeout 30m -v$${WHAT:+ -run="$$WHAT"} ./test/e2e/
 
 verify:
 	hack/verify.sh

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

@@ -156,7 +156,7 @@ func (g *Generator) removeObsoleteRoutes(cr *imageregistryv1.Config) error {
 		PropagationPolicy:  &propagationPolicy,
 	}
 	for _, route := range routes {
-		if !metaapi.IsControlledBy(route, cr) {
+		if !RouteIsCreatedByOperator(route) {
 			continue
 		}
 		if _, found := knownNames[route.Name]; found {
@@ -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
 
@@ -142,10 +147,7 @@ func (gic *generatorImageConfig) getRouteHostnames() ([]string, error) {
 	}
 	defaultHost := ""
 	for _, route := range routes {
-		routeOwner := metav1.GetControllerOf(route)
-
-		// ignore routes that weren't created by the registry operator
-		if routeOwner == nil || routeOwner.UID != gic.owner.UID {
+		if !RouteIsCreatedByOperator(route) {
 			continue
 		}
 		for _, ingress := range route.Status.Ingress {

pkg/resource/nodecadaemon.go

@@ -211,3 +211,8 @@ func (ds *generatorNodeCADaemonSet) Update(o runtime.Object) (bool, error) {
 func (ds *generatorNodeCADaemonSet) Delete(opts *metav1.DeleteOptions) error {
 	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 {

pkg/resource/route.go

@@ -11,9 +11,15 @@ 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"
 )
 
+const RouteOwnerAnnotation = "imageregistry.openshift.io"
+
+func RouteIsCreatedByOperator(route *routeapi.Route) bool {
+	_, ok := route.Annotations[RouteOwnerAnnotation]
+	return ok
+}
+
 var _ Mutator = &generatorRoute{}
 
 type generatorRoute struct {
@@ -22,7 +28,6 @@ type generatorRoute struct {
 	client       routeset.RouteV1Interface
 	namespace    string
 	serviceName  string
-	owner        metav1.OwnerReference
 	route        imageregistryv1.ImageRegistryConfigRoute
 }
 
@@ -33,7 +38,6 @@ func newGeneratorRoute(lister routelisters.RouteNamespaceLister, secretLister co
 		client:       client,
 		namespace:    params.Deployment.Namespace,
 		serviceName:  params.Service.Name,
-		owner:        util.AsOwner(cr),
 		route:        route,
 	}
 }
@@ -53,8 +57,9 @@ func (gr *generatorRoute) GetName() string {
 func (gr *generatorRoute) expected() (runtime.Object, error) {
 	r := &routeapi.Route{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      gr.GetName(),
-			Namespace: gr.GetNamespace(),
+			Name:        gr.GetName(),
+			Namespace:   gr.GetNamespace(),
+			Annotations: map[string]string{RouteOwnerAnnotation: "true"},
 		},
 		Spec: routeapi.RouteSpec{
 			Host: gr.route.Hostname,
@@ -83,9 +88,6 @@ func (gr *generatorRoute) expected() (runtime.Object, error) {
 			r.Spec.TLS.CACertificate = v
 		}
 	}
-
-	util.AddOwnerRefToObject(r, gr.owner)
-
 	return r, nil
 }
 
@@ -108,3 +110,7 @@ func (gr *generatorRoute) Update(o runtime.Object) (bool, error) {
 func (gr *generatorRoute) Delete(opts *metav1.DeleteOptions) error {
 	return gr.client.Routes(gr.GetNamespace()).Delete(gr.GetName(), opts)
 }
+
+func (g *generatorRoute) Owned() bool {
+	return true
+}

pkg/resource/secret.go

@@ -10,7 +10,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 = &generatorSecret{}
@@ -21,7 +20,6 @@ type generatorSecret struct {
 	driver    storage.Driver
 	name      string
 	namespace string
-	owner     metav1.OwnerReference
 }
 
 func newGeneratorSecret(lister corelisters.SecretNamespaceLister, client coreset.CoreV1Interface, driver storage.Driver, params *parameters.Globals, cr *imageregistryv1.Config) *generatorSecret {
@@ -31,7 +29,6 @@ func newGeneratorSecret(lister corelisters.SecretNamespaceLister, client coreset
 		driver:    driver,
 		name:      imageregistryv1.ImageRegistryPrivateConfiguration,
 		namespace: params.Deployment.Namespace,
-		owner:     util.AsOwner(cr),
 	}
 }
 
@@ -62,8 +59,6 @@ func (gs *generatorSecret) expected() (runtime.Object, error) {
 
 	sec.StringData = data
 
-	util.AddOwnerRefToObject(sec, gs.owner)
-
 	return sec, nil
 }
 
@@ -86,3 +81,7 @@ func (gs *generatorSecret) Update(o runtime.Object) (bool, error) {
 func (gs *generatorSecret) Delete(opts *metav1.DeleteOptions) error {
 	return gs.client.Secrets(gs.GetNamespace()).Delete(gs.GetName(), opts)
 }
+
+func (g *generatorSecret) Owned() bool {
+	return true
+}

pkg/resource/service.go

@@ -13,7 +13,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/resource/strategy"
-	"github.com/openshift/cluster-image-registry-operator/pkg/util"
 )
 
 var _ Mutator = &generatorService{}
@@ -26,7 +25,6 @@ type generatorService struct {
 	labels     map[string]string
 	port       int
 	secretName string
-	owner      metav1.OwnerReference
 }
 
 func newGeneratorService(lister corelisters.ServiceNamespaceLister, client coreset.CoreV1Interface, params *parameters.Globals, cr *imageregistryv1.Config) *generatorService {
@@ -38,7 +36,6 @@ func newGeneratorService(lister corelisters.ServiceNamespaceLister, client cores
 		labels:     params.Deployment.Labels,
 		port:       params.Container.Port,
 		secretName: imageregistryv1.ImageRegistryName + "-tls",
-		owner:      util.AsOwner(cr),
 	}
 }
 
@@ -78,8 +75,6 @@ func (gs *generatorService) expected() *corev1.Service {
 		"service.alpha.openshift.io/serving-cert-secret-name": gs.secretName,
 	}
 
-	util.AddOwnerRefToObject(svc, gs.owner)
-
 	return svc
 }
 
@@ -116,3 +111,7 @@ func (gs *generatorService) Update(o runtime.Object) (bool, error) {
 func (gs *generatorService) Delete(opts *metav1.DeleteOptions) error {
 	return gs.client.Services(gs.GetNamespace()).Delete(gs.GetName(), opts)
 }
+
+func (g *generatorService) Owned() bool {
+	return true
+}