Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do not setup ownerrefs for clusterscoped/cross-namespace objects #215

Merged
merged 3 commits into from
Feb 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 4 additions & 5 deletions pkg/resource/caconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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 {
Expand All @@ -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),
}
}

Expand Down Expand Up @@ -82,8 +79,6 @@ func (gcac *generatorCAConfig) expected() (runtime.Object, error) {
}
}

util.AddOwnerRefToObject(cm, gcac.owner)

return cm, nil
}

Expand All @@ -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
}
9 changes: 4 additions & 5 deletions pkg/resource/clusterrole.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}

Expand Down Expand Up @@ -94,8 +91,6 @@ func (gcr *generatorClusterRole) expected() (runtime.Object, error) {
},
}

util.AddOwnerRefToObject(role, gcr.owner)

return role, nil
}

Expand All @@ -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
}
9 changes: 4 additions & 5 deletions pkg/resource/clusterrolebinding.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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 {
Expand All @@ -28,7 +26,6 @@ func newGeneratorClusterRoleBinding(lister rbaclisters.ClusterRoleBindingLister,
client: client,
saName: params.Pod.ServiceAccount,
saNamespace: params.Deployment.Namespace,
owner: util.AsOwner(cr),
}
}

Expand Down Expand Up @@ -66,8 +63,6 @@ func (gcrb *generatorClusterRoleBinding) expected() (runtime.Object, error) {
},
}

util.AddOwnerRefToObject(crb, gcrb.owner)

return crb, nil
}

Expand All @@ -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
}
7 changes: 4 additions & 3 deletions pkg/resource/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -84,8 +83,6 @@ func (gd *generatorDeployment) expected() (runtime.Object, error) {
},
}

util.AddOwnerRefToObject(deploy, util.AsOwner(gd.cr))

return deploy, nil
}

Expand All @@ -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
}
5 changes: 4 additions & 1 deletion pkg/resource/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions pkg/resource/imageconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions pkg/resource/nodecadaemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
3 changes: 3 additions & 0 deletions pkg/resource/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
22 changes: 14 additions & 8 deletions pkg/resource/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -22,7 +28,6 @@ type generatorRoute struct {
client routeset.RouteV1Interface
namespace string
serviceName string
owner metav1.OwnerReference
route imageregistryv1.ImageRegistryConfigRoute
}

Expand All @@ -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,
}
}
Expand All @@ -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,
Expand Down Expand Up @@ -83,9 +88,6 @@ func (gr *generatorRoute) expected() (runtime.Object, error) {
r.Spec.TLS.CACertificate = v
}
}

util.AddOwnerRefToObject(r, gr.owner)

return r, nil
}

Expand All @@ -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
}
9 changes: 4 additions & 5 deletions pkg/resource/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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 {
Expand All @@ -31,7 +29,6 @@ func newGeneratorSecret(lister corelisters.SecretNamespaceLister, client coreset
driver: driver,
name: imageregistryv1.ImageRegistryPrivateConfiguration,
namespace: params.Deployment.Namespace,
owner: util.AsOwner(cr),
}
}

Expand Down Expand Up @@ -62,8 +59,6 @@ func (gs *generatorSecret) expected() (runtime.Object, error) {

sec.StringData = data

util.AddOwnerRefToObject(sec, gs.owner)

return sec, nil
}

Expand All @@ -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
}
9 changes: 4 additions & 5 deletions pkg/resource/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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 {
Expand All @@ -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),
}
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Loading