Skip to content

Commit 9186781

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #729 from ecordell/cross-ns-ownerrefs
fix(owners): remove cross-namespace and cluster->namespace ownerrefs
2 parents 1ac9ace + c1db6cd commit 9186781

File tree

14 files changed

+316
-160
lines changed

14 files changed

+316
-160
lines changed

‎go.mod

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,14 @@ require (
3232
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c // indirect
3333
google.golang.org/grpc v1.16.0
3434
k8s.io/api v0.0.0-20190118113203-912cbe2bfef3
35-
k8s.io/apiextensions-apiserver v0.0.0-20181204003618-e419c5771cdc
36-
k8s.io/apimachinery v0.0.0-20190208202428-1a579f8a7b42
35+
k8s.io/apiextensions-apiserver v0.0.0-20190223021643-57c81b676ab1
36+
k8s.io/apimachinery v0.0.0-20190223001710-c182ff3b9841
3737
k8s.io/apiserver v0.0.0-20181026151315-13cfe3978170
3838
k8s.io/client-go v8.0.0+incompatible
3939
k8s.io/code-generator v0.0.0-20181203235156-f8cba74510f3
4040
k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6 // indirect
4141
k8s.io/klog v0.2.0 // indirect
42-
k8s.io/kube-aggregator v0.0.0-20181204002017-122bac39d429
42+
k8s.io/kube-aggregator v0.0.0-20190223015803-f706565beac0
4343
k8s.io/kube-openapi v0.0.0-20181031203759-72693cb1fadd
44-
k8s.io/kubernetes v1.11.8-beta.0.0.20190214232326-4e0b35876724
44+
k8s.io/kubernetes v1.11.8-beta.0.0.20190223014307-4e209c9383fa
4545
)

‎go.sum

Lines changed: 55 additions & 4 deletions
Large diffs are not rendered by default.

‎pkg/api/wrappers/deployment_install_client_test.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,8 @@ func TestEnsureServiceAccount(t *testing.T) {
131131
namespace: "test-namespace",
132132
existingServiceAccount: &corev1.ServiceAccount{
133133
ObjectMeta: metav1.ObjectMeta{
134-
Name: "test-service-account",
134+
Name: "test-service-account",
135+
Namespace: "test-namespace",
135136
Labels: map[string]string{
136137
"test": "existing-service-account-found",
137138
},
@@ -161,7 +162,8 @@ func TestEnsureServiceAccount(t *testing.T) {
161162
},
162163
serviceAccountToUpdate: &corev1.ServiceAccount{
163164
ObjectMeta: metav1.ObjectMeta{
164-
Name: "test-service-account",
165+
Name: "test-service-account",
166+
Namespace: "test-namespace",
165167
Labels: map[string]string{
166168
"test": "existing-service-account-found",
167169
},
@@ -193,7 +195,8 @@ func TestEnsureServiceAccount(t *testing.T) {
193195
namespace: "test-namespace",
194196
existingServiceAccount: &corev1.ServiceAccount{
195197
ObjectMeta: metav1.ObjectMeta{
196-
Name: "test-service-account",
198+
Name: "test-service-account",
199+
Namespace: "test-namespace",
197200
Labels: map[string]string{
198201
"test": "existing-service-account-found",
199202
},
@@ -213,7 +216,8 @@ func TestEnsureServiceAccount(t *testing.T) {
213216
},
214217
serviceAccountToUpdate: &corev1.ServiceAccount{
215218
ObjectMeta: metav1.ObjectMeta{
216-
Name: "test-service-account",
219+
Name: "test-service-account",
220+
Namespace: "test-namespace",
217221
Labels: map[string]string{
218222
"test": "existing-service-account-found",
219223
},
@@ -235,7 +239,8 @@ func TestEnsureServiceAccount(t *testing.T) {
235239
namespace: "test-namespace",
236240
existingServiceAccount: &corev1.ServiceAccount{
237241
ObjectMeta: metav1.ObjectMeta{
238-
Name: "test-service-account",
242+
Name: "test-service-account",
243+
Namespace: "test-namespace",
239244
Labels: map[string]string{
240245
"test": "existing-service-account-create-conflict",
241246
},
@@ -259,7 +264,8 @@ func TestEnsureServiceAccount(t *testing.T) {
259264
expect: expect{
260265
returnedServiceAccount: &corev1.ServiceAccount{
261266
ObjectMeta: metav1.ObjectMeta{
262-
Name: "test-service-account",
267+
Name: "test-service-account",
268+
Namespace: "test-namespace",
263269
Labels: map[string]string{
264270
"test": "existing-service-account-create-conflict",
265271
},

‎pkg/controller/install/deployment.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,7 @@ func (i *StrategyDeploymentInstaller) installDeployments(deps []StrategyDeployme
7575
dep.Spec.Template.SetAnnotations(annotations)
7676

7777
ownerutil.AddNonBlockingOwner(dep, i.owner)
78-
if dep.Labels == nil {
79-
dep.SetLabels(map[string]string{})
80-
}
81-
dep.Labels["olm.owner"] = i.owner.GetName()
82-
dep.Labels["olm.owner.namespace"] = i.owner.GetNamespace()
78+
ownerutil.AddOwnerLabels(dep, i.owner)
8379
if _, err := i.strategyClient.CreateOrUpdateDeployment(dep); err != nil {
8480
return err
8581
}

‎pkg/controller/install/deployment_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import (
1717
)
1818

1919
func testDeployment(name, namespace string, mockOwner ownerutil.Owner) appsv1.Deployment {
20-
testDeploymentLabels := map[string]string{"olm.owner": mockOwner.GetName(), "olm.owner.namespace": mockOwner.GetNamespace()}
20+
testDeploymentLabels := map[string]string{"olm.owner": mockOwner.GetName(), "olm.owner.namespace": mockOwner.GetNamespace(), "olm.owner.kind": "ClusterServiceVersion"}
2121

2222
deployment := appsv1.Deployment{
2323
ObjectMeta: metav1.ObjectMeta{

‎pkg/controller/operators/olm/apiservices.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -496,8 +496,8 @@ func (a *Operator) installAPIServiceRequirements(desc v1alpha1.APIServiceDescrip
496496
existingAuthDelegatorClusterRoleBinding, err := a.lister.RbacV1().ClusterRoleBindingLister().Get(authDelegatorClusterRoleBinding.GetName())
497497
if err == nil {
498498
// Check if the only owners are this CSV or in this CSV's replacement chain.
499-
if ownerutil.Adoptable(csv, existingAuthDelegatorClusterRoleBinding.GetOwnerReferences()) {
500-
ownerutil.AddNonBlockingOwner(authDelegatorClusterRoleBinding, csv)
499+
if ownerutil.AdoptableLabels(csv, existingAuthDelegatorClusterRoleBinding.GetLabels()) {
500+
ownerutil.AddOwnerLabels(authDelegatorClusterRoleBinding, csv)
501501
}
502502

503503
// Attempt an update.
@@ -507,7 +507,7 @@ func (a *Operator) installAPIServiceRequirements(desc v1alpha1.APIServiceDescrip
507507
}
508508
} else if k8serrors.IsNotFound(err) {
509509
// Create the role.
510-
ownerutil.AddNonBlockingOwner(authDelegatorClusterRoleBinding, csv)
510+
ownerutil.AddOwnerLabels(authDelegatorClusterRoleBinding, csv)
511511
_, err = a.OpClient.CreateClusterRoleBinding(authDelegatorClusterRoleBinding)
512512
if err != nil {
513513
log.Warnf("could not create auth delegator clusterrolebinding %s", authDelegatorClusterRoleBinding.GetName())
@@ -539,18 +539,17 @@ func (a *Operator) installAPIServiceRequirements(desc v1alpha1.APIServiceDescrip
539539
existingAuthReaderRoleBinding, err := a.lister.RbacV1().RoleBindingLister().RoleBindings("kube-system").Get(authReaderRoleBinding.GetName())
540540
if err == nil {
541541
// Check if the only owners are this CSV or in this CSV's replacement chain.
542-
if ownerutil.Adoptable(csv, existingAuthReaderRoleBinding.GetOwnerReferences()) {
543-
ownerutil.AddNonBlockingOwner(authReaderRoleBinding, csv)
542+
if ownerutil.AdoptableLabels(csv, existingAuthReaderRoleBinding.GetLabels()) {
543+
ownerutil.AddOwnerLabels(authReaderRoleBinding, csv)
544544
}
545-
546545
// Attempt an update.
547546
if _, err := a.OpClient.UpdateRoleBinding(authReaderRoleBinding); err != nil {
548547
logger.Warnf("could not update auth reader role binding %s", authReaderRoleBinding.GetName())
549548
return nil, err
550549
}
551550
} else if k8serrors.IsNotFound(err) {
552551
// Create the role.
553-
ownerutil.AddNonBlockingOwner(authReaderRoleBinding, csv)
552+
ownerutil.AddOwnerLabels(authReaderRoleBinding, csv)
554553
_, err = a.OpClient.CreateRoleBinding(authReaderRoleBinding)
555554
if err != nil {
556555
log.Warnf("could not create auth reader role binding %s", authReaderRoleBinding.GetName())
@@ -641,13 +640,13 @@ func (a *Operator) installAPIServiceRequirements(desc v1alpha1.APIServiceDescrip
641640
apiService.SetName(apiServiceName)
642641
} else {
643642
// check if the APIService is adoptable
644-
if !ownerutil.Adoptable(csv, apiService.GetOwnerReferences()) {
643+
if !ownerutil.AdoptableLabels(csv, apiService.GetLabels()) {
645644
return nil, fmt.Errorf("pre-existing APIService %s is not adoptable", apiServiceName)
646645
}
647646
}
648647

649648
// Add the CSV as an owner
650-
ownerutil.AddNonBlockingOwner(apiService, csv)
649+
ownerutil.AddOwnerLabels(apiService, csv)
651650

652651
// update the ServiceReference
653652
apiService.Spec.Service = &apiregistrationv1.ServiceReference{

0 commit comments

Comments
 (0)