Merge pull request operator-framework#729 from ecordell/cross-ns-owne…

…rrefs fix(owners): remove cross-namespace and cluster->namespace ownerrefs
ecordell · Feb 26, 2019 · 86552ae · 86552ae
2 parents 360804f + 15b63f9
commit 86552ae
Show file tree

Hide file tree

Showing 14 changed files with 316 additions and 160 deletions.
diff --git a/go.mod b/go.mod
@@ -32,14 +32,14 @@ require (
  golang.org/x/time v0.0.0-20181108054448-85acf8d2951c // indirect
  google.golang.org/grpc v1.16.0
  k8s.io/api v0.0.0-20190118113203-912cbe2bfef3
- k8s.io/apiextensions-apiserver v0.0.0-20181204003618-e419c5771cdc
- k8s.io/apimachinery v0.0.0-20190208202428-1a579f8a7b42
+ k8s.io/apiextensions-apiserver v0.0.0-20190223021643-57c81b676ab1
+ k8s.io/apimachinery v0.0.0-20190223001710-c182ff3b9841
  k8s.io/apiserver v0.0.0-20181026151315-13cfe3978170
  k8s.io/client-go v8.0.0+incompatible
  k8s.io/code-generator v0.0.0-20181203235156-f8cba74510f3
  k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6 // indirect
  k8s.io/klog v0.2.0 // indirect
- k8s.io/kube-aggregator v0.0.0-20181204002017-122bac39d429
+ k8s.io/kube-aggregator v0.0.0-20190223015803-f706565beac0
  k8s.io/kube-openapi v0.0.0-20181031203759-72693cb1fadd
- k8s.io/kubernetes v1.11.8-beta.0.0.20190214232326-4e0b35876724
+ k8s.io/kubernetes v1.11.8-beta.0.0.20190223014307-4e209c9383fa
 )
diff --git a/go.sum b/go.sum
diff --git a/pkg/api/wrappers/deployment_install_client_test.go b/pkg/api/wrappers/deployment_install_client_test.go
@@ -131,7 +131,8 @@ func TestEnsureServiceAccount(t *testing.T) {
  namespace: "test-namespace",
  existingServiceAccount: &corev1.ServiceAccount{
  ObjectMeta: metav1.ObjectMeta{
- Name: "test-service-account",
+ Name: "test-service-account",
+ Namespace: "test-namespace",
  Labels: map[string]string{
  "test": "existing-service-account-found",
  },
@@ -161,7 +162,8 @@ func TestEnsureServiceAccount(t *testing.T) {
  },
  serviceAccountToUpdate: &corev1.ServiceAccount{
  ObjectMeta: metav1.ObjectMeta{
- Name: "test-service-account",
+ Name: "test-service-account",
+ Namespace: "test-namespace",
  Labels: map[string]string{
  "test": "existing-service-account-found",
  },
@@ -193,7 +195,8 @@ func TestEnsureServiceAccount(t *testing.T) {
  namespace: "test-namespace",
  existingServiceAccount: &corev1.ServiceAccount{
  ObjectMeta: metav1.ObjectMeta{
- Name: "test-service-account",
+ Name: "test-service-account",
+ Namespace: "test-namespace",
  Labels: map[string]string{
  "test": "existing-service-account-found",
  },
@@ -213,7 +216,8 @@ func TestEnsureServiceAccount(t *testing.T) {
  },
  serviceAccountToUpdate: &corev1.ServiceAccount{
  ObjectMeta: metav1.ObjectMeta{
- Name: "test-service-account",
+ Name: "test-service-account",
+ Namespace: "test-namespace",
  Labels: map[string]string{
  "test": "existing-service-account-found",
  },
@@ -235,7 +239,8 @@ func TestEnsureServiceAccount(t *testing.T) {
  namespace: "test-namespace",
  existingServiceAccount: &corev1.ServiceAccount{
  ObjectMeta: metav1.ObjectMeta{
- Name: "test-service-account",
+ Name: "test-service-account",
+ Namespace: "test-namespace",
  Labels: map[string]string{
  "test": "existing-service-account-create-conflict",
  },
@@ -259,7 +264,8 @@ func TestEnsureServiceAccount(t *testing.T) {
  expect: expect{
  returnedServiceAccount: &corev1.ServiceAccount{
  ObjectMeta: metav1.ObjectMeta{
- Name: "test-service-account",
+ Name: "test-service-account",
+ Namespace: "test-namespace",
  Labels: map[string]string{
  "test": "existing-service-account-create-conflict",
  },

diff --git a/pkg/controller/install/deployment.go b/pkg/controller/install/deployment.go
@@ -75,11 +75,7 @@ func (i *StrategyDeploymentInstaller) installDeployments(deps []StrategyDeployme
  dep.Spec.Template.SetAnnotations(annotations)
 
  ownerutil.AddNonBlockingOwner(dep, i.owner)
- if dep.Labels == nil {
- dep.SetLabels(map[string]string{})
- }
- dep.Labels["olm.owner"] = i.owner.GetName()
- dep.Labels["olm.owner.namespace"] = i.owner.GetNamespace()
+ ownerutil.AddOwnerLabels(dep, i.owner)
  if _, err := i.strategyClient.CreateOrUpdateDeployment(dep); err != nil {
  return err
  }

diff --git a/pkg/controller/install/deployment_test.go b/pkg/controller/install/deployment_test.go
@@ -17,7 +17,7 @@ import (
 )
 
 func testDeployment(name, namespace string, mockOwner ownerutil.Owner) appsv1.Deployment {
- testDeploymentLabels := map[string]string{"olm.owner": mockOwner.GetName(), "olm.owner.namespace": mockOwner.GetNamespace()}
+ testDeploymentLabels := map[string]string{"olm.owner": mockOwner.GetName(), "olm.owner.namespace": mockOwner.GetNamespace(), "olm.owner.kind": "ClusterServiceVersion"}
 
  deployment := appsv1.Deployment{
  ObjectMeta: metav1.ObjectMeta{

diff --git a/pkg/controller/operators/olm/apiservices.go b/pkg/controller/operators/olm/apiservices.go
@@ -496,8 +496,8 @@ func (a *Operator) installAPIServiceRequirements(desc v1alpha1.APIServiceDescrip
  existingAuthDelegatorClusterRoleBinding, err := a.lister.RbacV1().ClusterRoleBindingLister().Get(authDelegatorClusterRoleBinding.GetName())
  if err == nil {
  // Check if the only owners are this CSV or in this CSV's replacement chain.
- if ownerutil.Adoptable(csv, existingAuthDelegatorClusterRoleBinding.GetOwnerReferences()) {
- ownerutil.AddNonBlockingOwner(authDelegatorClusterRoleBinding, csv)
+ if ownerutil.AdoptableLabels(csv, existingAuthDelegatorClusterRoleBinding.GetLabels()) {
+ ownerutil.AddOwnerLabels(authDelegatorClusterRoleBinding, csv)
  }
 
  // Attempt an update.
@@ -507,7 +507,7 @@ func (a *Operator) installAPIServiceRequirements(desc v1alpha1.APIServiceDescrip
  }
  } else if k8serrors.IsNotFound(err) {
  // Create the role.
- ownerutil.AddNonBlockingOwner(authDelegatorClusterRoleBinding, csv)
+ ownerutil.AddOwnerLabels(authDelegatorClusterRoleBinding, csv)
  _, err = a.OpClient.CreateClusterRoleBinding(authDelegatorClusterRoleBinding)
  if err != nil {
  log.Warnf("could not create auth delegator clusterrolebinding %s", authDelegatorClusterRoleBinding.GetName())
@@ -539,18 +539,17 @@ func (a *Operator) installAPIServiceRequirements(desc v1alpha1.APIServiceDescrip
  existingAuthReaderRoleBinding, err := a.lister.RbacV1().RoleBindingLister().RoleBindings("kube-system").Get(authReaderRoleBinding.GetName())
  if err == nil {
  // Check if the only owners are this CSV or in this CSV's replacement chain.
- if ownerutil.Adoptable(csv, existingAuthReaderRoleBinding.GetOwnerReferences()) {
- ownerutil.AddNonBlockingOwner(authReaderRoleBinding, csv)
+ if ownerutil.AdoptableLabels(csv, existingAuthReaderRoleBinding.GetLabels()) {
+ ownerutil.AddOwnerLabels(authReaderRoleBinding, csv)
  }
-
  // Attempt an update.
  if _, err := a.OpClient.UpdateRoleBinding(authReaderRoleBinding); err != nil {
  logger.Warnf("could not update auth reader role binding %s", authReaderRoleBinding.GetName())
  return nil, err
  }
  } else if k8serrors.IsNotFound(err) {
  // Create the role.
- ownerutil.AddNonBlockingOwner(authReaderRoleBinding, csv)
+ ownerutil.AddOwnerLabels(authReaderRoleBinding, csv)
  _, err = a.OpClient.CreateRoleBinding(authReaderRoleBinding)
  if err != nil {
  log.Warnf("could not create auth reader role binding %s", authReaderRoleBinding.GetName())
@@ -641,13 +640,13 @@ func (a *Operator) installAPIServiceRequirements(desc v1alpha1.APIServiceDescrip
  apiService.SetName(apiServiceName)
  } else {
  // check if the APIService is adoptable
- if !ownerutil.Adoptable(csv, apiService.GetOwnerReferences()) {
+ if !ownerutil.AdoptableLabels(csv, apiService.GetLabels()) {
  return nil, fmt.Errorf("pre-existing APIService %s is not adoptable", apiServiceName)
  }
  }
 
  // Add the CSV as an owner
- ownerutil.AddNonBlockingOwner(apiService, csv)
+ ownerutil.AddOwnerLabels(apiService, csv)
 
  // update the ServiceReference
  apiService.Spec.Service = &apiregistrationv1.ServiceReference{