Skip to content

Commit

Permalink
add flag to ramain ownerRef (#2374)
Browse files Browse the repository at this point in the history
  • Loading branch information
Yisaer committed May 7, 2020
1 parent 3485e77 commit a072387
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 32 deletions.
64 changes: 35 additions & 29 deletions pkg/controller/generic_control.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ type TypedControlInterface interface {
// CreateOrUpdateDeployment create the desired deployment or update the current one to desired state if already existed
CreateOrUpdateDeployment(controller runtime.Object, deploy *appsv1.Deployment) (*appsv1.Deployment, error)
// CreateOrUpdatePVC create the desired pvc or update the current one to desired state if already existed
CreateOrUpdatePVC(controller runtime.Object, pvc *corev1.PersistentVolumeClaim) (*corev1.PersistentVolumeClaim, error)
CreateOrUpdatePVC(controller runtime.Object, pvc *corev1.PersistentVolumeClaim, setOwnerFlag bool) (*corev1.PersistentVolumeClaim, error)
// CreateOrUpdateIngress create the desired ingress or update the current one to desired state if already existed
CreateOrUpdateIngress(controller runtime.Object, ingress *extensionsv1beta1.Ingress) (*extensionsv1beta1.Ingress, error)
// UpdateStatus update the /status subresource of the object
Expand All @@ -77,14 +77,14 @@ func NewTypedControl(control GenericControlInterface) TypedControlInterface {
return &typedWrapper{control}
}

func (w *typedWrapper) CreateOrUpdatePVC(controller runtime.Object, pvc *corev1.PersistentVolumeClaim) (*corev1.PersistentVolumeClaim, error) {
func (w *typedWrapper) CreateOrUpdatePVC(controller runtime.Object, pvc *corev1.PersistentVolumeClaim, setOwnerFlag bool) (*corev1.PersistentVolumeClaim, error) {
result, err := w.GenericControlInterface.CreateOrUpdate(controller, pvc, func(existing, desired runtime.Object) error {
existingPVC := existing.(*corev1.PersistentVolumeClaim)
desiredPVC := desired.(*corev1.PersistentVolumeClaim)

existingPVC.Spec.Resources.Requests = desiredPVC.Spec.Resources.Requests
return nil
})
}, setOwnerFlag)
if err != nil {
return nil, err
}
Expand All @@ -100,7 +100,7 @@ func (w *typedWrapper) CreateOrUpdateClusterRoleBinding(controller runtime.Objec
existingCRB.RoleRef = desiredCRB.RoleRef
existingCRB.Subjects = desiredCRB.Subjects
return nil
})
}, true)
if err != nil {
return nil, err
}
Expand All @@ -115,7 +115,7 @@ func (w *typedWrapper) CreateOrUpdateClusterRole(controller runtime.Object, clus
existingCRole.Labels = desiredCRole.Labels
existingCRole.Rules = desiredCRole.Rules
return nil
})
}, true)
if err != nil {
return nil, err
}
Expand All @@ -133,7 +133,7 @@ func (w *typedWrapper) CreateOrUpdateSecret(controller runtime.Object, secret *c
existingSecret.Annotations[k] = v
}
return nil
})
}, true)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -180,7 +180,7 @@ func (w *typedWrapper) CreateOrUpdateDeployment(controller runtime.Object, deplo
existingDep.Spec.Template.Spec = desiredDep.Spec.Template.Spec
}
return nil
})
}, true)
if err != nil {
return nil, err
}
Expand All @@ -195,7 +195,7 @@ func (w *typedWrapper) CreateOrUpdateRole(controller runtime.Object, role *rbacv
existingRole.Labels = desiredCRole.Labels
existingRole.Rules = desiredCRole.Rules
return nil
})
}, true)
if err != nil {
return nil, err
}
Expand All @@ -211,7 +211,7 @@ func (w *typedWrapper) CreateOrUpdateRoleBinding(controller runtime.Object, rb *
existingRB.RoleRef = desiredRB.RoleRef
existingRB.Subjects = desiredRB.Subjects
return nil
})
}, true)
if err != nil {
return nil, err
}
Expand All @@ -225,7 +225,7 @@ func (w *typedWrapper) CreateOrUpdateServiceAccount(controller runtime.Object, s

existingSA.Labels = desiredSA.Labels
return nil
})
}, true)
if err != nil {
return nil, err
}
Expand All @@ -243,7 +243,7 @@ func (w *typedWrapper) CreateOrUpdateConfigMap(controller runtime.Object, cm *co
existingCm.Annotations[k] = v
}
return nil
})
}, true)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -297,7 +297,7 @@ func (w *typedWrapper) CreateOrUpdateService(controller runtime.Object, svc *cor
}
}
return nil
})
}, true)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -330,24 +330,24 @@ func (w *typedWrapper) CreateOrUpdateIngress(controller runtime.Object, ingress
existingIngress.Spec = desiredIngress.Spec
}
return nil
})
}, true)
if err != nil {
return nil, err
}
return result.(*extensionsv1beta1.Ingress), nil
}

func (w *typedWrapper) Create(controller, obj runtime.Object) error {
return w.GenericControlInterface.Create(controller, obj)
return w.GenericControlInterface.Create(controller, obj, true)
}
func (w *typedWrapper) Exist(key client.ObjectKey, obj runtime.Object) (bool, error) {
return w.GenericControlInterface.Exist(key, obj)
}

// GenericControlInterface manages generic object that managed by an arbitrary controller
type GenericControlInterface interface {
CreateOrUpdate(controller, obj runtime.Object, mergeFn MergeFn) (runtime.Object, error)
Create(controller, obj runtime.Object) error
CreateOrUpdate(controller, obj runtime.Object, mergeFn MergeFn, setOwnerFlag bool) (runtime.Object, error)
Create(controller, obj runtime.Object, setOwnerFlag bool) error
UpdateStatus(obj runtime.Object) error
Exist(key client.ObjectKey, obj runtime.Object) (bool, error)
Delete(controller, obj runtime.Object) error
Expand Down Expand Up @@ -399,14 +399,16 @@ func (c *realGenericControlInterface) Exist(key client.ObjectKey, obj runtime.Ob
// CreateOrUpdate create an object to the Kubernetes cluster for controller, if the object to create is existed,
// call mergeFn to merge the change in new object to the existing object, then update the existing object.
// The object will also be adopted by the given controller.
func (c *realGenericControlInterface) CreateOrUpdate(controller, obj runtime.Object, mergeFn MergeFn) (runtime.Object, error) {
func (c *realGenericControlInterface) CreateOrUpdate(controller, obj runtime.Object, mergeFn MergeFn, setOwnerFlag bool) (runtime.Object, error) {

// controller-runtime/client will mutate the object pointer in-place,
// to be consistent with other methods in our controller, we copy the object
// to avoid the in-place mutation here and hereafter.
desired := obj.DeepCopyObject()
if err := setControllerReference(controller, desired); err != nil {
return desired, err
if setOwnerFlag {
if err := setControllerReference(controller, desired); err != nil {
return desired, err
}
}

// 1. try to create and see if there is any conflicts
Expand All @@ -427,9 +429,11 @@ func (c *realGenericControlInterface) CreateOrUpdate(controller, obj runtime.Obj
return nil, err
}

// 3. try to adopt the existing object
if err := setControllerReference(controller, existing); err != nil {
return nil, err
if setOwnerFlag {
// 3. try to adopt the existing object
if err := setControllerReference(controller, existing); err != nil {
return nil, err
}
}

mutated := existing.DeepCopyObject()
Expand All @@ -455,13 +459,15 @@ func (c *realGenericControlInterface) CreateOrUpdate(controller, obj runtime.Obj
}

// Create create an object to the Kubernetes cluster for controller
func (c *realGenericControlInterface) Create(controller, obj runtime.Object) error {
func (c *realGenericControlInterface) Create(controller, obj runtime.Object, setOwnerFlag bool) error {
// controller-runtime/client will mutate the object pointer in-place,
// to be consistent with other methods in our controller, we copy the object
// to avoid the in-place mutation here and hereafter.
desired := obj.DeepCopyObject()
if err := setControllerReference(controller, desired); err != nil {
return err
if setOwnerFlag {
if err := setControllerReference(controller, desired); err != nil {
return err
}
}

err := c.client.Create(context.TODO(), desired)
Expand Down Expand Up @@ -548,14 +554,14 @@ func NewFakeGenericControl(initObjects ...runtime.Object) *FakeGenericControl {
RequestTracker{},
}
}
func (gc *FakeGenericControl) Create(controller, obj runtime.Object) error {
func (gc *FakeGenericControl) Create(controller, obj runtime.Object, setOwnerFlag bool) error {
defer gc.createTracker.Inc()
if gc.createTracker.ErrorReady() {
defer gc.createTracker.Reset()
return gc.createTracker.GetError()
}

return gc.control.Create(controller, obj)
return gc.control.Create(controller, obj, setOwnerFlag)
}

func (gc *FakeGenericControl) Exist(key client.ObjectKey, obj runtime.Object) (bool, error) {
Expand Down Expand Up @@ -602,14 +608,14 @@ func (gc *FakeGenericControl) UpdateStatus(obj runtime.Object) error {
return gc.FakeCli.Status().Update(context.TODO(), obj)
}

func (gc *FakeGenericControl) CreateOrUpdate(controller, obj runtime.Object, fn MergeFn) (runtime.Object, error) {
func (gc *FakeGenericControl) CreateOrUpdate(controller, obj runtime.Object, fn MergeFn, setOwnerFlag bool) (runtime.Object, error) {
defer gc.createOrUpdateTracker.Inc()
if gc.createOrUpdateTracker.ErrorReady() {
defer gc.createOrUpdateTracker.Reset()
return nil, gc.createOrUpdateTracker.GetError()
}

return gc.control.CreateOrUpdate(controller, obj, fn)
return gc.control.CreateOrUpdate(controller, obj, fn, setOwnerFlag)
}

func (gc *FakeGenericControl) Delete(controller, obj runtime.Object) error {
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/generic_control_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@ func TestGenericControlInterface_CreateOrUpdate(t *testing.T) {
control := NewRealGenericControl(withTracker, recorder)
controller := newTidbCluster()
if tt.existing != nil {
_, err := control.CreateOrUpdate(controller, tt.existing, tt.mergeFn)
_, err := control.CreateOrUpdate(controller, tt.existing, tt.mergeFn, true)
g.Expect(err).To(Succeed())
}
withTracker.UpdateTracker.SetRequests(0)
withTracker.CreateTracker.SetRequests(0)
result, err := control.CreateOrUpdate(controller, tt.desired, tt.mergeFn)
result, err := control.CreateOrUpdate(controller, tt.desired, tt.mergeFn, true)
tt.expectFn(g, withTracker, result.(*appsv1.Deployment), err)
}
mergeFn := func(existing, desired runtime.Object) error {
Expand Down
2 changes: 1 addition & 1 deletion pkg/monitor/monitor/monitor_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func (mm *MonitorManager) syncTidbMonitorService(monitor *v1alpha1.TidbMonitor)
func (mm *MonitorManager) syncTidbMonitorPVC(monitor *v1alpha1.TidbMonitor) (*corev1.PersistentVolumeClaim, error) {

pvc := getMonitorPVC(monitor)
pvc, err := mm.typedControl.CreateOrUpdatePVC(monitor, pvc)
pvc, err := mm.typedControl.CreateOrUpdatePVC(monitor, pvc, false)
if err != nil {
klog.Errorf("tm[%s/%s]'s pvc[%s] failed to sync,err: %v", monitor.Namespace, monitor.Name, pvc.Name, err)
return nil, err
Expand Down

0 comments on commit a072387

Please sign in to comment.