Skip to content

Commit

Permalink
chore: some reconciliation trace related corner cases fixed (#8543)
Browse files Browse the repository at this point in the history
(cherry picked from commit 61eb47e)
  • Loading branch information
free6om committed Nov 28, 2024
1 parent de5f495 commit e3eac8d
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 17 deletions.
18 changes: 17 additions & 1 deletion controllers/trace/informer_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (m *informerManager) processNextWorkItem() bool {
}
// get involved object if 'object' is an Event
if evt, ok := object.(*corev1.Event); ok {
ro, err := m.scheme.New(evt.InvolvedObject.GroupVersionKind())
ro, err := m.scheme.New(getGVK(&evt.InvolvedObject))
if err != nil {
m.logger.Error(err, "new an event involved object failed")
return true
Expand All @@ -145,6 +145,22 @@ func (m *informerManager) processNextWorkItem() bool {
return true
}

func getGVK(ref *corev1.ObjectReference) schema.GroupVersionKind {
if ref == nil {
return schema.GroupVersionKind{}
}
// handle core group
if ref.APIVersion == "" {
return schema.GroupVersionKind{
Group: "",
Version: "v1",
Kind: ref.Kind,
}
}
// handle other group
return schema.FromAPIVersionAndKind(ref.APIVersion, ref.Kind)
}

func (m *informerManager) createInformer(gvk schema.GroupVersionKind) error {
o, err := m.scheme.New(gvk)
if err != nil {
Expand Down
5 changes: 0 additions & 5 deletions controllers/trace/reconciler_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ var reconcilerFuncMap = map[tracev1.ObjectType]reconcilerFunc{
objectType(workloadsAPI.SchemeGroupVersion.String(), workloadsAPI.InstanceSetKind): newInstanceSetReconciler,
objectType(corev1.SchemeGroupVersion.String(), constant.ConfigMapKind): newConfigMapReconciler,
objectType(corev1.SchemeGroupVersion.String(), constant.PersistentVolumeClaimKind): newPVCReconciler,
objectType(rbacv1.SchemeGroupVersion.String(), constant.ClusterRoleBindingKind): newClusterRoleBindingReconciler,
objectType(rbacv1.SchemeGroupVersion.String(), constant.RoleBindingKind): newRoleBindingReconciler,
objectType(corev1.SchemeGroupVersion.String(), constant.ServiceAccountKind): newSAReconciler,
objectType(batchv1.SchemeGroupVersion.String(), constant.JobKind): newJobReconciler,
Expand Down Expand Up @@ -700,10 +699,6 @@ func newRoleBindingReconciler(c client.Client, recorder record.EventRecorder) re
return newDoNothingReconciler()
}

func newClusterRoleBindingReconciler(c client.Client, recorder record.EventRecorder) reconcile.Reconciler {
return newDoNothingReconciler()
}

func newConfigMapReconciler(c client.Client, recorder record.EventRecorder) reconcile.Reconciler {
return newDoNothingReconciler()
}
Expand Down
6 changes: 5 additions & 1 deletion controllers/trace/resources_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
kbappsv1 "github.com/apecloud/kubeblocks/apis/apps/v1"
tracev1 "github.com/apecloud/kubeblocks/apis/trace/v1"
"github.com/apecloud/kubeblocks/pkg/controller/kubebuilderx"
"github.com/apecloud/kubeblocks/pkg/controller/model"
)

type resourcesValidator struct {
Expand All @@ -46,6 +47,9 @@ func (r *resourcesValidator) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuild
if tree.GetRoot() == nil {
return kubebuilderx.Commit, nil
}
if model.IsObjectDeleting(tree.GetRoot()) {
return kubebuilderx.Continue, nil
}

// target object should exist
v, _ := tree.GetRoot().(*tracev1.ReconciliationTrace)
Expand All @@ -55,7 +59,7 @@ func (r *resourcesValidator) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuild
objectKey.Name = v.Spec.TargetObject.Name
}
if err := r.reader.Get(r.ctx, objectKey, &kbappsv1.Cluster{}); err != nil {
return kubebuilderx.Commit, err
return kubebuilderx.Commit, client.IgnoreNotFound(err)
}

return kubebuilderx.Continue, nil
Expand Down
1 change: 0 additions & 1 deletion controllers/trace/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ func mockObjects(k8sMock *mocks.MockClient) (*kbappsv1.Cluster, []kbappsv1.Compo
&corev1.SecretList{},
&corev1.ConfigMapList{},
&corev1.PersistentVolumeClaimList{},
&rbacv1.ClusterRoleBindingList{},
&rbacv1.RoleBindingList{},
&corev1.ServiceAccountList{},
&batchv1.JobList{},
Expand Down
4 changes: 0 additions & 4 deletions controllers/trace/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,6 @@ var (
Secondary: objectType(corev1.SchemeGroupVersion.String(), constant.PersistentVolumeClaimKind),
Criteria: componentCriteria,
},
{
Secondary: objectType(rbacv1.SchemeGroupVersion.String(), constant.ClusterRoleBindingKind),
Criteria: componentCriteria,
},
{
Secondary: objectType(rbacv1.SchemeGroupVersion.String(), constant.RoleBindingKind),
Criteria: componentCriteria,
Expand Down
1 change: 0 additions & 1 deletion pkg/constant/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ const (
PersistentVolumeClaimKind = "PersistentVolumeClaim"
PersistentVolumeKind = "PersistentVolume"
ConfigurationKind = "Configuration"
ClusterRoleBindingKind = "ClusterRoleBinding"
RoleBindingKind = "RoleBinding"
ServiceAccountKind = "ServiceAccount"
EventKind = "Event"
Expand Down
20 changes: 18 additions & 2 deletions pkg/controller/kubebuilderx/plan_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,31 @@ func (b *PlanBuilder) Build() (graph.Plan, error) {
}

func buildOrderedVertices(ctx context.Context, currentTree *ObjectTree, desiredTree *ObjectTree) []*model.ObjectVertex {
getStatusField := func(obj client.Object) interface{} {
objValue := reflect.ValueOf(obj)
if objValue.Kind() != reflect.Ptr || objValue.Elem().Kind() != reflect.Struct {
return nil
}
field := objValue.Elem().FieldByName("Status")
if !field.IsValid() {
return nil
}
return field.Interface()
}

var vertices []*model.ObjectVertex

// handle root object
if desiredTree.GetRoot() == nil {
root := model.NewObjectVertex(currentTree.GetRoot(), currentTree.GetRoot(), model.ActionDeletePtr())
vertices = append(vertices, root)
} else {
root := model.NewObjectVertex(currentTree.GetRoot(), desiredTree.GetRoot(), model.ActionStatusPtr())
vertices = append(vertices, root)
currentStatus := getStatusField(currentTree.GetRoot())
desiredStatus := getStatusField(desiredTree.GetRoot())
if !reflect.DeepEqual(currentStatus, desiredStatus) {
root := model.NewObjectVertex(currentTree.GetRoot(), desiredTree.GetRoot(), model.ActionStatusPtr())
vertices = append(vertices, root)
}
// if annotations, labels or finalizers updated, do both meta patch and status update.
if !reflect.DeepEqual(currentTree.GetRoot().GetAnnotations(), desiredTree.GetRoot().GetAnnotations()) ||
!reflect.DeepEqual(currentTree.GetRoot().GetLabels(), desiredTree.GetRoot().GetLabels()) ||
Expand Down
6 changes: 4 additions & 2 deletions pkg/controller/kubebuilderx/plan_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,15 +221,17 @@ var _ = Describe("plan builder test", func() {
env := builder.NewConfigMapBuilder(namespace, name+"-env").GetObject()

var verticesExpected []*model.ObjectVertex
verticesExpected = append(verticesExpected, newVertex(its.DeepCopy(), its, model.ActionStatusPtr()))
itsCopy := its.DeepCopy()
itsCopy.Status.Replicas = *itsCopy.Spec.Replicas
verticesExpected = append(verticesExpected, newVertex(its, itsCopy, model.ActionStatusPtr()))
verticesExpected = append(verticesExpected, newVertex(nil, pod, model.ActionCreatePtr()))
verticesExpected = append(verticesExpected, newVertex(nil, headlessSvc, model.ActionCreatePtr()))
verticesExpected = append(verticesExpected, newVertex(nil, svc, model.ActionCreatePtr()))
verticesExpected = append(verticesExpected, newVertex(nil, env, model.ActionCreatePtr()))

// build ordered vertices
currentTree.SetRoot(its)
desiredTree.SetRoot(its)
desiredTree.SetRoot(itsCopy)
Expect(desiredTree.Add(pod, headlessSvc, svc, env)).Should(Succeed())
vertices := buildOrderedVertices(ctx, currentTree, desiredTree)

Expand Down

0 comments on commit e3eac8d

Please sign in to comment.