Skip to content

Commit

Permalink
Remove the ownership on the clusterOperator
Browse files Browse the repository at this point in the history
A couple of reasons for doing this
1. no other clusterOperator has an owner other than clusterVersion
2. it does not seem effective as the cvo is setting ownership to ClusterVersion
3. then condition len(co.ObjectMeta.OwnerReferences) == 0 is totally ineffective - let's just remove it
  • Loading branch information
asalkeld committed Sep 14, 2021
1 parent fdbbee9 commit 8a42ed2
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 53 deletions.
17 changes: 1 addition & 16 deletions controllers/clusteroperator.go
Expand Up @@ -12,10 +12,8 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

osconfigv1 "github.com/openshift/api/config/v1"
metal3iov1alpha1 "github.com/openshift/cluster-baremetal-operator/api/v1alpha1"
"github.com/openshift/library-go/pkg/config/clusteroperator/v1helpers"
)

Expand Down Expand Up @@ -132,7 +130,7 @@ func (r *ProvisioningReconciler) createClusterOperator() (*osconfigv1.ClusterOpe
}

// ensureClusterOperator makes sure that the CO exists
func (r *ProvisioningReconciler) ensureClusterOperator(baremetalConfig *metal3iov1alpha1.Provisioning) error {
func (r *ProvisioningReconciler) ensureClusterOperator() error {
co, err := r.OSClient.ConfigV1().ClusterOperators().Get(context.Background(), clusterOperatorName, metav1.GetOptions{})
if k8serrors.IsNotFound(err) {
co, err = r.createClusterOperator()
Expand All @@ -141,19 +139,6 @@ func (r *ProvisioningReconciler) ensureClusterOperator(baremetalConfig *metal3io
return err
}

// if the CO has been created with the manifest then we need to update the ownership
if baremetalConfig != nil && len(co.ObjectMeta.OwnerReferences) == 0 {
err = controllerutil.SetControllerReference(baremetalConfig, co, r.Scheme)
if err != nil {
return err
}

co, err = r.OSClient.ConfigV1().ClusterOperators().Update(context.Background(), co, metav1.UpdateOptions{})
if err != nil {
return err
}
}

needsUpdate := false
if !equality.Semantic.DeepEqual(co.Status.RelatedObjects, relatedObjects()) {
needsUpdate = true
Expand Down
30 changes: 1 addition & 29 deletions controllers/clusteroperator_test.go
Expand Up @@ -10,8 +10,6 @@ import (
"github.com/google/go-cmp/cmp"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"

configv1 "github.com/openshift/api/config/v1"
osconfigv1 "github.com/openshift/api/config/v1"
Expand Down Expand Up @@ -157,15 +155,6 @@ func TestEnsureClusterOperator(t *testing.T) {
"include.release.openshift.io/self-managed-high-availability": "true",
"include.release.openshift.io/single-node-developer": "true",
},
OwnerReferences: []v1.OwnerReference{
{
APIVersion: "metal3.io/v1alpha1",
Kind: "Provisioning",
Name: "provisioning-configuration",
Controller: pointer.BoolPtr(true),
BlockOwnerDeletion: pointer.BoolPtr(true),
},
},
},
Status: osconfigv1.ClusterOperatorStatus{
Conditions: defaultConditions,
Expand Down Expand Up @@ -195,15 +184,6 @@ func TestEnsureClusterOperator(t *testing.T) {
"include.release.openshift.io/self-managed-high-availability": "true",
"include.release.openshift.io/single-node-developer": "true",
},
OwnerReferences: []v1.OwnerReference{
{
APIVersion: "metal3.io/v1alpha1",
Kind: "Provisioning",
Name: "provisioning-configuration",
Controller: pointer.BoolPtr(true),
BlockOwnerDeletion: pointer.BoolPtr(true),
},
},
},
Status: osconfigv1.ClusterOperatorStatus{
Conditions: conditions,
Expand All @@ -225,15 +205,7 @@ func TestEnsureClusterOperator(t *testing.T) {
reconciler.OSClient = osClient
reconciler.ReleaseVersion = "test-version"

err := reconciler.ensureClusterOperator(&metal3iov1alpha1.Provisioning{
TypeMeta: metav1.TypeMeta{
Kind: "Provisioning",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: metal3iov1alpha1.ProvisioningSingletonName,
},
})
err := reconciler.ensureClusterOperator()
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand Down
10 changes: 2 additions & 8 deletions controllers/provisioning_controller.go
Expand Up @@ -153,7 +153,7 @@ func (r *ProvisioningReconciler) Reconcile(ctx context.Context, req ctrl.Request
}

// Make sure ClusterOperator exists
err := r.ensureClusterOperator(nil)
err := r.ensureClusterOperator()
if err != nil {
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -195,12 +195,6 @@ func (r *ProvisioningReconciler) Reconcile(ctx context.Context, req ctrl.Request
"unable to put %q ClusterOperator in Available state", clusterOperatorName)
}

// Make sure ClusterOperator's ownership is updated
err = r.ensureClusterOperator(baremetalConfig)
if err != nil {
return ctrl.Result{}, err
}

// Read container images from Config Map
var containerImages provisioning.Images
if err := provisioning.GetContainerImages(&containerImages, r.ImagesFilename); err != nil {
Expand Down Expand Up @@ -464,7 +458,7 @@ func (r *ProvisioningReconciler) updateProvisioningMacAddresses(ctx context.Cont
// SetupWithManager configures the manager to run the controller
func (r *ProvisioningReconciler) SetupWithManager(mgr ctrl.Manager) error {
ctx := context.Background()
err := r.ensureClusterOperator(nil)
err := r.ensureClusterOperator()
if err != nil {
return errors.Wrap(err, "unable to set get baremetal ClusterOperator")
}
Expand Down

0 comments on commit 8a42ed2

Please sign in to comment.