Skip to content
This repository has been archived by the owner on Jun 8, 2022. It is now read-only.

fix status automatically removed bug #212

Merged
merged 2 commits into from
Sep 21, 2020

Conversation

ryanzhang-oss
Copy link
Collaborator

@ryanzhang-oss ryanzhang-oss commented Sep 19, 2020

The status fields in the workload disappear after a reconcile loop. The culprit is that the controller runtime merge function doesn't know how to patch a simple Json object... The solution is to manually patch it on the clientside (lame)

@ryanzhang-oss ryanzhang-oss force-pushed the fix-status branch 2 times, most recently from 5717426 to d72da93 Compare September 19, 2020 01:42
Signed-off-by: Ryan Zhang <yangzhangrice@hotmail.com>
Signed-off-by: Ryan Zhang <yangzhangrice@hotmail.com>
Copy link
Member

@hongchaodeng hongchaodeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code lgtm
Might need add some UT to pass codecov.

@@ -346,10 +345,36 @@ func (r *OAMApplicationReconciler) updateStatus(ctx context.Context, ac *v1alpha
}
}
ac.Status.Workloads = append(ac.Status.Workloads, revisionStatus...)

// patch the extra fields in the status that is wiped by the Status() function
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Status() is not specific enough to pinpoint the code location.
Better point out its parent function/object as well.

Copy link
Member

@wonderflow wonderflow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wonderflow
Copy link
Member

The Coverage seems to have some problem, merging

@wonderflow wonderflow merged commit fd7ae98 into crossplane:master Sep 21, 2020
xishengcai pushed a commit to xishengcai/oam-kubernetes-runtime that referenced this pull request Oct 25, 2020
fix status automatically removed bug

Signed-off-by: Xisheng Cai <xishengcai@XishengdeMacBook-Pro.local>
Signed-off-by: caixisheng <cc710917049@163.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants