Skip to content

fix(owners): remove cross-namespace and cluster->namespace ownerrefs #729

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 26, 2019
Merged

fix(owners): remove cross-namespace and cluster->namespace ownerrefs #729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
59 changes: 55 additions & 4 deletions go.sum
View file
Open in desktop

Large diffs are not rendered by default.

18 changes: 12 additions & 6 deletions pkg/api/wrappers/deployment_install_client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand Down Expand Up @@ -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",
},
Expand Down Expand Up @@ -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",
},
Expand All @@ -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",
},
Expand All @@ -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",
},
Expand All @@ -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",
},
Expand Down
6 changes: 1 addition & 5 deletions pkg/controller/install/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/install/deployment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
17 changes: 8 additions & 9 deletions pkg/controller/operators/olm/apiservices.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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{
Expand Down
Loading